feat(workhub): add coordination session lifecycle - #3764

Merged
Astro-Han merged 4 commits into
apache:mainfrom
ARE404:codex/workhub-coordination-session-lifecycle
Aug 25, 2026
Merged

feat(workhub): add coordination session lifecycle#3764
Astro-Han merged 4 commits into
apache:mainfrom
ARE404:codex/workhub-coordination-session-lifecycle

Conversation

@ARE404

@ARE404ARE404 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • provision and reuse one stable Coordination Session per Runtime Host through the existing Session/SQLite authority
  • revoke an old Host scope as soon as the default Host changes, and retry recoverable provisioning when model authority becomes available
  • reserve the Coordination identity/role pair from ordinary creation, copies, configuration, retirement, catalogs, and root execution

Refs #3492

Authority and deferred work

Session remains the only durable substrate: this adds no second WorkHub database, event store, lifecycle authority, or transcript copy. Each Runtime Host remains independent; cross-Host coordination is not supported.

Coordination transcript rendering and answer_here belong to Slice 3. Model dispositions, the deterministic Action Gate, bounded delegation links, and the deferred Work cardinality decision remain for later slices.

UI evidence

Missing default modelRecovered after model configuration
WorkHub shows a recoverable unavailable stateWorkHub recovers without changing surfaces

Verification

  • npm run lint
  • npm run format:check
  • git diff --check
  • npm run typecheck
  • npm run build
  • Storage focused regressions: 29 passed
  • Runtime Host: 1,156 passed
  • Runtime Host two-client branch/revision regression: 1 passed
  • Desktop: 1,506 passed
  • Electron default-model failure/recovery e2e: 1 passed
  • Standards and Spec adversarial review: no findings
  • GitHub CI: passed for exact head 35efcd6d9, including Desktop e2e, browser smoke, Storybook, and installed CLI validation

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex implemented and tested the lifecycle, storage boundary, protocol, Desktop integration, review fixes, and Standards/Spec reviews.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally or in clean CI

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Implement Slice 2 of apache#3492 on the existing Session substrate.
Generated-by: Codex
@ARE404
ARE404 marked this pull request as ready for review August 25, 2026 05:37

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this head and found blocking issues.

[P2] Default-Host switch while not ready leaves old generation active

Switching default Host to a reconnecting/unavailable Host does not increment the generation, so late results from the previous Host still pass checks and WorkHub continues using the old Host until the new one becomes ready — which may never happen.

[P2] Ordinary workspace relocation can permanently break Coordination identity

session.workspace.relocate can change the reserved maka_workhub_coordination cwd/projectId, after which the next resolve permanently fails with operation_conflict and the row is hidden with no repair path.

[P3] Write-only ref duplication

workHubCoordinationSessionIdRef is never read; state is the sole authority.

Checks on becd9f133e are test: success.

简体中文存在两项身份/代次阻断与一项冗余。

@ARE404

Copy link
Copy Markdown
ContributorAuthor

Addressed the review findings in 35efcd6d9:

  • every default-Host change now revokes the old generation immediately, including connecting, reconnecting, and unavailable targets; stale resolves and stale controllers cannot mutate
  • ordinary workspace/configuration/metadata/read-state/retirement authority rejects either half of the reserved Coordination identity/role pair, including corrupt persisted pairings
  • workHubCoordinationSessionIdRef now participates in the current-scope guard alongside the generation
  • also closed ordinary session.create, branch/revision copy, and generic root-execution paths over the reserved identity; missing-model startup is visible and retryable

Exact-head CI is green, and the final Standards/Spec adversarial re-review has no findings.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on 35efcd6d9b:

No blocking issues. The prior generation/identity P2s are now closed: default-Host changes correctly invalidate the old generation, and the Coordination identity is protected across copy paths.

[P3] Copy reservation only protects the target, not the source

session-revision-coordinator.ts:179 checks only targetSessionId. A valid Coordination Session supplied as sourceSessionId can still be copied into an ordinary Session, exposing hidden messages. This is latent until Slice 3 writes transcripts, but should be rejected before admission.

Checks on 35efcd6d9b are test: success.

简体中文该头无阻断,剩余 P3 为源侧拷贝保护。

@ARE404

Copy link
Copy Markdown
ContributorAuthor

Self-review pass over this slice, graded by severity. Fixes for all seven follow in the next commit.

P1 — correctness, must fix before merge

1. validCoordinationHeader treats cwd as part of the identity, and nothing can repair the mismatch.
workhub-coordination-coordinator.ts requires header.cwd === join(stateRoot, 'workhub-coordination'). The create fingerprint is a constant, so after the state root moves to a different absolute path — restoring the state directory on a new machine, a renamed home directory, a remote Host whose home path changed — probeStableSessionCreate still answers existing, the cwd comparison fails, and #resolve returns identityConflict() forever. This slice deliberately blocks session.remove, session.lifecycle.set, session.workspace.relocate, session.configuration.update and session.metadata.update for the reserved identity, so there is no product path left to repair or drop the stale row: WorkHub becomes permanently unopenable and the surface tells the user to check their default model, which is not the problem. cwd is repairable state, not identity — resolve should relocate it instead of rejecting.

2. The copy reservation covers the target but not the source.
session-revision-coordinator.ts rejects the reserved identity only as input.targetSessionId. The Coordination Session is still a legal source for session.conversation.copy (ordinary copy, revision, and side conversation): the source header is only checked for archived / subagentParent / Deep Research. #createInput does not carry role, so the identity/role pairing check in createStableSession passes and the copy lands as an ordinary Session holding the Coordination transcript — with an ordinary id and no role, so runtimeHostExecutionUnavailableReason also lets it run Turns. Any owner client can reach this: workhub.coordination.resolve hands out the id, and the copy does the rest.

P2 — should fix in this PR

3. The Coordination workspace directory is only guaranteed at provisioning time.
mkdir runs on the create branch only. If stateRoot/workhub-coordination is later removed — a pruned state directory, a database restored without its sibling directory — the existing branch compares the cwd string, reports success, and the directory never comes back. mkdir with recursive is idempotent; running it before the probe makes both branches converge on the same guarantee.

4. A default Host that is not ready leaves WorkHub on an indefinite spinner.
workhub-coordination-lifecycle.ts always revokes on a default-Host change but only re-resolves when readiness === 'ready', and revoke() clears failedGeneration. When the default Host goes unavailable (crash, failed respawn, onTargetRemoved), reportFailure never runs, so the surface stays on Preparing WorkHub… with no error and no Retry, and the connection_list_changed handler cannot restart it either because it is gated on failedGeneration === generation. Recovery waits for a ready event that may never arrive. connecting / reconnecting should keep the spinner; unavailable should report a failure.

P3 — cleanups

5. Every resolveCreateTarget() failure is reported as a missing default model.
The bare catch maps any error to operation_conflict / “requires an available default model”. resolveExternalSessionImportTarget also awaits the runtime policy read, which raises persistence_failed / “Runtime policy is unavailable”; the user is then told to fix a model setting that was never wrong, and the real persistence failure is never surfaced.

6. #updateMetadata reads the header snapshot twice.
The new snapshot read and Coordination guard duplicate the read and guard #replaceUserLabels already performs inside the same admission lease, on the same sessionId. A label update now costs two identical reads, and rename / flag updates pay a new one on a hot path (auto-titling renames land on most Turns). Threading the record through removes both.

7. Unused import.
registerRuntimeHostSessionCatalogIpc is imported in runtime-host-session-catalog-ipc-main.test.ts and never referenced — it is the file's only change. Biome's allowlist has no unused-import rule, so nothing caught it.

…on both sides
Review follow-ups on this slice.
The Coordination workspace is derived from the Host state root, so treating
its path as part of the identity stranded the one Session that no ordinary
lifecycle operation may relocate or retire: a state directory restored under
a different absolute path answered `operation_conflict` on every resolve,
with no product path left to repair it. Identity is now the id/role pair
alone, the durable path is relocated in place, and the workspace directory is
ensured on reuse as well as on provisioning.
The copy reservation covered the target identity but not the source, so the
Coordination transcript could be lifted into an ordinary — and therefore
executable — Session. Both halves of the reserved pair are refused as a copy
source now.
An unavailable default Host revoked WorkHub resolution without reporting a
failure, leaving the surface on a loading state no retry could reopen, since
the availability subscription only reopens a generation that failed.
Connecting and reconnecting still hold that state; unavailable no longer does.
Also: keep a failed model-authority read distinguishable from a missing
default model, drop a duplicate header read inside one admission lease, and
remove an unused test import.
@ARE404

Copy link
Copy Markdown
ContributorAuthor

Pushed cd4b60ed8, which addresses all seven.

P1-1 — Coordination workspace no longer acts as identity.validCoordinationHeader checks the id/role pair, lineage and archive state only. The durable cwd is relocated in place under the admission lease with the probed revision, so a state directory restored under a different absolute path keeps its Session instead of answering operation_conflict forever. New regression: relocates the durable workspace when the Host state root moves.

P1-2 — the copy reservation now holds on both sides.#copy refuses the reserved id as a source, and #copyAdmitted refuses either half of the reserved pair once the source header is known. The two-client UDS regression now asserts both directions for session.branch.create and session.revision.create, and that the would-be target never appears in the catalog.

P2-3 — the workspace directory is ensured on reuse.mkdir moved above the probe, so both branches converge on the same guarantee and a pruned directory comes back before the identity is handed out. This also simplified the create branch: everything after the probe is now a single outcome-unknown boundary. New regression: restores a Coordination workspace that was pruned after provisioning.

P2-4 — an unavailable default Host reports a failure.connecting and reconnecting still hold the loading state; unavailable (and removed) now reports through the same path a rejected resolve takes, so the surface shows the error with Retry and the availability subscription can reopen the generation. New regression: WorkHub reports an unavailable default Host instead of holding the loading state.

P3-5 — a persistence_failed model-authority read keeps its code and message; everything else still maps to the default-model conflict. Covered by separates an unreadable model authority from a missing default model.

P3-6#updateMetadata reads the admitted snapshot once and threads it into label replacement; #replaceUserLabels became the pure replaceUserLabels(current, …).

P3-7 — unused import removed.

Local: npm run typecheck, npm run format:check, npm run lint, git diff --check all clean. Runtime Host 1,159 passed, Desktop 1,487 passed.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on cd4b60ed86:

No blocking issues. The source-side copy guard now correctly rejects reserved source Sessions, and workspace/generation handling is sound.

Checks on cd4b60ed86 — code GO, no P0-P3.

简体中文该头未发现阻断。

Depth for the A-seat boundary review. Neither item was reachable; both were
places where the closure depended on a caller remembering something.
The id/role pairing was asserted in createStableSession() only, so the
creators that share buildSessionHeader() — createSubagent() and
createAgentGraphOperator() — did not inherit the invariant even though the
header they build carries the same field. It moves into the builder, where
every creator meets it. createStableSession() keeps its own assertion so a
malformed request is still refused before a durable create claim is written.
The metadata list role scope defaulted to the widest value, which made
closure a property of each caller remembering to narrow it. The default is
gone: list() now takes both the filter and the scope explicitly, and usage
statistics — the one reader that took the default — states 'all', which is
what a durable cost aggregate wants.
@ARE404

Copy link
Copy Markdown
ContributorAuthor

Both P3 depth items are in 9966f7ae7.

1 — the id/role pairing moved into the header builder.assertCoordinationIdentityPairing(sessionId, role) now runs inside buildSessionHeader(), so createSubagent() and createAgentGraphOperator() meet the same invariant as createStableSession() rather than depending on their inputs never carrying a role.

createStableSession() keeps its own call to the assertion, deliberately: it runs before claimStableSessionCreate(), and the builder runs after. Dropping the early one would let a malformed request write a durable create claim for the identity it names, which a later legitimate create would then see as a conflict. Same fact, asserted at the boundary that must fail before a write and at the low-level constructor that every creator shares.

Regression extended: requires the reserved WorkHub Coordination identity and role together now rejects a role claimed through both sibling creators and asserts the store stays empty afterwards.

2 — the metadata role scope has no default.SqliteSessionMetadataStore.list() takes (filter: SessionListFilter | undefined, roleScope: SessionMetadataRoleScope), both required, and the type is exported so a call site names its scope. The three readers now read as: catalog 'ordinary', recovery 'recoverable', usage statistics 'all' — the last stated explicitly with a note that a durable cost aggregate counts every Session that spent tokens, including rows a catalog hides.

Local: npm run typecheck, npm run format:check, npm run lint, git diff --check clean. Storage 940 passed / 14 skipped, Runtime Host 1,159 passed, Desktop 1,487 passed.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on 9966f7ae7b:

No blocking issues. The delta makes Coordination identity/role pairing pre-claim and shared across Session creators, correct for preventing ordinary copies from capturing the reserved Session.

Checks on 9966f7ae7b are test: success.

简体中文该头未发现阻断。

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE — 9966f7a, no P0-P3, identity pairing correctly pre-claimed.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ARE404@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

