Skip to content

Agent policy legibility, plus the Windows stall bisect - #120

Merged
b-macker merged 4 commits into
masterfrom
claude/naab-inadmissible-action-prevention-4cmn1m
Aug 4, 2026
Merged

Agent policy legibility, plus the Windows stall bisect#120
b-macker merged 4 commits into
masterfrom
claude/naab-inadmissible-action-prevention-4cmn1m

Conversation

@b-macker

@b-macker b-macker commented Aug 3, 2026

Copy link
Copy Markdown
Owner

Summary

Three usability defects on the agent policy surface, all reported from real application use and each traced to a line and verified against the agent stub before being changed. Three follow-up commits then chased a CI stall that blocked the PR and ended up locating it.

1. The shell response scan claimed an execution that never happened

agent_impl.cpp gates a response content scan on shell_allowed — the same flag checkShellAllowed() (governance_engine.cpp:1870) uses for real execution — then reported Shell execution is blocked for this agent role. Nothing was executed. Reproduced against the stub with capabilities.shell.enabled: true globally and only the agent's shell_allowed: false.

The scan matches $ <command> lines, so the blast radius is wider than "shell". Measured:

response content verdict
$ npm install express BLOCKED
Python fence containing # sudo apt install OK
prose: "you can use rm to remove files" OK
pure prose OK

An agent restricted from shell cannot write install instructions for any languagenpm, pip, apt, curl, wget all sit in that pattern. The message now names what was matched and says plainly that nothing ran. Splitting the flag is a schema change and is not done here.

2. Per-agent policy refusals did not name the agent

All seven omitted it, while 30 of 111 Agent error: throws in the same file do format in config_name — including the response-scan errors three functions away. Two conventions; the refusals picked the wrong one. With 13 agents configured, Action matrix does not include AGENT_SEND is unattributable.

The agent.create refusal additionally says which sense of AGENT_SEND it means, because the permission is overloaded: at agentSend:1789 it grants "may be sent to", at agentCreate:1072 "may spawn children".

3. Nine environment state keys vanished instead of defaulting

dict.get() returns null for both "absent" and "zero". Adjacent keys disagreed: tool_integrity_count has an explicit zero on its else path, tool_calls_total simply disappeared when tools were off.

The defaults are deliberately not uniform:

keys absent means default why
tool_calls_total, _blocked, _latency_ms tools off 0 no calls were made — 0 is honest
lease_remaining, _seconds no lease configured -1 0 is what an expired lease reports
turns_remaining, tokens_remaining no limit configured -1 0 is what an exhausted budget reports
risk_budget_remaining no budget configured -1 same

Defaulting the lease and budget keys to 0 replaces a silent absence with a confident lie. -1 is the sentinel already used by escalation_turn (837) and tokens_remaining's own unlimited case (697).

4–6. The Windows stall, and what actually found it

build-windows had stalled four times inside CLI tests — shell suites: step in_progress ~47 minutes, runner killed service-side, log archive 404. timeout-minutes failed to fire on two of those, so the runner could not enforce its own step timeout — the cause could not be read out, only excluded.

Two wrong turns, recorded because they cost the most time:

  • d05a65d hardened the stub launcher (port retry, longer readiness ceiling) after three Linux suites failed across two runs. It then stalled Windows, and a one-commit bisect (056e659 passed → d05a65d stalled, delta = launcher only) looked conclusive. It wasn't. 79a7f20 restored the Windows path to byte-equivalent prior behaviour and stalled anyway. The bisect was coincidence.
  • The set of "9 stub suites" came from grepping one launcher idiom rather than for agent_stub.py. The real population is 29. Excluding 9 would have left 20 running and made a negative result uninterpretable.

The answer was already in the tree. test_absorption_degenerate.sh has carried this since before any of this work:

"Stub-backed HTTP tests hang on Windows/MSYS2 due to signal propagation and process cleanup issues. Skip entirely — Linux CI validates the behavior."

The diagnosis was made once and applied to one file out of 29. e5c6f00 applies it to all of them, and the result is unambiguous:

