Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -108,16 +108,23 @@ final class BridgeClient: @unchecked Sendable {
private let mutationCoordinator = MutationCoordinator()
private let commandTimeoutMilliseconds: UInt64
private let importCommandTimeoutMilliseconds: UInt64
private let updateSourceTimeoutMilliseconds: UInt64
private let updateCommandMaximumTimeoutMilliseconds: UInt64
private let commandTimeoutGraceMilliseconds: UInt64

init(
commandTimeoutMilliseconds: UInt64 = 60_000,
// Network-heavy import/update/add work often needs more than 3 minutes on unstable links.
// Network-heavy import/add work often needs more than 3 minutes on unstable links.
importCommandTimeoutMilliseconds: UInt64 = 300_000,
// One source receives five minutes; selected updates scale to a 15-minute ceiling.
updateSourceTimeoutMilliseconds: UInt64 = 300_000,
updateCommandMaximumTimeoutMilliseconds: UInt64 = 900_000,
commandTimeoutGraceMilliseconds: UInt64 = 1_000
) {
self.commandTimeoutMilliseconds = commandTimeoutMilliseconds
self.importCommandTimeoutMilliseconds = importCommandTimeoutMilliseconds
self.updateSourceTimeoutMilliseconds = updateSourceTimeoutMilliseconds
self.updateCommandMaximumTimeoutMilliseconds = updateCommandMaximumTimeoutMilliseconds
self.commandTimeoutGraceMilliseconds = commandTimeoutGraceMilliseconds
}

Expand Down Expand Up @@ -425,7 +432,7 @@ final class BridgeClient: @unchecked Sendable {
inputPipe.fileHandleForWriting.write(requestData)
inputPipe.fileHandleForWriting.closeFile()

let activeTimeoutMilliseconds = timeoutMilliseconds(for: command)
let activeTimeoutMilliseconds = timeoutMilliseconds(for: command, payload: payload)
let didExit = await waitForProcessExit(
process,
state: exitWaitState,
Expand Down Expand Up @@ -493,8 +500,26 @@ final class BridgeClient: @unchecked Sendable {
throw BridgeClientError.commandFailed(message, response: response)
}

private func timeoutMilliseconds(for command: BridgeCommand) -> UInt64 {
command.usesExtendedNetworkTimeout
private func timeoutMilliseconds(
for command: BridgeCommand,
payload: [String: AnyCodable]?
) -> UInt64 {
if command == .update {
guard
let sourceIds = payload?["sourceIds"]?.value as? [String],
!sourceIds.isEmpty
else {
return updateCommandMaximumTimeoutMilliseconds
}
let sourceCount = UInt64(Set(sourceIds).count)
let (scaledTimeout, overflow) = updateSourceTimeoutMilliseconds
.multipliedReportingOverflow(by: sourceCount)
return min(
overflow ? updateCommandMaximumTimeoutMilliseconds : scaledTimeout,
updateCommandMaximumTimeoutMilliseconds
)
}
return command.usesExtendedNetworkTimeout
? importCommandTimeoutMilliseconds
: commandTimeoutMilliseconds
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,80 @@ final class BridgeClientExecutionTests: XCTestCase {
XCTAssertTrue(response.ok)
}

func testUpdateTimeoutScalesWithSelectedSourceCount() async throws {
let fixture = try SlowBridgeFixture.install(delayMilliseconds: 100)
self.fixture = fixture

let bridge = await MainActor.run {
BridgeClient(
commandTimeoutMilliseconds: 25,
importCommandTimeoutMilliseconds: 50,
updateSourceTimeoutMilliseconds: 75,
updateCommandMaximumTimeoutMilliseconds: 150
)
}

let response = try await bridge.updateSources(["alpha", "beta"])

XCTAssertEqual(response.command, BridgeCommand.update)
XCTAssertTrue(response.ok)
}

func testSingleSourceUpdateUsesOneSourceBudget() async throws {
let fixture = try SlowBridgeFixture.install(delayMilliseconds: 100)
self.fixture = fixture

let bridge = await MainActor.run {
BridgeClient(
updateSourceTimeoutMilliseconds: 50,
updateCommandMaximumTimeoutMilliseconds: 150
)
}

do {
_ = try await bridge.updateSources(["alpha"])
XCTFail("Expected one source update to use one source budget.")
} catch {
XCTAssertEqual(error.localizedDescription, "Operation timed out after 50ms.")
}
}

func testUpdateAllUsesMaximumUpdateBudget() async throws {
let fixture = try SlowBridgeFixture.install(delayMilliseconds: 100)
self.fixture = fixture

let bridge = await MainActor.run {
BridgeClient(
updateSourceTimeoutMilliseconds: 50,
updateCommandMaximumTimeoutMilliseconds: 150
)
}

let response = try await bridge.updateAll()

XCTAssertEqual(response.command, BridgeCommand.update)
XCTAssertTrue(response.ok)
}

func testSelectedUpdateBudgetDoesNotExceedMaximum() async throws {
let fixture = try SlowBridgeFixture.install(delayMilliseconds: 150)
self.fixture = fixture

let bridge = await MainActor.run {
BridgeClient(
updateSourceTimeoutMilliseconds: 50,
updateCommandMaximumTimeoutMilliseconds: 100
)
}

do {
_ = try await bridge.updateSources(["alpha", "beta", "gamma", "delta"])
XCTFail("Expected selected update budget to stop at its maximum.")
} catch {
XCTAssertEqual(error.localizedDescription, "Operation timed out after 100ms.")
}
}

func testTimedOutHelperIsForceKilledWhenItIgnoresTerminate() async throws {
let fixture = try StubbornBridgeFixture.install()
stubbornFixture = fixture
Expand Down
1 change: 1 addition & 0 deletions docs/FEATURE_INDEX.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
| Desktop DMG update flow | Planned | - | [plan](superpowers/plans/2026-06-24-desktop-dmg-update-flow.md) | - |
| Local scan import source path policy | Planned | [spec](superpowers/specs/2026-06-24-local-scan-import-source-path-policy-design.md) | [plan](superpowers/plans/2026-06-24-local-scan-import-source-path-policy.md) | - |
| CLI migration usability | Implemented | [spec](superpowers/specs/2026-06-26-cli-migration-usability-design.md) | [plan](superpowers/plans/2026-06-26-cli-migration-usability.md) | - |
| Git source update boundaries | Implemented | [spec](superpowers/specs/2026-08-18-git-source-update-boundaries-design.md) | - | Focused source, query, and desktop bridge tests |
| Plugin assets and target routing | Issue | - | [analysis](issues/ISSUE_8_plugin_assets_and_target_routing.md), [user report](issues/ISSUE_8_plugin_assets_and_target_routing_user_report.md) | - |
| Group card state matrix | Verification | - | - | [matrix](verification/GROUP_CARD_STATE_MATRIX.md) |
| Mount lifecycle edge cases | Verification | - | - | [matrix](verification/mount-lifecycle-matrix.md) |
3 changes: 2 additions & 1 deletion docs/PRODUCT.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ Skill Flow 用来把分散在不同来源的 AI agent skills 管理成可检查
- `lock.json` 记录解析结果、source snapshot 和部署投影。
- Target 目录是生成输出,不是事实源。
- macOS 桌面端依赖 CLI bridge 协议,不另建第二套状态模型。
- Managed update 只读写 `~/.skillflow/source/<kind>/<sourceId>` 的规范 checkout;lock 路径不匹配或 checkout 路径链包含符号链接时拒绝更新。
- 桌面端更新始终有界:每个明确选择的 source 预算 5 分钟,总上限 15 分钟;全部更新使用 15 分钟。

## 非目标

Expand All @@ -48,4 +50,3 @@ Skill Flow 用来把分散在不同来源的 AI agent skills 管理成可检查
- bridge protocol request/response。
- desktop bridge payload 或打包产物行为。
- README、release notes 中承诺的用户流程。

6 changes: 6 additions & 0 deletions docs/contracts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ save-settings

Bridge changes are external changes. Update parser tests, CLI bridge behavior, desktop bridge models, and release/user docs when changing this surface.

Desktop helper execution is always time-bounded. Ordinary commands use 60
seconds, import/add commands use 5 minutes, and managed update scales by the
number of explicitly selected sources at 5 minutes each with a 15-minute
ceiling. Update-all uses the 15-minute ceiling. These budgets are desktop
process behavior and do not change the protocol payload shape.

When adding, removing, or renaming a bridge command:

1. Update `BRIDGE_COMMAND_NAMES` in `packages/shared-types/src/protocol.ts`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,15 @@ The Import page seeds recommended cards from bundled `recommendations.json`. Car

### Bridge Timeout

Add a fixed timeout to `BridgeClient.send`, with a default of 60 seconds. If the helper process has not exited by then, terminate it, clear pipe handlers, and throw `BridgeClientError.timeout(timeoutMs)`.
Add a bounded timeout to `BridgeClient.send`. Ordinary commands use 60 seconds,
network-heavy import/add commands use 5 minutes, and managed update uses 5
minutes per explicitly selected source with a 15-minute ceiling. Update-all
uses the 15-minute ceiling. If the helper process has not exited by its active
budget, terminate it, clear pipe handlers, and throw
`BridgeClientError.timeout(timeoutMs)`.

The timeout should cover all bridge commands. This is acceptable because desktop actions should not wait indefinitely. Long commands can later receive command-specific budgets if needed.
The timeout must cover all bridge commands. Command-specific budgets may be
longer than the ordinary default, but no desktop action may wait indefinitely.

Implementation detail:

Expand Down Expand Up @@ -112,7 +118,7 @@ If the Swift test harness cannot easily spawn a hanging helper, cover bridge tim

## Risks

- A 60 second bridge timeout can interrupt a legitimately slow import on poor networks. This is acceptable for desktop UX because indefinite loading is worse, and the user can retry.
- A bounded bridge timeout can interrupt a legitimately slow network action. Import and update receive larger command-specific budgets, while indefinite loading remains disallowed.
- Timeout behavior in Swift subprocess handling must avoid leaving pipe readability handlers attached.
- Adding a fetch helper touches shared integration code. Keep the helper small and avoid broad refactors.

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
# Git Source Update Boundaries Design

## Goal

Make managed source updates faster without weakening the ownership, locator,
or desktop process boundaries documented by the upstream architecture.

## Scope

This change is limited to managed source update behavior:

- preserve the remote-ref precheck and reuse an existing managed Git object store;
- preserve an already-recorded `originBranch` throughout the update;
- reject update when a managed lock points outside its canonical checkout path
or that path uses symbolic links;
- keep every desktop bridge update bounded.

It does not change GitHub or GitLab locator parsing, reconcile, repair,
deployment, or external-source lifecycle rules.

## Contracts

### Managed checkout ownership

Before any update preflight, checkout reuse, rename, or replacement, a managed
lock path must equal:

```text
<stateRoot>/source/<sourceKind>/<sourceId>
```

The resolved path must also remain inside `<stateRoot>/source`. The managed
path components must not be symbolic links, and their real paths must remain
under the real source root. A mismatch returns
`SOURCE_CHECKOUT_PATH_INVALID` without reading or modifying the referenced
checkout. External sources continue to leave the managed update path before
this check.

### Git update path

When a lock has an `originBranch`, remote precheck, local-object fetch, clean
clone, and archive fallback use that exact branch. Failure must not silently
switch to `main` or `master`. Locks without `originBranch` retain the upstream
default-branch behavior.

For a managed Git checkout, first attempt a local no-checkout clone to reuse
its object store, then fetch the remote ref and detach at `FETCH_HEAD`. If this
optimization fails, remove the temporary checkout and use the upstream clean
clone, HTTPS fallback, and archive fallback sequence.

### Mutation lock ownership

A lock with a live PID is never reclaimed solely because its timestamp has
crossed the stale threshold. A dead owner may be reclaimed immediately, while
missing or unreadable ownership metadata retains the upstream age-based rule.

### Desktop update budget

Every bridge command remains time-bounded. Managed update uses a dedicated
budget based on the distinct selected source count:

- one source: 5 minutes;
- two sources: 10 minutes;
- three or more sources: 15 minutes;
- update all: 15 minutes.

Tests may inject shorter budgets. Timeout termination and output handling stay
the same as other bridge commands.

## Acceptance tests

1. An unchanged remote commit skips checkout preparation.
2. A changed Git source reuses the existing managed object store.
3. A failed local reuse falls back to the upstream clean-fetch sequence.
4. A recorded `originBranch` is used by every update fetch path without
switching to a default branch.
5. Managed update rejects mismatched and symbolic-link checkout paths before
checkout preparation and leaves referenced external directories untouched.
6. A live process lock is not reclaimed after five minutes.
7. One- and two-source desktop updates receive 5- and 10-minute budgets, while
update-all and larger selections never exceed 15 minutes.
8. External sources remain excluded from managed update before any checkout
operation.
67 changes: 67 additions & 0 deletions packages/core-engine/src/services/source-authority-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,12 +327,26 @@ export class SourceAuthorityService {
continue;
}

const sourceRoot = path.join(this.options.stateStore.rootPath, "source");
const expectedCheckoutPath = path.join(sourceRoot, source.kind, sourceId);
if (!await this.isManagedCheckoutPathValid(
sourceRoot,
expectedCheckoutPath,
lock.localPath,
)) {
return fail({
code: "SOURCE_CHECKOUT_PATH_INVALID",
message: `Refusing to update checkout with invalid managed path: ${lock.localPath}`,
}, warnings);
}

const lockedCommit = this.readLockedCommitSha(lock.revision);
let repairReason: SourceRepairReason | undefined;
if (source.kind === "git" && lockedCommit) {
try {
const remoteCommit = await this.options.checkoutService.readGitRemoteHeadCommit(
source.locator,
lock.originBranch ? { branch: lock.originBranch } : {},
);
if (!remoteCommit) {
precheckFallbackSourceIds.push(sourceId);
Expand Down Expand Up @@ -374,8 +388,11 @@ export class SourceAuthorityService {
options: {
sourceIdOverride: sourceId,
displayNameOverride: source.displayName,
...(lock.originBranch ? { originBranch: lock.originBranch } : {}),
},
checkoutPath: tempCheckoutPath,
existingCheckoutPath: lock.localPath,
...(lock.originBranch ? { updateBranch: lock.originBranch } : {}),
allowEmptyLeafs: true,
});
if (!prepared.ok) {
Expand Down Expand Up @@ -472,6 +489,56 @@ export class SourceAuthorityService {
}, warnings);
}

private async isManagedCheckoutPathValid(
sourceRoot: string,
expectedCheckoutPath: string,
localPath: string,
): Promise<boolean> {
const normalizedLocalPath = path.resolve(localPath);
if (
normalizedLocalPath !== path.resolve(expectedCheckoutPath)
|| !isPathInside(sourceRoot, normalizedLocalPath)
) {
return false;
}

const kindRoot = path.dirname(expectedCheckoutPath);
for (const managedPath of [sourceRoot, kindRoot]) {
try {
if ((await fs.lstat(managedPath)).isSymbolicLink()) {
return false;
}
} catch {
return false;
}
}

const checkoutExists = await pathExists(expectedCheckoutPath);
if (checkoutExists) {
try {
if ((await fs.lstat(expectedCheckoutPath)).isSymbolicLink()) {
return false;
}
} catch {
return false;
}
}

try {
const realSourceRoot = await fs.realpath(sourceRoot);
const realKindRoot = await fs.realpath(kindRoot);
if (!isPathInside(realSourceRoot, realKindRoot)) {
return false;
}
if (!checkoutExists) {
return true;
}
return isPathInside(realSourceRoot, await fs.realpath(expectedCheckoutPath));
} catch {
return false;
}
}

async reconcileInventory(
sourceIds?: string[],
options: { force?: boolean } = {},
Expand Down
Loading