Skip to content

fix(air): drop catalog ids removed since a session was created instead of failing its resume or fork - #1258

Merged
tadasant merged 4 commits into
mainfrom
fix/unarchive-catalog-drift
Oct 8, 2026
Merged

tadasant merged 4 commits into
mainfrom
fix/unarchive-catalog-drift

Conversation

@tadasant

@tadasant tadasant commented Oct 8, 2026 •

Copy link
Copy Markdown
Owner

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 prepare exited 1 on the stale id (Unknown MCP server ID "gmail-tadas412-readonly", session 22934, GlitchTip zimmer-backend #185), so UnarchiveSessionService and then AgentSessionJob both failed and paged #alerts.

What changed

  • AirPrepareService#reconciled_catalog_selection replaces the skills-only scrubbed_catalog_skills. It reconciles all four columns (mcp_servers, catalog_skills, catalog_hooks, catalog_plugins) against the catalog before air prepare. It iterates Session.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.
  • WARN, not ERROR. Drift logs one WARN line. The skills path used to ErrorReporter.report_message(level: :error) a "Session self-healed" alert. That alert is removed, because catalog drift is expected and should not page anyone.
  • Recorded. The pruned column is persisted with update_column. The dropped ids accumulate in custom_metadata["dropped_unknown_catalog_ids"], keyed by column, and Session#dropped_unknown_catalog_ids reads 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 empties mcp_servers, the empty list is marked deliberate (record_explicit_mcp_servers!([])) so McpServerBackfill does not refill it with root defaults the session never chose. The dropped servers' mcp_servers_status entries are forgotten, so they are not re-reported as lost on every regeneration.
  • The agent is told. AgentSessionJob#build_prompt_with_goal appends 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, and sessions/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.
  • Forks too. ForkSessionService, which also makes the status-summary fork, copied the source's stale ids into a new row, and create! 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's dropped_unknown_catalog_ids record over and adds to it anything it dropped itself, logs at WARN, and marks a server list emptied by drift as explicitly empty.
  • Fresh creation is unchanged. A start_session that names an unknown id is still rejected by CatalogArtifactReferences validation. That is a caller error, not drift.
  • No alias map. A hardcoded rename table would go stale, and CatalogArtifactReferences already documents that a successor cannot be derived. This is recorded as a new limitations.md entry, which points at the backfill-migration pattern for renames that should carry live sessions.
  • Docs: air/zimmer-integration.md is rewritten for the generalized guard, and a new limitations entry is added. Comment references to the renamed method were updated mechanically, including in the zimmer-change-ai-artifact skill.

Verification

  • Searched for rival work on Unarchive/resume hard-fails when a session names an MCP server since removed from the AIR catalog (AirPrepareError: Unknown MCP server ID) #1257: no PR carries a closing keyword, no open PR mentions 1257 or "Unknown MCP server ID", and no remote branch names it.
  • Tests added. air_prepare_service_test.rb covers an MCP server id dropped, persisted and recorded with the stale id kept out of the air prepare argv; hook and plugin drops that accumulate with earlier records and leave no dangling --plugin flag; 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.rb covers the unarchive path: the real prepare! runs down to the subprocess seam with gmail-tadas412-readonly, which is dropped and recorded. agent_session_job_test.rb covers the resume/follow-up path: the real prepare!, 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.rb covers 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.
  • CI green. These tests run in CI only: this container has no Postgres for a local run.
  • bin/rubocop is clean on all touched Ruby files.
  • Self-review and a fresh-eyes subagent review are done. All four warnings were fixed: the whole-catalog load guard, an emptied server list kept empty, the notice clearing for re-added ids, and the spawning.md row. 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-type config.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

Zimmer Agent and others added 2 commits October 8, 2026 16:32
…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>
@tadasant tadasant added ready to merge Agent-authored PR: reviewed and CI-green; ready to merge and removed ready to merge Agent-authored PR: reviewed and CI-green; ready to merge labels Oct 8, 2026
Zimmer Agent and others added 2 commits October 8, 2026 16:50
…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>
@tadasant tadasant changed the title fix(air): drop catalog ids removed since a session was created instead of failing its resume fix(air): drop catalog ids removed since a session was created instead of failing its resume or fork Oct 8, 2026
@tadasant tadasant added the ready to merge Agent-authored PR: reviewed and CI-green; ready to merge label Oct 8, 2026
@tadasant

tadasant commented Oct 8, 2026

Copy link
Copy Markdown
Owner Author

🚀 Merge gate

Verdict: 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: e13a6167735c4484d0a63a2bfd7d98db7cefe0f0

  • H2/H3 considered and cleared (zimmer hold-list row 3, what a session's environment contains): the change can only remove ids that the catalog no longer resolves. There is no alias map, and effective_mcp_servers is still mcp_servers + plugin_mcp_servers, minus the dropped ids.
Rating detail

Head: e13a616 · Base: main

Problem (#1257): when a session names an MCP server, skill, hook or plugin that the AIR catalog has since removed, air prepare exits 1 and the unarchive or resume fails, which pages #alerts (session 22934, gmail-tadas412-readonly).
Solution: AirPrepareService#reconciled_catalog_selection reconciles all four catalog columns before air prepare. It drops unknown ids, writes the pruned column, records the dropped ids in custom_metadata["dropped_unknown_catalog_ids"], and adds a <dropped-catalog-artifacts> prompt block. ForkSessionService copies only ids the catalog still resolves. Creating a fresh session with an unknown id is still rejected.
Aligned: yes. Self-contained: yes. Merging it fixes the resume and fork failures, with nothing else needed.

Hold tests:

  • H1 no. Row 1 is not touched: no auth, API-key or tool-group scoping change. The selection can only shrink, and an emptied server list is marked explicitly empty so McpServerBackfill does not refill it with root defaults the session never chose (a tightening).
  • H2 no. Row 3 is considered and cleared as above. Row 2 does not apply, since no mcp.json entry changes. Rows 4 and 5 do not apply: what gets recorded is catalog ids in the session's own custom_metadata and a WARN log line, and nothing new leaves the box.
  • H2b no. The removed ErrorReporter "self-healed" alert is an alert, not a recovery mechanism, and real air prepare failures still raise.
  • H3 no. Nothing new becomes reachable, persisted beyond ids, or exported.
  • S1 is not engaged: no UI or MCP capability is added or changed, and the new metadata key shows in both through the existing session views and get_session.

The four-axis rating is recorded for the ledger. It did not decide this.

Axis Rating Why
Requirement complexity small A clear, observed failure with a known trigger: catalog drift.
Requirement impact medium Hits resume, unarchive and fork of any session after a catalog rename. Not every session, but it pages when it fires.
Solution complexity medium Generalizes a skills-only scrub to four columns across prepare and fork, with persistence, the prompt block and docs.
Implementation risk medium Sits on the session-startup path (air prepare) for every resume. A bug here breaks startups loudly, and a revert fixes it.

Worth noting: the PR body's Noticed, not filed names the same per-type guard gap in CatalogArtifactReferences#heal_stale_catalog_reference! on the trigger path, which this PR leaves as it is.

@tadasant
tadasant merged commit 91ce25e into main Oct 8, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready to merge Agent-authored PR: reviewed and CI-green; ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unarchive/resume hard-fails when a session names an MCP server since removed from the AIR catalog (AirPrepareError: Unknown MCP server ID)

1 participant