feat(workhub): add coordination session lifecycle - #3764

Merged
Astro-Han merged 4 commits into
apache:mainfrom
ARE404:codex/workhub-coordination-session-lifecycle
Aug 25, 2026
Merged

feat(workhub): add coordination session lifecycle#3764
Astro-Han merged 4 commits into
apache:mainfrom
ARE404:codex/workhub-coordination-session-lifecycle

Conversation

@ARE404

@ARE404ARE404 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • provision and reuse one stable Coordination Session per Runtime Host through the existing Session/SQLite authority
  • revoke an old Host scope as soon as the default Host changes, and retry recoverable provisioning when model authority becomes available
  • reserve the Coordination identity/role pair from ordinary creation, copies, configuration, retirement, catalogs, and root execution

Refs #3492

Authority and deferred work

Session remains the only durable substrate: this adds no second WorkHub database, event store, lifecycle authority, or transcript copy. Each Runtime Host remains independent; cross-Host coordination is not supported.

Coordination transcript rendering and answer_here belong to Slice 3. Model dispositions, the deterministic Action Gate, bounded delegation links, and the deferred Work cardinality decision remain for later slices.

UI evidence

Missing default modelRecovered after model configuration
WorkHub shows a recoverable unavailable stateWorkHub recovers without changing surfaces

Verification

  • npm run lint
  • npm run format:check
  • git diff --check
  • npm run typecheck
  • npm run build
  • Storage focused regressions: 29 passed
  • Runtime Host: 1,156 passed
  • Runtime Host two-client branch/revision regression: 1 passed
  • Desktop: 1,506 passed
  • Electron default-model failure/recovery e2e: 1 passed
  • Standards and Spec adversarial review: no findings
  • GitHub CI: passed for exact head 35efcd6d9, including Desktop e2e, browser smoke, Storybook, and installed CLI validation

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex implemented and tested the lifecycle, storage boundary, protocol, Desktop integration, review fixes, and Standards/Spec reviews.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally or in clean CI

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Implement Slice 2 of apache#3492 on the existing Session substrate.
Generated-by: Codex
@ARE404
ARE404 marked this pull request as ready for review August 25, 2026 05:37

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this head and found blocking issues.

[P2] Default-Host switch while not ready leaves old generation active

Switching default Host to a reconnecting/unavailable Host does not increment the generation, so late results from the previous Host still pass checks and WorkHub continues using the old Host until the new one becomes ready — which may never happen.

[P2] Ordinary workspace relocation can permanently break Coordination identity

session.workspace.relocate can change the reserved maka_workhub_coordination cwd/projectId, after which the next resolve permanently fails with operation_conflict and the row is hidden with no repair path.

[P3] Write-only ref duplication

workHubCoordinationSessionIdRef is never read; state is the sole authority.

Checks on becd9f133e are test: success.

简体中文存在两项身份/代次阻断与一项冗余。

@ARE404

Copy link
Copy Markdown
ContributorAuthor

Addressed the review findings in 35efcd6d9:

  • every default-Host change now revokes the old generation immediately, including connecting, reconnecting, and unavailable targets; stale resolves and stale controllers cannot mutate
  • ordinary workspace/configuration/metadata/read-state/retirement authority rejects either half of the reserved Coordination identity/role pair, including corrupt persisted pairings
  • workHubCoordinationSessionIdRef now participates in the current-scope guard alongside the generation
  • also closed ordinary session.create, branch/revision copy, and generic root-execution paths over the reserved identity; missing-model startup is visible and retryable

Exact-head CI is green, and the final Standards/Spec adversarial re-review has no findings.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on 35efcd6d9b:

No blocking issues. The prior generation/identity P2s are now closed: default-Host changes correctly invalidate the old generation, and the Coordination identity is protected across copy paths.

[P3] Copy reservation only protects the target, not the source

session-revision-coordinator.ts:179 checks only targetSessionId. A valid Coordination Session supplied as sourceSessionId can still be copied into an ordinary Session, exposing hidden messages. This is latent until Slice 3 writes transcripts, but should be rejected before admission.

Checks on 35efcd6d9b are test: success.

简体中文该头无阻断,剩余 P3 为源侧拷贝保护。

@ARE404

Copy link
Copy Markdown
ContributorAuthor

Self-review pass over this slice, graded by severity. Fixes for all seven follow in the next commit.

P1 — correctness, must fix before merge

1. validCoordinationHeader treats cwd as part of the identity, and nothing can repair the mismatch.
workhub-coordination-coordinator.ts requires header.cwd === join(stateRoot, 'workhub-coordination'). The create fingerprint is a constant, so after the state root moves to a different absolute path — restoring the state directory on a new machine, a renamed home directory, a remote Host whose home path changed — probeStableSessionCreate still answers existing, the cwd comparison fails, and #resolve returns identityConflict() forever. This slice deliberately blocks session.remove, session.lifecycle.set, session.workspace.relocate, session.configuration.update and session.metadata.update for the reserved identity, so there is no product path left to repair or drop the stale row: WorkHub becomes permanently unopenable and the surface tells the user to check their default model, which is not the problem. cwd is repairable state, not identity — resolve should relocate it instead of rejecting.

2. The copy reservation covers the target but not the source.
session-revision-coordinator.ts rejects the reserved identity only as input.targetSessionId. The Coordination Session is still a legal source for session.conversation.copy (ordinary copy, revision, and side conversation): the source header is only checked for archived / subagentParent / Deep Research. #createInput does not carry role, so the identity/role pairing check in createStableSession passes and the copy lands as an ordinary Session holding the Coordination transcript — with an ordinary id and no role, so runtimeHostExecutionUnavailableReason also lets it run Turns. Any owner client can reach this: workhub.coordination.resolve hands out the id, and the copy does the rest.

P2 — should fix in this PR

3. The Coordination workspace directory is only guaranteed at provisioning time.
mkdir runs on the create branch only. If stateRoot/workhub-coordination is later removed — a pruned state directory, a database restored without its sibling directory — the existing branch compares the cwd string, reports success, and the directory never comes back. mkdir with recursive is idempotent; running it before the probe makes both branches converge on the same guarantee.

4. A default Host that is not ready leaves WorkHub on an indefinite spinner.
workhub-coordination-lifecycle.ts always revokes on a default-Host change but only re-resolves when readiness === 'ready', and revoke() clears failedGeneration. When the default Host goes unavailable (crash, failed respawn, onTargetRemoved), reportFailure never runs, so the surface stays on Preparing WorkHub… with no error and no Retry, and the connection_list_changed handler cannot restart it either because it is gated on failedGeneration === generation. Recovery waits for a ready event that may never arrive. connecting / reconnecting should keep the spinner; unavailable should report a failure.

P3 — cleanups

5. Every resolveCreateTarget() failure is reported as a missing default model.
The bare catch maps any error to operation_conflict / “requires an available default model”. resolveExternalSessionImportTarget also awaits the runtime policy read, which raises persistence_failed / “Runtime policy is unavailable”; the user is then told to fix a model setting that was never wrong, and the real persistence failure is never surfaced.

6. #updateMetadata reads the header snapshot twice.
The new snapshot read and Coordination guard duplicate the read and guard #replaceUserLabels already performs inside the same admission lease, on the same sessionId. A label update now costs two identical reads, and rename / flag updates pay a new one on a hot path (auto-titling renames land on most Turns). Threading the record through removes both.

7. Unused import.
registerRuntimeHostSessionCatalogIpc is imported in runtime-host-session-catalog-ipc-main.test.ts and never referenced — it is the file's only change. Biome's allowlist has no unused-import rule, so nothing caught it.

…on both sides
Review follow-ups on this slice.
The Coordination workspace is derived from the Host state root, so treating
its path as part of the identity stranded the one Session that no ordinary
lifecycle operation may relocate or retire: a state directory restored under
a different absolute path answered `operation_conflict` on every resolve,
with no product path left to repair it. Identity is now the id/role pair
alone, the durable path is relocated in place, and the workspace directory is
ensured on reuse as well as on provisioning.
The copy reservation covered the target identity but not the source, so the
Coordination transcript could be lifted into an ordinary — and therefore
executable — Session. Both halves of the reserved pair are refused as a copy
source now.
An unavailable default Host revoked WorkHub resolution without reporting a
failure, leaving the surface on a loading state no retry could reopen, since
the availability subscription only reopens a generation that failed.
Connecting and reconnecting still hold that state; unavailable no longer does.
Also: keep a failed model-authority read distinguishable from a missing
default model, drop a duplicate header read inside one admission lease, and
remove an unused test import.
@ARE404

Copy link
Copy Markdown
ContributorAuthor

Pushed cd4b60ed8, which addresses all seven.

P1-1 — Coordination workspace no longer acts as identity.validCoordinationHeader checks the id/role pair, lineage and archive state only. The durable cwd is relocated in place under the admission lease with the probed revision, so a state directory restored under a different absolute path keeps its Session instead of answering operation_conflict forever. New regression: relocates the durable workspace when the Host state root moves.

P1-2 — the copy reservation now holds on both sides.#copy refuses the reserved id as a source, and #copyAdmitted refuses either half of the reserved pair once the source header is known. The two-client UDS regression now asserts both directions for session.branch.create and session.revision.create, and that the would-be target never appears in the catalog.

P2-3 — the workspace directory is ensured on reuse.mkdir moved above the probe, so both branches converge on the same guarantee and a pruned directory comes back before the identity is handed out. This also simplified the create branch: everything after the probe is now a single outcome-unknown boundary. New regression: restores a Coordination workspace that was pruned after provisioning.

P2-4 — an unavailable default Host reports a failure.connecting and reconnecting still hold the loading state; unavailable (and removed) now reports through the same path a rejected resolve takes, so the surface shows the error with Retry and the availability subscription can reopen the generation. New regression: WorkHub reports an unavailable default Host instead of holding the loading state.

P3-5 — a persistence_failed model-authority read keeps its code and message; everything else still maps to the default-model conflict. Covered by separates an unreadable model authority from a missing default model.

P3-6#updateMetadata reads the admitted snapshot once and threads it into label replacement; #replaceUserLabels became the pure replaceUserLabels(current, …).

P3-7 — unused import removed.

Local: npm run typecheck, npm run format:check, npm run lint, git diff --check all clean. Runtime Host 1,159 passed, Desktop 1,487 passed.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on cd4b60ed86:

No blocking issues. The source-side copy guard now correctly rejects reserved source Sessions, and workspace/generation handling is sound.

Checks on cd4b60ed86 — code GO, no P0-P3.

简体中文该头未发现阻断。

Depth for the A-seat boundary review. Neither item was reachable; both were
places where the closure depended on a caller remembering something.
The id/role pairing was asserted in createStableSession() only, so the
creators that share buildSessionHeader() — createSubagent() and
createAgentGraphOperator() — did not inherit the invariant even though the
header they build carries the same field. It moves into the builder, where
every creator meets it. createStableSession() keeps its own assertion so a
malformed request is still refused before a durable create claim is written.
The metadata list role scope defaulted to the widest value, which made
closure a property of each caller remembering to narrow it. The default is
gone: list() now takes both the filter and the scope explicitly, and usage
statistics — the one reader that took the default — states 'all', which is
what a durable cost aggregate wants.
@ARE404

Copy link
Copy Markdown
ContributorAuthor

Both P3 depth items are in 9966f7ae7.

1 — the id/role pairing moved into the header builder.assertCoordinationIdentityPairing(sessionId, role) now runs inside buildSessionHeader(), so createSubagent() and createAgentGraphOperator() meet the same invariant as createStableSession() rather than depending on their inputs never carrying a role.

createStableSession() keeps its own call to the assertion, deliberately: it runs before claimStableSessionCreate(), and the builder runs after. Dropping the early one would let a malformed request write a durable create claim for the identity it names, which a later legitimate create would then see as a conflict. Same fact, asserted at the boundary that must fail before a write and at the low-level constructor that every creator shares.

Regression extended: requires the reserved WorkHub Coordination identity and role together now rejects a role claimed through both sibling creators and asserts the store stays empty afterwards.

2 — the metadata role scope has no default.SqliteSessionMetadataStore.list() takes (filter: SessionListFilter | undefined, roleScope: SessionMetadataRoleScope), both required, and the type is exported so a call site names its scope. The three readers now read as: catalog 'ordinary', recovery 'recoverable', usage statistics 'all' — the last stated explicitly with a note that a durable cost aggregate counts every Session that spent tokens, including rows a catalog hides.

Local: npm run typecheck, npm run format:check, npm run lint, git diff --check clean. Storage 940 passed / 14 skipped, Runtime Host 1,159 passed, Desktop 1,487 passed.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on 9966f7ae7b:

No blocking issues. The delta makes Coordination identity/role pairing pre-claim and shared across Session creators, correct for preventing ordinary copies from capturing the reserved Session.

Checks on 9966f7ae7b are test: success.

简体中文该头未发现阻断。

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE — 9966f7a, no P0-P3, identity pairing correctly pre-claimed.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ARE404@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(workhub): add coordination session lifecycle - #3764

