fix(sa-mode): emit caller-side SA guard for Layer 0 enforcement#530
fix(sa-mode): emit caller-side SA guard for Layer 0 enforcement#530RyderFreeman4Logos merged 2 commits intomainfrom
Conversation
…ment --sa-mode true previously only constrained the callee (AI tool inside CSA session) via anti-recursion guards. The caller (e.g., Claude Code) received no signal to operate as a pure orchestrator, allowing it to read/edit code directly — violating the Layer 0 Manager contract. Now when --sa-mode true is active at root depth (CSA_DEPTH=0), a structured <csa-caller-sa-guard> block is emitted to stdout at two points: 1. Pre-session — immediately on CSA startup (before session work begins) 2. Post-session — after command handler returns (reminder before next action) The guard block clearly states FORBIDDEN and ALLOWED actions, reinforcing that all code work must be delegated to CSA sub-agents. All 6 SA-capable commands (run, review, debate, batch, plan run, claude-sub-agent) emit the post-session guard consistently. Closes #162. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request implements a 'SA mode caller guard' mechanism within the cli-sub-agent. It introduces a structured XML block that defines constraints for the 'Layer 0 Manager' orchestrator, ensuring implementation work is delegated to sub-agents. This guard is emitted to stdout at the start and completion of various commands (Run, Review, Debate, etc.) when SA mode is active at the root depth. The changes also include unit tests for the emission logic and a version bump to 0.1.196. Feedback suggests refactoring the repeated guard emission and process exit logic into a helper function to reduce code duplication across command handlers.
| crate::pipeline::prompt_guard::emit_sa_mode_caller_guard(sa_mode_active, current_depth); | ||
| let _ = std::io::stdout().flush(); | ||
| let _ = std::io::stderr().flush(); | ||
| std::process::exit(exit_code); |
There was a problem hiding this comment.
This block of code for emitting the SA guard, flushing streams, and exiting is duplicated across several command arms (Run, Review, Debate, ClaudeSubAgent). To improve maintainability and reduce code duplication, consider extracting this logic into a helper function.
For example, you could create a function exit_with_sa_guard(exit_code: i32, sa_mode_active: bool, current_depth: u32) -> ! that handles these final steps. This would make the code cleaner and less error-prone when adding new commands in the future.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71891160cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if !sa_mode || depth > 0 { | ||
| return false; | ||
| } | ||
| println!("{SA_MODE_CALLER_GUARD}"); |
There was a problem hiding this comment.
Keep daemon session ID output machine-readable
emit_sa_mode_caller_guard now always prints to stdout, which breaks the csa run --daemon --sa-mode true output contract: the daemon path documents stdout as a machine-readable session ID (run_cmd_daemon::spawn_and_exit prints only the ID for script capture). With the new guard emitted first, stdout becomes multi-line XML+ID, so automation that expects a raw session ID will parse the wrong value or fail.
Useful? React with 👍 / 👎.
| if !sa_mode || depth > 0 { | ||
| return false; | ||
| } | ||
| println!("{SA_MODE_CALLER_GUARD}"); |
There was a problem hiding this comment.
Avoid corrupting JSON stdout with SA guard output
The new println! in emit_sa_mode_caller_guard is unconditional for SA mode at depth 0, so execution commands that support --format json now emit XML on stdout before/after their JSON payload. This regresses machine parsing for run/review/debate in SA mode, and it conflicts with existing code paths that explicitly suppress side-output to avoid polluting JSON CLI output (e.g., hooks runner).
Useful? React with 👍 / 👎.
…tion Address bot review findings on PR #530: - P1 (codex): guard XML on stdout corrupts --format json output - Add text_mode parameter to emit_sa_mode_caller_guard - Only emit when output_format is Text - Add test for JSON mode skip Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Bot Finding Responsesgemini-code-assist — Refactor repeated guard+flush+exit (medium)Acknowledged, deferred. The repetition (7 call sites) is cosmetic — each site has correct args and the pattern is clear. A helper closure could reduce it, but the function call is the right granularity for a 1-line emission. Will revisit if more SA-capable commands are added. codex — P1: SA guard corrupts JSON stdoutFixed in commit e27bea3. Added codex — P1: Daemon session ID output contractNot an issue. In daemon mode ( All findings addressed or explicitly dismissed with rationale. |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the workspace version to 0.1.196 and introduces a 'Sub-Agent (SA) mode prompt guard' to enforce role constraints on calling agents. The guard is implemented as a structured XML block emitted to stdout at the start and end of command executions when running at root depth in text mode. Review feedback includes a suggestion to add a leading newline to the guard output for more robust parsing and several recommendations to inline the guard emission logic across various command branches to reduce boilerplate and improve code conciseness.
| if !sa_mode || depth > 0 || !text_mode { | ||
| return false; | ||
| } | ||
| println!("{SA_MODE_CALLER_GUARD}"); |
There was a problem hiding this comment.
The guard block is emitted to stdout and may merge with the preceding tool output if that output lacks a trailing newline. Adding a leading newline ensures the XML tags are correctly placed on a new line, which is important for structured parsing by the calling agent.
| println!("{SA_MODE_CALLER_GUARD}"); | |
| println!("\n{SA_MODE_CALLER_GUARD}"); |
| let text_mode = matches!(output_format, OutputFormat::Text); | ||
| crate::pipeline::prompt_guard::emit_sa_mode_caller_guard( | ||
| sa_mode_enabled, | ||
| current_depth, | ||
| text_mode, | ||
| ); |
There was a problem hiding this comment.
This multi-line call is repeated across several command branches in this file. Inlining the text_mode check and the function call into a single line would significantly reduce boilerplate and improve the readability of the command dispatch logic.
| let text_mode = matches!(output_format, OutputFormat::Text); | |
| crate::pipeline::prompt_guard::emit_sa_mode_caller_guard( | |
| sa_mode_enabled, | |
| current_depth, | |
| text_mode, | |
| ); | |
| crate::pipeline::prompt_guard::emit_sa_mode_caller_guard(sa_mode_enabled, current_depth, matches!(output_format, OutputFormat::Text)); |
| crate::pipeline::prompt_guard::emit_sa_mode_caller_guard( | ||
| sa_mode_active, | ||
| current_depth, | ||
| matches!(output_format, OutputFormat::Text), | ||
| ); |
There was a problem hiding this comment.
This block is repeated in 6 different command branches. Inlining it to a single line reduces noise in the run() function. Consider centralizing this logic if more commands are added in the future.
| crate::pipeline::prompt_guard::emit_sa_mode_caller_guard( | |
| sa_mode_active, | |
| current_depth, | |
| matches!(output_format, OutputFormat::Text), | |
| ); | |
| crate::pipeline::prompt_guard::emit_sa_mode_caller_guard(sa_mode_active, current_depth, matches!(output_format, OutputFormat::Text)); |
| crate::pipeline::prompt_guard::emit_sa_mode_caller_guard( | ||
| sa_mode_active, | ||
| current_depth, | ||
| matches!(output_format, OutputFormat::Text), | ||
| ); |
There was a problem hiding this comment.
Inlining this call to a single line would improve readability and consistency with other branches.
| crate::pipeline::prompt_guard::emit_sa_mode_caller_guard( | |
| sa_mode_active, | |
| current_depth, | |
| matches!(output_format, OutputFormat::Text), | |
| ); | |
| crate::pipeline::prompt_guard::emit_sa_mode_caller_guard(sa_mode_active, current_depth, matches!(output_format, OutputFormat::Text)); |
| crate::pipeline::prompt_guard::emit_sa_mode_caller_guard( | ||
| sa_mode_active, | ||
| current_depth, | ||
| matches!(output_format, OutputFormat::Text), | ||
| ); |
There was a problem hiding this comment.
Inlining this call to a single line would improve readability and consistency with other branches.
| crate::pipeline::prompt_guard::emit_sa_mode_caller_guard( | |
| sa_mode_active, | |
| current_depth, | |
| matches!(output_format, OutputFormat::Text), | |
| ); | |
| crate::pipeline::prompt_guard::emit_sa_mode_caller_guard(sa_mode_active, current_depth, matches!(output_format, OutputFormat::Text)); |
| crate::pipeline::prompt_guard::emit_sa_mode_caller_guard( | ||
| sa_mode_active, | ||
| current_depth, | ||
| matches!(output_format, OutputFormat::Text), | ||
| ); |
There was a problem hiding this comment.
Inlining this call to a single line would improve readability and consistency with other branches.
| crate::pipeline::prompt_guard::emit_sa_mode_caller_guard( | |
| sa_mode_active, | |
| current_depth, | |
| matches!(output_format, OutputFormat::Text), | |
| ); | |
| crate::pipeline::prompt_guard::emit_sa_mode_caller_guard(sa_mode_active, current_depth, matches!(output_format, OutputFormat::Text)); |
| crate::pipeline::prompt_guard::emit_sa_mode_caller_guard( | ||
| sa_mode_active, | ||
| current_depth, | ||
| matches!(output_format, OutputFormat::Text), | ||
| ); |
There was a problem hiding this comment.
Inlining this call to a single line would improve readability and consistency with other branches.
| crate::pipeline::prompt_guard::emit_sa_mode_caller_guard( | |
| sa_mode_active, | |
| current_depth, | |
| matches!(output_format, OutputFormat::Text), | |
| ); | |
| crate::pipeline::prompt_guard::emit_sa_mode_caller_guard(sa_mode_active, current_depth, matches!(output_format, OutputFormat::Text)); |
| crate::pipeline::prompt_guard::emit_sa_mode_caller_guard( | ||
| sa_mode_active, | ||
| current_depth, | ||
| matches!(output_format, OutputFormat::Text), | ||
| ); |
There was a problem hiding this comment.
Inlining this call to a single line would improve readability and consistency with other branches.
| crate::pipeline::prompt_guard::emit_sa_mode_caller_guard( | |
| sa_mode_active, | |
| current_depth, | |
| matches!(output_format, OutputFormat::Text), | |
| ); | |
| crate::pipeline::prompt_guard::emit_sa_mode_caller_guard(sa_mode_active, current_depth, matches!(output_format, OutputFormat::Text)); |
Re-review Response (Round 2)gemini-code-assist — Repeated guard emission calls (medium x7)Acknowledged, deferred. Same feedback as round 1. The repetition is cosmetic; each call site correctly passes gemini-code-assist — Guard may merge with preceding tool output (medium)Not an issue. The guard is wrapped in All findings are cosmetic. No blocking issues. Proceeding to merge. |
|
Thank you for the detailed follow-up and for clarifying the rationale behind the current implementation. I understand that the repetition is a conscious design choice for granularity and that the XML tags provide sufficient structural boundaries for the calling agent. Given the context provided and the verification that all findings have been addressed or deferred with clear reasoning, I have no further concerns. This looks ready for merge. |
…tion Address bot review findings on PR #530: - P1 (codex): guard XML on stdout corrupts --format json output - Add text_mode parameter to emit_sa_mode_caller_guard - Only emit when output_format is Text - Add test for JSON mode skip Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fix(sa-mode): emit caller-side SA guard for Layer 0 enforcement
Summary
<csa-caller-sa-guard>stdout emission when--sa-mode trueis active at root depthapply_sa_mode_prompt_guardreturn type changed fromResult<()>toResult<bool>to propagate SA stateProblem
--sa-mode trueonly constrained the callee (AI tool inside CSA session) via anti-recursion guards. The caller (e.g., Claude Code) had no enforcement signal, allowing direct code reads/edits that violate the Layer 0 Manager contract.Solution
When SA mode is active at depth 0, a structured XML guard block is emitted to stdout so the calling agent sees it in the Bash tool output. The block clearly states FORBIDDEN and ALLOWED actions, reinforcing that all code work must be delegated to CSA sub-agents.
Test plan
just pre-commitexit 0csa review --range main...HEADverdict: PASSCloses #162
🤖 Generated with Claude Code