Skip to content

Commit 1f12e89

Browse files
committed
feat: enhance crew integration checks and recovery mechanisms
- Improved handling of interrupted checks in CrewMergeJournal, including detailed execution states and outputs. - Added functionality to reconcile check executions and validate applied checkouts post-crash. - Introduced a new prompt mechanism for handling stale bridges in crew lanes, allowing for retries and recovery. - Updated UI components to reflect the state of integration checks and custody halts more clearly. - Added tests for new functionalities, including check process liveness and custody scope validation.
1 parent 4645ab7 commit 1f12e89

27 files changed

Lines changed: 533 additions & 75 deletions

‎docs/behavioral-merge-review.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ The sidebar reports the result as a **candidate decision**, not as whether CrewC
3535

3636
The main process atomically persists a merge journal under CrewCode's user-data directory before combination begins and at every phase transition. Each record contains the session, base branch/SHA, lane label, branch, worktree path, owned files and SHA, current phase, discovered check commands, outputs, status, and timestamps.
3737

38-
After restart, an in-flight operation is shown as `interrupted` instead of implying that its process survived. Any running check is likewise marked interrupted. A candidate that had already passed is reconciled against Git; moved base/lane/ref inputs make it stale. If CrewCode stopped while applying but the base now equals the retained integration SHA, reconciliation records the operation as applied. The base is never inferred to be updated merely because a subprocess had started.
38+
After restart, an in-flight operation is shown as `interrupted` instead of implying that it completed. Every check process receives a random custody token plus a persisted local PID or remote PID file. Restart reconciliation probes the PID and verifies the token through the process environment where the operating system exposes it, reporting `running`, `exited`, or `unknown` rather than treating a reused PID as evidence. A still-running or unresolved interrupted check blocks another verification run. Platforms that do not expose enough process identity evidence remain `unknown` and require manual resolution; CrewCode does not guess that the process exited.
39+
40+
A candidate that had already passed is reconciled against Git; moved base/lane/ref inputs make it stale. If CrewCode stopped while applying and the base ref now equals the retained integration SHA, that SHA alone is not sufficient evidence of success. Reconciliation also requires `HEAD` to be attached to the expected base branch at the exact integration commit, a clean index and worktree (including untracked files), and no remaining `MERGE_HEAD`. Only then is the operation recorded as applied; otherwise it remains interrupted with the failed checkout/index invariant shown in the sidebar. The base is never inferred to be updated merely because a subprocess had started.
3941

4042
Crew session ownership is also persisted locally. Each lane has an explicit **enabled / paused** switch and an editable **next action** checkpoint, automatically seeded from its latest assignment. Pausing stops the lane runtime but retains its worktree, transcript, and checkpoint; resuming does not auto-submit work. Process-local bridge and terminal IDs are cleared on recovery, lanes that previously said `running` recover as `ready`, and persisted pause/checkpoint state remains visible. The UI never claims an agent process survived without evidence.
4143

‎docs/current-state.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ Supervisor reporting is incremental, not batched: `useCrewSupervisor` feeds each
2424

2525
Task distribution (`session.distribution`, default `split`) is a live header toggle separate from `mode`. `split` = each worker gets a distinct sub-task: the supervisor's per-turn run-selection snapshot carries `distributionDirective()`, and `validateDirectivePolicy()` hard-blocks `"to":"all"`, targets that resolve to multiple workers, unavailable/skipped targets, and exact duplicate task text before dispatch. No-supervisor shared mode renders one composer per worker in `CrewTimeline`, and split timeline rounds show each lane's own prompt inside that lane card rather than collapsing to one shared prompt. `broadcast` = same message to all (`handleBroadcast`) and the supervisor may use `"to":"all"`. `set_distribution` is legal at any phase, so it can flip mid-run.
2626

