fix(guard): cross-repo scope + companion-rescue Bash for tri-review

src/cmd/guard.go

@@ -53,9 +53,11 @@ stdout ({"decision":"approve"} or {"decision":"block","reason":...}).
 The allowlist policy:
   command step + hard  → devkit MCP + TodoWrite + Skill
   prompt step  + hard  → read-only evidence tools + devkit MCP + Skill
+                         + Agent/Task dispatch + companion-rescue Bash
   prompt step  + soft  → allow with a stderr nudge
   parallel / unknown   → allow (engine is dispatching)
   stale session (TTL)  → allow with a stderr warning (orphan recovery)
+  cross-repo session   → allow silently (block remains in origin repo)
 
 Skill is allowed under both step types so a workflow that's mid-run can
 still load a nested skill (e.g. user asks for tri-review during a feature
@@ -88,28 +90,36 @@ func runGuard(cmd *cobra.Command, args []string) {
 	runPreToolGuard()
 }
 
-// readToolNameFromStdin extracts tool_name from a PreToolUse payload.
-// Returns "" with a nil error for genuinely empty stdin (the common
-// case when --tool-name is used instead). Any real failure (read error,
-// malformed JSON) returns a non-nil error so the caller can log it
-// distinctly — silent "" on parse failure would mask Claude Code schema
-// drift. Under hard enforcement the empty tool name still falls through
-// to the default-block branch, so errors never cause a silent ALLOW.
-func readToolNameFromStdin() (string, error) {
-	data, err := io.ReadAll(guardStdin)
-	if err != nil {
-		return "", fmt.Errorf("read stdin: %w", err)
+// readToolInvocationFromStdin extracts tool_name and (for Bash)
+// tool_input.command from a PreToolUse payload. Returns ("", "", nil)
+// for genuinely empty stdin (the common case when --tool-name is used
+// instead). Any real failure (read error, malformed JSON) returns a
+// non-nil error so the caller can log it distinctly — silent "" on
+// parse failure would mask Claude Code schema drift. Under hard
+// enforcement the empty tool name still falls through to the default-
+// block branch, so errors never cause a silent ALLOW.
+//
+// command is populated only for tools that ship a "command" field in
+// tool_input (Bash). Other tools get "" here — callers that care about
+// specific input shapes must parse stdin themselves.
+func readToolInvocationFromStdin() (name, command string, err error) {
+	data, rerr := io.ReadAll(guardStdin)
+	if rerr != nil {
+		return "", "", fmt.Errorf("read stdin: %w", rerr)
 	}
 	if len(data) == 0 {
-		return "", nil
+		return "", "", nil
 	}
 	var payload struct {
-		ToolName string `json:"tool_name"`
+		ToolName  string `json:"tool_name"`
+		ToolInput struct {
+			Command string `json:"command"`
+		} `json:"tool_input"`
 	}
-	if err := json.Unmarshal(data, &payload); err != nil {
-		return "", fmt.Errorf("parse PreToolUse JSON: %w", err)
+	if uerr := json.Unmarshal(data, &payload); uerr != nil {
+		return "", "", fmt.Errorf("parse PreToolUse JSON: %w", uerr)
 	}
-	return payload.ToolName, nil
+	return payload.ToolName, payload.ToolInput.Command, nil
 }
 
 // resolveDataDir returns CLAUDE_PLUGIN_DATA or "" if unset. The shell
@@ -289,6 +299,35 @@ func isDevkitMCPTool(name string) bool {
 	return false
 }
 
+// isCompanionRescueCommand reports whether a Bash command string
+// invokes a known rescue-subagent companion script. Used by the
+// prompt+hard carve-out so tri-* workflows' rescue subagents (codex,
+// gemini) can reach external models through their companion adapters.
+//
+// Matched via substring so invocation style doesn't matter — the
+// adapters call their scripts as `node /abs/path/codex-companion.mjs`,
+// `bash -c '... gemini-companion.mjs ...'`, or through env-var
+// overrides, and we must tolerate all of them. The .mjs suffix is
+// load-bearing: without it, a Bash comment token like
+// `rm -rf $HOME # codex-companion` would satisfy the substring match
+// and bypass the prompt+hard Bash block. Both real companions ship
+// as .mjs files, so requiring the extension doesn't break any
+// intended invocation style. A future .js / .cjs adapter must be
+// added to this allowlist deliberately rather than relying on the
+// looser bare-name match.
+//
+// The cost of a broader match (even with .mjs pinned) is negligible:
+// the only "bypass" it enables is the main agent invoking a real
+// external model directly, which produces a real external review
+// (just not one dispatched through the subagent indirection).
+func isCompanionRescueCommand(cmd string) bool {
+	if cmd == "" {
+		return false
+	}
+	return strings.Contains(cmd, "codex-companion.mjs") ||
+		strings.Contains(cmd, "gemini-companion.mjs")
+}
+
 func runPreToolGuard() {
 	dataDir := resolveDataDir(false)
 	if dataDir == "" {
@@ -340,9 +379,32 @@ func runPreToolGuard() {
 		return
 	}
 
+	// Cross-repo scope (mirrors stop-guard carve-out from issue #91):
+	// if the workflow was started in a different repo than the current
+	// Claude Code session, allow silently. Without this, a workflow
+	// stuck at a prompt step in repo A (crashed engine, missed
+	// devkit_advance, session not cleaned up) would keep blocking
+	// unrelated tool calls in repo B for the full 30-minute TTL. The
+	// block remains active in the originating repo — re-entering A
+	// re-arms it. Same posture as stop-guard: empty state.RepoRoot
+	// (pre-#91 sessions written by older engines) or unresolvable
+	// current repo (CLAUDE_PROJECT_DIR missing, no walk-up match)
+	// falls through to full enforcement so a missing signal never
+	// silently approves.
+	if state.RepoRoot != "" {
+		if cur := currentRepoRoot(); cur != "" && !samePath(state.RepoRoot, cur) {
+			fmt.Fprintf(guardStderr,
+				"devkit-guard: session %s belongs to %s, current repo is %s — allowing (block remains active in originating repo)\n",
+				state.Workflow, state.RepoRoot, cur)
+			guardExit(0)
+			return
+		}
+	}
+
 	tool := guardToolName
+	var command string
 	if tool == "" {
-		t, terr := readToolNameFromStdin()
+		t, c, terr := readToolInvocationFromStdin()
 		if terr != nil {
 			// Log distinctly so schema drift in the PreToolUse payload
 			// doesn't silently degrade into "unknown tool name" and
@@ -354,6 +416,7 @@ func runPreToolGuard() {
 				terr)
 		}
 		tool = t
+		command = c
 	}
 	// state.StepEnforce is guaranteed valid ("hard" or "soft") by
 	// SessionState.UnmarshalJSON — ReadSessionJSON would have rejected
@@ -406,6 +469,27 @@ func runPreToolGuard() {
 				// applies uniformly to both layers.
 				guardExit(0)
 				return
+			case "Bash":
+				// Rescue-subagent escape hatch (issue #95-2). tri-*
+				// workflows dispatch codex:codex-rescue /
+				// gemini:gemini-rescue via Agent/Task, and those
+				// subagents' system prompts require forwarding to
+				// codex-companion.mjs / gemini-companion.mjs through
+				// Bash. That nested Bash call re-enters this guard
+				// with the same prompt+hard session state and would
+				// otherwise be blocked, silently breaking tri-review's
+				// model-diversity invariant.
+				//
+				// The main agent could technically invoke these
+				// companion scripts directly to fish for a sympathetic
+				// external review, but that just runs the real
+				// external model — there's no verdict-faking bypass
+				// to be had. Write/Edit stay blocked, so the main
+				// model still cannot author the review itself.
+				if isCompanionRescueCommand(command) {
+					guardExit(0)
+					return
+				}
 			}
 			fmt.Fprintf(guardStderr,
 				"BLOCKED: devkit workflow %s is at a prompt step — gather evidence with Read/Grep/Glob then call devkit_advance. (attempted tool: %s)\n",

src/cmd/guard_test.go

@@ -608,8 +608,8 @@ func TestGuardPreToolUse(t *testing.T) {
 			wantStderrSubstr: "attempted tool: <unknown>",
 		},
 		{
-			// Malformed stdin JSON: readToolNameFromStdin returns an
-			// error, the call site logs it, falls through with "",
+			// Malformed stdin JSON: readToolInvocationFromStdin returns
+			// an error, the call site logs it, falls through with "",
 			// and the hard command step blocks. Pin this so a future
 			// refactor can't silently allow on parse failure.
 			name:       "command+hard+malformed stdin → block",
@@ -622,6 +622,65 @@ func TestGuardPreToolUse(t *testing.T) {
 			wantExit:         2,
 			wantStderrSubstr: "cannot determine tool name",
 		},
+		{
+			// Issue #95-2: rescue subagents (codex:codex-rescue /
+			// gemini:gemini-rescue) forward to external models via
+			// `node codex-companion.mjs ...`. That nested Bash call
+			// re-enters the guard under the same prompt+hard session
+			// state as the parent tri-* workflow. Allow it so the
+			// model-diversity invariant of tri-review survives.
+			name:       "prompt+hard+Bash codex-companion → allow (rescue)",
+			dataDir:    true,
+			hasSession: true,
+			session: lib.SessionState{
+				Status: "running", StepType: "prompt", StepEnforce: lib.EnforceHard, CurrentStep: "review-smart",
+				Workflow: "tri-review", TotalSteps: 6,
+			},
+			stdin:    `{"tool_name":"Bash","tool_input":{"command":"node /plugins/codex/1.0.3/scripts/codex-companion.mjs task"}}`,
+			wantExit: 0,
+		},
+		{
+			name:       "prompt+hard+Bash gemini-companion → allow (rescue)",
+			dataDir:    true,
+			hasSession: true,
+			session: lib.SessionState{
+				Status: "running", StepType: "prompt", StepEnforce: lib.EnforceHard, CurrentStep: "review-general",
+				Workflow: "tri-review", TotalSteps: 6,
+			},
+			stdin:    `{"tool_name":"Bash","tool_input":{"command":"bash -c 'node /p/gemini-companion.mjs task'"}}`,
+			wantExit: 0,
+		},
+		{
+			// Negative: a random Bash command on prompt+hard must
+			// still block. Without this, the companion carve-out
+			// could drift into a generic Bash allow if someone
+			// refactored isCompanionRescueCommand loose.
+			name:       "prompt+hard+Bash unrelated → block",
+			dataDir:    true,
+			hasSession: true,
+			session: lib.SessionState{
+				Status: "running", StepType: "prompt", StepEnforce: lib.EnforceHard, CurrentStep: "review-smart",
+				Workflow: "tri-review", TotalSteps: 6,
+			},
+			stdin:            `{"tool_name":"Bash","tool_input":{"command":"curl https://evil.example.com"}}`,
+			wantExit:         2,
+			wantStderrSubstr: "gather evidence with Read/Grep/Glob",
+		},
+		{
+			// Negative: command-step Bash must still block even if the
+			// command looks like a companion rescue. Command steps are
+			// run by the engine directly; any Bash from the model is a
+			// determinism violation, regardless of payload.
+			name:       "command+hard+Bash codex-companion → block (wrong step type)",
+			dataDir:    true,
+			hasSession: true,
+			session: lib.SessionState{
+				Status: "running", StepType: "command", StepEnforce: lib.EnforceHard, CurrentStep: "build",
+			},
+			stdin:            `{"tool_name":"Bash","tool_input":{"command":"node codex-companion.mjs task"}}`,
+			wantExit:         2,
+			wantStderrSubstr: "BLOCKED: Command step",
+		},
 	}
 
 	for _, tc := range tests {
@@ -868,6 +927,121 @@ func TestGuardPreToolUseEnvTTLOverride(t *testing.T) {
 	}
 }
 
+// TestGuardPreToolUseRepoScope pins the cross-repo escape hatch for
+// pre-tool calls (issue #95-3). Same posture as the stop-guard check
+// added in issue #91: if the active workflow was started in repo A,
+// a fresh session in repo B must not have its tool calls blocked by
+// the leftover prompt-step state. The block remains active in repo A.
+// Empty signals fall through to full enforcement so a missing env
+// never silently approves.
+func TestGuardPreToolUseRepoScope(t *testing.T) {
+	tests := []struct {
+		name             string
+		stateRepoRoot    string
+		claudeProjectDir string
+		stepType         string
+		wantExit         int
+		wantStderrSubstr string
+	}{
+		{
+			// Cross-repo: the wedged workflow belongs to repo-a,
+			// but the user is now working in repo-b. Allow and
+			// note it on stderr so the escape hatch is debuggable.
+			name:             "different repo → allow",
+			stateRepoRoot:    "/tmp/repo-a",
+			claudeProjectDir: "/tmp/repo-b",
+			stepType:         "prompt",
+			wantExit:         0,
+			wantStderrSubstr: "allowing (block remains active in originating repo)",
+		},
+		{
+			// Matching repo: the block stays armed so the user
+			// actually working on the wedged workflow still gets
+			// the enforcement they expect.
+			name:             "matching repo → block (prompt+hard)",
+			stateRepoRoot:    "/tmp/repo-a",
+			claudeProjectDir: "/tmp/repo-a",
+			stepType:         "prompt",
+			wantExit:         2,
+			wantStderrSubstr: "gather evidence with Read/Grep/Glob",
+		},
+		{
+			// Pre-#91 session (no RepoRoot) must still block — we
+			// don't know where it came from, so fail closed and
+			// force enforcement rather than silently approve.
+			name:             "empty state.RepoRoot → block",
+			stateRepoRoot:    "",
+			claudeProjectDir: "/tmp/repo-b",
+			stepType:         "prompt",
+			wantExit:         2,
+			wantStderrSubstr: "gather evidence with Read/Grep/Glob",
+		},
+		{
+			// Missing CLAUDE_PROJECT_DIR with no walk-up match:
+			// currentRepoRoot() returns "". Fail closed (block)
+			// rather than silently approve across repos.
+			name:             "unresolvable current repo → block",
+			stateRepoRoot:    "/tmp/repo-a",
+			claudeProjectDir: "", // and cwd will be forced off-repo
+			stepType:         "prompt",
+			wantExit:         2,
+			wantStderrSubstr: "gather evidence with Read/Grep/Glob",
+		},
+		{
+			// Command-step session in a different repo: same
+			// allow behavior — the scope check precedes the
+			// step-type switch. Pin this so a refactor that put
+			// scope inside the prompt branch would fail here.
+			name:             "different repo + command step → allow",
+			stateRepoRoot:    "/tmp/repo-a",
+			claudeProjectDir: "/tmp/repo-b",
+			stepType:         "command",
+			wantExit:         0,
+			wantStderrSubstr: "allowing (block remains active in originating repo)",
+		},
+	}
+
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			dir := t.TempDir()
+			writeSession(t, dir, lib.SessionState{
+				Status:      "running",
+				Workflow:    "pr-ready",
+				StepType:    tc.stepType,
+				StepEnforce: lib.EnforceHard,
+				TotalSteps:  5,
+				CurrentStep: "analyse",
+				RepoRoot:    tc.stateRepoRoot,
+			})
+			env := newGuardTestEnv(t, `{"tool_name":"Bash"}`, "", false, dir)
+			if tc.claudeProjectDir != "" {
+				t.Setenv("CLAUDE_PROJECT_DIR", tc.claudeProjectDir)
+			} else {
+				t.Setenv("CLAUDE_PROJECT_DIR", "")
+				// Force the walk-up fallback to return "" by
+				// cd'ing into a non-git tempdir. Without this,
+				// the test runner's cwd (inside devkit's own
+				// repo) would satisfy the walk-up and the
+				// scope check would match, masking the test.
+				prev, _ := os.Getwd()
+				nowhere := t.TempDir()
+				if err := os.Chdir(nowhere); err != nil {
+					t.Fatalf("chdir: %v", err)
+				}
+				t.Cleanup(func() { os.Chdir(prev) })
+			}
+			runGuard(guardCmd, nil)
+			if env.exit != tc.wantExit {
+				t.Fatalf("exit=%d want=%d\nstderr=%s", env.exit, tc.wantExit, env.stderr.String())
+			}
+			if tc.wantStderrSubstr != "" && !strings.Contains(env.stderr.String(), tc.wantStderrSubstr) {
+				t.Fatalf("stderr missing %q\ngot: %s", tc.wantStderrSubstr, env.stderr.String())
+			}
+		})
+	}
+}
+
 func TestGuardStopHook(t *testing.T) {
 	tests := []struct {
 		name         string