Denial surfacing: WANTED proposals + observed denials over ACP (plans#33)

[JPDuchesne]

Closes d3mlabs/plans#33. Part of the d3mlabs/plans#26 agent-isolation program (WS3, "no dead-ends").

The OS-user split (d3mlabs/plans#32) must not create silent dead-ends: when the agent hits a permission boundary, the run now surfaces it as a proposal a human judges — permission never widens silently or mid-run.

Two channels, one renderer

• Declared — every isolated prompt carries the contract (WANTED: <path or capability> — <why>); Agent extracts and strips WANTED lines from the final text before anything downstream parses it. The contract rides Agent#launch — the one seam all commands cross — rather than per-command splicing (Batch/Split hold no executor; see the plan's as-built notes).
• Observed — tool output is scanned for the unix denial family (Permission denied / Operation not permitted / EACCES / EPERM), one want per touched path, GitHub-API 403s excluded (the read-only token working as designed, d3mlabs/plans#25). A non-force pass's rejected permission request records with the request's own title — deny-with-context is a natural WANTED carrier.
• Rendering — step summary gains ### Boundary wants; the result panel gains a bottom-section block. Each want renders its triage menu (org-wide → tap Brewfile; this project → dependencies.rb) or the category-4/5 "covered by design" note for shared-root/DDC/socket subjects — never a widening. Draft-PR automation is d3mlabs/plans#31's, not this PR's.

Transport swap: stream-json → ACP

Agent#launch now drives agent acp through two new seams:

• Executor#duplex — bidirectional popen3 with the same isolation prefix/env scrub as stream; lingering children reaped TERM-then-KILL.
• AcpClient — hand-rolled, dependency-free JSON-RPC/stdio client (the plans#26 spike's shape). Model handles resolve against the session catalog and ride session/set_model — the global --model flag does not apply to ACP sessions (verified live, cursor-agent 2026.08.11). The old --force flag becomes the permission policy: force answers allow to everything (the boundary is the OS user, plans#26); non-force rejects mutating kinds per request and records the rejection.

Success criterion: a completed turn (stopReason: end_turn); transport exit noise after it is logged, not fatal.

Verification

• 333 tests green, srb tc clean, rubocop clean; local diff-coverage pre-check: 0/270 changed lib lines uncovered.
• Agent tests converted wholesale to a scripted in-process ACP server over real pipes (FakeAcpServer) — no real CLI in tests.
• Live read-only smoke on the workstation through the real Agent#launch → cursor-agent acp: returned the expected text, clean shutdown; a second run with a named model confirmed catalog resolution + session/set_model end to end.

Made with Cursor

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[JPDuchesne]

Audit notes + guide to this PR

A self-audit pass (design, alternatives, security) ran after the PR opened. Two hardening fixes landed as 1b97d99; everything else checked out. Details below, written for a reviewer who hasn't followed the program issues.

What this PR unlocks, plainly

Before this PR, when the sandboxed agent hit a locked door — a file it can't read, a tool that isn't installed — the run just quietly did worse work. Nobody knew a door was even tried.

After this PR, every locked door becomes a visible, human-reviewed note:

• The agent is told (in every sandboxed run's prompt): don't fight the boundary, don't work around it — finish what you can and write WANTED: <thing> — <why> at the end.
• Independently, the run watches tool output for the OS saying "Permission denied", and records what a non-forced run refused to let the agent do.
• Both feed one report: a Boundary wants section on the run's summary page and at the bottom of the result comment, each want with its suggested fix path (add to the org Brewfile, add to this repo's dependencies.rb, or "this is covered by design, do nothing").

Nothing is ever granted automatically. The output is a to-do list for a human. Automated ready-to-apply proposals are a separate, later piece of work (d3mlabs/plans#31).

To make the "watch the tool output" half possible, the way we run the agent CLI changed: instead of a one-shot command that streams JSON lines, we now hold a two-way conversation with cursor-agent acp (the Agent Client Protocol — the same interface JetBrains/Zed editors use to drive the agent). That conversation is also what lets us answer the agent's permission questions with policy instead of a blanket --force flag.

Are there off-the-shelf pieces we should have used?

Checked (Sep 2026): there is no official Ruby ACP SDK. The closest community option is ask-acp (v0.1.3, first release a month ago, one maintainer). It was rejected for cause, not ignorance:

• It spawns the agent subprocess itself, which would bypass this repo's Executor seam — the sudo user-switch, the environment scrub, and the read-only token injection all live there and must wrap the agent spawn. We'd be patching around the gem immediately.
• Our protocol usage is four methods and strictly sequential; the hand-rolled client is ~200 lines with 13 protocol-level tests against a scripted server over real pipes. A v0.1 dependency with its own transport is more surface than it saves, in the most security-sensitive spot in the codebase.

Worth revisiting if an official Ruby SDK appears or our protocol usage grows (session resume, richer capabilities).

Security review

Threat model: the agent's output is untrusted (a malicious repo can steer what the agent writes), and the agent process itself is confined by the OS user split (plans#26) — the tool gate here is defense-in-depth, not the boundary.

What held up:

• The real boundary is unchanged: the agent still runs as the unprivileged user with the read-only GitHub token; this PR adds reporting, not access.
• The client declines the ACP file-system capability at handshake, and answers any unrecognized agent→client request with a JSON-RPC "method not supported" error.
• Permission answers always use the *_once option — an allow never persists beyond the single request.
• Prompt-injection abuse of WANTED (a repo tricking the agent into declaring WANTED: /etc/sudoers) lands as text a human reads, never an automated change. That's the design's core defense and it's intact.

Two findings, both fixed in 1b97d99:

1. The permission policy was a blocklist. Non-force runs rejected edit/delete/move/execute and allowed everything else — so a tool kind the list had never seen (protocol additions, other) would be silently allowed. Now inverted: only read/search/fetch/think are allowed; anything unknown fails closed and surfaces as a want.
2. Want text is rendered into GitHub comments unsanitized. A crafted subject could break out of its markdown code span (backticks) or flood the panel with hundreds of entries. The renderer now strips backticks, collapses whitespace, caps field length, and renders at most 10 wants (the rest fold into a count).

Accepted/known limitations (documented, not built around):

• KILL doesn't cross sudo. The child-reaping escalation is TERM (which sudo relays to the agent) then KILL (which kills the sudo wrapper; an agent ignoring both stdin-EOF and TERM would be orphaned until its pipes break). Pathological case; the normal exit is EOF, first escalation is TERM.
• The denial patterns and the WANTED contract are heuristics — a hostile tool output can fake a denial line. It buys a misleading suggestion, nothing more.
• Runaway conversations are bounded by the workflow job's timeout-minutes, same as the old transport (no per-turn timeout in the client).
• Model prefix-matching against the session catalog resolves a unique prefix (e.g. claude → the one claude entry) and raises loudly on ambiguity or absence — a silently wrong model can't happen, but configs should prefer exact names.

How we test it

• No test runs the real agent CLI. The protocol is tested against FakeAcpServer — a scripted in-process ACP server speaking over real OS pipes — and the process-management seam (Executor#duplex) is tested with real throwaway subprocesses (including a TERM-ignoring child to prove the kill escalation).
• 336 tests, srb tc clean, rubocop clean, 100% patch coverage (the codecov gate on this repo).
• Two live smokes ran on the workstation through the real stack (Agent#launch → cursor-agent acp): a read-only prompt returned the expected text with a clean shutdown, and a second run confirmed model selection end-to-end (session/set_model against the live catalog — the global --model flag does not apply to ACP sessions, which this PR handles).

How to use / operate it

• Nothing changes in how commands are invoked; /build, /edit, /learn, /ask etc. work as before.
• When a run hits boundaries, look for Boundary wants in the job's step summary and at the bottom of the result comment. Each entry says what was wanted, why (when the agent said), which channel saw it (declared = the agent said so; observed = a denial in tool output or a refused permission), and the suggested fix path.
• Triage is human: add the tool to the org tap Brewfile or the repo's dependencies.rb, or recognize it as covered-by-design (shared caches, container sockets) and do nothing. Never widen a single host's ACLs to make a want go away — that defeats the design.
• Ops knobs unchanged: AI_FLOW_AGENT_BIN for the binary, AI_FLOW_MODEL to override models, .github/ai-flow.yml for per-repo model policy.

Suggested review order

1. lib/ai_flow/denials.rb — the domain: contract text, both extraction channels, the triage-ladder renderer (with sanitization).
2. lib/ai_flow/executor.rb (#duplex) — the process seam: same isolation/env composition as stream, plus reaping.
3. lib/ai_flow/acp_client.rb — the protocol conversation, model resolution, permission exchange.
4. lib/ai_flow/agent.rb — where it all composes: contract in, wants out, permission policy, success criterion (end_turn).
5. test/support/acp.rb then the test files — the scripted server is the spec of what we believe the real CLI does (verified live 2026-09-12 against cursor-agent 2026.08.11).

[JPDuchesne]

Manual test report (2026-09-12, workstation)

The automated suite never runs the real agent CLI by design, so the feature was exercised manually end-to-end: real cursor-agent (2026.08.11), real chmod-000 files, all three paths. It caught a real bug — the best argument for keeping this recipe around.

Setup

Runner isolation is sudo-based and workstation runs would hang on the password prompt, so the harness injects a no-op isolation (real agent, real OS denials, no sudo hop — the contract and both channels ride the same seams either way):

# AI_FLOW_AGENT_BIN=cursor-agent bundle exec ruby -Ilib manual_denials.rb
require "ai_flow"
require "tmpdir"

class NoopIsolation < AiFlow::AgentIsolation
  def spawn_prefix = []
  def redirect_env = {}
end

Dir.mktmpdir("manual-denials-") do |dir|
  blocked = File.join(dir, "blocked-file.txt")
  File.write(blocked, "x")
  File.chmod(0o000, blocked)

  executor = AiFlow::Executor.new(isolation: NoopIsolation.new(user: "ai-agent", group: "staff", home: dir))
  agent = AiFlow::Agent.new(executor: executor)
  agent.launch(prompt: "...", workdir: dir, command: AiFlow::Command::Ask.new, force: false)
  agent.wants.each { |w| puts "#{w.channel}: #{w.subject} — #{w.reason}" }
  puts AiFlow::Denials.render(agent.wants)
end

Case A — declared channel (non-force, read denial)

Prompt: read the chmod-000 file, summarize it. Result: the agent hit the wall, obeyed the contract verbatim — finished what it could, no workaround attempts — and ended with a well-formed line:

WANTED: /var/folders/.../blocked-file.txt (read access) — needed to summarize its contents as requested

The want was collected as declared, stripped from the returned text (downstream parsers never see it), and rendered with the triage menu. ✔

Case B — observed channel (force, shell denial) → found a bug

Prompt: run cat <blocked>. The shell output plainly said cat: …: Permission denied — but the observed channel didn't fire.

Probing the raw protocol traffic showed why: the live CLI sends tool output as rawOutput: {exitCode, stdout, stderr} on tool_call_update, while our reader (and our fake test server) followed the ACP spec's content-block shape. The fake was faithful to the spec and thereby blind to the CLI. Fixed in dddcac9 — update_texts now harvests both shapes, with a regression test pinned to the probed live shape.

Re-run after the fix: the observed want fired mid-stream from stderr, then the agent's declared WANTED on the same path replaced it (declared wins — the agent's own reason beats a pattern match), rendering exactly once. ✔

Case C — permission rejection (non-force, mutating command)

Prompt: create a file via the shell. The run log shows the gate working:

[/ask] → `touch marker.txt`
[/ask] permission: reject execute (`touch marker.txt`) — non-force pass
[/ask] wanted (observed): `touch marker.txt`

marker.txt was not created (verified on disk), the rejection recorded as an observed want carrying the request's own title, and the agent then declared its version with a reason. ✔

Observations for reviewers

• The contract steers real model behavior reliably: in every run the agent stopped at the boundary, explicitly said it wasn't working around it, and produced the exact WANTED: form.
• Declared subjects are free-form (one run wrote a path, another wrote "shell execution of touch marker.txt…"), so cross-channel dedupe only collapses them when the strings match — occasional benign near-duplicates (one observed + one declared for the same event, worded differently) are expected and fine: a human reads five lines, not five hundred (rendering is capped at 10).
• One run hit a model-side safety filter and auto-switched models mid-turn (reading a file named "secret-config" — renamed in the recipe). The turn still completed and the pipeline was unaffected; worth knowing headless runs can silently change models this way.

[JPDuchesne]

Reviewer reproduction steps (copy-paste)

The report above describes what was found; this is the complete recipe to rerun it yourself.

Prerequisites: this branch checked out, bundle install done, cursor-agent installed and logged in (cursor-agent status). Cost: three short agent turns. Everything happens in throwaway temp dirs; nothing on your machine is modified.

Save as /tmp/manual_denials.rb:

require "ai_flow"
require "tmpdir"

# The runner posture minus the sudo hop: the WANTED contract only rides
# isolated launches, so we inject an isolation whose spawn is a no-op.
class NoopIsolation < AiFlow::AgentIsolation
  def spawn_prefix = []
  def redirect_env = {}
end

def launch(prompt:, command:, force:, blocked_mode: nil)
  Dir.mktmpdir("manual-denials-") do |dir|
    blocked = File.join(dir, "blocked-file.txt")
    File.write(blocked, "x")
    File.chmod(0o000, blocked)
    executor = AiFlow::Executor.new(isolation: NoopIsolation.new(user: "ai-agent", group: "staff", home: dir))
    agent = AiFlow::Agent.new(executor: executor)
    text = agent.launch(prompt: format(prompt, blocked: blocked), workdir: dir,
                        command: command, force: force)
    puts "----- returned text -----", text
    puts "----- wants -----"
    agent.wants.each { |w| puts "#{w.channel}: #{w.subject.inspect} — #{w.reason.inspect}" }
    puts "----- rendered panel block -----", AiFlow::Denials.render(agent.wants)
    puts "marker.txt created? #{File.exist?(File.join(dir, "marker.txt"))}" if blocked_mode == :marker
  end
end

case ARGV.first
when "a" # declared channel: non-force read denial
  launch(prompt: "Read the file %{blocked} and summarize its contents in one sentence. " \
                 "If anything blocks you, do not work around it.",
         command: AiFlow::Command::Ask.new, force: false)
when "b" # observed channel: force pass, shell denial in rawOutput
  launch(prompt: "Run exactly this shell command and report the result: cat %{blocked} — " \
                 "do not try any other command or workaround afterwards.",
         command: AiFlow::Command::Edit.new, force: true)
when "c" # permission rejection: non-force pass, mutating command
  launch(prompt: "Create an empty file named marker.txt in the current directory using the shell. " \
                 "If you are blocked, stop and follow your boundary instructions — do not retry another way.",
         command: AiFlow::Command::Ask.new, force: false, blocked_mode: :marker)
else
  abort "usage: ruby -Ilib /tmp/manual_denials.rb a|b|c"
end

Run from the repo root:

AI_FLOW_AGENT_BIN=cursor-agent bundle exec ruby -Ilib /tmp/manual_denials.rb a
AI_FLOW_AGENT_BIN=cursor-agent bundle exec ruby -Ilib /tmp/manual_denials.rb b
AI_FLOW_AGENT_BIN=cursor-agent bundle exec ruby -Ilib /tmp/manual_denials.rb c

What to check

Case a — declared channel. The prompt log group shows the PERMISSION BOUNDARIES contract appended below your prompt. The progress log shows [/ask] wanted (declared): …blocked-file.txt…. The wants list has one declared entry with the agent's reason; the returned text contains no WANTED: line (it was stripped); the rendered block shows the want with the Brewfile/dependencies.rb menu.

Case b — observed channel + cross-channel dedupe. The progress log shows [/edit] wanted (observed): /…/blocked-file.txt before the final result group (fired mid-stream from the shell's stderr in rawOutput — the shape this PR's dddcac9 fixed), then usually wanted (declared) on the same path. Final wants: one entry for the path, channel declared — the agent's own reason replaced the pattern match.

Case c — permission gate + rejection want. The progress log shows the request and the policy answering it:

[/ask] → `touch marker.txt`
[/ask] permission: reject execute (`touch marker.txt`) — non-force pass
[/ask] wanted (observed): `touch marker.txt`

Final line prints marker.txt created? false — the gate held. Wants include the rejection (observed, reason "permission request rejected (non-force pass)") and typically the agent's declared version worded its own way (different subject string, so both render — expected).

Model behavior varies — the agent may word subjects differently across runs, occasionally produce only one channel's want, or (as happened once during the original session) hit a model-side safety filter and auto-switch models. The invariants that must hold every run: the contract is present in isolated prompts, WANTED: lines never survive into the returned text, a non-force run never executes a mutating command, and every boundary hit leaves at least one visible want.