CLI tests — shell suites
last two runs in_progress 47 min → runner killed
last passing run (29 suites running) 4m04s
this run (29 excluded) 2m04s ✅

Coverage is not lost: build-linux and Build & Test run all 29 in full, and agent-governance semantics are platform-neutral. Reversible in one commit.

Retraction

An earlier revision of this description claimed a security gap: that agent.commit() re-checks lease and CRITICAL but not allowed_actions, so a proposal could be committed after AGENT_SEND was ratcheted away. That is wrong and the gap does not exist.

agentCommit already carries a reload-generation check (getReloadCount() != selected.reload_count) stamped at propose time. reload_count_++ fires only on an accepted reload (governance_config.cpp:3873), and allowed_actions is config-derived, so it cannot change without invalidating the proposal first. s_pending_proposals has exactly one insert — inside agentPropose, behind its own AGENT_SEND check — so a caller cannot supply a proposal either. Covered by test_propose_commit.sh H-01/H-02, which pass 25/25.

I missed it by grepping allowed_actions|AGENT_SEND|checkCriticalSuspension|leaseExpiredLocked|checkAdmissionreload_count was not in the pattern, so the guard was invisible. Same grep-shaped-model error as the 9-vs-29 count above.

Test Plan

  • Ran bash run-all-tests.sh with no new failures — 441 tests, 0 unexpected failures
  • bash tests/security/test_error_msg_leaks.sh874 checks, 0 failures (error text changed, so this one matters)
  • Added/updated tests for new functionality — n/a for the fixes; the CI commits change test infrastructure only
  • Tested manually in the REPL — n/a

Verified per fix against the agent stub, before and after:

fix before after
1 "contains shell commands / Shell execution is blocked" "shell command syntax / nothing was executed", names the $ <cmd> pattern
2 Action matrix does not include AGENT_SEND Agent 'deploy_lead' — action matrix does not include AGENT_SEND
3 3 keys missing on a no-tool no-lease agent tool_calls_total=0, lease_remaining=-1, risk_budget_remaining=-1

All 29 guards were observed taking the skip under a simulated MINGW uname, not assumed to fire. A static coverage check flagged 4 as unguarded; that was a false positive matching a header comment above the guard, and the dynamic check settled it.

Follow-ups not in this PR

  • The pending stall bisect comments in the 29 guards describe a hypothesis that is now resolved; they should be rewritten to state the finding and consolidated into a shared guard.
  • Fixing the launcher's Windows signal/cleanup behaviour rather than excluding the suites, per the original diagnosis.
  • AGENT_SEND and shell_allowed are both overloaded; splitting either is a govern.json schema break.
  • Narrowing the $ <cmd> pattern is a behaviour change to a security check and needs a control proving real shell content still blocks.

Generated by Claude Code

Three usability defects found while tracing the agent policy surface. All
three were reported from real application use; each is traced to a line and
each fix was verified against a stub, not just built.

1. The shell response scan claimed execution that never happened.
   agent_impl.cpp gates a RESPONSE CONTENT scan on shell_allowed, the same
   flag checkShellAllowed() uses for real execution, then reports "Shell
   execution is blocked for this agent role". Nothing was executed. The scan
   matches '$ <command>' lines, so an agent restricted from shell cannot
   write install instructions for ANY language — verified: a response
   containing "$ npm install express" is HARD-blocked. The message now says
   what was actually matched and states that nothing ran. Splitting the flag
   into execution and content halves is a schema change and is NOT done here.

2. Per-agent policy refusals did not name the agent. All seven omitted it
   while 30 of 111 "Agent error:" throws in the same file DO format in
   config_name — including the response-scan errors three functions away.
   Two conventions, and the refusals picked the wrong one. With 13 agents
   configured, "Action matrix does not include AGENT_SEND" is unattributable.
   Now names the agent and the exact govern.json path. The agent name is
   already in the config the operator wrote, so this leaks nothing.

   The agent.create refusal also now says WHICH sense of AGENT_SEND it means:
   the permission is overloaded, granting "may be sent to" at agentSend and
   "may spawn children" at agentCreate, and the old text did not distinguish.