Merged
Astro-Han merged 4 commits into
apache:mainfrom
ARE404:codex/workhub-coordination-session-lifecycle
Aug 25, 2026
Merged

feat(workhub): add coordination session lifecycle#3764
Astro-Han merged 4 commits into
apache:mainfrom
ARE404:codex/workhub-coordination-session-lifecycle

Conversation

@ARE404

@ARE404ARE404 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • provision and reuse one stable Coordination Session per Runtime Host through the existing Session/SQLite authority
  • revoke an old Host scope as soon as the default Host changes, and retry recoverable provisioning when model authority becomes available
  • reserve the Coordination identity/role pair from ordinary creation, copies, configuration, retirement, catalogs, and root execution

Refs #3492

Authority and deferred work

Session remains the only durable substrate: this adds no second WorkHub database, event store, lifecycle authority, or transcript copy. Each Runtime Host remains independent; cross-Host coordination is not supported.

Coordination transcript rendering and answer_here belong to Slice 3. Model dispositions, the deterministic Action Gate, bounded delegation links, and the deferred Work cardinality decision remain for later slices.

UI evidence

Missing default modelRecovered after model configuration
WorkHub shows a recoverable unavailable stateWorkHub recovers without changing surfaces

Verification

  • npm run lint
  • npm run format:check
  • git diff --check
  • npm run typecheck
  • npm run build
  • Storage focused regressions: 29 passed
  • Runtime Host: 1,156 passed
  • Runtime Host two-client branch/revision regression: 1 passed
  • Desktop: 1,506 passed
  • Electron default-model failure/recovery e2e: 1 passed
  • Standards and Spec adversarial review: no findings
  • GitHub CI: passed for exact head 35efcd6d9, including Desktop e2e, browser smoke, Storybook, and installed CLI validation

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex implemented and tested the lifecycle, storage boundary, protocol, Desktop integration, review fixes, and Standards/Spec reviews.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally or in clean CI

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Implement Slice 2 of apache#3492 on the existing Session substrate.
Generated-by: Codex
@ARE404
ARE404 marked this pull request as ready for review August 25, 2026 05:37

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this head and found blocking issues.

[P2] Default-Host switch while not ready leaves old generation active

Switching default Host to a reconnecting/unavailable Host does not increment the generation, so late results from the previous Host still pass checks and WorkHub continues using the old Host until the new one becomes ready — which may never happen.

[P2] Ordinary workspace relocation can permanently break Coordination identity

session.workspace.relocate can change the reserved maka_workhub_coordination cwd/projectId, after which the next resolve permanently fails with operation_conflict and the row is hidden with no repair path.

[P3] Write-only ref duplication

workHubCoordinationSessionIdRef is never read; state is the sole authority.

Checks on becd9f133e are test: success.

简体中文存在两项身份/代次阻断与一项冗余。

@ARE404

Copy link
Copy Markdown
ContributorAuthor

Addressed the review findings in 35efcd6d9:

  • every default-Host change now revokes the old generation immediately, including connecting, reconnecting, and unavailable targets; stale resolves and stale controllers cannot mutate
  • ordinary workspace/configuration/metadata/read-state/retirement authority rejects either half of the reserved Coordination identity/role pair, including corrupt persisted pairings
  • workHubCoordinationSessionIdRef now participates in the current-scope guard alongside the generation
  • also closed ordinary session.create, branch/revision copy, and generic root-execution paths over the reserved identity; missing-model startup is visible and retryable

Exact-head CI is green, and the final Standards/Spec adversarial re-review has no findings.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on 35efcd6d9b:

No blocking issues. The prior generation/identity P2s are now closed: default-Host changes correctly invalidate the old generation, and the Coordination identity is protected across copy paths.

[P3] Copy reservation only protects the target, not the source

session-revision-coordinator.ts:179 checks only targetSessionId. A valid Coordination Session supplied as sourceSessionId can still be copied into an ordinary Session, exposing hidden messages. This is latent until Slice 3 writes transcripts, but should be rejected before admission.

Checks on 35efcd6d9b are test: success.

简体中文该头无阻断,剩余 P3 为源侧拷贝保护。

@ARE404

Copy link
Copy Markdown
ContributorAuthor

Self-review pass over this slice, graded by severity. Fixes for all seven follow in the next commit.

P1 — correctness, must fix before merge

1. validCoordinationHeader treats cwd as part of the identity, and nothing can repair the mismatch.
workhub-coordination-coordinator.ts requires header.cwd === join(stateRoot, 'workhub-coordination'). The create fingerprint is a constant, so after the state root moves to a different absolute path — restoring the state directory on a new machine, a renamed home directory, a remote Host whose home path changed — probeStableSessionCreate still answers existing, the cwd comparison fails, and #resolve returns identityConflict() forever. This slice deliberately blocks session.remove, session.lifecycle.set, session.workspace.relocate, session.configuration.update and session.metadata.update for the reserved identity, so there is no product path left to repair or drop the stale row: WorkHub becomes permanently unopenable and the surface tells the user to check their default model, which is not the problem. cwd is repairable state, not identity — resolve should relocate it instead of rejecting.

2. The copy reservation covers the target but not the source.
session-revision-coordinator.ts rejects the reserved identity only as input.targetSessionId. The Coordination Session is still a legal source for session.conversation.copy (ordinary copy, revision, and side conversation): the source header is only checked for archived / subagentParent / Deep Research. #createInput does not carry role, so the identity/role pairing check in createStableSession passes and the copy lands as an ordinary Session holding the Coordination transcript — with an ordinary id and no role, so runtimeHostExecutionUnavailableReason also lets it run Turns. Any owner client can reach this: workhub.coordination.resolve hands out the id, and the copy does the rest.

P2 — should fix in this PR

3. The Coordination workspace directory is only guaranteed at provisioning time.
mkdir runs on the create branch only. If stateRoot/workhub-coordination is later removed — a pruned state directory, a database restored without its sibling directory — the existing branch compares the cwd string, reports success, and the directory never comes back. mkdir with recursive is idempotent; running it before the probe makes both branches converge on the same guarantee.

4. A default Host that is not ready leaves WorkHub on an indefinite spinner.
workhub-coordination-lifecycle.ts always revokes on a default-Host change but only re-resolves when readiness === 'ready', and revoke() clears failedGeneration. When the default Host goes unavailable (crash, failed respawn, onTargetRemoved), reportFailure never runs, so the surface stays on Preparing WorkHub… with no error and no Retry, and the connection_list_changed handler cannot restart it either because it is gated on failedGeneration === generation. Recovery waits for a ready event that may never arrive. connecting / reconnecting should keep the spinner; unavailable should report a failure.

P3 — cleanups

5. Every resolveCreateTarget() failure is reported as a missing default model.
The bare catch maps any error to operation_conflict / “requires an available default model”. resolveExternalSessionImportTarget also awaits the runtime policy read, which raises persistence_failed / “Runtime policy is unavailable”; the user is then told to fix a model setting that was never wrong, and the real persistence failure is never surfaced.

6. #updateMetadata reads the header snapshot twice.
The new snapshot read and Coordination guard duplicate the read and guard #replaceUserLabels already performs inside the same admission lease, on the same sessionId. A label update now costs two identical reads, and rename / flag updates pay a new one on a hot path (auto-titling renames land on most Turns). Threading the record through removes both.

7. Unused import.
registerRuntimeHostSessionCatalogIpc is imported in runtime-host-session-catalog-ipc-main.test.ts and never referenced — it is the file's only change. Biome's allowlist has no unused-import rule, so nothing caught it.

…on both sides
Review follow-ups on this slice.
The Coordination workspace is derived from the Host state root, so treating
its path as part of the identity stranded the one Session that no ordinary
lifecycle operation may relocate or retire: a state directory restored under
a different absolute path answered `operation_conflict` on every resolve,
with no product path left to repair it. Identity is now the id/role pair
alone, the durable path is relocated in place, and the workspace directory is
ensured on reuse as well as on provisioning.
The copy reservation covered the target identity but not the source, so the
Coordination transcript could be lifted into an ordinary — and therefore
executable — Session. Both halves of the reserved pair are refused as a copy
source now.
An unavailable default Host revoked WorkHub resolution without reporting a
failure, leaving the surface on a loading state no retry could reopen, since
the availability subscription only reopens a generation that failed.
Connecting and reconnecting still hold that state; unavailable no longer does.
Also: keep a failed model-authority read distinguishable from a missing
default model, drop a duplicate header read inside one admission lease, and
remove an unused test import.
@ARE404

Copy link
Copy Markdown
ContributorAuthor

Pushed cd4b60ed8, which addresses all seven.

P1-1 — Coordination workspace no longer acts as identity.validCoordinationHeader checks the id/role pair, lineage and archive state only. The durable cwd is relocated in place under the admission lease with the probed revision, so a state directory restored under a different absolute path keeps its Session instead of answering operation_conflict forever. New regression: relocates the durable workspace when the Host state root moves.

P1-2 — the copy reservation now holds on both sides.#copy refuses the reserved id as a source, and #copyAdmitted refuses either half of the reserved pair once the source header is known. The two-client UDS regression now asserts both directions for session.branch.create and session.revision.create, and that the would-be target never appears in the catalog.

P2-3 — the workspace directory is ensured on reuse.mkdir moved above the probe, so both branches converge on the same guarantee and a pruned directory comes back before the identity is handed out. This also simplified the create branch: everything after the probe is now a single outcome-unknown boundary. New regression: restores a Coordination workspace that was pruned after provisioning.

P2-4 — an unavailable default Host reports a failure.connecting and reconnecting still hold the loading state; unavailable (and removed) now reports through the same path a rejected resolve takes, so the surface shows the error with Retry and the availability subscription can reopen the generation. New regression: WorkHub reports an unavailable default Host instead of holding the loading state.

P3-5 — a persistence_failed model-authority read keeps its code and message; everything else still maps to the default-model conflict. Covered by separates an unreadable model authority from a missing default model.

P3-6#updateMetadata reads the admitted snapshot once and threads it into label replacement; #replaceUserLabels became the pure replaceUserLabels(current, …).

P3-7 — unused import removed.

Local: npm run typecheck, npm run format:check, npm run lint, git diff --check all clean. Runtime Host 1,159 passed, Desktop 1,487 passed.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on cd4b60ed86:

No blocking issues. The source-side copy guard now correctly rejects reserved source Sessions, and workspace/generation handling is sound.

Checks on cd4b60ed86 — code GO, no P0-P3.

简体中文该头未发现阻断。

Depth for the A-seat boundary review. Neither item was reachable; both were
places where the closure depended on a caller remembering something.
The id/role pairing was asserted in createStableSession() only, so the
creators that share buildSessionHeader() — createSubagent() and
createAgentGraphOperator() — did not inherit the invariant even though the
header they build carries the same field. It moves into the builder, where
every creator meets it. createStableSession() keeps its own assertion so a
malformed request is still refused before a durable create claim is written.
The metadata list role scope defaulted to the widest value, which made
closure a property of each caller remembering to narrow it. The default is
gone: list() now takes both the filter and the scope explicitly, and usage
statistics — the one reader that took the default — states 'all', which is
what a durable cost aggregate wants.
@ARE404

Copy link
Copy Markdown
ContributorAuthor

Both P3 depth items are in 9966f7ae7.

1 — the id/role pairing moved into the header builder.assertCoordinationIdentityPairing(sessionId, role) now runs inside buildSessionHeader(), so createSubagent() and createAgentGraphOperator() meet the same invariant as createStableSession() rather than depending on their inputs never carrying a role.

createStableSession() keeps its own call to the assertion, deliberately: it runs before claimStableSessionCreate(), and the builder runs after. Dropping the early one would let a malformed request write a durable create claim for the identity it names, which a later legitimate create would then see as a conflict. Same fact, asserted at the boundary that must fail before a write and at the low-level constructor that every creator shares.

Regression extended: requires the reserved WorkHub Coordination identity and role together now rejects a role claimed through both sibling creators and asserts the store stays empty afterwards.

2 — the metadata role scope has no default.SqliteSessionMetadataStore.list() takes (filter: SessionListFilter | undefined, roleScope: SessionMetadataRoleScope), both required, and the type is exported so a call site names its scope. The three readers now read as: catalog 'ordinary', recovery 'recoverable', usage statistics 'all' — the last stated explicitly with a note that a durable cost aggregate counts every Session that spent tokens, including rows a catalog hides.

Local: npm run typecheck, npm run format:check, npm run lint, git diff --check clean. Storage 940 passed / 14 skipped, Runtime Host 1,159 passed, Desktop 1,487 passed.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on 9966f7ae7b:

No blocking issues. The delta makes Coordination identity/role pairing pre-claim and shared across Session creators, correct for preventing ordinary copies from capturing the reserved Session.

Checks on 9966f7ae7b are test: success.

简体中文该头未发现阻断。

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE — 9966f7a, no P0-P3, identity pairing correctly pre-claimed.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ARE404@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(workhub): add coordination session lifecycle - #3764

