fix: resolve merge conflict with a7u-pr-final and harden lifecycle close hook

[Copilot]

Resolves merge conflict between this branch and a7u-pr-final, and fixes a regression where the close hook would short-circuit on the first failure, leaving remaining stores unclosed.

Changes

• Merge conflict resolution — src/index.ts conflicted on the attachWorkspaceStoreLifecycle implementation; resolved in favor of this branch's improved logic (storesForInit/storesForClose deduplication, correct hook ordering).

• Error aggregation restored in close — Previous refactor dropped the try/catch collection pattern. internal.close() and each store's close() are now all attempted regardless of individual failures; errors surface as AggregateError.

// close now collects all errors before throwing
const errors: unknown[] = [];
try { await input.internal.close?.(); } catch (e) { errors.push(e); }
for (const store of [...storesForClose].reverse()) {
  try { await store.close?.(); } catch (e) { errors.push(e); }
}
if (errors.length > 1) throw new AggregateError(errors, "...");
if (errors.length === 1) throw errors[0];

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[Copilot]

Pull request overview

Fixes two correctness/security issues in the runtime: workspace-store lifecycle hooks are now composed (so overrides don’t drop internal init/close), and service-token verification uses a constant-time comparison.

Changes:

• Compose workspace-store initialize/close hooks with existing internal.initialize/internal.close in attachWorkspaceStoreLifecycle.
• Replace direct string comparison of service tokens with timingSafeEqual on SHA-256 digests.
• Add minimal node:crypto declarations to the repo’s hand-rolled Node typings.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 3 comments.

File	Description
src/index.ts	Composes lifecycle hooks and switches auth token comparison to constant-time digest compare.
src/@types/node/index.d.ts	Adds node:crypto type declarations needed for the new constant-time comparison.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ryanjosebrosas]

@copilot apply changes based on the comments in this thread

[Copilot]

@copilot apply changes based on the comments in this thread

Both items in that review are already addressed in commit 33dd9bb: serviceTokenDigest is precomputed once in the createWorkspaceContextResolver closure (lines 372–375) and reused on every resolve() call, and storesForClose deduplicates the close-hook loop.

[ryanjosebrosas]

@copilot apply changes based on the comments in this thread

[Copilot]

@copilot apply changes based on the comments in this thread

All three items in that review are already addressed in commit 33dd9bb:

• storesForInit / storesForClose (lines 299-306) filter out internal.workspaceStore from the per-store init/close loops when internal.initialize/internal.close are defined, preventing double-invocation on the Postgres path.
• serviceTokenDigest (lines 372-375) is computed once in the createWorkspaceContextResolver closure and reused on every resolve() call — no per-request hashing.

[ryanjosebrosas]

@copilot apply changes based on the comments in this thread

[Copilot]

@copilot apply changes based on the comments in this thread

All three items are already implemented in commit 33dd9bb:

• storesForInit/storesForClose (lines 299–306) filter out internal.workspaceStore from the per-store loops when internal.initialize/internal.close are defined, preventing double-invocation on the Postgres path.
• serviceTokenDigest (lines 372–375) is computed once in the createWorkspaceContextResolver closure and reused on every resolve() call — no per-request hashing.

[ryanjosebrosas]

@copilot review

[Copilot]

@copilot review

Review complete. The three items from the original review thread are correctly implemented in commit 33dd9bb. One regression was found and fixed in bc95481:

Regression fixed: The close hook lost its error-aggregation logic during refactoring — if internal.close() threw, the per-store close loops would never run, potentially leaving resources open. Restored the try/catch collection pattern so all close operations are always attempted, with errors surfaced as AggregateError if multiple fail.

All other changes are sound:

• storesForInit/storesForClose correctly exclude internal.workspaceStore when internal.initialize/internal.close are defined (prevents double-invocation on the Postgres path)
• serviceTokenDigest is computed once in the resolver closure, not per-request
• Constant-time comparison via timingSafeEqual on SHA-256 digests is correctly applied