27-
Crew control stops are scoped intentionally: `stop all` in `CrewSurface` aborts every runtime, the supervisor composer stop button calls `abortSupervisor()` only, and lane composer stop buttons call `restartLane()` for that lane only. The per-lane enabled/paused switch is durable: pausing releases that runtime while retaining the worktree, transcript, and editable `nextAction`; every assignment seeds the checkpoint, restart recovery preserves it, and supervisor status snapshots include paused lanes without making them delegation targets. The supervisor sidebar width is local UI state in `CrewSurface` and is resized with the shared `Splitter`; the supervisor thread also has a scroll-to-bottom affordance. Shared timeline lane groups (`crew-lane-group`) are locally collapsible with chevrons so dense multi-agent rounds remain scannable. Every direct lane composer lazily supports `@` file search scoped to that lane's effective worktree/base path and auto-grows from one line to a bounded 220px before scrolling.
27+
Crew control stops are scoped intentionally: `stop all` in `CrewSurface` aborts every runtime, the supervisor composer stop button calls `abortSupervisor()` only, and lane composer stop buttons call `restartLane()` for that lane only. A lane stop must release bridge runtimes through `useBridgeRegistry.dropBridge()` rather than raw stop IPC so the cached `(tab, agent)` id is removed; lane prompt dispatch also replaces and retries a `bridge not found` runtime once, re-priming the replacement before sending. The per-lane enabled/paused switch is durable: pausing releases that runtime while retaining the worktree, transcript, and editable `nextAction`; every assignment seeds the checkpoint, restart recovery preserves it, and supervisor status snapshots include paused lanes without making them delegation targets. The supervisor sidebar width is local UI state in `CrewSurface` and is resized with the shared `Splitter`; the supervisor thread also has a scroll-to-bottom affordance. Shared timeline lane groups (`crew-lane-group`) are locally collapsible with chevrons so dense multi-agent rounds remain scannable. Every direct lane composer lazily supports `@` file search scoped to that lane's effective worktree/base path and auto-grows from one line to a bounded 220px before scrolling. Combined-check processes carry durable custody tokens and PID evidence; restart reconciliation reports them as running, exited, or unresolved and gates a replacement verification accordingly. A crash during apply is called applied only after validating the attached base checkout, exact HEAD, clean index/worktree, and absence of unfinished merge state.
2828

2929
Isolated crew review treats clean Git merges as unverified. Cross-lane Diff shows branch/commit ownership, runtime state, exact file overlap, and narrow cross-file contract heuristics; the crew merge sidebar requires explicit acknowledgment when signals exist. Crew sessions persist locally so restart retains lane/worktree provenance while clearing process-local runtime ids. Compare/Merge Git fetches are keyed to a stable ownership fingerprint rather than the once-per-second runtime/usage updates, preventing loaded review evidence from flashing back to a loading state. A merge audit is written before Git starts and recovers a leftover `running` operation as `interrupted`. After a clean merge, the sidebar discovers and visibly presents allowlisted package `typecheck`/`test` scripts; main runs only those discovered ids with `CI=1`, and persists pass/fail/interrupted evidence for local and SSH workspaces. See `docs/behavioral-merge-review.md`.
3030

‎docs/execution-custody.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# Execution Custody
22

3+
> Scope: interactive halt-and-reauthorize custody is enabled for synthetic crew lane threads only. Ordinary solo chats and the crew supervisor use their normal provider error/retry behavior and do not receive custody-loss banners or gates.
4+
35
> Status: living document. `docs/security-model.md` covers **granting** authority —
46
> whether it may cross the next boundary. This document covers **withdrawing** it:
57
> what happens when authority that was already granted stops being knowable while

‎docs/security-model.md‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -208,14 +208,15 @@ cannot be prevented, not what cannot be *contained after the fact*.
208208
test that read-only/ask/plan block writes and that `full`/bypass is reachable only by
209209
explicit opt-in. Ollama and OpenRouter expose **no** tool surface at all (pure chat
210210
streamers), so they cannot write or exec regardless of mode.
211-
4. **Authority can be withdrawn after it was granted.** Previously CrewCode could only
212-
deny the *next* action; a grant already in flight was good until the session ended,
213-
and an execution whose outcome was never observed left no trace. Now a persisted
214-
custody journal records every bridge execution, an interrupted turn is recovered as
215-
halted rather than assumed complete, mid-turn authority mutations are refused and
216-
deferred, and a tripped invariant refuses privileged actions until a human explicitly
217-
reauthorizes — reporting the exact failed invariant and affected scope, with the
218-
interrupted prompt and partial response preserved. See hop 5 and
211+
4. **Crew-lane authority can be withdrawn after it was granted.** Previously CrewCode
212+
could only deny the *next* action; a grant already in flight was good until the lane
213+
ended, and an execution whose outcome was never observed left no trace. For synthetic
214+
crew lane bridge threads, a persisted custody journal records execution, an interrupted
215+
turn is recovered as halted rather than assumed complete, mid-turn authority mutations
216+
are refused and deferred, and a tripped invariant refuses privileged actions until a
217+
human explicitly reauthorizes — reporting the exact failed invariant and affected
218+
scope, with the interrupted prompt and partial response preserved. Ordinary solo chats
219+
and crew supervisors deliberately retain normal provider error/retry behavior. See hop 5 and
219220
[`execution-custody.md`](execution-custody.md). Coverage gaps (remote transport, PTY,
220221
plugin sessions) are named there rather than implied away.
221222