Merged
Astro-Han merged 4 commits into
apache:mainfrom
ARE404:codex/workhub-coordination-session-lifecycle
Aug 25, 2026
Merged

feat(workhub): add coordination session lifecycle#3764
Astro-Han merged 4 commits into
apache:mainfrom
ARE404:codex/workhub-coordination-session-lifecycle

Conversation

@ARE404

@ARE404ARE404 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • provision and reuse one stable Coordination Session per Runtime Host through the existing Session/SQLite authority
  • revoke an old Host scope as soon as the default Host changes, and retry recoverable provisioning when model authority becomes available
  • reserve the Coordination identity/role pair from ordinary creation, copies, configuration, retirement, catalogs, and root execution

Refs #3492

Authority and deferred work

Session remains the only durable substrate: this adds no second WorkHub database, event store, lifecycle authority, or transcript copy. Each Runtime Host remains independent; cross-Host coordination is not supported.

Coordination transcript rendering and answer_here belong to Slice 3. Model dispositions, the deterministic Action Gate, bounded delegation links, and the deferred Work cardinality decision remain for later slices.

UI evidence

Missing default modelRecovered after model configuration
WorkHub shows a recoverable unavailable stateWorkHub recovers without changing surfaces

Verification

  • npm run lint
  • npm run format:check
  • git diff --check
  • npm run typecheck
  • npm run build
  • Storage focused regressions: 29 passed
  • Runtime Host: 1,156 passed
  • Runtime Host two-client branch/revision regression: 1 passed
  • Desktop: 1,506 passed
  • Electron default-model failure/recovery e2e: 1 passed
  • Standards and Spec adversarial review: no findings
  • GitHub CI: passed for exact head 35efcd6d9, including Desktop e2e, browser smoke, Storybook, and installed CLI validation

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex implemented and tested the lifecycle, storage boundary, protocol, Desktop integration, review fixes, and Standards/Spec reviews.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally or in clean CI

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Implement Slice 2 of apache#3492 on the existing Session substrate.
Generated-by: Codex
@ARE404
ARE404 marked this pull request as ready for review August 25, 2026 05:37

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this head and found blocking issues.

[P2] Default-Host switch while not ready leaves old generation active

Switching default Host to a reconnecting/unavailable Host does not increment the generation, so late results from the previous Host still pass checks and WorkHub continues using the old Host until the new one becomes ready — which may never happen.

[P2] Ordinary workspace relocation can permanently break Coordination identity

session.workspace.relocate can change the reserved maka_workhub_coordination cwd/projectId, after which the next resolve permanently fails with operation_conflict and the row is hidden with no repair path.

[P3] Write-only ref duplication

workHubCoordinationSessionIdRef is never read; state is the sole authority.

Checks on becd9f133e are test: success.

简体中文存在两项身份/代次阻断与一项冗余。

@ARE404

Copy link
Copy Markdown
ContributorAuthor

Addressed the review findings in 35efcd6d9:

  • every default-Host change now revokes the old generation immediately, including connecting, reconnecting, and unavailable targets; stale resolves and stale controllers cannot mutate
  • ordinary workspace/configuration/metadata/read-state/retirement authority rejects either half of the reserved Coordination identity/role pair, including corrupt persisted pairings
  • workHubCoordinationSessionIdRef now participates in the current-scope guard alongside the generation
  • also closed ordinary session.create, branch/revision copy, and generic root-execution paths over the reserved identity; missing-model startup is visible and retryable

Exact-head CI is green, and the final Standards/Spec adversarial re-review has no findings.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on 35efcd6d9b:

No blocking issues. The prior generation/identity P2s are now closed: default-Host changes correctly invalidate the old generation, and the Coordination identity is protected across copy paths.

[P3] Copy reservation only protects the target, not the source

session-revision-coordinator.ts:179 checks only targetSessionId. A valid Coordination Session supplied as sourceSessionId can still be copied into an ordinary Session, exposing hidden messages. This is latent until Slice 3 writes transcripts, but should be rejected before admission.

Checks on 35efcd6d9b are test: success.

简体中文该头无阻断,剩余 P3 为源侧拷贝保护。

@ARE404

Copy link
Copy Markdown
ContributorAuthor

Self-review pass over this slice, graded by severity. Fixes for all seven follow in the next commit.

P1 — correctness, must fix before merge

1. validCoordinationHeader treats cwd as part of the identity, and nothing can repair the mismatch.
workhub-coordination-coordinator.ts requires header.cwd === join(stateRoot, 'workhub-coordination'). The create fingerprint is a constant, so after the state root moves to a different absolute path — restoring the state directory on a new machine, a renamed home directory, a remote Host whose home path changed — probeStableSessionCreate still answers existing, the cwd comparison fails, and #resolve returns identityConflict() forever. This slice deliberately blocks session.remove, session.lifecycle.set, session.workspace.relocate, session.configuration.update and session.metadata.update for the reserved identity, so there is no product path left to repair or drop the stale row: WorkHub becomes permanently unopenable and the surface tells the user to check their default model, which is not the problem. cwd is repairable state, not identity — resolve should relocate it instead of rejecting.

2. The copy reservation covers the target but not the source.
session-revision-coordinator.ts rejects the reserved identity only as input.targetSessionId. The Coordination Session is still a legal source for session.conversation.copy (ordinary copy, revision, and side conversation): the source header is only checked for archived / subagentParent / Deep Research. #createInput does not carry role, so the identity/role pairing check in createStableSession passes and the copy lands as an ordinary Session holding the Coordination transcript — with an ordinary id and no role, so runtimeHostExecutionUnavailableReason also lets it run Turns. Any owner client can reach this: workhub.coordination.resolve hands out the id, and the copy does the rest.

P2 — should fix in this PR

3. The Coordination workspace directory is only guaranteed at provisioning time.
mkdir runs on the create branch only. If stateRoot/workhub-coordination is later removed — a pruned state directory, a database restored without its sibling directory — the existing branch compares the cwd string, reports success, and the directory never comes back. mkdir with recursive is idempotent; running it before the probe makes both branches converge on the same guarantee.

4. A default Host that is not ready leaves WorkHub on an indefinite spinner.
workhub-coordination-lifecycle.ts always revokes on a default-Host change but only re-resolves when readiness === 'ready', and revoke() clears failedGeneration. When the default Host goes unavailable (crash, failed respawn, onTargetRemoved), reportFailure never runs, so the surface stays on Preparing WorkHub… with no error and no Retry, and the connection_list_changed handler cannot restart it either because it is gated on failedGeneration === generation. Recovery waits for a ready event that may never arrive. connecting / reconnecting should keep the spinner; unavailable should report a failure.

P3 — cleanups

5. Every resolveCreateTarget() failure is reported as a missing default model.
The bare catch maps any error to operation_conflict / “requires an available default model”. resolveExternalSessionImportTarget also awaits the runtime policy read, which raises persistence_failed / “Runtime policy is unavailable”; the user is then told to fix a model setting that was never wrong, and the real persistence failure is never surfaced.

6. #updateMetadata reads the header snapshot twice.
The new snapshot read and Coordination guard duplicate the read and guard #replaceUserLabels already performs inside the same admission lease, on the same sessionId. A label update now costs two identical reads, and rename / flag updates pay a new one on a hot path (auto-titling renames land on most Turns). Threading the record through removes both.

7. Unused import.
registerRuntimeHostSessionCatalogIpc is imported in runtime-host-session-catalog-ipc-main.test.ts and never referenced — it is the file's only change. Biome's allowlist has no unused-import rule, so nothing caught it.

…on both sides
Review follow-ups on this slice.
The Coordination workspace is derived from the Host state root, so treating
its path as part of the identity stranded the one Session that no ordinary
lifecycle operation may relocate or retire: a state directory restored under
a different absolute path answered `operation_conflict` on every resolve,
with no product path left to repair it. Identity is now the id/role pair
alone, the durable path is relocated in place, and the workspace directory is
ensured on reuse as well as on provisioning.
The copy reservation covered the target identity but not the source, so the
Coordination transcript could be lifted into an ordinary — and therefore
executable — Session. Both halves of the reserved pair are refused as a copy
source now.
An unavailable default Host revoked WorkHub resolution without reporting a
failure, leaving the surface on a loading state no retry could reopen, since
the availability subscription only reopens a generation that failed.
Connecting and reconnecting still hold that state; unavailable no longer does.
Also: keep a failed model-authority read distinguishable from a missing
default model, drop a duplicate header read inside one admission lease, and
remove an unused test import.
@ARE404

Copy link
Copy Markdown
ContributorAuthor

Pushed cd4b60ed8, which addresses all seven.

P1-1 — Coordination workspace no longer acts as identity.validCoordinationHeader checks the id/role pair, lineage and archive state only. The durable cwd is relocated in place under the admission lease with the probed revision, so a state directory restored under a different absolute path keeps its Session instead of answering operation_conflict forever. New regression: relocates the durable workspace when the Host state root moves.

P1-2 — the copy reservation now holds on both sides.#copy refuses the reserved id as a source, and #copyAdmitted refuses either half of the reserved pair once the source header is known. The two-client UDS regression now asserts both directions for session.branch.create and session.revision.create, and that the would-be target never appears in the catalog.

P2-3 — the workspace directory is ensured on reuse.mkdir moved above the probe, so both branches converge on the same guarantee and a pruned directory comes back before the identity is handed out. This also simplified the create branch: everything after the probe is now a single outcome-unknown boundary. New regression: restores a Coordination workspace that was pruned after provisioning.

P2-4 — an unavailable default Host reports a failure.connecting and reconnecting still hold the loading state; unavailable (and removed) now reports through the same path a rejected resolve takes, so the surface shows the error with Retry and the availability subscription can reopen the generation. New regression: WorkHub reports an unavailable default Host instead of holding the loading state.

P3-5 — a persistence_failed model-authority read keeps its code and message; everything else still maps to the default-model conflict. Covered by separates an unreadable model authority from a missing default model.

P3-6#updateMetadata reads the admitted snapshot once and threads it into label replacement; #replaceUserLabels became the pure replaceUserLabels(current, …).

P3-7 — unused import removed.

Local: npm run typecheck, npm run format:check, npm run lint, git diff --check all clean. Runtime Host 1,159 passed, Desktop 1,487 passed.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on cd4b60ed86:

No blocking issues. The source-side copy guard now correctly rejects reserved source Sessions, and workspace/generation handling is sound.

Checks on cd4b60ed86 — code GO, no P0-P3.

简体中文该头未发现阻断。

Depth for the A-seat boundary review. Neither item was reachable; both were
places where the closure depended on a caller remembering something.
The id/role pairing was asserted in createStableSession() only, so the
creators that share buildSessionHeader() — createSubagent() and
createAgentGraphOperator() — did not inherit the invariant even though the
header they build carries the same field. It moves into the builder, where
every creator meets it. createStableSession() keeps its own assertion so a
malformed request is still refused before a durable create claim is written.
The metadata list role scope defaulted to the widest value, which made
closure a property of each caller remembering to narrow it. The default is
gone: list() now takes both the filter and the scope explicitly, and usage
statistics — the one reader that took the default — states 'all', which is
what a durable cost aggregate wants.
@ARE404

Copy link
Copy Markdown
ContributorAuthor

Both P3 depth items are in 9966f7ae7.

1 — the id/role pairing moved into the header builder.assertCoordinationIdentityPairing(sessionId, role) now runs inside buildSessionHeader(), so createSubagent() and createAgentGraphOperator() meet the same invariant as createStableSession() rather than depending on their inputs never carrying a role.

createStableSession() keeps its own call to the assertion, deliberately: it runs before claimStableSessionCreate(), and the builder runs after. Dropping the early one would let a malformed request write a durable create claim for the identity it names, which a later legitimate create would then see as a conflict. Same fact, asserted at the boundary that must fail before a write and at the low-level constructor that every creator shares.

Regression extended: requires the reserved WorkHub Coordination identity and role together now rejects a role claimed through both sibling creators and asserts the store stays empty afterwards.

2 — the metadata role scope has no default.SqliteSessionMetadataStore.list() takes (filter: SessionListFilter | undefined, roleScope: SessionMetadataRoleScope), both required, and the type is exported so a call site names its scope. The three readers now read as: catalog 'ordinary', recovery 'recoverable', usage statistics 'all' — the last stated explicitly with a note that a durable cost aggregate counts every Session that spent tokens, including rows a catalog hides.

Local: npm run typecheck, npm run format:check, npm run lint, git diff --check clean. Storage 940 passed / 14 skipped, Runtime Host 1,159 passed, Desktop 1,487 passed.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on 9966f7ae7b:

No blocking issues. The delta makes Coordination identity/role pairing pre-claim and shared across Session creators, correct for preventing ordinary copies from capturing the reserved Session.

Checks on 9966f7ae7b are test: success.

简体中文该头未发现阻断。

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE — 9966f7a, no P0-P3, identity pairing correctly pre-claimed.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ARE404@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

