Skip to content

Fix what running the built wheel found - #487

Closed
bar-capsule wants to merge 8 commits into
refimpl/adapters-11-examples-and-importsfrom
refimpl/adapters-12-live-run-fixes
Closed

bar-capsule wants to merge 8 commits into
refimpl/adapters-11-examples-and-importsfrom
refimpl/adapters-12-live-run-fixes

Conversation

@bar-capsule

@bar-capsule bar-capsule commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on #486. Next: #488. The full stack is listed in #22.

What changed

From running the wheel in Claude Code, Cursor and NAT:

  • A blocked Cursor prompt shows a readable message (exit 0 with user_message).
  • Each session sends its handshake and sessionStart once, before its other steps: the first hook process holds a per-session startup lock while it starts the session, and the others wait.
  • The sessionStart decision is enforced: a deny or a refusal refuses the session.
  • acs-nat-wire wires every NAT function, and --check rejects one without the middleware.

Eight commits.

Which issue does this implement

Part of #134

Base branch

Stacked on #486, not on integration. It lands through #22; see the stack in #22.

Type of change

  • Reference implementation or adapter

I tested this

At this slice's top (e42f2771), with claude removed from PATH and ACS_LIVE_CLAUDE and ACS_SPEC_DIR unset: 475 passed and 3 expected skips with the NAT extra, 408 passed and 3 expected skips without it. The skips are the two opt-in live Claude Code tests and the manual Cursor procedure. CI passes on this PR, including lint-imports and the installed-wheel check on Python 3.11 and 3.12.

Checklist

  • Commits are signed off with git commit -s (required by the DCO)
  • Prose follows STYLE.md
  • No secrets, tokens, or internal URLs in the diff

Security

A denied sessionStart refuses the session, sessionStart goes out once before other steps, and every NAT function is wired.

A blocked beforeSubmitPrompt exits 0 with continue false and
user_message, so Cursor shows the message.

Signed-off-by: Bar Kaduri <bar@capsule.security>
The hook that opens a session holds a startup lock while it handshakes
and sends sessionStart; the session's other hooks wait and reuse the
stored decision.

Signed-off-by: Bar Kaduri <bar@capsule.security>
The NAT README and mapping page state that only a run that returns sends
agentResponse and sessionEnd.

Signed-off-by: Bar Kaduri <bar@capsule.security>
acs-nat-wire adds acs_guardian to every function's middleware and
--check reports any function without it, so nested calls reach the
Guardian.

Signed-off-by: Bar Kaduri <bar@capsule.security>
A denied or refused sessionStart refuses the session. The outcome is
kept in a signed startup record written under the startup lock.

Signed-off-by: Bar Kaduri <bar@capsule.security>
A hook whose ACS_REQUIRE_PROFILES is not met denies its own step before
sessionStart; a later hook of the session starts it.

Signed-off-by: Bar Kaduri <bar@capsule.security>
Both tests store their session records in a temporary directory.

Signed-off-by: Bar Kaduri <bar@capsule.security>
One network mock covers both threads, so no mock outlives the test.

Signed-off-by: Bar Kaduri <bar@capsule.security>

