Repository navigation
fix(air): drop catalog ids removed since a session was created instead of failing its resume or fork - #1258
Conversation
…d of failing its resume A session that names an MCP server, skill, hook or plugin the AIR catalog has since renamed or removed failed every later unarchive and follow-up: `air prepare` exits 1 on an unknown id (zimmer#1257, session 22934 with `gmail-tadas412-readonly`). AirPrepareService#reconciled_catalog_selection generalises the skill-only scrub to all four columns: drop ids the catalog does not know, log at WARN (no ErrorReporter page), persist the pruned column, and record the dropped ids in custom_metadata["dropped_unknown_catalog_ids"]. AgentSessionJob renders them into a <dropped-catalog-artifacts> prompt block so the agent knows what it lost. Fresh session creation still rejects unknown ids at validation. Closes #1257 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…server list empty, clear notice for re-added ids Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ot fail its create! Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…iltering a fork's selection Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
🚀 Merge gateVerdict: MERGE: reversible. Unknown catalog ids are now dropped from a session's own selection on resume and fork, where before they failed it. Nothing is added or widened. No gate, auth or tool-group scope, credential or data boundary moves. Rated head:
Rating detailHead: e13a616 · Base: main Problem (#1257): when a session names an MCP server, skill, hook or plugin that the AIR catalog has since removed, Hold tests:
The four-axis rating is recorded for the ledger. It did not decide this.
Worth noting: the PR body's Noticed, not filed names the same per-type guard gap in |
Summary
Closes #1257. A session that names an MCP server, skill, hook or plugin that the AIR catalog renamed or removed after the session was created no longer fails its unarchive or follow-up. Before this change,
air prepareexited 1 on the stale id (Unknown MCP server ID "gmail-tadas412-readonly", session 22934, GlitchTip zimmer-backend #185), soUnarchiveSessionServiceand thenAgentSessionJobboth failed and paged#alerts.What changed
AirPrepareService#reconciled_catalog_selectionreplaces the skills-onlyscrubbed_catalog_skills. It reconciles all four columns (mcp_servers,catalog_skills,catalog_hooks,catalog_plugins) against the catalog beforeair prepare. It iteratesSession.catalog_artifact_references, the declarations zimmer#537-style validation already uses, and each*Config.exists?is the same check that validation runs. Every id the catalog does not know is dropped and the survivors are prepared.ErrorReporter.report_message(level: :error)a "Session self-healed" alert. That alert is removed, because catalog drift is expected and should not page anyone.update_column. The dropped ids accumulate incustom_metadata["dropped_unknown_catalog_ids"], keyed by column, andSession#dropped_unknown_catalog_idsreads them. The guards: a catalog that failed to load entirely (every facade empty) strips nothing. That check looks at the whole catalog rather than one type at a time, so a type that legitimately becomes empty, such as the only hook being removed, still gets reconciled. A degraded catalog drops ids in memory only and records nothing. A failed write degrades to a WARN, and the record is written before the column, so a partial failure is retried on the next prepare rather than lost. When drift emptiesmcp_servers, the empty list is marked deliberate (record_explicit_mcp_servers!([])) soMcpServerBackfilldoes not refill it with root defaults the session never chose. The dropped servers'mcp_servers_statusentries are forgotten, so they are not re-reported as lost on every regeneration.AgentSessionJob#build_prompt_with_goalappends a<dropped-catalog-artifacts>block, listing the dropped ids by kind, to every later prompt, leaving out any id the session names again. It sits next to the existing<unavailable-mcp-servers>block, andsessions/spawning.md's prompt-block table lists it. It asks the agent to use a successor and say which one, or to say plainly that it has none.ForkSessionService, which also makes the status-summary fork, copied the source's stale ids into a new row, andcreate!failed validation (Mcp servers contains invalid server(s): gmail-tadas412-readonly, session 22934, 16:21:35Z). It now copies only the ids the catalog still knows; whether the catalog loaded at all is judged across the whole catalog, not one type at a time. It carries the source'sdropped_unknown_catalog_idsrecord over and adds to it anything it dropped itself, logs at WARN, and marks a server list emptied by drift as explicitly empty.start_sessionthat names an unknown id is still rejected byCatalogArtifactReferencesvalidation. That is a caller error, not drift.CatalogArtifactReferencesalready documents that a successor cannot be derived. This is recorded as a newlimitations.mdentry, which points at the backfill-migration pattern for renames that should carry live sessions.air/zimmer-integration.mdis rewritten for the generalized guard, and a new limitations entry is added. Comment references to the renamed method were updated mechanically, including in thezimmer-change-ai-artifactskill.Verification
air_prepare_service_test.rbcovers an MCP server id dropped, persisted and recorded with the stale id kept out of theair prepareargv; hook and plugin drops that accumulate with earlier records and leave no dangling--pluginflag; skill drop at WARN with no ErrorReporter call; a one-time drop across repeated prepares; a degraded catalog recording nothing; a stale hook dropped even when the hook catalog is empty; and an emptied server list marked explicitly empty with its status forgotten.unarchive_session_service_test.rbcovers the unarchive path: the realprepare!runs down to the subprocess seam withgmail-tadas412-readonly, which is dropped and recorded.agent_session_job_test.rbcovers the resume/follow-up path: the realprepare!, the session not failed, and the spawned prompt carrying the<dropped-catalog-artifacts>block naming the server. A prompt-builder test checks that an id the session names again is left out.fork_session_service_test.rbcovers the fork path: a source with a stale MCP server and skill forks successfully, the stale ids are not copied, and the dropped-id record is carried and extended. It also covers a stale hook dropped when the hook catalog is empty, and a server list emptied by drift marked explicitly empty.bin/rubocopis clean on all touched Ruby files.spawning.mdrow. The useful nits were fixed too: write ordering, forgetting dropped server status, the stale skill reference, and deriving labels from the references. A second fresh-eyes review of the fork change flagged that the per-type guard would bring back the empty-type gap, so the fork now uses the whole-catalog check, and a blank-entry edge case is fixed.Noticed, not filed
CatalogArtifactReferences#heal_stale_catalog_reference!(trigger path) has the same per-typeconfig.all.empty?guard, so a type whose catalog becomes empty passes stale names through on a trigger fire. That code is not touched here.🤖 Generated with Claude Code