feat(workhub): add coordination session lifecycle - #3764

Merged
Astro-Han merged 4 commits into
apache:mainfrom
ARE404:codex/workhub-coordination-session-lifecycle
Aug 25, 2026
Merged

feat(workhub): add coordination session lifecycle#3764
Astro-Han merged 4 commits into
apache:mainfrom
ARE404:codex/workhub-coordination-session-lifecycle

Conversation

@ARE404

@ARE404ARE404 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • provision and reuse one stable Coordination Session per Runtime Host through the existing Session/SQLite authority
  • revoke an old Host scope as soon as the default Host changes, and retry recoverable provisioning when model authority becomes available
  • reserve the Coordination identity/role pair from ordinary creation, copies, configuration, retirement, catalogs, and root execution

Refs #3492

Authority and deferred work

Session remains the only durable substrate: this adds no second WorkHub database, event store, lifecycle authority, or transcript copy. Each Runtime Host remains independent; cross-Host coordination is not supported.

Coordination transcript rendering and answer_here belong to Slice 3. Model dispositions, the deterministic Action Gate, bounded delegation links, and the deferred Work cardinality decision remain for later slices.

UI evidence

Missing default modelRecovered after model configuration
WorkHub shows a recoverable unavailable stateWorkHub recovers without changing surfaces

Verification

  • npm run lint
  • npm run format:check
  • git diff --check
  • npm run typecheck
  • npm run build
  • Storage focused regressions: 29 passed
  • Runtime Host: 1,156 passed
  • Runtime Host two-client branch/revision regression: 1 passed
  • Desktop: 1,506 passed
  • Electron default-model failure/recovery e2e: 1 passed
  • Standards and Spec adversarial review: no findings
  • GitHub CI: passed for exact head 35efcd6d9, including Desktop e2e, browser smoke, Storybook, and installed CLI validation

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex implemented and tested the lifecycle, storage boundary, protocol, Desktop integration, review fixes, and Standards/Spec reviews.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally or in clean CI

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Implement Slice 2 of apache#3492 on the existing Session substrate.
Generated-by: Codex
@ARE404
ARE404 marked this pull request as ready for review August 25, 2026 05:37

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this head and found blocking issues.

[P2] Default-Host switch while not ready leaves old generation active

Switching default Host to a reconnecting/unavailable Host does not increment the generation, so late results from the previous Host still pass checks and WorkHub continues using the old Host until the new one becomes ready — which may never happen.

[P2] Ordinary workspace relocation can permanently break Coordination identity

session.workspace.relocate can change the reserved maka_workhub_coordination cwd/projectId, after which the next resolve permanently fails with operation_conflict and the row is hidden with no repair path.

[P3] Write-only ref duplication

workHubCoordinationSessionIdRef is never read; state is the sole authority.

Checks on becd9f133e are test: success.

简体中文存在两项身份/代次阻断与一项冗余。

@ARE404

Copy link
Copy Markdown
ContributorAuthor

Addressed the review findings in 35efcd6d9:

  • every default-Host change now revokes the old generation immediately, including connecting, reconnecting, and unavailable targets; stale resolves and stale controllers cannot mutate
  • ordinary workspace/configuration/metadata/read-state/retirement authority rejects either half of the reserved Coordination identity/role pair, including corrupt persisted pairings
  • workHubCoordinationSessionIdRef now participates in the current-scope guard alongside the generation
  • also closed ordinary session.create, branch/revision copy, and generic root-execution paths over the reserved identity; missing-model startup is visible and retryable

Exact-head CI is green, and the final Standards/Spec adversarial re-review has no findings.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on 35efcd6d9b:

No blocking issues. The prior generation/identity P2s are now closed: default-Host changes correctly invalidate the old generation, and the Coordination identity is protected across copy paths.

[P3] Copy reservation only protects the target, not the source

session-revision-coordinator.ts:179 checks only targetSessionId. A valid Coordination Session supplied as sourceSessionId can still be copied into an ordinary Session, exposing hidden messages. This is latent until Slice 3 writes transcripts, but should be rejected before admission.

Checks on 35efcd6d9b are test: success.

简体中文该头无阻断,剩余 P3 为源侧拷贝保护。

@ARE404

Copy link
Copy Markdown
ContributorAuthor

Self-review pass over this slice, graded by severity. Fixes for all seven follow in the next commit.

P1 — correctness, must fix before merge

1. validCoordinationHeader treats cwd as part of the identity, and nothing can repair the mismatch.
workhub-coordination-coordinator.ts requires header.cwd === join(stateRoot, 'workhub-coordination'). The create fingerprint is a constant, so after the state root moves to a different absolute path — restoring the state directory on a new machine, a renamed home directory, a remote Host whose home path changed — probeStableSessionCreate still answers existing, the cwd comparison fails, and #resolve returns identityConflict() forever. This slice deliberately blocks session.remove, session.lifecycle.set, session.workspace.relocate, session.configuration.update and session.metadata.update for the reserved identity, so there is no product path left to repair or drop the stale row: WorkHub becomes permanently unopenable and the surface tells the user to check their default model, which is not the problem. cwd is repairable state, not identity — resolve should relocate it instead of rejecting.

2. The copy reservation covers the target but not the source.
session-revision-coordinator.ts rejects the reserved identity only as input.targetSessionId. The Coordination Session is still a legal source for session.conversation.copy (ordinary copy, revision, and side conversation): the source header is only checked for archived / subagentParent / Deep Research. #createInput does not carry role, so the identity/role pairing check in createStableSession passes and the copy lands as an ordinary Session holding the Coordination transcript — with an ordinary id and no role, so runtimeHostExecutionUnavailableReason also lets it run Turns. Any owner client can reach this: workhub.coordination.resolve hands out the id, and the copy does the rest.

P2 — should fix in this PR

3. The Coordination workspace directory is only guaranteed at provisioning time.
mkdir runs on the create branch only. If stateRoot/workhub-coordination is later removed — a pruned state directory, a database restored without its sibling directory — the existing branch compares the cwd string, reports success, and the directory never comes back. mkdir with recursive is idempotent; running it before the probe makes both branches converge on the same guarantee.

4. A default Host that is not ready leaves WorkHub on an indefinite spinner.
workhub-coordination-lifecycle.ts always revokes on a default-Host change but only re-resolves when readiness === 'ready', and revoke() clears failedGeneration. When the default Host goes unavailable (crash, failed respawn, onTargetRemoved), reportFailure never runs, so the surface stays on Preparing WorkHub… with no error and no Retry, and the connection_list_changed handler cannot restart it either because it is gated on failedGeneration === generation. Recovery waits for a ready event that may never arrive. connecting / reconnecting should keep the spinner; unavailable should report a failure.

P3 — cleanups

5. Every resolveCreateTarget() failure is reported as a missing default model.
The bare catch maps any error to operation_conflict / “requires an available default model”. resolveExternalSessionImportTarget also awaits the runtime policy read, which raises persistence_failed / “Runtime policy is unavailable”; the user is then told to fix a model setting that was never wrong, and the real persistence failure is never surfaced.

6. #updateMetadata reads the header snapshot twice.
The new snapshot read and Coordination guard duplicate the read and guard #replaceUserLabels already performs inside the same admission lease, on the same sessionId. A label update now costs two identical reads, and rename / flag updates pay a new one on a hot path (auto-titling renames land on most Turns). Threading the record through removes both.

7. Unused import.
registerRuntimeHostSessionCatalogIpc is imported in runtime-host-session-catalog-ipc-main.test.ts and never referenced — it is the file's only change. Biome's allowlist has no unused-import rule, so nothing caught it.

…on both sides
Review follow-ups on this slice.
The Coordination workspace is derived from the Host state root, so treating
its path as part of the identity stranded the one Session that no ordinary
lifecycle operation may relocate or retire: a state directory restored under
a different absolute path answered `operation_conflict` on every resolve,
with no product path left to repair it. Identity is now the id/role pair
alone, the durable path is relocated in place, and the workspace directory is
ensured on reuse as well as on provisioning.
The copy reservation covered the target identity but not the source, so the
Coordination transcript could be lifted into an ordinary — and therefore
executable — Session. Both halves of the reserved pair are refused as a copy
source now.
An unavailable default Host revoked WorkHub resolution without reporting a
failure, leaving the surface on a loading state no retry could reopen, since
the availability subscription only reopens a generation that failed.
Connecting and reconnecting still hold that state; unavailable no longer does.
Also: keep a failed model-authority read distinguishable from a missing
default model, drop a duplicate header read inside one admission lease, and
remove an unused test import.
@ARE404

Copy link
Copy Markdown
ContributorAuthor

Pushed cd4b60ed8, which addresses all seven.

P1-1 — Coordination workspace no longer acts as identity.validCoordinationHeader checks the id/role pair, lineage and archive state only. The durable cwd is relocated in place under the admission lease with the probed revision, so a state directory restored under a different absolute path keeps its Session instead of answering operation_conflict forever. New regression: relocates the durable workspace when the Host state root moves.

P1-2 — the copy reservation now holds on both sides.#copy refuses the reserved id as a source, and #copyAdmitted refuses either half of the reserved pair once the source header is known. The two-client UDS regression now asserts both directions for session.branch.create and session.revision.create, and that the would-be target never appears in the catalog.

P2-3 — the workspace directory is ensured on reuse.mkdir moved above the probe, so both branches converge on the same guarantee and a pruned directory comes back before the identity is handed out. This also simplified the create branch: everything after the probe is now a single outcome-unknown boundary. New regression: restores a Coordination workspace that was pruned after provisioning.

P2-4 — an unavailable default Host reports a failure.connecting and reconnecting still hold the loading state; unavailable (and removed) now reports through the same path a rejected resolve takes, so the surface shows the error with Retry and the availability subscription can reopen the generation. New regression: WorkHub reports an unavailable default Host instead of holding the loading state.

P3-5 — a persistence_failed model-authority read keeps its code and message; everything else still maps to the default-model conflict. Covered by separates an unreadable model authority from a missing default model.

P3-6#updateMetadata reads the admitted snapshot once and threads it into label replacement; #replaceUserLabels became the pure replaceUserLabels(current, …).

P3-7 — unused import removed.

Local: npm run typecheck, npm run format:check, npm run lint, git diff --check all clean. Runtime Host 1,159 passed, Desktop 1,487 passed.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on cd4b60ed86:

No blocking issues. The source-side copy guard now correctly rejects reserved source Sessions, and workspace/generation handling is sound.

Checks on cd4b60ed86 — code GO, no P0-P3.

简体中文该头未发现阻断。

Depth for the A-seat boundary review. Neither item was reachable; both were
places where the closure depended on a caller remembering something.
The id/role pairing was asserted in createStableSession() only, so the
creators that share buildSessionHeader() — createSubagent() and
createAgentGraphOperator() — did not inherit the invariant even though the
header they build carries the same field. It moves into the builder, where
every creator meets it. createStableSession() keeps its own assertion so a
malformed request is still refused before a durable create claim is written.
The metadata list role scope defaulted to the widest value, which made
closure a property of each caller remembering to narrow it. The default is
gone: list() now takes both the filter and the scope explicitly, and usage
statistics — the one reader that took the default — states 'all', which is
what a durable cost aggregate wants.
@ARE404

Copy link
Copy Markdown
ContributorAuthor

Both P3 depth items are in 9966f7ae7.

1 — the id/role pairing moved into the header builder.assertCoordinationIdentityPairing(sessionId, role) now runs inside buildSessionHeader(), so createSubagent() and createAgentGraphOperator() meet the same invariant as createStableSession() rather than depending on their inputs never carrying a role.

createStableSession() keeps its own call to the assertion, deliberately: it runs before claimStableSessionCreate(), and the builder runs after. Dropping the early one would let a malformed request write a durable create claim for the identity it names, which a later legitimate create would then see as a conflict. Same fact, asserted at the boundary that must fail before a write and at the low-level constructor that every creator shares.

Regression extended: requires the reserved WorkHub Coordination identity and role together now rejects a role claimed through both sibling creators and asserts the store stays empty afterwards.

2 — the metadata role scope has no default.SqliteSessionMetadataStore.list() takes (filter: SessionListFilter | undefined, roleScope: SessionMetadataRoleScope), both required, and the type is exported so a call site names its scope. The three readers now read as: catalog 'ordinary', recovery 'recoverable', usage statistics 'all' — the last stated explicitly with a note that a durable cost aggregate counts every Session that spent tokens, including rows a catalog hides.

Local: npm run typecheck, npm run format:check, npm run lint, git diff --check clean. Storage 940 passed / 14 skipped, Runtime Host 1,159 passed, Desktop 1,487 passed.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on 9966f7ae7b:

No blocking issues. The delta makes Coordination identity/role pairing pre-claim and shared across Session creators, correct for preventing ordinary copies from capturing the reserved Session.

Checks on 9966f7ae7b are test: success.

简体中文该头未发现阻断。

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE — 9966f7a, no P0-P3, identity pairing correctly pre-claimed.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ARE404@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(workhub): add coordination session lifecycle - #3764

