Skip to content

Bind approvals to reviewed evidence, sandbox validation network, fix flaky test - #15

Merged
christopherjnelson merged 3 commits into
mainfrom
claude/optimistic-shannon-4dh7gs
Sep 28, 2026
Merged

christopherjnelson merged 3 commits into
mainfrom
claude/optimistic-shannon-4dh7gs

Conversation

@christopherjnelson

Copy link
Copy Markdown
Member

Third batch of audit fixes. Three commits:

Bind the human decision to the evidence the operator reviewed (3a1e627)

  • New GET /api/runs/:id/decision returns the current evidence identity: evidenceDigest, pinnedBaseCommit, workerResponseId, validationId and recommendationId.
  • API change: the approve and reject calls now require evidenceDigest (64 hex characters).
    • A missing or malformed digest gets 400.
    • A digest that no longer matches the run's current evidence gets 409 with "The evidence changed since you reviewed it; refresh and review the current result before deciding."
    • This closes the race where a retry or revalidation lands between the operator reading the result and clicking Approve.
  • UI: the new ui/decision-digest.ts fetches the digest keyed on the evidence identity that is on screen.
    • It refuses to send if the identity changed.
    • It uses a generation counter to drop stale responses.
    • It turns a 409 into a refresh prompt.
    • Both Approve/Reject button pairs use it.
  • docs/workspace-bridge.md is updated to cover the approve step.

Run validation commands without network unless they need it (906cd75)

  • With the bwrap sandbox, each validation command runs with --unshare-net unless it needs network. A check therefore cannot reach services on the host (the bridge, local databases) or cloud credential endpoints.
  • Validation commands have an optional network flag.
    • When the flag is absent, the default is applied at execution time: npm, pnpm and yarn installs get network, everything else does not.
    • Because the default is not written into stored commands, older recorded runs keep the same evidence digest and still promote. A regression test in tests/recorded-integration.test.ts covers this.
  • Repository inspection sets network: true on the suggested install, cargo test and go test checks.
  • The open-repository dialog shows a network toggle per suggested check, and the checks pipeline shows a network badge.
  • With FOREMAN_VALIDATION_SANDBOX=none nothing changes: every command keeps host networking.

Fix the flaky live Worker persistence test (734d19e)

  • The test now polls the persisted file on disk instead of the in-memory store, so it no longer races the debounced write.

Verification

  • Typecheck is clean.
  • vitest: 514/514 passing.
  • pnpm test:bridge: 54/54 passing.
  • Build succeeds.
  • The open-repository dialog was smoke-tested in Chromium against a real backend: 3 network toggles rendered and there were no console errors.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DXCvrGWmmRNDLmy7nYS2QP


Generated by Claude Code

The test waited for the Worker responseId by polling the in-memory
store, then built a second controller that reads the state file from
disk. The store publishes a mutation in memory before its atomic write
lands, so recover() could see the stale 'submitting' assignment, replay
the held POST with the same idempotency key, and block until the 5 s
timeout (17/20 failures under CPU load). Poll the on-disk state instead
(0/90 under the same load).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DXCvrGWmmRNDLmy7nYS2QP
approveRun computed the evidence digest at click time, so a decision
applied to whatever evidence existed when the request arrived, even if
it changed after the operator looked (a validation retry, a correction
cycle, another tab). The GitHub merge path already required the
reviewed head SHA; do the same for the Foreman decision.

GET /api/runs/:id/decision returns the evidence digest and identity for
a run at a decision checkpoint. Approve and reject now require that
evidenceDigest: missing or malformed is a 400, and a digest that no
longer matches the stored evidence is a 409 with nothing recorded.

The UI fetches the digest when it renders a decidable run, keyed on the
Worker response, validation and Reviewer recommendation it shows, never
at click time. It ignores out-of-order responses and refuses a digest
issued for evidence other than what is on screen. Both Approve/Reject
pairs are disabled until the digest is ready; a 409 shows the server's
message and refetches.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DXCvrGWmmRNDLmy7nYS2QP
Validation commands now run inside a network namespace with no
interfaces by default, so a check cannot reach services on the host
(the bridge, local databases) or cloud credential endpoints.

- Each validation command carries an optional `network` flag. When it is
  absent, the default is decided at execution time from the command:
  npm, pnpm and yarn installs get network, everything else does not.
  Older recorded runs therefore keep the same evidence digest and still
  promote.
- Repository inspection sets `network: true` explicitly on the suggested
  install, cargo test and go test checks.
- The open-repository dialog shows each suggested check with a network
  toggle, and the checks pipeline shows a network badge on checks that
  run with network.
- With FOREMAN_VALIDATION_SANDBOX=none the flag has no effect and every
  command keeps host networking, as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DXCvrGWmmRNDLmy7nYS2QP
@christopherjnelson
christopherjnelson merged commit dd6b98d into main Sep 28, 2026
1 check passed
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.

2 participants