@@ -240,13 +241,20 @@ cannot be prevented, not what cannot be *contained after the fact*.
240241
- **Claude** — full coverage. Full Access routes through `canUseTool` (no more native
241242
`bypassPermissions`), so every command is classified; denylisted ones pause. Highest
242243
priority because Claude's Full Access is an unsandboxed shell.
244+
- **Grok** — full coverage. Full Access now routes through `session/request_permission`
245+
(`--permission-mode default` instead of `bypassPermissions`); benign commands
246+
auto-approve, denylisted ones pause. Also an unsandboxed shell, so high priority.
243247
- **CrewCoder(ACP), Hermes(ACP)** — covered via the tool call's `rawInput`; denylisted
244248
commands fall through to a confirmation prompt instead of auto-approving.
245-
- **Codex** — **not yet routed through the tripwire**, but Full Access still runs under a
246-
`workspace-write` sandbox (writes scoped to the workspace, network off), so its blast
247-
radius is already the smallest. Tracked as follow-up.
248-
- **Grok, pi** — **not yet covered**; their Full Access uses engine-native bypass / a
249-
confirmation channel that does not carry the command string. Tracked as follow-up.
249+
- **Codex** — defense-in-depth tripwire wired into its approval handler, AND Full Access
250+
runs under a `workspace-write` sandbox (writes scoped to the workspace, network off).
251+
The sandbox alone already blocks most of the denylist (`curl|sh`, force-push, `dd`,
252+
`mkfs`, `sudo`, `chmod /` all need network or out-of-workspace access it denies); the
253+
only residual is destroying files *inside* the workspace, which is git-recoverable.
254+
- **pi** — **not covered.** pi's protocol has no pre-execution permission request that
255+
carries the command string (its `confirm` channel omits it, and `tool_execution_start`
256+
is informational — it cannot block). A tripwire here needs a pi protocol change; a
257+
best-effort hook would be false confidence, so it is deliberately omitted and tracked.
250258
- **Ollama, OpenRouter** — N/A (no tool surface; cannot exec).
251259
7. **MCP servers run with user privilege.** A user-configured MCP server is trusted code on
252260
the host; CrewCode gates *which* sessions may use it, not what the server binary itself

‎src/main/agents/codex-bridge.ts‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { spawnAgentProcess } from './agent-spawn'
22
import type { AgentBridge, BridgeStartOpts, EmitFn, ModeLevel, RequestUserFn, TurnUsage } from './bridge-types'
33
import { buildUsage, contextWindowFor } from './model-context'
4+
import { tripwireForToolCall } from './dangerous-command'
45