Merged
Astro-Han merged 4 commits into
apache:mainfrom
ARE404:codex/workhub-coordination-session-lifecycle
Aug 25, 2026
Merged

feat(workhub): add coordination session lifecycle#3764
Astro-Han merged 4 commits into
apache:mainfrom
ARE404:codex/workhub-coordination-session-lifecycle

Conversation

@ARE404

@ARE404ARE404 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • provision and reuse one stable Coordination Session per Runtime Host through the existing Session/SQLite authority
  • revoke an old Host scope as soon as the default Host changes, and retry recoverable provisioning when model authority becomes available
  • reserve the Coordination identity/role pair from ordinary creation, copies, configuration, retirement, catalogs, and root execution

Refs #3492

Authority and deferred work

Session remains the only durable substrate: this adds no second WorkHub database, event store, lifecycle authority, or transcript copy. Each Runtime Host remains independent; cross-Host coordination is not supported.

Coordination transcript rendering and answer_here belong to Slice 3. Model dispositions, the deterministic Action Gate, bounded delegation links, and the deferred Work cardinality decision remain for later slices.

UI evidence

Missing default modelRecovered after model configuration
WorkHub shows a recoverable unavailable stateWorkHub recovers without changing surfaces

Verification

  • npm run lint
  • npm run format:check
  • git diff --check
  • npm run typecheck
  • npm run build
  • Storage focused regressions: 29 passed
  • Runtime Host: 1,156 passed
  • Runtime Host two-client branch/revision regression: 1 passed
  • Desktop: 1,506 passed
  • Electron default-model failure/recovery e2e: 1 passed
  • Standards and Spec adversarial review: no findings
  • GitHub CI: passed for exact head 35efcd6d9, including Desktop e2e, browser smoke, Storybook, and installed CLI validation

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex implemented and tested the lifecycle, storage boundary, protocol, Desktop integration, review fixes, and Standards/Spec reviews.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally or in clean CI

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Implement Slice 2 of apache#3492 on the existing Session substrate.
Generated-by: Codex
@ARE404
ARE404 marked this pull request as ready for review August 25, 2026 05:37

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this head and found blocking issues.

[P2] Default-Host switch while not ready leaves old generation active

Switching default Host to a reconnecting/unavailable Host does not increment the generation, so late results from the previous Host still pass checks and WorkHub continues using the old Host until the new one becomes ready — which may never happen.

[P2] Ordinary workspace relocation can permanently break Coordination identity

session.workspace.relocate can change the reserved maka_workhub_coordination cwd/projectId, after which the next resolve permanently fails with operation_conflict and the row is hidden with no repair path.

[P3] Write-only ref duplication

workHubCoordinationSessionIdRef is never read; state is the sole authority.

Checks on becd9f133e are test: success.

简体中文存在两项身份/代次阻断与一项冗余。

@ARE404

Copy link
Copy Markdown
ContributorAuthor

Addressed the review findings in 35efcd6d9:

  • every default-Host change now revokes the old generation immediately, including connecting, reconnecting, and unavailable targets; stale resolves and stale controllers cannot mutate
  • ordinary workspace/configuration/metadata/read-state/retirement authority rejects either half of the reserved Coordination identity/role pair, including corrupt persisted pairings
  • workHubCoordinationSessionIdRef now participates in the current-scope guard alongside the generation
  • also closed ordinary session.create, branch/revision copy, and generic root-execution paths over the reserved identity; missing-model startup is visible and retryable

Exact-head CI is green, and the final Standards/Spec adversarial re-review has no findings.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on 35efcd6d9b:

No blocking issues. The prior generation/identity P2s are now closed: default-Host changes correctly invalidate the old generation, and the Coordination identity is protected across copy paths.

[P3] Copy reservation only protects the target, not the source

session-revision-coordinator.ts:179 checks only targetSessionId. A valid Coordination Session supplied as sourceSessionId can still be copied into an ordinary Session, exposing hidden messages. This is latent until Slice 3 writes transcripts, but should be rejected before admission.

Checks on 35efcd6d9b are test: success.

简体中文该头无阻断,剩余 P3 为源侧拷贝保护。

@ARE404

Copy link
Copy Markdown
ContributorAuthor

Self-review pass over this slice, graded by severity. Fixes for all seven follow in the next commit.

P1 — correctness, must fix before merge

1. validCoordinationHeader treats cwd as part of the identity, and nothing can repair the mismatch.
workhub-coordination-coordinator.ts requires header.cwd === join(stateRoot, 'workhub-coordination'). The create fingerprint is a constant, so after the state root moves to a different absolute path — restoring the state directory on a new machine, a renamed home directory, a remote Host whose home path changed — probeStableSessionCreate still answers existing, the cwd comparison fails, and #resolve returns identityConflict() forever. This slice deliberately blocks session.remove, session.lifecycle.set, session.workspace.relocate, session.configuration.update and session.metadata.update for the reserved identity, so there is no product path left to repair or drop the stale row: WorkHub becomes permanently unopenable and the surface tells the user to check their default model, which is not the problem. cwd is repairable state, not identity — resolve should relocate it instead of rejecting.

2. The copy reservation covers the target but not the source.
session-revision-coordinator.ts rejects the reserved identity only as input.targetSessionId. The Coordination Session is still a legal source for session.conversation.copy (ordinary copy, revision, and side conversation): the source header is only checked for archived / subagentParent / Deep Research. #createInput does not carry role, so the identity/role pairing check in createStableSession passes and the copy lands as an ordinary Session holding the Coordination transcript — with an ordinary id and no role, so runtimeHostExecutionUnavailableReason also lets it run Turns. Any owner client can reach this: workhub.coordination.resolve hands out the id, and the copy does the rest.

P2 — should fix in this PR

3. The Coordination workspace directory is only guaranteed at provisioning time.
mkdir runs on the create branch only. If stateRoot/workhub-coordination is later removed — a pruned state directory, a database restored without its sibling directory — the existing branch compares the cwd string, reports success, and the directory never comes back. mkdir with recursive is idempotent; running it before the probe makes both branches converge on the same guarantee.

4. A default Host that is not ready leaves WorkHub on an indefinite spinner.
workhub-coordination-lifecycle.ts always revokes on a default-Host change but only re-resolves when readiness === 'ready', and revoke() clears failedGeneration. When the default Host goes unavailable (crash, failed respawn, onTargetRemoved), reportFailure never runs, so the surface stays on Preparing WorkHub… with no error and no Retry, and the connection_list_changed handler cannot restart it either because it is gated on failedGeneration === generation. Recovery waits for a ready event that may never arrive. connecting / reconnecting should keep the spinner; unavailable should report a failure.

P3 — cleanups

5. Every resolveCreateTarget() failure is reported as a missing default model.
The bare catch maps any error to operation_conflict / “requires an available default model”. resolveExternalSessionImportTarget also awaits the runtime policy read, which raises persistence_failed / “Runtime policy is unavailable”; the user is then told to fix a model setting that was never wrong, and the real persistence failure is never surfaced.

6. #updateMetadata reads the header snapshot twice.
The new snapshot read and Coordination guard duplicate the read and guard #replaceUserLabels already performs inside the same admission lease, on the same sessionId. A label update now costs two identical reads, and rename / flag updates pay a new one on a hot path (auto-titling renames land on most Turns). Threading the record through removes both.

7. Unused import.
registerRuntimeHostSessionCatalogIpc is imported in runtime-host-session-catalog-ipc-main.test.ts and never referenced — it is the file's only change. Biome's allowlist has no unused-import rule, so nothing caught it.

…on both sides
Review follow-ups on this slice.
The Coordination workspace is derived from the Host state root, so treating
its path as part of the identity stranded the one Session that no ordinary
lifecycle operation may relocate or retire: a state directory restored under
a different absolute path answered `operation_conflict` on every resolve,
with no product path left to repair it. Identity is now the id/role pair
alone, the durable path is relocated in place, and the workspace directory is
ensured on reuse as well as on provisioning.
The copy reservation covered the target identity but not the source, so the
Coordination transcript could be lifted into an ordinary — and therefore
executable — Session. Both halves of the reserved pair are refused as a copy
source now.
An unavailable default Host revoked WorkHub resolution without reporting a
failure, leaving the surface on a loading state no retry could reopen, since
the availability subscription only reopens a generation that failed.
Connecting and reconnecting still hold that state; unavailable no longer does.
Also: keep a failed model-authority read distinguishable from a missing
default model, drop a duplicate header read inside one admission lease, and
remove an unused test import.
@ARE404

Copy link
Copy Markdown
ContributorAuthor

Pushed cd4b60ed8, which addresses all seven.

P1-1 — Coordination workspace no longer acts as identity.validCoordinationHeader checks the id/role pair, lineage and archive state only. The durable cwd is relocated in place under the admission lease with the probed revision, so a state directory restored under a different absolute path keeps its Session instead of answering operation_conflict forever. New regression: relocates the durable workspace when the Host state root moves.

P1-2 — the copy reservation now holds on both sides.#copy refuses the reserved id as a source, and #copyAdmitted refuses either half of the reserved pair once the source header is known. The two-client UDS regression now asserts both directions for session.branch.create and session.revision.create, and that the would-be target never appears in the catalog.

P2-3 — the workspace directory is ensured on reuse.mkdir moved above the probe, so both branches converge on the same guarantee and a pruned directory comes back before the identity is handed out. This also simplified the create branch: everything after the probe is now a single outcome-unknown boundary. New regression: restores a Coordination workspace that was pruned after provisioning.

P2-4 — an unavailable default Host reports a failure.connecting and reconnecting still hold the loading state; unavailable (and removed) now reports through the same path a rejected resolve takes, so the surface shows the error with Retry and the availability subscription can reopen the generation. New regression: WorkHub reports an unavailable default Host instead of holding the loading state.

P3-5 — a persistence_failed model-authority read keeps its code and message; everything else still maps to the default-model conflict. Covered by separates an unreadable model authority from a missing default model.

P3-6#updateMetadata reads the admitted snapshot once and threads it into label replacement; #replaceUserLabels became the pure replaceUserLabels(current, …).

P3-7 — unused import removed.

Local: npm run typecheck, npm run format:check, npm run lint, git diff --check all clean. Runtime Host 1,159 passed, Desktop 1,487 passed.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on cd4b60ed86:

No blocking issues. The source-side copy guard now correctly rejects reserved source Sessions, and workspace/generation handling is sound.

Checks on cd4b60ed86 — code GO, no P0-P3.

简体中文该头未发现阻断。

Depth for the A-seat boundary review. Neither item was reachable; both were
places where the closure depended on a caller remembering something.
The id/role pairing was asserted in createStableSession() only, so the
creators that share buildSessionHeader() — createSubagent() and
createAgentGraphOperator() — did not inherit the invariant even though the
header they build carries the same field. It moves into the builder, where
every creator meets it. createStableSession() keeps its own assertion so a
malformed request is still refused before a durable create claim is written.
The metadata list role scope defaulted to the widest value, which made
closure a property of each caller remembering to narrow it. The default is
gone: list() now takes both the filter and the scope explicitly, and usage
statistics — the one reader that took the default — states 'all', which is
what a durable cost aggregate wants.
@ARE404

Copy link
Copy Markdown
ContributorAuthor

Both P3 depth items are in 9966f7ae7.

1 — the id/role pairing moved into the header builder.assertCoordinationIdentityPairing(sessionId, role) now runs inside buildSessionHeader(), so createSubagent() and createAgentGraphOperator() meet the same invariant as createStableSession() rather than depending on their inputs never carrying a role.

createStableSession() keeps its own call to the assertion, deliberately: it runs before claimStableSessionCreate(), and the builder runs after. Dropping the early one would let a malformed request write a durable create claim for the identity it names, which a later legitimate create would then see as a conflict. Same fact, asserted at the boundary that must fail before a write and at the low-level constructor that every creator shares.

Regression extended: requires the reserved WorkHub Coordination identity and role together now rejects a role claimed through both sibling creators and asserts the store stays empty afterwards.

2 — the metadata role scope has no default.SqliteSessionMetadataStore.list() takes (filter: SessionListFilter | undefined, roleScope: SessionMetadataRoleScope), both required, and the type is exported so a call site names its scope. The three readers now read as: catalog 'ordinary', recovery 'recoverable', usage statistics 'all' — the last stated explicitly with a note that a durable cost aggregate counts every Session that spent tokens, including rows a catalog hides.

Local: npm run typecheck, npm run format:check, npm run lint, git diff --check clean. Storage 940 passed / 14 skipped, Runtime Host 1,159 passed, Desktop 1,487 passed.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on 9966f7ae7b:

No blocking issues. The delta makes Coordination identity/role pairing pre-claim and shared across Session creators, correct for preventing ordinary copies from capturing the reserved Session.

Checks on 9966f7ae7b are test: success.

简体中文该头未发现阻断。

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE — 9966f7a, no P0-P3, identity pairing correctly pre-claimed.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ARE404@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(workhub): add coordination session lifecycle - #3764