@drk-ram drk-ram left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed at head e42f2771dc74915e00cb36b9b849fe8558a2507d (stacked on #486, base refimpl/adapters-11-examples-and-imports, which is c59c00e). I read the full diff of the 28 files and ran the adapters suite. I am approving. Three notes follow, none of them blocking.

What I checked in the startup protocol (client/_handshake.py).

  • open_session(start=…) takes the per-session lock, re-reads the signed session record inside the lock, then writes a signed .started claim with os.link(). link() fails if any startup record exists, so exactly one process sends sessionStart. The outcome then replaces the claim through os.replace(). Every other hook either finds the outcome on its unlocked fast path or waits on the lock and then follows what was recorded. That is the right shape, and it removes the race behind the handshake and sessionStart that two Cursor hooks used to send.
  • The two record kinds cannot stand in for each other. _verified() checks a distinct marker (acs_session_record or acs_session_startup) plus the session id and Guardian URL under the HMAC, and _read_record() refuses a session record that carries start_failure.
  • Config.gate() turns a recorded start_failure into a fail-closed resolution only when the negotiated posture is deny. A deny, a refusal or a non-allow sessionStart disposition refuses the session for every later hook, and a hook whose own ACS_REQUIRE_PROFILES refuses leaves the start to the next hook. The new tests cover each of those, including test_the_session_start_decision_governs_every_hook_of_the_session as a host × case matrix.
  • Claude Code emits the opening hook's source into sessionStart, and a later SessionStart with resume or compact sends nothing. test_a_resumed_or_compacted_claude_session_keeps_its_one_session_start pins that.
  • Cursor's beforeSubmitPrompt block now exits 0 with {"continue": false, "user_message": …}. Cursor's hooks page says exit 2 is the block, and it documents the {continue, user_message} output for this hook, so the new shape is the documented one. See the first note below.
  • acs-nat-wire --check now flags every function with no acs_guardian, and --write adds it. A call to a function runs only that function's own middleware, so a function that omits it bypasses the Guardian, and test_wire.py covers both directions.

Verification (Windows, Python 3.11.9, uv sync --frozen --extra nat --group test).

The harness does not run on Windows as it stands: launch_guardian() calls select.select() on a pipe, and example_guardian.py imports fcntl. I used a thread-based line read and an fcntl stub in my working copy only, and committed nothing. With that, pytest -q on e42f277 gives 452 passed, 37 failed and 3 skipped. I ran the same suite with the same two edits on the base c59c00e: 432 passed, 38 failed, 3 skipped. Every failure on e42f277 is also in the base's failure list. They are POSIX-only assertions: 0600/0700 modes, symlink rejection, /-style paths in test_wire_duplicates.py, and connect-refused timing on 127.0.0.1:1. None is in the code this PR touches. All of the new startup tests pass: the parallel-startup, one-refusal and unanswered-handshake tests, the resume/compact test, and the decision matrix for both hosts. I did not run the Linux-only lint or wheel checks.

Notes (non-blocking).

  1. Cursor no longer exits 2 on a blocked prompt, so a Cursor release that stopped honouring continue: false on beforeSubmitPrompt would let the prompt through with nothing to catch it. Documented output and the live run are good evidence today. The live check in docs/cursor/live_verification.md is the right guard, and it would be worth keeping it in the release checklist.
  2. _locked() waits differently by platform. POSIX flock(LOCK_EX) waits indefinitely. On Windows msvcrt.locking(LK_LOCK) retries about ten times at one-second intervals and then raises, which open_session turns into session_startup_lock_unavailable, a refused session. A handshake (5 s default) followed by a sessionStart (5 s default) can run past ten seconds when the Guardian is slow, so the hooks waiting behind the opener would refuse on Windows only. A deadline derived from the negotiated timeouts, applied on both platforms, would make the two behave the same.
  3. On this PR alone, a hook that the host kills between the claim and the outcome leaves the record pending, and every later hook of that session gets session_start_incomplete as a refusal, even under on_decision_failure: proceed. I see #489 settles that case under the session's posture, so this is fine given the stack lands through #22. It is worth saying in #22 that this slice must not land alone.

Approving e42f2771.

@drk-ram drk-ram mentioned this pull request Nov 5, 2026
4 tasks done
@rocklambros

Copy link
Copy Markdown
Collaborator

Closing this slice because PR #22 (the reference adapters for Claude Code, Cursor and NAT) merged into integration as 2630ec2, and that merge carries every slice in this stack. The top of the stack had the same files as PR #22's final commit, so nothing in this slice was left behind. Thanks, Bar, for cutting the adapters into pieces this size, because it made the review possible.

@rocklambros rocklambros closed this Nov 5, 2026
@github-project-automation github-project-automation Bot moved this from In progress to Done in ACS Project Tracker Nov 5, 2026
@bar-capsule
bar-capsule deleted the refimpl/adapters-12-live-run-fixes branch November 8, 2026 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants