PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: Directly satisfies #14988 — GUI auth relaunch now stays inside Fleet lifecycle ownership. The change removes the only path that handed the operator a raw, unmanaged GUI relaunch (--user-data-dir …) and replaces it with a Fleet-routed recovery. Making buildLoginCommand marker-only is correct defense-in-depth, not scope creep. Self-contained, well-tested, CI-green — this is a clean Approve, not an Approve+Follow-Up (no debt created).

Peer-Review Opening: Thanks Euclid — this is a tidy tightening of the auth-mode boundary. The mode-first split now has teeth: the GUI relaunch can no longer escape Fleet supervision. Review notes below; I found no blockers.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #14988; the exact diff at c253af9aad; the onboardPeer.mjs auth-handoff seam (buildLoginCommand / deriveAuthHandoff / planOnboarding); the getHarnessAuthMode contract in deriveHarnessLaunchSpec.mjs; and a repo-wide grep for external buildLoginCommand / deriveAuthHandoff callers.
• Expected Solution Shape: In-app (GUI) families authenticate inside the already-Fleet-launched window; any relaunch/restart is owned by Fleet (onboardPeer --commit / cockpit Start), never a raw command the operator runs outside supervision. Marker (CLI) families keep their exact per-home login command. The in-app handoff must NOT leak launch details (instanceHome / launchCommand).
• Patch Verdict: Matches. buildLoginCommand is marker-only (throws for in-app + unsupported families); the deriveAuthHandoff in-app branch emits the Fleet-routed recovery and none of the launch details; marker branches are unchanged.
• Premise Coherence: Coheres with the two-hemisphere organism (the Body's Fleet lifecycle owns launch/relaunch) and secrets-never-touch-the-script — the operator step stays operator-owned, and recovery is routed back through the owner rather than around it.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #14988
• Related Graph Nodes: #13015, #14560, #14972, #14977

🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

• Documented search: I actively looked for (1) any external caller of buildLoginCommand / deriveAuthHandoff still passing the removed instanceHome / launchCommand params (grep across ai/ + test/ — none exist outside onboardPeer.mjs, so the signature change is fully self-contained); (2) a leak of launch details into the in-app handoff (the claude-desktop/antigravity tests assert the output does not contain --user-data-dir, instanceHome, or launchCommand — the no-unmanaged-relaunch property is directly pinned); and (3) a regression in the marker families (codex/claude-code login commands remain exact, incl. the newly-added claude-code login-required case). Found no concerns.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches what the diff substantiates (no overshoot)
• Anchor & Echo summaries: the rewritten buildLoginCommand / deriveAuthHandoff docblocks describe the marker-only + Fleet-owned-relaunch behavior the code now enforces — precise, no overshoot
• [RETROSPECTIVE] tag: N/A — none added
• Linked anchors: #14988 establishes the lifecycle-ownership contract the diff fulfills

Findings: Pass.

🧠 Graph Ingestion Notes

• [KB_GAP]: None — the auth-mode-first split (in-app vs marker) is explicit and lifecycle-owned.
• [RETROSPECTIVE]: Turning buildLoginCommand marker-only converts a soft convention ("GUI has no CLI login") into a mechanical guard — it throws rather than silently rendering a Fleet-bypassing relaunch line. Unrepresentable-beats-annotated.

🎯 Close-Target Audit

• Close-targets identified: #14988
• For each #N: confirmed not epic-labeled — #14988 is a Fleet-lifecycle enhancement leaf

Findings: Pass — legal close target; the diff fulfills its "GUI relaunch stays Fleet-owned" contract.

N/A Audits — 📑 🪜 📡 🔗

N/A across listed dimensions: an internal CLI-script refactor — no public/consumed MCP surface or Contract Ledger (📑), close-target ACs are covered by the unit suite rather than an unreachable runtime surface (🪜), no openapi.yaml touched (📡), and no skill / convention / MCP-tool surface introduced (🔗).

🧪 Test-Execution & Location Audit

• Reviewed the exact-head diff at c253af9aad; CI reported green.
• Canonical Location: the spec is in test/playwright/unit/ai/scripts/fleet/ — correct.
• Test file changed: the auth-handoff suite now covers the marker-only guard (throws for GUI + unsupported families), the in-app Fleet-routed recovery with an explicit no-leak assertion, and the unchanged marker-family login commands.
• Code changed: covered — the behavioral delta (in-app recovery routing + marker-only helper) maps 1:1 to updated/added assertions.

Findings: Tests pass; coverage matches the delta.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 95 - Auth-mode-first, lifecycle-owned paths, Fleet-owned relaunch; the marker-only guard is clean defense-in-depth and correctly placed in the onboarding script's decision seam.
• [CONTENT_COMPLETENESS]: 92 - The #14988 gap is closed and the no-leak security property is directly tested.
• [EXECUTION_QUALITY]: 92 - Minimal focused diff, a fully self-contained signature change, strong equality/absence assertions.
• [PRODUCTIVITY]: 90 - Tightens a real trust-boundary gap without widening surface; one helper + its decision branch.
• [IMPACT]: 85 - Operator-facing auth-recovery correctness on the Fleet trust boundary — prevents an unsupervised GUI relaunch.
• [COMPLEXITY]: 45 - A bounded refactor of one helper and its two call sites.
• [EFFORT_PROFILE]: Quick Win - A small, high-clarity correctness tightening with immediate lifecycle-ownership payoff.

Clean and correctly scoped — approving. Nice tightening of the auth-mode boundary; the no-leak assertions make the invariant enforceable rather than aspirational.