Merged
Astro-Han merged 4 commits into
apache:mainfrom
ARE404:codex/workhub-coordination-session-lifecycle
Aug 25, 2026
Merged

feat(workhub): add coordination session lifecycle#3764
Astro-Han merged 4 commits into
apache:mainfrom
ARE404:codex/workhub-coordination-session-lifecycle

Conversation

@ARE404

@ARE404ARE404 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • provision and reuse one stable Coordination Session per Runtime Host through the existing Session/SQLite authority
  • revoke an old Host scope as soon as the default Host changes, and retry recoverable provisioning when model authority becomes available
  • reserve the Coordination identity/role pair from ordinary creation, copies, configuration, retirement, catalogs, and root execution

Refs #3492

Authority and deferred work

Session remains the only durable substrate: this adds no second WorkHub database, event store, lifecycle authority, or transcript copy. Each Runtime Host remains independent; cross-Host coordination is not supported.

Coordination transcript rendering and answer_here belong to Slice 3. Model dispositions, the deterministic Action Gate, bounded delegation links, and the deferred Work cardinality decision remain for later slices.

UI evidence

Missing default modelRecovered after model configuration
WorkHub shows a recoverable unavailable stateWorkHub recovers without changing surfaces

Verification

  • npm run lint
  • npm run format:check
  • git diff --check
  • npm run typecheck
  • npm run build
  • Storage focused regressions: 29 passed
  • Runtime Host: 1,156 passed
  • Runtime Host two-client branch/revision regression: 1 passed
  • Desktop: 1,506 passed
  • Electron default-model failure/recovery e2e: 1 passed
  • Standards and Spec adversarial review: no findings
  • GitHub CI: passed for exact head 35efcd6d9, including Desktop e2e, browser smoke, Storybook, and installed CLI validation

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex implemented and tested the lifecycle, storage boundary, protocol, Desktop integration, review fixes, and Standards/Spec reviews.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally or in clean CI

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Implement Slice 2 of apache#3492 on the existing Session substrate.
Generated-by: Codex
@ARE404
ARE404 marked this pull request as ready for review August 25, 2026 05:37

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this head and found blocking issues.

[P2] Default-Host switch while not ready leaves old generation active

Switching default Host to a reconnecting/unavailable Host does not increment the generation, so late results from the previous Host still pass checks and WorkHub continues using the old Host until the new one becomes ready — which may never happen.

[P2] Ordinary workspace relocation can permanently break Coordination identity

session.workspace.relocate can change the reserved maka_workhub_coordination cwd/projectId, after which the next resolve permanently fails with operation_conflict and the row is hidden with no repair path.

[P3] Write-only ref duplication

workHubCoordinationSessionIdRef is never read; state is the sole authority.

Checks on becd9f133e are test: success.

简体中文存在两项身份/代次阻断与一项冗余。

@ARE404

Copy link
Copy Markdown
ContributorAuthor

Addressed the review findings in 35efcd6d9:

  • every default-Host change now revokes the old generation immediately, including connecting, reconnecting, and unavailable targets; stale resolves and stale controllers cannot mutate
  • ordinary workspace/configuration/metadata/read-state/retirement authority rejects either half of the reserved Coordination identity/role pair, including corrupt persisted pairings
  • workHubCoordinationSessionIdRef now participates in the current-scope guard alongside the generation
  • also closed ordinary session.create, branch/revision copy, and generic root-execution paths over the reserved identity; missing-model startup is visible and retryable

Exact-head CI is green, and the final Standards/Spec adversarial re-review has no findings.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on 35efcd6d9b:

No blocking issues. The prior generation/identity P2s are now closed: default-Host changes correctly invalidate the old generation, and the Coordination identity is protected across copy paths.

[P3] Copy reservation only protects the target, not the source

session-revision-coordinator.ts:179 checks only targetSessionId. A valid Coordination Session supplied as sourceSessionId can still be copied into an ordinary Session, exposing hidden messages. This is latent until Slice 3 writes transcripts, but should be rejected before admission.

Checks on 35efcd6d9b are test: success.

简体中文该头无阻断,剩余 P3 为源侧拷贝保护。

@ARE404

Copy link
Copy Markdown
ContributorAuthor

Self-review pass over this slice, graded by severity. Fixes for all seven follow in the next commit.

P1 — correctness, must fix before merge

1. validCoordinationHeader treats cwd as part of the identity, and nothing can repair the mismatch.
workhub-coordination-coordinator.ts requires header.cwd === join(stateRoot, 'workhub-coordination'). The create fingerprint is a constant, so after the state root moves to a different absolute path — restoring the state directory on a new machine, a renamed home directory, a remote Host whose home path changed — probeStableSessionCreate still answers existing, the cwd comparison fails, and #resolve returns identityConflict() forever. This slice deliberately blocks session.remove, session.lifecycle.set, session.workspace.relocate, session.configuration.update and session.metadata.update for the reserved identity, so there is no product path left to repair or drop the stale row: WorkHub becomes permanently unopenable and the surface tells the user to check their default model, which is not the problem. cwd is repairable state, not identity — resolve should relocate it instead of rejecting.

2. The copy reservation covers the target but not the source.
session-revision-coordinator.ts rejects the reserved identity only as input.targetSessionId. The Coordination Session is still a legal source for session.conversation.copy (ordinary copy, revision, and side conversation): the source header is only checked for archived / subagentParent / Deep Research. #createInput does not carry role, so the identity/role pairing check in createStableSession passes and the copy lands as an ordinary Session holding the Coordination transcript — with an ordinary id and no role, so runtimeHostExecutionUnavailableReason also lets it run Turns. Any owner client can reach this: workhub.coordination.resolve hands out the id, and the copy does the rest.

P2 — should fix in this PR

3. The Coordination workspace directory is only guaranteed at provisioning time.
mkdir runs on the create branch only. If stateRoot/workhub-coordination is later removed — a pruned state directory, a database restored without its sibling directory — the existing branch compares the cwd string, reports success, and the directory never comes back. mkdir with recursive is idempotent; running it before the probe makes both branches converge on the same guarantee.

4. A default Host that is not ready leaves WorkHub on an indefinite spinner.
workhub-coordination-lifecycle.ts always revokes on a default-Host change but only re-resolves when readiness === 'ready', and revoke() clears failedGeneration. When the default Host goes unavailable (crash, failed respawn, onTargetRemoved), reportFailure never runs, so the surface stays on Preparing WorkHub… with no error and no Retry, and the connection_list_changed handler cannot restart it either because it is gated on failedGeneration === generation. Recovery waits for a ready event that may never arrive. connecting / reconnecting should keep the spinner; unavailable should report a failure.

P3 — cleanups

5. Every resolveCreateTarget() failure is reported as a missing default model.
The bare catch maps any error to operation_conflict / “requires an available default model”. resolveExternalSessionImportTarget also awaits the runtime policy read, which raises persistence_failed / “Runtime policy is unavailable”; the user is then told to fix a model setting that was never wrong, and the real persistence failure is never surfaced.

6. #updateMetadata reads the header snapshot twice.
The new snapshot read and Coordination guard duplicate the read and guard #replaceUserLabels already performs inside the same admission lease, on the same sessionId. A label update now costs two identical reads, and rename / flag updates pay a new one on a hot path (auto-titling renames land on most Turns). Threading the record through removes both.

7. Unused import.
registerRuntimeHostSessionCatalogIpc is imported in runtime-host-session-catalog-ipc-main.test.ts and never referenced — it is the file's only change. Biome's allowlist has no unused-import rule, so nothing caught it.

…on both sides
Review follow-ups on this slice.
The Coordination workspace is derived from the Host state root, so treating
its path as part of the identity stranded the one Session that no ordinary
lifecycle operation may relocate or retire: a state directory restored under
a different absolute path answered `operation_conflict` on every resolve,
with no product path left to repair it. Identity is now the id/role pair
alone, the durable path is relocated in place, and the workspace directory is
ensured on reuse as well as on provisioning.
The copy reservation covered the target identity but not the source, so the
Coordination transcript could be lifted into an ordinary — and therefore
executable — Session. Both halves of the reserved pair are refused as a copy
source now.
An unavailable default Host revoked WorkHub resolution without reporting a
failure, leaving the surface on a loading state no retry could reopen, since
the availability subscription only reopens a generation that failed.
Connecting and reconnecting still hold that state; unavailable no longer does.
Also: keep a failed model-authority read distinguishable from a missing
default model, drop a duplicate header read inside one admission lease, and
remove an unused test import.
@ARE404

Copy link
Copy Markdown
ContributorAuthor

Pushed cd4b60ed8, which addresses all seven.

P1-1 — Coordination workspace no longer acts as identity.validCoordinationHeader checks the id/role pair, lineage and archive state only. The durable cwd is relocated in place under the admission lease with the probed revision, so a state directory restored under a different absolute path keeps its Session instead of answering operation_conflict forever. New regression: relocates the durable workspace when the Host state root moves.

P1-2 — the copy reservation now holds on both sides.#copy refuses the reserved id as a source, and #copyAdmitted refuses either half of the reserved pair once the source header is known. The two-client UDS regression now asserts both directions for session.branch.create and session.revision.create, and that the would-be target never appears in the catalog.

P2-3 — the workspace directory is ensured on reuse.mkdir moved above the probe, so both branches converge on the same guarantee and a pruned directory comes back before the identity is handed out. This also simplified the create branch: everything after the probe is now a single outcome-unknown boundary. New regression: restores a Coordination workspace that was pruned after provisioning.

P2-4 — an unavailable default Host reports a failure.connecting and reconnecting still hold the loading state; unavailable (and removed) now reports through the same path a rejected resolve takes, so the surface shows the error with Retry and the availability subscription can reopen the generation. New regression: WorkHub reports an unavailable default Host instead of holding the loading state.

P3-5 — a persistence_failed model-authority read keeps its code and message; everything else still maps to the default-model conflict. Covered by separates an unreadable model authority from a missing default model.

P3-6#updateMetadata reads the admitted snapshot once and threads it into label replacement; #replaceUserLabels became the pure replaceUserLabels(current, …).

P3-7 — unused import removed.

Local: npm run typecheck, npm run format:check, npm run lint, git diff --check all clean. Runtime Host 1,159 passed, Desktop 1,487 passed.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on cd4b60ed86:

No blocking issues. The source-side copy guard now correctly rejects reserved source Sessions, and workspace/generation handling is sound.

Checks on cd4b60ed86 — code GO, no P0-P3.

简体中文该头未发现阻断。

Depth for the A-seat boundary review. Neither item was reachable; both were
places where the closure depended on a caller remembering something.
The id/role pairing was asserted in createStableSession() only, so the
creators that share buildSessionHeader() — createSubagent() and
createAgentGraphOperator() — did not inherit the invariant even though the
header they build carries the same field. It moves into the builder, where
every creator meets it. createStableSession() keeps its own assertion so a
malformed request is still refused before a durable create claim is written.
The metadata list role scope defaulted to the widest value, which made
closure a property of each caller remembering to narrow it. The default is
gone: list() now takes both the filter and the scope explicitly, and usage
statistics — the one reader that took the default — states 'all', which is
what a durable cost aggregate wants.
@ARE404

Copy link
Copy Markdown
ContributorAuthor

Both P3 depth items are in 9966f7ae7.

1 — the id/role pairing moved into the header builder.assertCoordinationIdentityPairing(sessionId, role) now runs inside buildSessionHeader(), so createSubagent() and createAgentGraphOperator() meet the same invariant as createStableSession() rather than depending on their inputs never carrying a role.

createStableSession() keeps its own call to the assertion, deliberately: it runs before claimStableSessionCreate(), and the builder runs after. Dropping the early one would let a malformed request write a durable create claim for the identity it names, which a later legitimate create would then see as a conflict. Same fact, asserted at the boundary that must fail before a write and at the low-level constructor that every creator shares.

Regression extended: requires the reserved WorkHub Coordination identity and role together now rejects a role claimed through both sibling creators and asserts the store stays empty afterwards.

2 — the metadata role scope has no default.SqliteSessionMetadataStore.list() takes (filter: SessionListFilter | undefined, roleScope: SessionMetadataRoleScope), both required, and the type is exported so a call site names its scope. The three readers now read as: catalog 'ordinary', recovery 'recoverable', usage statistics 'all' — the last stated explicitly with a note that a durable cost aggregate counts every Session that spent tokens, including rows a catalog hides.

Local: npm run typecheck, npm run format:check, npm run lint, git diff --check clean. Storage 940 passed / 14 skipped, Runtime Host 1,159 passed, Desktop 1,487 passed.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on 9966f7ae7b:

No blocking issues. The delta makes Coordination identity/role pairing pre-claim and shared across Session creators, correct for preventing ordinary copies from capturing the reserved Session.

Checks on 9966f7ae7b are test: success.

简体中文该头未发现阻断。

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE — 9966f7a, no P0-P3, identity pairing correctly pre-claimed.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ARE404@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

