fix(sessions): agents start without a broken managed skill instead of not starting at all

[Juliusolsson05]

Problem

SessionManager runs the managed-skills reconcile before every agent spawn. When it threw, if (hasReportingDomain(options.builtInMcpDomains)) throw error aborted the spawn for any session with TLDR or Goal enabled, and that is nearly every pane. So one managed-skill failure meant no agent in any provider could start. The panes still restored their transcripts, so the workspace looked healthy while nothing was running. The journal then recorded recover.failed code=start-failed with no cause.

This happened twice in two days, from unrelated causes. On Sept 18, "TLDR skill deployment failed" appeared on a build before grok, and the cause was lost. On Sept 19, grok's unsupported-provider health hit the gate (#1014, fixed in #1017 and #1037). In the Sept 19 journals, every reconcile error is followed at seq+1 by recover.failed (15/15 and 14/14). #1017 and #1037 fixed specific failure modes. This PR limits the blast radius for any future cause.

Implemented behavior

• A reconcile failure never aborts a spawn now. The agent launches without the broken skill. Its MCP token stays registered, and the TLDR/Goal tools still bring their own server instructions.
• Independent steps. New AgentCodeManagedSkillsService.prepareForAgentSpawn(domains) runs the audit, TLDR and Goal as separate steps and returns the failures. Before, a broken TLDR skill (or a failed audit) also skipped Goal.
• Visible warning. For each requested skill that failed, SessionManager emits managed-skills-unavailable. This happens only after the post-reconcile cancellation check, so a restore cancelled during the reconcile gets no toast and no false row. The forwarder broadcasts managed-skills:unavailable { skills }, and GlobalToastProvider shows a 10 s click-to-dismiss toast. The same skill set is not re-toasted within 60 s. For example: "TLDR skill could not be prepared, so agents started without it. Review Settings › Agents › Custom Skills." That Settings section is where TLDR/Goal appear as "Managed by … MCP" rows with their health and target status.
• Journal. Each failure is recorded under the existing conventions.pre_spawn_reconcile.error name, now with data.skill and ids.sessionId, so current greps keep working. A new conventions.pre_spawn_reconcile.degraded (warn, same ids.sessionId) records which skills the session is launching without, so the two rows join per pane. AppRunJournal.recordError gained an optional ids argument for this.
• recover.failed has a typed cause. SESSION_START_FAILURE_CAUSES in src/shared/lifecycle/events.ts has four values: missing-workspace, cli-not-found, provider-launch and unknown. Raw exception text still stays out of the stream and out of IPC.

Design decisions and tradeoffs

• Launching without the skill is the default the user accepted in bug(sessions): one failing managed-skill reconcile stops every agent from starting #1133. It can be changed. The old strict rule treated the skill as part of the capability. The rewritten WHY comment (runPreSpawnSkillReconcile) records the tradeoff: a missing skill costs guidance about when to call the tools, while the old rule turned any skill failure into a workspace-wide outage. If a strict mode is ever wanted, it should be an explicit setting, not the silent default.

• Why the global toast. I checked the existing channels first:

  • Provider conditions are interactive, provider-specific TUI states with actions, so they are the wrong shape.
  • Pane toasts are driven by the renderer and would repeat the same banner in every restoring pane.
  • Routing through session:* / sendToSessionWindow would quarantine the event for ids that no window has claimed, and record a false routing gap.
  • GlobalToast already takes a main→renderer push (extensions:notification) and is app-wide. That fits, because skill health is machine-wide. Its single slot collapses the N identical warnings from a restore into one.

  The new IPC channel only feeds this existing UI.

• A hook that throws still warns. The production hook reports failures and does not throw. If it throws anyway, no one knows which skills were prepared, so every requested reporting skill is treated as unavailable. Under-warning would bring back the silent omission the strict rule was meant to prevent.

• Audit-only failures do not toast. Before this PR that was not nonfatal. An audit throw shared the hook's single catch, so it also aborted every TLDR/Goal launch. Only sessions without reporting domains survived it. The Sept 18 outage could therefore have been an audit failure as easily as a skill deploy failure; the .error row's message tells them apart. Now it is journal-only: the audit never removes a skill the agent asked for (the product skills run as their own steps), and its degraded health already shows in Settings.

• Repeat window. Main warns on every launch while a skill stays broken. GlobalToast holds back a repeat of the same skill set for 60 s, per window, so the toast does not come back all day or push out extension notifications. A new skill set shows at once, and the journal still records every occurrence.

• No conventions-gate cause. After this change the gate cannot fail a start, so the value could never be emitted. A dead value would look like a live failure mode during triage.

• How causes are classified. ProviderCliNotFoundError is a new class that keeps the old message unchanged. provider-launch is attached through a WeakMap tag at the provider start() catch sites instead of wrapping the error, because Codex replacement compensation and callers depend on the original error's identity and message. Nothing matches on message substrings.

Tests and verification

Each new test fails on origin/main (verified by stashing the source changes) and passes on this branch:

• sessionManager.wake.test.ts: the two tests that pinned the old abort now pin the new contract:
  • a TLDR or Goal failure still spawns, does not revoke the MCP token, emits the warning and journals .error and .degraded;
  • a throwing hook still spawns and warns for ['tldr','goal'] in fixed order;
  • a restore cancelled during the reconcile gets no warning and no .degraded row, but still gets the .error row;
  • an audit-only failure does not warn.
• sessionManager.lifecycle.test.ts: recover.failed.cause is missing-workspace, cli-not-found, provider-launch or unknown, and the provider error text (TOKEN=secret-value) does not appear in the event.
• AgentCodeCustomSkillsService.system.test.ts (real service, real files): with a user-owned file blocking TLDR, prepareForAgentSpawn(['tldr','goal']) reports the TLDR failure, leaves the user's file alone, and still gets Goal to active.
• forwarder.test.ts: the warning is broadcast with domain names only.
• src/shared/types/tldr.test.ts: the message text names the skill(s) and points to Settings.

Verification on Node 24.14.1:

• npx tsc -b: clean.
• vitest run on the touched suites plus the recover and codexReplacement suites: 132 passed. AppRunJournal.completeness: 4 passed.

The app was not launched.

Limitations

• The toast is transient. The lasting record is the skill's health row in Settings, which the toast points to. Spawn and recover results do not carry the warning, so remote clients and automation get no warning beyond the journal.
• The toast fires once the reconcile has failed and the spawn is still live, but before the provider starts. If the provider launch then fails too, the user sees the skill toast followed by the usual start-failure message (recover.failed with its own cause).
• The 60 s repeat window is renderer-only and has no test (it is a few lines of ref logic in GlobalToast).
• cause is added only to recover.failed. Fresh session:spawn failures still reject to the caller as before, and there is no lifecycle event for them.
• The Sept 18 root cause ("TLDR skill deployment failed" before grok) is still unknown. It would now show up as a toast plus a journaled .error with skill: 'tldr', not as an outage.

Implementation plan: docs/superpowers/plans/2026-09-23-skill-gate-blast-radius.md (first commit, updated for the review round). Review fixes: 7bef296e.

Fixes #1133

🤖 Generated with Claude Code