3. Nine environment state keys vanished instead of defaulting. dict.get()
   returns null for both "absent" and "zero", so no NAAb script can tell them
   apart. Adjacent keys disagreed: tool_integrity_count has an explicit zero
   on its else path, tool_calls_total simply disappeared when tools were off.

   The defaults are NOT uniform, deliberately. 0 is honest for the tool
   counters — tools disabled means no calls were made. For the lease and
   budget keys 0 is a lie: it is exactly what an EXPIRED lease and an
   EXHAUSTED budget report, so those default to -1, the sentinel already used
   by escalation_turn and by tokens_remaining's own unlimited case.

Verified per fix against the agent stub: the shell error prints the new
wording, the refusal prints "Agent 'deploy_lead' — action matrix ...", and a
no-tool no-lease agent now reports tool_calls_total=0, lease_remaining=-1,
risk_budget_remaining=-1 instead of three missing keys.

441 tests / 0 unexpected, 874 leak checks / 0 failures.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ELUfjXZvx8kzXo1UJjrAhC
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

NAAb Governance Report

Metric Count
Files checked 16
Passed 16
Failed 0

All governance checks passed!

Generated by NAAb Governance Engine v4.0

claude added 3 commits August 3, 2026 05:12
Three consecutive CI runs failed in a stub-backed suite, each time a
DIFFERENT one, none reproducible locally:

  #119  Build & Test  test_quarantine_corroboration.sh
  #120  build-linux   test_failure_mode_coverage.sh
  #120  Build & Test  test_absorption_degenerate.sh

A code regression fails the same test every run. A different test each time,
all sharing one start_stub idiom copied across 9 files, is the launcher.

Two defects in it, both of which bite only under load:

  - the port is picked at random with no bind check and no retry, so a
    collision or a lingering TIME_WAIT socket leaves the stub dead
  - the readiness wait was a flat 5s, and python3 startup + bind exceeds
    that often enough to matter on a busy runner

Either way start_stub returned 1, the caller printed STUB_FAIL, and every
assertion in the suite then failed for a reason unrelated to what the test
measures — with nothing in the output saying so.

Now retries across 3 ports, waits 30s per attempt, and prints the stub log
tail naming the real cause. The kill -0 check is what keeps the longer bound
cheap: a stub that died on bind is detected immediately and retried on a
fresh port instead of waiting out the ceiling.

Verified the retry path by making every bind fail: 3 attempts, the actual
error surfaced ("Address already in use"), rc=1 — in 0 seconds, not 90. The
old code burned 5s silently and said nothing.

All 9 patched suites pass individually (93 assertions), 441 tests /
0 unexpected, 874 leak checks / 0 failures.

Not claimed: that this is the whole cause. It is the mechanism that fits
every observation, and the next failure now names itself either way.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ELUfjXZvx8kzXo1UJjrAhC
d05a65d caused the Windows stall it was meant to help diagnose. The bisect
is one commit wide and unambiguous:

  056e659  policy refusals + env keys, no stub change   build-windows PASSED
  d05a65d  stub launcher hardening, 9 test files only   build-windows STALLED

The entire delta between them is the launcher. Two defects, both confined to
the FAILURE path — a stub that comes up promptly never enters it, which is
why Linux stayed green and why Windows was fine until the retry existed:

  - `kill` (TERM) followed by an unbounded `wait`. run-all-tests.sh already
    warns in its own comments that native Windows binaries under MSYS2 ignore
    TERM and that plain `timeout` "can wait forever". A `wait` on a child that
    never dies does not return, and a process tree holding an unkillable child
    is also why the runner could neither enforce `timeout-minutes: 25` (it ran
    21m past the bound) nor finalize the step nor upload logs.

  - Spawn amplification. 50 iterations of grep+sleep became 3x300 iterations
    of grep+kill+sleep — roughly 100 process spawns to ~2700. At the ~50-100ms
    fork/exec cost of MSYS2 the loop is dominated by spawning, not by the 30s
    of intended waiting.

