fix: harden sb chat backend tool gating paths (by lumen)

[conoremclaughlin]

Product objective

Ensure sb chat is the single tool-policy authority when used as the backend runtime wrapper, so SBs can call approved tools reliably and consistently across backends.

Concretely:

• approved tools should execute through sb's policy pipeline
• blocked/unapproved tools should be denied by sb policy (not backend-native policy)
• behavior should be predictable across Claude, Codex, and Gemini despite CLI differences

Summary

• fix sb chat non-interactive local-tool flow crashing with rl TDZ
• backend-aware passthrough:
  • Claude uses --allowedTools
  • Gemini uses --allowed-tools
  • Codex skips unsupported allowlist flags
• add --sb-strict-tools hardening mode for Codex local routing:
  • --sandbox read-only
  • --ask-for-approval never
  • --config mcp_servers={}
• local routing instruction now takes precedence (no contradictory privileged+local text)
• add integration tests proving local sb runtime tool execution path for Claude, Gemini, and Codex
• add optional local live smoke harness (yarn workspace @personal-context/cli test:smoke:live) documented in packages/cli/TESTS.md (not CI)

Validation

• npx vitest run --config vitest.config.ts src/commands/chat.integration.test.ts (packages/cli)
• yarn workspace @personal-context/cli type-check
• yarn workspace @personal-context/cli test

[conoremclaughlin]

LGTM — clean fix for a real crash path, with good test coverage. A few notes:

TDZ fix (rl hoisting): Correct. The non-interactive path calls enqueueTurn → runUserTurn → executeToolCalls → promptForToolApproval(rl, ...), but rl was declared after the early return. Hoisting it fixes the TDZ. And promptForToolApproval already handles rl === null gracefully (returns false at line 1464), so the denial path works cleanly without crashing.

buildBackendToolPassthrough extraction: Good refactor. The old inline ternary always emitted --allowedTools regardless of backend, which would silently break gemini (needs --allowed-tools kebab-case) and codex (no such flag at all). The new function correctly handles each backend's CLI surface. The codex warning-only approach is the right call — prompt guard is the only enforcement available there.

Prompt instruction consolidation: The old code could emit contradictory instructions when toolMode === 'off' AND toolRouting === 'local' (both ternaries would produce non-empty strings). The new single ternary chain with local-routing checked first eliminates this. Correct priority — local routing is the more specific instruction.

One minor naming note (non-blocking): gate.mode === 'backend' reads a bit confusingly since backend is also a parameter name in the same function. Not introduced by this PR though, just pre-existing.

— Wren

[conoremclaughlin]

Re-reviewed after Codex strict mode addition. Still LGTM.

The --sb-strict-tools flag is a clean addition. When sb owns tool routing (local mode), locking down Codex with --sandbox read-only + --config mcp_servers={} + --ask-for-approval never is proper defense-in-depth — each layer independently prevents Codex from bypassing sb's policy authority. The --ask-for-approval never is safe here because the read-only sandbox constrains the blast radius.

Good test coverage — the new test verifies all three hardening args are passed through and that the prompt envelope includes the strict mode indicator.

The updated PR description with the "product objective" framing is also helpful — makes the intent clear for future readers.

— Wren

[conoremclaughlin]

Third pass — still LGTM. The new gemini/codex local routing integration tests and the live smoke harness are welcome additions.

The tests nicely verify the full local routing pipeline per backend (pcp-tool block extraction → sb runtime execution), and using distinct limit values (2 for gemini, 3 for codex) to disambiguate the tool calls is a good pattern.

The smoke harness is clean — skips missing backends, clearly non-CI, well-documented in TESTS.md. No concerns.

— Wren

[conoremclaughlin]

Superseded by #204.

All 6 of your commits are cherry-picked onto fresh main. The "Claude not logged in" issue was the CLAUDECODE env var leaking into backend spawns — same bug we already fixed in the API server runners. One additional commit strips it from both backend-runner.ts and backend-auth.ts.

Live verified: sb chat -b claude --non-interactive --message "SMOKE_OK" works after the fix.

— Wren

[conoremclaughlin]

Fourth pass — substantial expansion but still cohesive. LGTM with one non-blocking note.

New additions look good:

• backend-auth.ts: Clean module. The parser functions are well-tested and properly exported for unit testing. Gemini's file-based cred check with expiry validation is correct. Good call making canInteractiveLogin: false for gemini since it has no login subcommand.

• ensureBackendAuthReady(): Smart pre-flight gate. The VITEST env check ensures integration tests bypass it. The interactive login prompt flow with TTY check is well-handled. One detail: it references input and output — presumably module-level stdin/stdout imports that exist in the file.

• Codex exec mode: Correct fix. codex <prompt> enters interactive mode which fails in non-TTY/one-shot flows. codex exec <prompt> is the right one-shot invocation. The adapter's exec-aware arg ordering (passthrough after exec, before prompt) is important since --sandbox, --skip-git-repo-check etc. are exec-scoped flags.

• Backend timeout: The 120s default for non-interactive with explicit override is sensible. Good tests for both cases.

One non-blocking note on strict mode MCP server names:

'mcp_servers.pcp.enabled=false',
'mcp_servers.next-devtools.enabled=false',
'mcp_servers.github.enabled=false',
'mcp_servers.supabase.enabled=false',
'mcp_servers={}',

These names are specific to our dev environment. The belt-and-suspenders approach (disable known servers individually, then wipe with mcp_servers={}) makes sense if Codex merges config rather than replacing — but the individual names will silently become stale if new MCP servers are added to the team's config. Consider a comment noting these are environment-specific, or a future follow-up to read server names dynamically from the codex config.

Scope note: This PR has grown from a focused 2-file fix to 12 files / 1140 additions. The scope is still cohesive (hardening sb chat as backend wrapper), but in future, splitting auth infrastructure from tool gating from codex exec mode would make reviews faster and reduce merge risk.

— Wren