feat(crit): PR review integration (v1, flag-gated)

[Ecko95]

Crit PR Review Integration (v1 — flag-gated, experimental)

Embeds the crit review binary as a GITS-managed sidecar and surfaces its review UI inside the PR view. "Send to agent" routes into the active GITS thread (so the user's default provider — codex — answers), rather than crit spawning its own agent.

Design: docs/superpowers/specs/2026-06-07-crit-pr-review-integration-design.md
Plan: docs/superpowers/plans/2026-06-07-crit-pr-review-integration.md

Architecture

apps/server spawns one crit process per workspace and sets crit's agent_cmd to a thin GITS wrapper CLI. The wrapper reads crit's comment on stdin, POSTs a thread.turn.start to /api/orchestration/dispatch, blocks while polling /api/orchestration/snapshot until the turn completes, then prints the assistant reply on stdout for crit to thread. apps/web renders crit's loopback URL in an <iframe> panel that replaces the sidebar diff for PR review only, behind an off-by-defaultcritReviewEnabled flag with native-DiffPanel fallback.

Why crit's missing codex support is a non-issue: crit's agent_cmd points at the GITS wrapper, never at codex. The wrapper satisfies crit's stdin→stdout contract; the actual codex run happens inside GITS via the codex app-server. crit never spawns an agent.

What's included

Layer	Component
shared	review-comment-block.ts — builds the <review_comment> block the existing web parser reads
server (CLI)	crit-agent-cli.ts — the agent_cmd bridge (parse → thread.turn.start → block-and-return poll → reply)
server	crit-binary-resolver.ts, crit-sidecar-manager.ts (Effect service: spawn + HTTP health-check + ref-count + Scope teardown), crit-sidecar-request.ts
contracts	crit.ts schemas + crit.ensureSidecar/crit.sidecarStatus RPC, critReviewEnabled setting
client/web	EnvironmentApi.crit, CritReviewPanel.tsx, PR-view route wiring behind the flag

Verification

• ~27 unit tests passing (builder 4, wrapper CLI 5, resolver 3, sidecar manager 4, contracts 7, request helper 3, panel 1).
• Typecheck clean across packages/{shared,contracts,client-runtime} and apps/{server,web} for all touched files.
• End-to-end RPC identity verified: client transport.request(WS_METHODS.critEnsureSidecar) ↔ server ws.ts handler share the same Rpc.make contract.
• Flag-off default is safe — with critReviewEnabled false, the native diff/PR path is unchanged and no crit code runs.
• Two-stage review per task caught & fixed: a parser-truncation bug (escaping <), the wrapper's typing/lint issues, and a sidecar crash path that threw at runtime + a non-load-bearing test.

⚠️ Deferred (require building crit; Go was unavailable in the build env)

Tracked in docs/crit-integration-notes.md, marked PENDING in-code:

• 0b real crit CLI flags — crit-sidecar-manager.ts uses placeholder --repo/--branch/--host/--port/--agent-cmd.
• 0c crit's agent_cmd stdin format — crit-agent-cli.ts assumes JSON {comment,quoted,filePath,startLine,endLine} with a plain-text fallback.
• Phase 7 per-platform binary bundling (electron-builder extraResources) + the wrapper's production build path (GITS_CRIT_AGENT_CMD env override for now).
• Phase 8 end-to-end test against a live crit.

🔎 Follow-ups from final holistic review (not blockers for this flag-gated v1)

1. Block-and-return stale-reply race — after dispatch, latestTurn doesn't flip to running synchronously, so a fast first poll can return the prior turn's reply. Fix: baseline the pre-dispatch turn id and only accept a new completed turn.
2. Token is role:"owner", never revoked — deviates from the design's "workspace-scoped, dispatch-only, revoked on teardown." Currently necessary because dispatch/snapshot are owner-gated; needs a scoped-dispatch capability + revocation.
3. release_sidecar not called in production — process/token leak; wire it to CritReviewPanel cleanup via a crit.releaseSidecar WS method (or release on WS disconnect).

🤖 Generated with Claude Code

[Ecko95]

Follow-up #1 (block-and-return stale-reply race) is now fixed in 0bf5d30b: the wrapper baselines the thread's latestTurn id before dispatch and only accepts a turn whose id differs, so a fast first poll can no longer surface the previous (completed) turn's reply. Added a load-bearing test that reproduces the race (fails against the old code). Follow-ups #2 (owner-token scoping/revocation) and #3 (release_sidecar wiring) remain open.

[Ecko95]

Follow-ups #2 and #3 landed in c7f1d152:

• feat(gits): review pipeline (mechanical gate → semantic verifier → triage) #3 — sidecar release wiring (leak fixed): added a crit.releaseSidecar WS method end-to-end (contracts/rpc → client-runtime → EnvironmentApi → ws.ts handler). CritReviewPanel's effect cleanup now calls it, so unmount/input-change releases the refCount its ensureSidecar took.
• feat(gits): acceptance-criteria plumbing for the verifier-critic #2 — token lifecycle (revoke + no-leak): token issuance moved intoCritSidecarManager — minted only on actual spawn (reuse path mints nothing, fixing the per-call leak), with a bounded 2h TTL, and revoked on every teardown path (release-to-0, crash, readiness-timeout) via a finalizer on the per-sidecar scope. Load-bearing tests assert: no second mint on reuse, not-revoked-while-alive, and revoked-on-teardown (normal + crash).

Verification: 16 server-crit + 9 web + contracts tests pass; typecheck clean across contracts/client-runtime/apps-server/apps-web.

Remaining (deliberately not in this change): true least-privilege scoping (owner→thread-bound). /api/orchestration/{dispatch,snapshot} are owner-gated and accept any command, so real scoping needs a dedicated crit endpoint (thread-bound, client-role) + repointing the wrapper — a focused, security-reviewed change better done once crit is buildable for E2E. The 2h-TTL + revoke-on-teardown is the v1 mitigation (loopback-only, flag-off).

[Ecko95]

Crit PR Review Integration (v1 — flag-gated, experimental)

Embeds the crit review binary as a GITS-managed sidecar and surfaces its review UI inside the PR view. "Send to agent" routes into the active GITS thread (so the user's default provider — codex — answers), rather than crit spawning its own agent.

Design: docs/superpowers/specs/2026-06-07-crit-pr-review-integration-design.md
Plan: docs/superpowers/plans/2026-06-07-crit-pr-review-integration.md

Architecture

apps/server spawns one crit process per workspace and sets crit's agent_cmd to a thin GITS wrapper CLI. The wrapper reads crit's comment on stdin, POSTs a thread.turn.start to /api/orchestration/dispatch, blocks while polling /api/orchestration/snapshot until the turn completes, then prints the assistant reply on stdout for crit to thread. apps/web renders crit's loopback URL in an <iframe> panel that replaces the sidebar diff for PR review only, behind an off-by-defaultcritReviewEnabled flag with native-DiffPanel fallback.

Why crit's missing codex support is a non-issue: crit's agent_cmd points at the GITS wrapper, never at codex. The wrapper satisfies crit's stdin→stdout contract; the actual codex run happens inside GITS via the codex app-server. crit never spawns an agent.

What's included

Layer	Component
shared	review-comment-block.ts — builds the <review_comment> block the existing web parser reads
server (CLI)	crit-agent-cli.ts — the agent_cmd bridge (parse → thread.turn.start → block-and-return poll → reply)
server	crit-binary-resolver.ts, crit-sidecar-manager.ts (Effect service: spawn + HTTP health-check + ref-count + Scope teardown), crit-sidecar-request.ts
contracts	crit.ts schemas + crit.ensureSidecar/crit.sidecarStatus RPC, critReviewEnabled setting
client/web	EnvironmentApi.crit, CritReviewPanel.tsx, PR-view route wiring behind the flag

Verification

• ~27 unit tests passing (builder 4, wrapper CLI 5, resolver 3, sidecar manager 4, contracts 7, request helper 3, panel 1).
• Typecheck clean across packages/{shared,contracts,client-runtime} and apps/{server,web} for all touched files.
• End-to-end RPC identity verified: client transport.request(WS_METHODS.critEnsureSidecar) ↔ server ws.ts handler share the same Rpc.make contract.
• Flag-off default is safe — with critReviewEnabled false, the native diff/PR path is unchanged and no crit code runs.
• Two-stage review per task caught & fixed: a parser-truncation bug (escaping <), the wrapper's typing/lint issues, and a sidecar crash path that threw at runtime + a non-load-bearing test.

⚠️ Deferred (require building crit; Go was unavailable in the build env)

Tracked in docs/crit-integration-notes.md, marked PENDING in-code:

• 0b real crit CLI flags — crit-sidecar-manager.ts uses placeholder --repo/--branch/--host/--port/--agent-cmd.
• 0c crit's agent_cmd stdin format — crit-agent-cli.ts assumes JSON {comment,quoted,filePath,startLine,endLine} with a plain-text fallback.
• Phase 7 per-platform binary bundling (electron-builder extraResources) + the wrapper's production build path (GITS_CRIT_AGENT_CMD env override for now).
• Phase 8 end-to-end test against a live crit.

🔎 Follow-ups from final holistic review (not blockers for this flag-gated v1)

1. Block-and-return stale-reply race — after dispatch, latestTurn doesn't flip to running synchronously, so a fast first poll can return the prior turn's reply. Fix: baseline the pre-dispatch turn id and only accept a new completed turn.
2. Token is role:"owner", never revoked — deviates from the design's "workspace-scoped, dispatch-only, revoked on teardown." Currently necessary because dispatch/snapshot are owner-gated; needs a scoped-dispatch capability + revocation.
3. release_sidecar not called in production — process/token leak; wire it to CritReviewPanel cleanup via a crit.releaseSidecar WS method (or release on WS disconnect).

🤖 Generated with Claude Code

[Ecko95]

Follow-up #1 (block-and-return stale-reply race) is now fixed in 0bf5d30b: the wrapper baselines the thread's latestTurn id before dispatch and only accepts a turn whose id differs, so a fast first poll can no longer surface the previous (completed) turn's reply. Added a load-bearing test that reproduces the race (fails against the old code). Follow-ups #2 (owner-token scoping/revocation) and #3 (release_sidecar wiring) remain open.

[Ecko95]

Follow-ups #2 and #3 landed in c7f1d152:

• feat(gits): review pipeline (mechanical gate → semantic verifier → triage) #3 — sidecar release wiring (leak fixed): added a crit.releaseSidecar WS method end-to-end (contracts/rpc → client-runtime → EnvironmentApi → ws.ts handler). CritReviewPanel's effect cleanup now calls it, so unmount/input-change releases the refCount its ensureSidecar took.
• feat(gits): acceptance-criteria plumbing for the verifier-critic #2 — token lifecycle (revoke + no-leak): token issuance moved intoCritSidecarManager — minted only on actual spawn (reuse path mints nothing, fixing the per-call leak), with a bounded 2h TTL, and revoked on every teardown path (release-to-0, crash, readiness-timeout) via a finalizer on the per-sidecar scope. Load-bearing tests assert: no second mint on reuse, not-revoked-while-alive, and revoked-on-teardown (normal + crash).

Verification: 16 server-crit + 9 web + contracts tests pass; typecheck clean across contracts/client-runtime/apps-server/apps-web.

Remaining (deliberately not in this change): true least-privilege scoping (owner→thread-bound). /api/orchestration/{dispatch,snapshot} are owner-gated and accept any command, so real scoping needs a dedicated crit endpoint (thread-bound, client-role) + repointing the wrapper — a focused, security-reviewed change better done once crit is buildable for E2E. The 2h-TTL + revoke-on-teardown is the v1 mitigation (loopback-only, flag-off).

[Ecko95]

Crit PR Review Integration (v1 — flag-gated, experimental)

Embeds the crit review binary as a GITS-managed sidecar and surfaces its review UI inside the PR view. "Send to agent" routes into the active GITS thread (so the user's default provider — codex — answers), rather than crit spawning its own agent.

Design: docs/superpowers/specs/2026-06-07-crit-pr-review-integration-design.md
Plan: docs/superpowers/plans/2026-06-07-crit-pr-review-integration.md

Architecture

apps/server spawns one crit process per workspace and sets crit's agent_cmd to a thin GITS wrapper CLI. The wrapper reads crit's comment on stdin, POSTs a thread.turn.start to /api/orchestration/dispatch, blocks while polling /api/orchestration/snapshot until the turn completes, then prints the assistant reply on stdout for crit to thread. apps/web renders crit's loopback URL in an <iframe> panel that replaces the sidebar diff for PR review only, behind an off-by-defaultcritReviewEnabled flag with native-DiffPanel fallback.

Why crit's missing codex support is a non-issue: crit's agent_cmd points at the GITS wrapper, never at codex. The wrapper satisfies crit's stdin→stdout contract; the actual codex run happens inside GITS via the codex app-server. crit never spawns an agent.

What's included

Layer	Component
shared	review-comment-block.ts — builds the <review_comment> block the existing web parser reads
server (CLI)	crit-agent-cli.ts — the agent_cmd bridge (parse → thread.turn.start → block-and-return poll → reply)
server	crit-binary-resolver.ts, crit-sidecar-manager.ts (Effect service: spawn + HTTP health-check + ref-count + Scope teardown), crit-sidecar-request.ts
contracts	crit.ts schemas + crit.ensureSidecar/crit.sidecarStatus RPC, critReviewEnabled setting
client/web	EnvironmentApi.crit, CritReviewPanel.tsx, PR-view route wiring behind the flag

Verification

• ~27 unit tests passing (builder 4, wrapper CLI 5, resolver 3, sidecar manager 4, contracts 7, request helper 3, panel 1).
• Typecheck clean across packages/{shared,contracts,client-runtime} and apps/{server,web} for all touched files.
• End-to-end RPC identity verified: client transport.request(WS_METHODS.critEnsureSidecar) ↔ server ws.ts handler share the same Rpc.make contract.
• Flag-off default is safe — with critReviewEnabled false, the native diff/PR path is unchanged and no crit code runs.
• Two-stage review per task caught & fixed: a parser-truncation bug (escaping <), the wrapper's typing/lint issues, and a sidecar crash path that threw at runtime + a non-load-bearing test.

⚠️ Deferred (require building crit; Go was unavailable in the build env)

Tracked in docs/crit-integration-notes.md, marked PENDING in-code:

• 0b real crit CLI flags — crit-sidecar-manager.ts uses placeholder --repo/--branch/--host/--port/--agent-cmd.
• 0c crit's agent_cmd stdin format — crit-agent-cli.ts assumes JSON {comment,quoted,filePath,startLine,endLine} with a plain-text fallback.
• Phase 7 per-platform binary bundling (electron-builder extraResources) + the wrapper's production build path (GITS_CRIT_AGENT_CMD env override for now).
• Phase 8 end-to-end test against a live crit.

🔎 Follow-ups from final holistic review (not blockers for this flag-gated v1)

1. Block-and-return stale-reply race — after dispatch, latestTurn doesn't flip to running synchronously, so a fast first poll can return the prior turn's reply. Fix: baseline the pre-dispatch turn id and only accept a new completed turn.
2. Token is role:"owner", never revoked — deviates from the design's "workspace-scoped, dispatch-only, revoked on teardown." Currently necessary because dispatch/snapshot are owner-gated; needs a scoped-dispatch capability + revocation.
3. release_sidecar not called in production — process/token leak; wire it to CritReviewPanel cleanup via a crit.releaseSidecar WS method (or release on WS disconnect).

🤖 Generated with Claude Code

[Ecko95]

Follow-up #1 (block-and-return stale-reply race) is now fixed in 0bf5d30b: the wrapper baselines the thread's latestTurn id before dispatch and only accepts a turn whose id differs, so a fast first poll can no longer surface the previous (completed) turn's reply. Added a load-bearing test that reproduces the race (fails against the old code). Follow-ups #2 (owner-token scoping/revocation) and #3 (release_sidecar wiring) remain open.

[Ecko95]

Follow-ups #2 and #3 landed in c7f1d152:

• feat(gits): review pipeline (mechanical gate → semantic verifier → triage) #3 — sidecar release wiring (leak fixed): added a crit.releaseSidecar WS method end-to-end (contracts/rpc → client-runtime → EnvironmentApi → ws.ts handler). CritReviewPanel's effect cleanup now calls it, so unmount/input-change releases the refCount its ensureSidecar took.
• feat(gits): acceptance-criteria plumbing for the verifier-critic #2 — token lifecycle (revoke + no-leak): token issuance moved intoCritSidecarManager — minted only on actual spawn (reuse path mints nothing, fixing the per-call leak), with a bounded 2h TTL, and revoked on every teardown path (release-to-0, crash, readiness-timeout) via a finalizer on the per-sidecar scope. Load-bearing tests assert: no second mint on reuse, not-revoked-while-alive, and revoked-on-teardown (normal + crash).

Verification: 16 server-crit + 9 web + contracts tests pass; typecheck clean across contracts/client-runtime/apps-server/apps-web.

Remaining (deliberately not in this change): true least-privilege scoping (owner→thread-bound). /api/orchestration/{dispatch,snapshot} are owner-gated and accept any command, so real scoping needs a dedicated crit endpoint (thread-bound, client-role) + repointing the wrapper — a focused, security-reviewed change better done once crit is buildable for E2E. The 2h-TTL + revoke-on-teardown is the v1 mitigation (loopback-only, flag-off).

[Ecko95]

Crit PR Review Integration (v1 — flag-gated, experimental)

Embeds the crit review binary as a GITS-managed sidecar and surfaces its review UI inside the PR view. "Send to agent" routes into the active GITS thread (so the user's default provider — codex — answers), rather than crit spawning its own agent.

Design: docs/superpowers/specs/2026-06-07-crit-pr-review-integration-design.md
Plan: docs/superpowers/plans/2026-06-07-crit-pr-review-integration.md

Architecture

apps/server spawns one crit process per workspace and sets crit's agent_cmd to a thin GITS wrapper CLI. The wrapper reads crit's comment on stdin, POSTs a thread.turn.start to /api/orchestration/dispatch, blocks while polling /api/orchestration/snapshot until the turn completes, then prints the assistant reply on stdout for crit to thread. apps/web renders crit's loopback URL in an <iframe> panel that replaces the sidebar diff for PR review only, behind an off-by-defaultcritReviewEnabled flag with native-DiffPanel fallback.

Why crit's missing codex support is a non-issue: crit's agent_cmd points at the GITS wrapper, never at codex. The wrapper satisfies crit's stdin→stdout contract; the actual codex run happens inside GITS via the codex app-server. crit never spawns an agent.

What's included

Layer	Component
shared	review-comment-block.ts — builds the <review_comment> block the existing web parser reads
server (CLI)	crit-agent-cli.ts — the agent_cmd bridge (parse → thread.turn.start → block-and-return poll → reply)
server	crit-binary-resolver.ts, crit-sidecar-manager.ts (Effect service: spawn + HTTP health-check + ref-count + Scope teardown), crit-sidecar-request.ts
contracts	crit.ts schemas + crit.ensureSidecar/crit.sidecarStatus RPC, critReviewEnabled setting
client/web	EnvironmentApi.crit, CritReviewPanel.tsx, PR-view route wiring behind the flag

Verification

• ~27 unit tests passing (builder 4, wrapper CLI 5, resolver 3, sidecar manager 4, contracts 7, request helper 3, panel 1).
• Typecheck clean across packages/{shared,contracts,client-runtime} and apps/{server,web} for all touched files.
• End-to-end RPC identity verified: client transport.request(WS_METHODS.critEnsureSidecar) ↔ server ws.ts handler share the same Rpc.make contract.
• Flag-off default is safe — with critReviewEnabled false, the native diff/PR path is unchanged and no crit code runs.
• Two-stage review per task caught & fixed: a parser-truncation bug (escaping <), the wrapper's typing/lint issues, and a sidecar crash path that threw at runtime + a non-load-bearing test.

⚠️ Deferred (require building crit; Go was unavailable in the build env)

Tracked in docs/crit-integration-notes.md, marked PENDING in-code:

• 0b real crit CLI flags — crit-sidecar-manager.ts uses placeholder --repo/--branch/--host/--port/--agent-cmd.
• 0c crit's agent_cmd stdin format — crit-agent-cli.ts assumes JSON {comment,quoted,filePath,startLine,endLine} with a plain-text fallback.
• Phase 7 per-platform binary bundling (electron-builder extraResources) + the wrapper's production build path (GITS_CRIT_AGENT_CMD env override for now).
• Phase 8 end-to-end test against a live crit.

🔎 Follow-ups from final holistic review (not blockers for this flag-gated v1)

1. Block-and-return stale-reply race — after dispatch, latestTurn doesn't flip to running synchronously, so a fast first poll can return the prior turn's reply. Fix: baseline the pre-dispatch turn id and only accept a new completed turn.
2. Token is role:"owner", never revoked — deviates from the design's "workspace-scoped, dispatch-only, revoked on teardown." Currently necessary because dispatch/snapshot are owner-gated; needs a scoped-dispatch capability + revocation.
3. release_sidecar not called in production — process/token leak; wire it to CritReviewPanel cleanup via a crit.releaseSidecar WS method (or release on WS disconnect).

🤖 Generated with Claude Code

[Ecko95]

Follow-up #1 (block-and-return stale-reply race) is now fixed in 0bf5d30b: the wrapper baselines the thread's latestTurn id before dispatch and only accepts a turn whose id differs, so a fast first poll can no longer surface the previous (completed) turn's reply. Added a load-bearing test that reproduces the race (fails against the old code). Follow-ups #2 (owner-token scoping/revocation) and #3 (release_sidecar wiring) remain open.

[Ecko95]

Follow-ups #2 and #3 landed in c7f1d152:

• feat(gits): review pipeline (mechanical gate → semantic verifier → triage) #3 — sidecar release wiring (leak fixed): added a crit.releaseSidecar WS method end-to-end (contracts/rpc → client-runtime → EnvironmentApi → ws.ts handler). CritReviewPanel's effect cleanup now calls it, so unmount/input-change releases the refCount its ensureSidecar took.
• feat(gits): acceptance-criteria plumbing for the verifier-critic #2 — token lifecycle (revoke + no-leak): token issuance moved intoCritSidecarManager — minted only on actual spawn (reuse path mints nothing, fixing the per-call leak), with a bounded 2h TTL, and revoked on every teardown path (release-to-0, crash, readiness-timeout) via a finalizer on the per-sidecar scope. Load-bearing tests assert: no second mint on reuse, not-revoked-while-alive, and revoked-on-teardown (normal + crash).

Verification: 16 server-crit + 9 web + contracts tests pass; typecheck clean across contracts/client-runtime/apps-server/apps-web.

Remaining (deliberately not in this change): true least-privilege scoping (owner→thread-bound). /api/orchestration/{dispatch,snapshot} are owner-gated and accept any command, so real scoping needs a dedicated crit endpoint (thread-bound, client-role) + repointing the wrapper — a focused, security-reviewed change better done once crit is buildable for E2E. The 2h-TTL + revoke-on-teardown is the v1 mitigation (loopback-only, flag-off).

[Ecko95]

Crit PR Review Integration (v1 — flag-gated, experimental)

Embeds the crit review binary as a GITS-managed sidecar and surfaces its review UI inside the PR view. "Send to agent" routes into the active GITS thread (so the user's default provider — codex — answers), rather than crit spawning its own agent.

Design: docs/superpowers/specs/2026-06-07-crit-pr-review-integration-design.md
Plan: docs/superpowers/plans/2026-06-07-crit-pr-review-integration.md

Architecture

apps/server spawns one crit process per workspace and sets crit's agent_cmd to a thin GITS wrapper CLI. The wrapper reads crit's comment on stdin, POSTs a thread.turn.start to /api/orchestration/dispatch, blocks while polling /api/orchestration/snapshot until the turn completes, then prints the assistant reply on stdout for crit to thread. apps/web renders crit's loopback URL in an <iframe> panel that replaces the sidebar diff for PR review only, behind an off-by-defaultcritReviewEnabled flag with native-DiffPanel fallback.

Why crit's missing codex support is a non-issue: crit's agent_cmd points at the GITS wrapper, never at codex. The wrapper satisfies crit's stdin→stdout contract; the actual codex run happens inside GITS via the codex app-server. crit never spawns an agent.

What's included

Layer	Component
shared	review-comment-block.ts — builds the <review_comment> block the existing web parser reads
server (CLI)	crit-agent-cli.ts — the agent_cmd bridge (parse → thread.turn.start → block-and-return poll → reply)
server	crit-binary-resolver.ts, crit-sidecar-manager.ts (Effect service: spawn + HTTP health-check + ref-count + Scope teardown), crit-sidecar-request.ts
contracts	crit.ts schemas + crit.ensureSidecar/crit.sidecarStatus RPC, critReviewEnabled setting
client/web	EnvironmentApi.crit, CritReviewPanel.tsx, PR-view route wiring behind the flag

Verification

• ~27 unit tests passing (builder 4, wrapper CLI 5, resolver 3, sidecar manager 4, contracts 7, request helper 3, panel 1).
• Typecheck clean across packages/{shared,contracts,client-runtime} and apps/{server,web} for all touched files.
• End-to-end RPC identity verified: client transport.request(WS_METHODS.critEnsureSidecar) ↔ server ws.ts handler share the same Rpc.make contract.
• Flag-off default is safe — with critReviewEnabled false, the native diff/PR path is unchanged and no crit code runs.
• Two-stage review per task caught & fixed: a parser-truncation bug (escaping <), the wrapper's typing/lint issues, and a sidecar crash path that threw at runtime + a non-load-bearing test.

⚠️ Deferred (require building crit; Go was unavailable in the build env)

Tracked in docs/crit-integration-notes.md, marked PENDING in-code:

• 0b real crit CLI flags — crit-sidecar-manager.ts uses placeholder --repo/--branch/--host/--port/--agent-cmd.
• 0c crit's agent_cmd stdin format — crit-agent-cli.ts assumes JSON {comment,quoted,filePath,startLine,endLine} with a plain-text fallback.
• Phase 7 per-platform binary bundling (electron-builder extraResources) + the wrapper's production build path (GITS_CRIT_AGENT_CMD env override for now).
• Phase 8 end-to-end test against a live crit.

🔎 Follow-ups from final holistic review (not blockers for this flag-gated v1)

1. Block-and-return stale-reply race — after dispatch, latestTurn doesn't flip to running synchronously, so a fast first poll can return the prior turn's reply. Fix: baseline the pre-dispatch turn id and only accept a new completed turn.
2. Token is role:"owner", never revoked — deviates from the design's "workspace-scoped, dispatch-only, revoked on teardown." Currently necessary because dispatch/snapshot are owner-gated; needs a scoped-dispatch capability + revocation.
3. release_sidecar not called in production — process/token leak; wire it to CritReviewPanel cleanup via a crit.releaseSidecar WS method (or release on WS disconnect).

🤖 Generated with Claude Code

[Ecko95]

Follow-up #1 (block-and-return stale-reply race) is now fixed in 0bf5d30b: the wrapper baselines the thread's latestTurn id before dispatch and only accepts a turn whose id differs, so a fast first poll can no longer surface the previous (completed) turn's reply. Added a load-bearing test that reproduces the race (fails against the old code). Follow-ups #2 (owner-token scoping/revocation) and #3 (release_sidecar wiring) remain open.

[Ecko95]

Follow-ups #2 and #3 landed in c7f1d152:

• feat(gits): review pipeline (mechanical gate → semantic verifier → triage) #3 — sidecar release wiring (leak fixed): added a crit.releaseSidecar WS method end-to-end (contracts/rpc → client-runtime → EnvironmentApi → ws.ts handler). CritReviewPanel's effect cleanup now calls it, so unmount/input-change releases the refCount its ensureSidecar took.
• feat(gits): acceptance-criteria plumbing for the verifier-critic #2 — token lifecycle (revoke + no-leak): token issuance moved intoCritSidecarManager — minted only on actual spawn (reuse path mints nothing, fixing the per-call leak), with a bounded 2h TTL, and revoked on every teardown path (release-to-0, crash, readiness-timeout) via a finalizer on the per-sidecar scope. Load-bearing tests assert: no second mint on reuse, not-revoked-while-alive, and revoked-on-teardown (normal + crash).

Verification: 16 server-crit + 9 web + contracts tests pass; typecheck clean across contracts/client-runtime/apps-server/apps-web.

Remaining (deliberately not in this change): true least-privilege scoping (owner→thread-bound). /api/orchestration/{dispatch,snapshot} are owner-gated and accept any command, so real scoping needs a dedicated crit endpoint (thread-bound, client-role) + repointing the wrapper — a focused, security-reviewed change better done once crit is buildable for E2E. The 2h-TTL + revoke-on-teardown is the v1 mitigation (loopback-only, flag-off).

[Ecko95]

Crit PR Review Integration (v1 — flag-gated, experimental)

Embeds the crit review binary as a GITS-managed sidecar and surfaces its review UI inside the PR view. "Send to agent" routes into the active GITS thread (so the user's default provider — codex — answers), rather than crit spawning its own agent.

Design: docs/superpowers/specs/2026-06-07-crit-pr-review-integration-design.md
Plan: docs/superpowers/plans/2026-06-07-crit-pr-review-integration.md

Architecture

apps/server spawns one crit process per workspace and sets crit's agent_cmd to a thin GITS wrapper CLI. The wrapper reads crit's comment on stdin, POSTs a thread.turn.start to /api/orchestration/dispatch, blocks while polling /api/orchestration/snapshot until the turn completes, then prints the assistant reply on stdout for crit to thread. apps/web renders crit's loopback URL in an <iframe> panel that replaces the sidebar diff for PR review only, behind an off-by-defaultcritReviewEnabled flag with native-DiffPanel fallback.

Why crit's missing codex support is a non-issue: crit's agent_cmd points at the GITS wrapper, never at codex. The wrapper satisfies crit's stdin→stdout contract; the actual codex run happens inside GITS via the codex app-server. crit never spawns an agent.

What's included

Layer	Component
shared	review-comment-block.ts — builds the <review_comment> block the existing web parser reads
server (CLI)	crit-agent-cli.ts — the agent_cmd bridge (parse → thread.turn.start → block-and-return poll → reply)
server	crit-binary-resolver.ts, crit-sidecar-manager.ts (Effect service: spawn + HTTP health-check + ref-count + Scope teardown), crit-sidecar-request.ts
contracts	crit.ts schemas + crit.ensureSidecar/crit.sidecarStatus RPC, critReviewEnabled setting
client/web	EnvironmentApi.crit, CritReviewPanel.tsx, PR-view route wiring behind the flag

Verification

• ~27 unit tests passing (builder 4, wrapper CLI 5, resolver 3, sidecar manager 4, contracts 7, request helper 3, panel 1).
• Typecheck clean across packages/{shared,contracts,client-runtime} and apps/{server,web} for all touched files.
• End-to-end RPC identity verified: client transport.request(WS_METHODS.critEnsureSidecar) ↔ server ws.ts handler share the same Rpc.make contract.
• Flag-off default is safe — with critReviewEnabled false, the native diff/PR path is unchanged and no crit code runs.
• Two-stage review per task caught & fixed: a parser-truncation bug (escaping <), the wrapper's typing/lint issues, and a sidecar crash path that threw at runtime + a non-load-bearing test.

⚠️ Deferred (require building crit; Go was unavailable in the build env)

Tracked in docs/crit-integration-notes.md, marked PENDING in-code:

• 0b real crit CLI flags — crit-sidecar-manager.ts uses placeholder --repo/--branch/--host/--port/--agent-cmd.
• 0c crit's agent_cmd stdin format — crit-agent-cli.ts assumes JSON {comment,quoted,filePath,startLine,endLine} with a plain-text fallback.
• Phase 7 per-platform binary bundling (electron-builder extraResources) + the wrapper's production build path (GITS_CRIT_AGENT_CMD env override for now).
• Phase 8 end-to-end test against a live crit.

🔎 Follow-ups from final holistic review (not blockers for this flag-gated v1)

1. Block-and-return stale-reply race — after dispatch, latestTurn doesn't flip to running synchronously, so a fast first poll can return the prior turn's reply. Fix: baseline the pre-dispatch turn id and only accept a new completed turn.
2. Token is role:"owner", never revoked — deviates from the design's "workspace-scoped, dispatch-only, revoked on teardown." Currently necessary because dispatch/snapshot are owner-gated; needs a scoped-dispatch capability + revocation.
3. release_sidecar not called in production — process/token leak; wire it to CritReviewPanel cleanup via a crit.releaseSidecar WS method (or release on WS disconnect).

🤖 Generated with Claude Code

[Ecko95]

Follow-up #1 (block-and-return stale-reply race) is now fixed in 0bf5d30b: the wrapper baselines the thread's latestTurn id before dispatch and only accepts a turn whose id differs, so a fast first poll can no longer surface the previous (completed) turn's reply. Added a load-bearing test that reproduces the race (fails against the old code). Follow-ups #2 (owner-token scoping/revocation) and #3 (release_sidecar wiring) remain open.

[Ecko95]

Follow-ups #2 and #3 landed in c7f1d152:

• feat(gits): review pipeline (mechanical gate → semantic verifier → triage) #3 — sidecar release wiring (leak fixed): added a crit.releaseSidecar WS method end-to-end (contracts/rpc → client-runtime → EnvironmentApi → ws.ts handler). CritReviewPanel's effect cleanup now calls it, so unmount/input-change releases the refCount its ensureSidecar took.
• feat(gits): acceptance-criteria plumbing for the verifier-critic #2 — token lifecycle (revoke + no-leak): token issuance moved intoCritSidecarManager — minted only on actual spawn (reuse path mints nothing, fixing the per-call leak), with a bounded 2h TTL, and revoked on every teardown path (release-to-0, crash, readiness-timeout) via a finalizer on the per-sidecar scope. Load-bearing tests assert: no second mint on reuse, not-revoked-while-alive, and revoked-on-teardown (normal + crash).

Verification: 16 server-crit + 9 web + contracts tests pass; typecheck clean across contracts/client-runtime/apps-server/apps-web.

Remaining (deliberately not in this change): true least-privilege scoping (owner→thread-bound). /api/orchestration/{dispatch,snapshot} are owner-gated and accept any command, so real scoping needs a dedicated crit endpoint (thread-bound, client-role) + repointing the wrapper — a focused, security-reviewed change better done once crit is buildable for E2E. The 2h-TTL + revoke-on-teardown is the v1 mitigation (loopback-only, flag-off).

[Ecko95]

Crit PR Review Integration (v1 — flag-gated, experimental)

Embeds the crit review binary as a GITS-managed sidecar and surfaces its review UI inside the PR view. "Send to agent" routes into the active GITS thread (so the user's default provider — codex — answers), rather than crit spawning its own agent.

Design: docs/superpowers/specs/2026-06-07-crit-pr-review-integration-design.md
Plan: docs/superpowers/plans/2026-06-07-crit-pr-review-integration.md

Architecture

apps/server spawns one crit process per workspace and sets crit's agent_cmd to a thin GITS wrapper CLI. The wrapper reads crit's comment on stdin, POSTs a thread.turn.start to /api/orchestration/dispatch, blocks while polling /api/orchestration/snapshot until the turn completes, then prints the assistant reply on stdout for crit to thread. apps/web renders crit's loopback URL in an <iframe> panel that replaces the sidebar diff for PR review only, behind an off-by-defaultcritReviewEnabled flag with native-DiffPanel fallback.

Why crit's missing codex support is a non-issue: crit's agent_cmd points at the GITS wrapper, never at codex. The wrapper satisfies crit's stdin→stdout contract; the actual codex run happens inside GITS via the codex app-server. crit never spawns an agent.

What's included

Layer	Component
shared	review-comment-block.ts — builds the <review_comment> block the existing web parser reads
server (CLI)	crit-agent-cli.ts — the agent_cmd bridge (parse → thread.turn.start → block-and-return poll → reply)
server	crit-binary-resolver.ts, crit-sidecar-manager.ts (Effect service: spawn + HTTP health-check + ref-count + Scope teardown), crit-sidecar-request.ts
contracts	crit.ts schemas + crit.ensureSidecar/crit.sidecarStatus RPC, critReviewEnabled setting
client/web	EnvironmentApi.crit, CritReviewPanel.tsx, PR-view route wiring behind the flag

Verification

• ~27 unit tests passing (builder 4, wrapper CLI 5, resolver 3, sidecar manager 4, contracts 7, request helper 3, panel 1).
• Typecheck clean across packages/{shared,contracts,client-runtime} and apps/{server,web} for all touched files.
• End-to-end RPC identity verified: client transport.request(WS_METHODS.critEnsureSidecar) ↔ server ws.ts handler share the same Rpc.make contract.
• Flag-off default is safe — with critReviewEnabled false, the native diff/PR path is unchanged and no crit code runs.
• Two-stage review per task caught & fixed: a parser-truncation bug (escaping <), the wrapper's typing/lint issues, and a sidecar crash path that threw at runtime + a non-load-bearing test.

⚠️ Deferred (require building crit; Go was unavailable in the build env)

Tracked in docs/crit-integration-notes.md, marked PENDING in-code:

• 0b real crit CLI flags — crit-sidecar-manager.ts uses placeholder --repo/--branch/--host/--port/--agent-cmd.
• 0c crit's agent_cmd stdin format — crit-agent-cli.ts assumes JSON {comment,quoted,filePath,startLine,endLine} with a plain-text fallback.
• Phase 7 per-platform binary bundling (electron-builder extraResources) + the wrapper's production build path (GITS_CRIT_AGENT_CMD env override for now).
• Phase 8 end-to-end test against a live crit.

🔎 Follow-ups from final holistic review (not blockers for this flag-gated v1)

1. Block-and-return stale-reply race — after dispatch, latestTurn doesn't flip to running synchronously, so a fast first poll can return the prior turn's reply. Fix: baseline the pre-dispatch turn id and only accept a new completed turn.
2. Token is role:"owner", never revoked — deviates from the design's "workspace-scoped, dispatch-only, revoked on teardown." Currently necessary because dispatch/snapshot are owner-gated; needs a scoped-dispatch capability + revocation.
3. release_sidecar not called in production — process/token leak; wire it to CritReviewPanel cleanup via a crit.releaseSidecar WS method (or release on WS disconnect).

🤖 Generated with Claude Code

[Ecko95]

Follow-up #1 (block-and-return stale-reply race) is now fixed in 0bf5d30b: the wrapper baselines the thread's latestTurn id before dispatch and only accepts a turn whose id differs, so a fast first poll can no longer surface the previous (completed) turn's reply. Added a load-bearing test that reproduces the race (fails against the old code). Follow-ups #2 (owner-token scoping/revocation) and #3 (release_sidecar wiring) remain open.

[Ecko95]

Follow-ups #2 and #3 landed in c7f1d152:

• feat(gits): review pipeline (mechanical gate → semantic verifier → triage) #3 — sidecar release wiring (leak fixed): added a crit.releaseSidecar WS method end-to-end (contracts/rpc → client-runtime → EnvironmentApi → ws.ts handler). CritReviewPanel's effect cleanup now calls it, so unmount/input-change releases the refCount its ensureSidecar took.
• feat(gits): acceptance-criteria plumbing for the verifier-critic #2 — token lifecycle (revoke + no-leak): token issuance moved intoCritSidecarManager — minted only on actual spawn (reuse path mints nothing, fixing the per-call leak), with a bounded 2h TTL, and revoked on every teardown path (release-to-0, crash, readiness-timeout) via a finalizer on the per-sidecar scope. Load-bearing tests assert: no second mint on reuse, not-revoked-while-alive, and revoked-on-teardown (normal + crash).

Verification: 16 server-crit + 9 web + contracts tests pass; typecheck clean across contracts/client-runtime/apps-server/apps-web.

Remaining (deliberately not in this change): true least-privilege scoping (owner→thread-bound). /api/orchestration/{dispatch,snapshot} are owner-gated and accept any command, so real scoping needs a dedicated crit endpoint (thread-bound, client-role) + repointing the wrapper — a focused, security-reviewed change better done once crit is buildable for E2E. The 2h-TTL + revoke-on-teardown is the v1 mitigation (loopback-only, flag-off).

[Ecko95]

Crit PR Review Integration (v1 — flag-gated, experimental)

Embeds the crit review binary as a GITS-managed sidecar and surfaces its review UI inside the PR view. "Send to agent" routes into the active GITS thread (so the user's default provider — codex — answers), rather than crit spawning its own agent.

Design: docs/superpowers/specs/2026-06-07-crit-pr-review-integration-design.md
Plan: docs/superpowers/plans/2026-06-07-crit-pr-review-integration.md

Architecture

apps/server spawns one crit process per workspace and sets crit's agent_cmd to a thin GITS wrapper CLI. The wrapper reads crit's comment on stdin, POSTs a thread.turn.start to /api/orchestration/dispatch, blocks while polling /api/orchestration/snapshot until the turn completes, then prints the assistant reply on stdout for crit to thread. apps/web renders crit's loopback URL in an <iframe> panel that replaces the sidebar diff for PR review only, behind an off-by-defaultcritReviewEnabled flag with native-DiffPanel fallback.

Why crit's missing codex support is a non-issue: crit's agent_cmd points at the GITS wrapper, never at codex. The wrapper satisfies crit's stdin→stdout contract; the actual codex run happens inside GITS via the codex app-server. crit never spawns an agent.

What's included

Layer	Component
shared	review-comment-block.ts — builds the <review_comment> block the existing web parser reads
server (CLI)	crit-agent-cli.ts — the agent_cmd bridge (parse → thread.turn.start → block-and-return poll → reply)
server	crit-binary-resolver.ts, crit-sidecar-manager.ts (Effect service: spawn + HTTP health-check + ref-count + Scope teardown), crit-sidecar-request.ts
contracts	crit.ts schemas + crit.ensureSidecar/crit.sidecarStatus RPC, critReviewEnabled setting
client/web	EnvironmentApi.crit, CritReviewPanel.tsx, PR-view route wiring behind the flag

Verification

• ~27 unit tests passing (builder 4, wrapper CLI 5, resolver 3, sidecar manager 4, contracts 7, request helper 3, panel 1).
• Typecheck clean across packages/{shared,contracts,client-runtime} and apps/{server,web} for all touched files.
• End-to-end RPC identity verified: client transport.request(WS_METHODS.critEnsureSidecar) ↔ server ws.ts handler share the same Rpc.make contract.
• Flag-off default is safe — with critReviewEnabled false, the native diff/PR path is unchanged and no crit code runs.
• Two-stage review per task caught & fixed: a parser-truncation bug (escaping <), the wrapper's typing/lint issues, and a sidecar crash path that threw at runtime + a non-load-bearing test.

⚠️ Deferred (require building crit; Go was unavailable in the build env)

Tracked in docs/crit-integration-notes.md, marked PENDING in-code:

• 0b real crit CLI flags — crit-sidecar-manager.ts uses placeholder --repo/--branch/--host/--port/--agent-cmd.
• 0c crit's agent_cmd stdin format — crit-agent-cli.ts assumes JSON {comment,quoted,filePath,startLine,endLine} with a plain-text fallback.
• Phase 7 per-platform binary bundling (electron-builder extraResources) + the wrapper's production build path (GITS_CRIT_AGENT_CMD env override for now).
• Phase 8 end-to-end test against a live crit.

🔎 Follow-ups from final holistic review (not blockers for this flag-gated v1)

1. Block-and-return stale-reply race — after dispatch, latestTurn doesn't flip to running synchronously, so a fast first poll can return the prior turn's reply. Fix: baseline the pre-dispatch turn id and only accept a new completed turn.
2. Token is role:"owner", never revoked — deviates from the design's "workspace-scoped, dispatch-only, revoked on teardown." Currently necessary because dispatch/snapshot are owner-gated; needs a scoped-dispatch capability + revocation.
3. release_sidecar not called in production — process/token leak; wire it to CritReviewPanel cleanup via a crit.releaseSidecar WS method (or release on WS disconnect).

🤖 Generated with Claude Code

[Ecko95]

Follow-up #1 (block-and-return stale-reply race) is now fixed in 0bf5d30b: the wrapper baselines the thread's latestTurn id before dispatch and only accepts a turn whose id differs, so a fast first poll can no longer surface the previous (completed) turn's reply. Added a load-bearing test that reproduces the race (fails against the old code). Follow-ups #2 (owner-token scoping/revocation) and #3 (release_sidecar wiring) remain open.

[Ecko95]

Follow-ups #2 and #3 landed in c7f1d152:

• feat(gits): review pipeline (mechanical gate → semantic verifier → triage) #3 — sidecar release wiring (leak fixed): added a crit.releaseSidecar WS method end-to-end (contracts/rpc → client-runtime → EnvironmentApi → ws.ts handler). CritReviewPanel's effect cleanup now calls it, so unmount/input-change releases the refCount its ensureSidecar took.
• feat(gits): acceptance-criteria plumbing for the verifier-critic #2 — token lifecycle (revoke + no-leak): token issuance moved intoCritSidecarManager — minted only on actual spawn (reuse path mints nothing, fixing the per-call leak), with a bounded 2h TTL, and revoked on every teardown path (release-to-0, crash, readiness-timeout) via a finalizer on the per-sidecar scope. Load-bearing tests assert: no second mint on reuse, not-revoked-while-alive, and revoked-on-teardown (normal + crash).

Verification: 16 server-crit + 9 web + contracts tests pass; typecheck clean across contracts/client-runtime/apps-server/apps-web.

Remaining (deliberately not in this change): true least-privilege scoping (owner→thread-bound). /api/orchestration/{dispatch,snapshot} are owner-gated and accept any command, so real scoping needs a dedicated crit endpoint (thread-bound, client-role) + repointing the wrapper — a focused, security-reviewed change better done once crit is buildable for E2E. The 2h-TTL + revoke-on-teardown is the v1 mitigation (loopback-only, flag-off).