Windows now takes the single 5s attempt that ran green for many jobs: no
retry, no kill, no wait. POSIX keeps the retry, with sleep 0.5 over 60
iterations so the longer ceiling costs no more spawns than the original, and
SIGKILL with no wait at all — reaping is not worth a hang.

Verified both paths against a stub rigged to fail every bind: POSIX retries
3 ports, surfaces the real error and returns in 2s; Windows takes one 5s
attempt and returns, byte-for-byte the pre-d05a65d behaviour. All 9 suites
pass (93 assertions), 441 tests / 0 unexpected, 874 leak checks / 0 failures.

The three stalls that predate d05a65d are NOT explained by this and remain
open.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ELUfjXZvx8kzXo1UJjrAhC
…the tree

test_absorption_degenerate.sh has carried this comment since before any of
today's work:

  "Stub-backed HTTP tests hang on Windows/MSYS2 due to signal propagation and
   process cleanup issues. Skip entirely — Linux CI validates the behavior."

The diagnosis was made once and the guard applied to ONE file. 29 suites
launch the stub; 28 of them still ran on Windows. build-windows has stalled
four times since, always inside "CLI tests — shell suites", each time sitting
in_progress ~47 minutes before the runner is killed service-side with the log
archive 404ing. timeout-minutes failed to fire on two of those, so the runner
cannot enforce its own step timeout either: the cause cannot be read out, only
excluded.

All 29 are now excluded on Windows, on both `uname -s` and $WINDIR.

Read the next Windows run as the experiment's result:
  green   -> stub-backed suites are the cause; narrow from 29
  stalls  -> they are exonerated and the cause is elsewhere in the phase

Guarding only the 9 I first found would have made a "stalls" result
uninterpretable, since 20 unguarded launchers would still have been running.
That set of 9 was an artifact of grepping for ONE launcher idiom rather than
for agent_stub.py — the same mistake that hid the reload_count guard earlier
today.

Coverage is not lost: build-linux and Build & Test run all 29 in full, and
agent-governance semantics are platform-neutral. Reversible in one commit.

Verified: all 29 observed taking the skip under a simulated MINGW uname (not
merely assumed to), 441 tests / 0 unexpected and 874 leak checks / 0 failures
unchanged on Linux.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ELUfjXZvx8kzXo1UJjrAhC
@b-macker b-macker changed the title Make agent policy refusals and environment keys legible Agent policy legibility, plus the Windows stall bisect Aug 4, 2026
@b-macker
b-macker marked this pull request as ready for review August 4, 2026 01:31
@b-macker
b-macker merged commit 9b031f3 into master Aug 4, 2026
23 checks passed
@b-macker
b-macker deleted the claude/naab-inadmissible-action-prevention-4cmn1m branch August 4, 2026 01:32
b-macker added a commit that referenced this pull request Aug 4, 2026
The 29 guards added in #120 carried "pending stall bisect" in their comments.
The bisect is finished: excluding all 29 took "CLI tests — shell suites" from a
47-minute hang to 2m04s. Replaces the near-copies with
tests/helpers/stub_platform.sh — 762 lines removed, 58 added.

The line count is not the point. The reason the stall took four occurrences to
diagnose is that the finding was recorded in ONE file's comment, where the 28
other callers of the same launcher could not see it. A single definition is what
makes the next such finding reach all of them. The helper states what is known
rather than what was suspected, and says plainly that it is a workaround: the
launcher's signal and cleanup behaviour under MSYS2 is the actual defect and
remains open.

Also records the episode in docs/governance-campaign-findings.md, which exists
so the next person does not rediscover what was already settled — precisely the
failure this entry is about. New section "A fix that reached one caller", with
measured figures: the Windows guard reached 1 of 29 suites, and 081f460's stub
port retry reached 2 of 29. Two withdrawn proposals (the agent.commit() gap that
does not exist; the launcher hardening whose one-commit bisect was coincidence),
and a fifth method note on how a grep defines what you are able to see.

Verified in both directions: all 29 observed taking the skip under a simulated
MINGW uname and under $WINDIR, and confirmed NOT firing on Linux. 441 tests /
0 unexpected, 874 leak checks / 0 failures, suite total unchanged.
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