Skip to content

feat(harness): persist bootstrap claims [Agent Map 05/15] - #824

Merged
ynadge merged 1 commit into
mainfrom
review/agent-map-05-bootstrap-state
Sep 6, 2026
Merged

feat(harness): persist bootstrap claims [Agent Map 05/15]#824
ynadge merged 1 commit into
mainfrom
review/agent-map-05-bootstrap-state

Conversation

@ynadge

@ynadge ynadge commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Primary change type

  • Feature

Problem and motivation

Project bootstrap and queued user input need durable identities and claims so retries or restarts cannot create duplicate first sessions or discard accepted requests.

Summary and scope

Extract state decoding and migration, project intents, first-session claims, accepted-input receipts, and the bootstrap outbox into a persistence boundary that can be tested before activation.

Ignore unrelated outbox files such as .DS_Store while rejecting malformed reserved records. Failed state writes remove their temporary file and preserve the last committed state.

How this increment fits

This is internal persistence groundwork with no active server behavior. The complete coordinator follows in part 6 and activates in part 7.

Stack and review boundary

  • Part 05 of 15 in the Agent Map review stack; review this increment against its predecessor.
  • Base: review/agent-map-04-session-input.
  • Current head: 2b5ee346d6a49b783033256d4e3a8ae04949fb20; 2,350 changed lines across 8 files, counting additions and deletions including tests.
  • Repackages the corresponding final behavior from #804. Original code and review history remain preserved.
  • Complete coworker testing branch: fix/studio-onboarding-followups.
  • The stack remains unmerged. Dependent PRs target their predecessor, so their diffs do not repeat earlier increments.

Related work

Agent Map checkpoint SAP-3147; relevant work SAP-3148. This packaging follows the maintainer-approved 15-PR split.

Validation

Root checks ran against cc19a8a0d4bde84feb5568a50d01801a784fa76e. The final head changes only README terminology or commit ancestry; a complete tracked-file comparison confirms identical executable source and build inputs. The terminology gate was rerun on 2b5ee346d6a49b783033256d4e3a8ae04949fb20.

pnpm build — passed (exit 0)
pnpm typecheck — passed (exit 0)
pnpm lint — passed (exit 0)
pnpm test — passed (exit 0)

Tests and documentation

Regression coverage: Legacy state decoding, durable claims and receipts, duplicate intents, corrupt reserved entries, unrelated files, failed writes, and restart recovery.

See part 15 for integrated browser, native CLI, and Mac journey validation. The checks above were run independently on this PR’s own commit.

Linux tests run with ordinary user filesystem permissions; the sandbox's extra ambient capabilities are dropped. Hosted CI and automated review are separate from these recorded local results.

Compatibility and release impact

  • Compatibility: Existing startup state remains readable. No user-facing behavior or public API activation occurs in this increment.
  • Changeset: Included: .changeset/bootstrap-storage.md

Security

  • No secrets, credentials, private user data, or unsanitized logs are included.
  • This PR does not publicly disclose a suspected vulnerability.

AI assistance

  • Codex assembled the implementation, addressed reproduced defects, supplied tests and documentation, inspected the diff, and ran the checks above. Reviews are handled by hosted PR automation.

Checklist

  • Read CONTRIBUTING.md; implementation follows the requested 15-PR split.
  • Description reflects this PR's actual predecessor-relative diff.
  • Relevant tests accompany the changed behavior.
  • Root build, typecheck, lint, and test evidence matches the final implementation; any documentation-only update is identified above.
  • Release/documentation treatment is explained above.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Review — PR #824 (round 1)

No confidentiality findings. Changeset, JSDoc, and comments are provider- and
customer-neutral; nothing in the diff names a third party, a business arrangement, or an
internal host/incident/employee.

Findings

1. Post-commit hook rejection fails a create that already committed

packages/harness/src/core/studio-project-catalog.ts:534 (and the same shape at
:713): afterProjectsCreatedCommit is awaited after await this.persist(next) and
this.projects = next, with no try/catch. Its rejection propagates straight out of
create() / reconcile(), through enqueue, to the caller.

Failure scenario: the slot-7 consumer's post-commit delivery throws (outbox complete()
hits EACCES, intent write fails). POST project-create rejects and Studio reports the
creation failed, but the project is durably in studio-projects.json and in the
in-memory this.projects, so the very next list call shows it. The user sees a project
they were told was not created; the caller has no way to distinguish "not created" from
"created, delivery pending".

The hook's own JSDoc says "Delivery may be retried, so consumers must be project-keyed and
idempotent" — that contract only holds if the catalog treats the post-commit call as
best-effort. Wrap it (await hook(...).catch(...), or void), since the write-ahead
marker staged by beforeProjectsCreatedCommit is exactly the mechanism that makes recovery
possible without the caller learning about the failure. The pre-commit hook's abort path is
tested (project-bootstrap-outbox.test.ts:142); the post-commit failure path has no test.

2. Changeset is patch but the PR widens the published type surface

.changeset/bootstrap-storage.md is patch and describes the change as "internal", yet
packages/harness/src/index.ts:26 adds ProjectBootstrapInputReceipt to the package's
public exports. A new exported type is a contract commitment and, by this repo's own
practice (.changeset/ordinary-project-session-identity.md,
.changeset/unified-project-agents.md are both minor for additions), needs minor.

Additionally: nothing in this PR — no API response, no public function signature — uses
ProjectBootstrapInputReceipt. PersistedProjectBootstrapInputReceipt in
project-bootstrap-store.ts:21 extends it internally, and that file is unexported. So the
cheaper fix is to drop the index.ts line and keep the type internal until slot 7 has a
public consumer for it; then the patch level is correct as written.