feat(workhub): add coordination session lifecycle - #3764

Merged
Astro-Han merged 4 commits into
apache:mainfrom
ARE404:codex/workhub-coordination-session-lifecycle
Aug 25, 2026
Merged

feat(workhub): add coordination session lifecycle#3764
Astro-Han merged 4 commits into
apache:mainfrom
ARE404:codex/workhub-coordination-session-lifecycle

Conversation

@ARE404

@ARE404ARE404 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • provision and reuse one stable Coordination Session per Runtime Host through the existing Session/SQLite authority
  • revoke an old Host scope as soon as the default Host changes, and retry recoverable provisioning when model authority becomes available
  • reserve the Coordination identity/role pair from ordinary creation, copies, configuration, retirement, catalogs, and root execution

Refs #3492

Authority and deferred work

Session remains the only durable substrate: this adds no second WorkHub database, event store, lifecycle authority, or transcript copy. Each Runtime Host remains independent; cross-Host coordination is not supported.

Coordination transcript rendering and answer_here belong to Slice 3. Model dispositions, the deterministic Action Gate, bounded delegation links, and the deferred Work cardinality decision remain for later slices.

UI evidence

Missing default modelRecovered after model configuration
WorkHub shows a recoverable unavailable stateWorkHub recovers without changing surfaces

Verification

  • npm run lint
  • npm run format:check
  • git diff --check
  • npm run typecheck
  • npm run build
  • Storage focused regressions: 29 passed
  • Runtime Host: 1,156 passed
  • Runtime Host two-client branch/revision regression: 1 passed
  • Desktop: 1,506 passed
  • Electron default-model failure/recovery e2e: 1 passed
  • Standards and Spec adversarial review: no findings
  • GitHub CI: passed for exact head 35efcd6d9, including Desktop e2e, browser smoke, Storybook, and installed CLI validation

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex implemented and tested the lifecycle, storage boundary, protocol, Desktop integration, review fixes, and Standards/Spec reviews.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally or in clean CI

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Implement Slice 2 of apache#3492 on the existing Session substrate.
Generated-by: Codex
@ARE404
ARE404 marked this pull request as ready for review August 25, 2026 05:37

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this head and found blocking issues.

[P2] Default-Host switch while not ready leaves old generation active

Switching default Host to a reconnecting/unavailable Host does not increment the generation, so late results from the previous Host still pass checks and WorkHub continues using the old Host until the new one becomes ready — which may never happen.

[P2] Ordinary workspace relocation can permanently break Coordination identity

session.workspace.relocate can change the reserved maka_workhub_coordination cwd/projectId, after which the next resolve permanently fails with operation_conflict and the row is hidden with no repair path.

[P3] Write-only ref duplication

workHubCoordinationSessionIdRef is never read; state is the sole authority.

Checks on becd9f133e are test: success.

简体中文存在两项身份/代次阻断与一项冗余。

@ARE404

Copy link
Copy Markdown
ContributorAuthor

Addressed the review findings in 35efcd6d9:

  • every default-Host change now revokes the old generation immediately, including connecting, reconnecting, and unavailable targets; stale resolves and stale controllers cannot mutate
  • ordinary workspace/configuration/metadata/read-state/retirement authority rejects either half of the reserved Coordination identity/role pair, including corrupt persisted pairings
  • workHubCoordinationSessionIdRef now participates in the current-scope guard alongside the generation
  • also closed ordinary session.create, branch/revision copy, and generic root-execution paths over the reserved identity; missing-model startup is visible and retryable

Exact-head CI is green, and the final Standards/Spec adversarial re-review has no findings.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on 35efcd6d9b:

No blocking issues. The prior generation/identity P2s are now closed: default-Host changes correctly invalidate the old generation, and the Coordination identity is protected across copy paths.

[P3] Copy reservation only protects the target, not the source

session-revision-coordinator.ts:179 checks only targetSessionId. A valid Coordination Session supplied as sourceSessionId can still be copied into an ordinary Session, exposing hidden messages. This is latent until Slice 3 writes transcripts, but should be rejected before admission.

Checks on 35efcd6d9b are test: success.

简体中文该头无阻断,剩余 P3 为源侧拷贝保护。

@ARE404

Copy link
Copy Markdown
ContributorAuthor

Self-review pass over this slice, graded by severity. Fixes for all seven follow in the next commit.

P1 — correctness, must fix before merge

1. validCoordinationHeader treats cwd as part of the identity, and nothing can repair the mismatch.
workhub-coordination-coordinator.ts requires header.cwd === join(stateRoot, 'workhub-coordination'). The create fingerprint is a constant, so after the state root moves to a different absolute path — restoring the state directory on a new machine, a renamed home directory, a remote Host whose home path changed — probeStableSessionCreate still answers existing, the cwd comparison fails, and #resolve returns identityConflict() forever. This slice deliberately blocks session.remove, session.lifecycle.set, session.workspace.relocate, session.configuration.update and session.metadata.update for the reserved identity, so there is no product path left to repair or drop the stale row: WorkHub becomes permanently unopenable and the surface tells the user to check their default model, which is not the problem. cwd is repairable state, not identity — resolve should relocate it instead of rejecting.

2. The copy reservation covers the target but not the source.
session-revision-coordinator.ts rejects the reserved identity only as input.targetSessionId. The Coordination Session is still a legal source for session.conversation.copy (ordinary copy, revision, and side conversation): the source header is only checked for archived / subagentParent / Deep Research. #createInput does not carry role, so the identity/role pairing check in createStableSession passes and the copy lands as an ordinary Session holding the Coordination transcript — with an ordinary id and no role, so runtimeHostExecutionUnavailableReason also lets it run Turns. Any owner client can reach this: workhub.coordination.resolve hands out the id, and the copy does the rest.

P2 — should fix in this PR

3. The Coordination workspace directory is only guaranteed at provisioning time.
mkdir runs on the create branch only. If stateRoot/workhub-coordination is later removed — a pruned state directory, a database restored without its sibling directory — the existing branch compares the cwd string, reports success, and the directory never comes back. mkdir with recursive is idempotent; running it before the probe makes both branches converge on the same guarantee.

4. A default Host that is not ready leaves WorkHub on an indefinite spinner.
workhub-coordination-lifecycle.ts always revokes on a default-Host change but only re-resolves when readiness === 'ready', and revoke() clears failedGeneration. When the default Host goes unavailable (crash, failed respawn, onTargetRemoved), reportFailure never runs, so the surface stays on Preparing WorkHub… with no error and no Retry, and the connection_list_changed handler cannot restart it either because it is gated on failedGeneration === generation. Recovery waits for a ready event that may never arrive. connecting / reconnecting should keep the spinner; unavailable should report a failure.

P3 — cleanups

5. Every resolveCreateTarget() failure is reported as a missing default model.
The bare catch maps any error to operation_conflict / “requires an available default model”. resolveExternalSessionImportTarget also awaits the runtime policy read, which raises persistence_failed / “Runtime policy is unavailable”; the user is then told to fix a model setting that was never wrong, and the real persistence failure is never surfaced.

6. #updateMetadata reads the header snapshot twice.
The new snapshot read and Coordination guard duplicate the read and guard #replaceUserLabels already performs inside the same admission lease, on the same sessionId. A label update now costs two identical reads, and rename / flag updates pay a new one on a hot path (auto-titling renames land on most Turns). Threading the record through removes both.

7. Unused import.
registerRuntimeHostSessionCatalogIpc is imported in runtime-host-session-catalog-ipc-main.test.ts and never referenced — it is the file's only change. Biome's allowlist has no unused-import rule, so nothing caught it.

…on both sides
Review follow-ups on this slice.
The Coordination workspace is derived from the Host state root, so treating
its path as part of the identity stranded the one Session that no ordinary
lifecycle operation may relocate or retire: a state directory restored under
a different absolute path answered `operation_conflict` on every resolve,
with no product path left to repair it. Identity is now the id/role pair
alone, the durable path is relocated in place, and the workspace directory is
ensured on reuse as well as on provisioning.
The copy reservation covered the target identity but not the source, so the
Coordination transcript could be lifted into an ordinary — and therefore
executable — Session. Both halves of the reserved pair are refused as a copy
source now.
An unavailable default Host revoked WorkHub resolution without reporting a
failure, leaving the surface on a loading state no retry could reopen, since
the availability subscription only reopens a generation that failed.
Connecting and reconnecting still hold that state; unavailable no longer does.
Also: keep a failed model-authority read distinguishable from a missing
default model, drop a duplicate header read inside one admission lease, and
remove an unused test import.
@ARE404

Copy link
Copy Markdown
ContributorAuthor

Pushed cd4b60ed8, which addresses all seven.

P1-1 — Coordination workspace no longer acts as identity.validCoordinationHeader checks the id/role pair, lineage and archive state only. The durable cwd is relocated in place under the admission lease with the probed revision, so a state directory restored under a different absolute path keeps its Session instead of answering operation_conflict forever. New regression: relocates the durable workspace when the Host state root moves.

P1-2 — the copy reservation now holds on both sides.#copy refuses the reserved id as a source, and #copyAdmitted refuses either half of the reserved pair once the source header is known. The two-client UDS regression now asserts both directions for session.branch.create and session.revision.create, and that the would-be target never appears in the catalog.

P2-3 — the workspace directory is ensured on reuse.mkdir moved above the probe, so both branches converge on the same guarantee and a pruned directory comes back before the identity is handed out. This also simplified the create branch: everything after the probe is now a single outcome-unknown boundary. New regression: restores a Coordination workspace that was pruned after provisioning.

P2-4 — an unavailable default Host reports a failure.connecting and reconnecting still hold the loading state; unavailable (and removed) now reports through the same path a rejected resolve takes, so the surface shows the error with Retry and the availability subscription can reopen the generation. New regression: WorkHub reports an unavailable default Host instead of holding the loading state.

P3-5 — a persistence_failed model-authority read keeps its code and message; everything else still maps to the default-model conflict. Covered by separates an unreadable model authority from a missing default model.

P3-6#updateMetadata reads the admitted snapshot once and threads it into label replacement; #replaceUserLabels became the pure replaceUserLabels(current, …).

P3-7 — unused import removed.

Local: npm run typecheck, npm run format:check, npm run lint, git diff --check all clean. Runtime Host 1,159 passed, Desktop 1,487 passed.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on cd4b60ed86:

No blocking issues. The source-side copy guard now correctly rejects reserved source Sessions, and workspace/generation handling is sound.

Checks on cd4b60ed86 — code GO, no P0-P3.

简体中文该头未发现阻断。

Depth for the A-seat boundary review. Neither item was reachable; both were
places where the closure depended on a caller remembering something.
The id/role pairing was asserted in createStableSession() only, so the
creators that share buildSessionHeader() — createSubagent() and
createAgentGraphOperator() — did not inherit the invariant even though the
header they build carries the same field. It moves into the builder, where
every creator meets it. createStableSession() keeps its own assertion so a
malformed request is still refused before a durable create claim is written.
The metadata list role scope defaulted to the widest value, which made
closure a property of each caller remembering to narrow it. The default is
gone: list() now takes both the filter and the scope explicitly, and usage
statistics — the one reader that took the default — states 'all', which is
what a durable cost aggregate wants.
@ARE404

Copy link
Copy Markdown
ContributorAuthor

Both P3 depth items are in 9966f7ae7.

1 — the id/role pairing moved into the header builder.assertCoordinationIdentityPairing(sessionId, role) now runs inside buildSessionHeader(), so createSubagent() and createAgentGraphOperator() meet the same invariant as createStableSession() rather than depending on their inputs never carrying a role.

createStableSession() keeps its own call to the assertion, deliberately: it runs before claimStableSessionCreate(), and the builder runs after. Dropping the early one would let a malformed request write a durable create claim for the identity it names, which a later legitimate create would then see as a conflict. Same fact, asserted at the boundary that must fail before a write and at the low-level constructor that every creator shares.

Regression extended: requires the reserved WorkHub Coordination identity and role together now rejects a role claimed through both sibling creators and asserts the store stays empty afterwards.

2 — the metadata role scope has no default.SqliteSessionMetadataStore.list() takes (filter: SessionListFilter | undefined, roleScope: SessionMetadataRoleScope), both required, and the type is exported so a call site names its scope. The three readers now read as: catalog 'ordinary', recovery 'recoverable', usage statistics 'all' — the last stated explicitly with a note that a durable cost aggregate counts every Session that spent tokens, including rows a catalog hides.

Local: npm run typecheck, npm run format:check, npm run lint, git diff --check clean. Storage 940 passed / 14 skipped, Runtime Host 1,159 passed, Desktop 1,487 passed.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on 9966f7ae7b:

No blocking issues. The delta makes Coordination identity/role pairing pre-claim and shared across Session creators, correct for preventing ordinary copies from capturing the reserved Session.

Checks on 9966f7ae7b are test: success.

简体中文该头未发现阻断。

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE — 9966f7a, no P0-P3, identity pairing correctly pre-claimed.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ARE404@Astro-Han