56
// Codex app-server JSON-RPC over stdio.
67
// Protocol: newline-delimited JSON (`"jsonrpc": "2.0"` header omitted on the wire).
@@ -516,15 +517,26 @@ export async function createCodexBridge(
516517
// Mode switches mutate opts on the live bridge; honor them even when the
517518
// Codex thread was started with a different approval policy.
518519
const modeDecision = codexApprovalDecisionForMode(opts.mode, opts.toolPolicy)
519-
if (modeDecision || !requestUser) {
520+
// Full Access tripwire: a denylisted command must confirm even when the mode
521+
// would auto-accept. Codex's workspace-write sandbox (no network, scoped
522+
// writes) already blocks most of the denylist; this is defense-in-depth for
523+
// in-workspace destruction whenever Codex does surface an approval request.
524+
const itemRecord = (msg.params.item && typeof msg.params.item === 'object') ? msg.params.item as Record<string, unknown> : undefined
525+
const codexCommand = msg.params.command ?? itemRecord?.command
526+
const codexVerdict = codexCommand !== undefined ? tripwireForToolCall('shell', { command: codexCommand }) : { dangerous: false as const }
527+
if (!codexVerdict.dangerous && (modeDecision || !requestUser)) {
520528
respond(msg.id, { decision: modeDecision ?? 'accept' })
521529
return
522530
}
531+
if (!requestUser) {
532+
respond(msg.id, { decision: codexVerdict.dangerous ? 'decline' : (modeDecision ?? 'accept') })
533+
return
534+
}
523535
const response = await requestUser({
524536
kind: 'permission',
525537
turnId: currentTurnId ?? undefined,
526-
title: msg.method.includes('commandExecution') ? 'approve command execution' : 'approve file change',
527-
message: typeof msg.params.reason === 'string' ? msg.params.reason : undefined,
538+
title: codexVerdict.dangerous ? 'Full Access tripwire — confirm dangerous command' : (msg.method.includes('commandExecution') ? 'approve command execution' : 'approve file change'),
539+
message: codexVerdict.dangerous ? codexVerdict.reason : (typeof msg.params.reason === 'string' ? msg.params.reason : undefined),
528540
detail: requestDetail(msg.params),
529541
dangerous: true,
530542
source: 'codex',

‎src/main/agents/grok-bridge.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,8 @@ describe('grokPermissionMode', () => {
9191

9292
it('maps build to interactive approval and full to bypass', () => {
9393
expect(grokPermissionMode({ mode: 'build' })).toBe('default')
94-
expect(grokPermissionMode({ mode: 'full' })).toBe('bypassPermissions')
94+
// Full Access routes through 'default' (not native bypass) so the tripwire can inspect commands.
95+
expect(grokPermissionMode({ mode: 'full' })).toBe('default')
9596
})
9697

9798
it('defaults an absent mode to interactive approval rather than bypass', () => {

‎src/main/agents/grok-bridge.ts‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import type {
1717
} from './bridge-types'
1818
import { buildUsage } from './model-context'
1919
import { enrichUsageContextWindow } from './openrouter-model-context'
20+
import { tripwireForToolCall } from './dangerous-command'
2021

2122
// Grok Build via ACP (Agent Client Protocol) — newline-delimited JSON-RPC 2.0
2223
// over stdio. CrewCode is the client and spawns `grok agent stdio`.
@@ -140,7 +141,11 @@ export function grokPermissionMode(opts: Pick<BridgeStartOpts, 'mode' | 'toolPol
140141
// A constrained role (supervisor, completion) outranks the composer mode.
141142
if (opts.toolPolicy === 'read-only') return 'dontAsk'
142143
if (opts.mode === 'ask' || opts.mode === 'plan') return 'dontAsk'
143-
if (opts.mode === 'full') return 'bypassPermissions'
144+
// Full Access used to send 'bypassPermissions', which made Grok run every tool
145+
// silently — leaving no chance to inspect commands. Route through 'default' so
146+
// Grok asks via session/request_permission; CrewCode auto-approves everything
147+
// except the denylist (the Full Access tripwire, see handlePermissionRequest).
148+
if (opts.mode === 'full') return 'default'
144149
return 'default'
145150
}
146151

@@ -709,14 +714,23 @@ export async function createGrokBridge(
709714

710715
async function handlePermissionRequest(requestMessage: AcpRequestIn, params: Record<string, unknown>): Promise<void> {
711716
const options = grokPermissionOptions(params)
712-
if (opts.mode === 'full' && opts.toolPolicy !== 'read-only') {
717+
const fullToolCall = record(params.toolCall)
718+
const fullVerdict = tripwireForToolCall(typeof fullToolCall?.kind === 'string' ? fullToolCall.kind : undefined, fullToolCall?.rawInput)
719+
if (opts.mode === 'full' && opts.toolPolicy !== 'read-only' && !fullVerdict.dangerous) {
720+
// Auto-approve in Full Access — except denylisted catastrophic commands,
721+
// which fall through to the confirmation prompt (the Full Access tripwire).
713722
send({
714723
jsonrpc: '2.0',
715724
id: requestMessage.id,
716725
result: { outcome: { outcome: 'selected', optionId: grokSelectedOption({ requestId: '', action: 'accept' }, params, options) } },
717726
})
718727
return
719728
}
729+
if (opts.mode === 'full' && fullVerdict.dangerous && !requestUser) {
730+
// Tripwire tripped but no human to confirm — fail safe by refusing.
731+
send({ jsonrpc: '2.0', id: requestMessage.id, result: { outcome: { outcome: 'cancelled' } } })
732+
return
733+
}
720734
if (writeBlocked(opts) || !requestUser) {
721735
send({ jsonrpc: '2.0', id: requestMessage.id, result: { outcome: { outcome: 'cancelled' } } })
722736
return
@@ -729,11 +743,11 @@ export async function createGrokBridge(
729743
const response = await requestUser({
730744
kind: 'permission',
731745
turnId: currentTurnId ?? undefined,
732-
title: typeof toolCall?.title === 'string' ? toolCall.title : 'Permission required',
733-
message: typeof params.message === 'string' ? params.message : undefined,
746+
title: fullVerdict.dangerous ? 'Full Access tripwire — confirm dangerous command' : (typeof toolCall?.title === 'string' ? toolCall.title : 'Permission required'),
747+
message: fullVerdict.dangerous ? fullVerdict.reason : (typeof params.message === 'string' ? params.message : undefined),
734748
detail: JSON.stringify(toolCall ?? params, null, 2).slice(0, 1600),
735749
options,
736-
dangerous: meta.readOnly !== true,
750+
dangerous: fullVerdict.dangerous || meta.readOnly !== true,
737751
source: 'grok',
738752
})
739753
const optionId = grokSelectedOption(response, params, options)

0 commit comments

Comments
 (0)