3. ProjectBootstrapOutbox.pending() is permanently bricked by one stray file

packages/harness/src/core/project-bootstrap-outbox.ts:149-152 throws
ProjectBootstrapOutboxError for any directory entry that is not project_*.json or the
exact writer-temp shape, and pending() has no path that removes or ignores it.

Failure scenario: the shipping desktop host is a macOS .dmg, and state lives under
~/.sapiom. A user opens the outbox directory in Finder, Finder writes .DS_Store, and
from then on every pending() call throws — bootstrap recovery for all staged projects
is dead until someone manually deletes a hidden file. Editor swapfiles and
Zone.Identifier/Thumbs.db on Windows are the same story.

Fail-closed on an unparseable project_*.json is right and worth keeping; fail-closed on
an unrelated filename is not. Skip entries that don't match /^project_.*\.json$/ (or at
minimum dotfiles) rather than throwing.

4. writeState orphans its temp file on failure

packages/harness/src/core/project-bootstrap-store.ts:764-770 is the only atomic writer in
this PR without the try { write; rename } finally { fs.rm(temporary, {force:true}) }
cleanup that writeIntent (:779-787), writeAcceptedInputIds (:1081-1090) and the
outbox's stage (:119-127) all use. A full disk or an EACCES during writeFile/rename
leaves input-queue.json.tmp-<pid>-<uuid> in the session directory, and every retry adds
another. Match the sibling writers.

Nits

  • ProjectBootstrapRegistrationMode (shared/agent-map.ts:503) is referenced by nothing
    in the repo — not source, not tests. Drop it until its consumer lands.
  • shared/agent-map.ts uses triple blank lines between the new declarations (:477,
    :486, :500, :510); Prettier collapses those, so pnpm format will churn the file.
  • project-bootstrap-store.ts:470-479 (receipt subsequence check) is subsumed by the exact
    trailing-suffix check immediately after it at :480-483; one of the two can go.

Verdict

Request changes: finding 1 is a user-visible inconsistency in the hook contract this PR
exists to introduce, and finding 2 mislabels a public API addition as a patch. Findings 3
and 4 are durability cleanups in new code that is cheap to fix now and expensive to fix
after slot 7 depends on it.

@ynadge
ynadge force-pushed the review/agent-map-04-session-input branch from d824dc0 to 023c09d Compare September 5, 2026 11:57
@ynadge
ynadge force-pushed the review/agent-map-05-bootstrap-state branch from 37ad9f0 to cc19a8a Compare September 5, 2026 11:57
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Review — PR #824 (round 2, follow-up)

Delta since 37ad9f05: changeset reworded, outbox entry filtering, writeState temp
cleanup, plus tests. studio-project-catalog.ts, index.ts and shared/agent-map.ts are
untouched since round 1. No confidentiality findings in the new copy — the rewritten
changeset names no company, arrangement, or internal host.

Fixed

  • langchain SDK config cleanup #3 outbox bricked by a stray fileproject-bootstrap-outbox.ts:150 now skips
    anything outside project_*.json and still fails closed on a malformed reserved marker;
    covered by both new tests (.DS_Store/notes.txt/dir ignored, malformed marker throws).
  • langchain v1 support + fixes #4 orphaned temp stateproject-bootstrap-store.ts:766-774 now matches the sibling
    writers' try { write; rename } finally { rm }, with injected write- and rename-failure
    tests asserting the durable file survives.

Not fixed

  1. Post-commit hook rejection fails an already-committed create
    studio-project-catalog.ts:535 and :717 still await afterProjectsCreatedCommit(...) uncaught after persist(next) and this.projects = next. A throwing slot-7 consumer still makes create()/reconcile() reject for a
    project that is durably present and visible on the next list call. Unchanged from round 1.
  2. patch changeset still ships a new public exportindex.ts:26 still exports
    ProjectBootstrapInputReceipt (shared/agent-map.ts:494) while
    .changeset/bootstrap-storage.md remains patch and now asserts "No user-facing
    behavior changes". Nothing in this PR consumes the type publicly, so either drop the
    index.ts line (keeps patch honest) or go minor.
  3. Nits 1 and 3 stand: ProjectBootstrapRegistrationMode (shared/agent-map.ts:503) is
    still referenced by nothing, and the receipt subsequence check
    (project-bootstrap-store.ts:470-479) is still subsumed by the trailing-suffix check at
    :480-483.

Round 1 correction

Nit 2 was wrong: packages/harness has no format script (package.json:43-64), so
pnpm format never reaches shared/agent-map.ts and the triple blank lines will not churn.
Cosmetic only — no action needed.

Verdict

Request changes: the two substantive round-1 findings (uncaught post-commit hook, patch-level
public export) are unaddressed; the durability fixes landed correctly.

@ynadge
ynadge force-pushed the review/agent-map-04-session-input branch from 023c09d to 3d105bc Compare September 5, 2026 12:17
@ynadge
ynadge force-pushed the review/agent-map-05-bootstrap-state branch from cc19a8a to 2b5ee34 Compare September 5, 2026 12:17
Base automatically changed from review/agent-map-04-session-input to main September 6, 2026 22:20
@ynadge
ynadge merged commit 9f2ef62 into main Sep 6, 2026
2 checks passed
@ynadge
ynadge deleted the review/agent-map-05-bootstrap-state branch September 6, 2026 22:21
Sign up for free to 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.

1 participant