fix(collaboration): stop reporting unknown catalog state as a catalog change

[abhisheksharma2411]

Fixes #1354.

Summary

v2 collaboration guidance mapped unknown to the same text as stale:

// src/server/responses/collaboration.ts
if (catalogState.state === "stale" || catalogState.state === "unknown") {
  return "<multi_agent_mode>The model catalog changed after Codex started; do not set "
    + "model or reasoning_effort overrides until Codex restarts.</multi_agent_mode>";
}

unknown does not mean the catalog changed. It means the comparison could not be made at all. collectCodexAppServerCatalogState returns unknown when the catalog mtime is unreadable, when process enumeration fails, or when any app-server's start time is unreadable:

// src/codex/app-server-processes.ts
if (catalogMtimeMs === null || withStarts.some(proc => proc.startedAtMs === null)) {
  return { state: "unknown", processes: withStarts, catalogMtimeMs };
}

A transient code-mode-host process that exits between enumeration and the start-time read lands exactly there, which is why the message appears intermittently and alternates with normal guidance from turn to turn, as the reporter describes.

So the text asserted a cause that was never established, and prescribed a remedy that cannot work — restarting Codex does not clear unknown, because the state is a failed measurement rather than a stale process.

Suppressing overrides while the state is unknown is still correct and is unchanged. Only the claim changes. unknown now gets its own message, mirroring the honest phrasing ocx doctor already uses for the same state (src/cli/doctor.ts):

Could not verify whether Codex's model catalog is current (app-server start time or catalog timestamp unreadable); do not set model or reasoning_effort overrides for this turn.

An operator reading a turn can now tell "the catalog changed" apart from "we could not check", and is no longer told to perform a restart that would not help.

Verification

Reproduced with a failing test before the fix — unknown returned the stale text verbatim:

expect(unknown).not.toContain("The model catalog changed after Codex started")
  Received: "<multi_agent_mode>The model catalog changed after Codex started; do not set
             model or reasoning_effort overrides until Codex restarts.</multi_agent_mode>"
(fail) v2 guidance does not assert a catalog change or a restart remedy when the state is unknown (#1354)

Commands run against the dev base (branch is on 3976d34):

bun test tests/multi-agent-compat.test.ts   → 42 pass, 0 fail
bun x tsc --noEmit                          → clean
coverage src/server/responses/collaboration.ts → 84.00% lines

The new test pins four things: stale keeps its exact wording; unknown asserts neither the cause nor the restart remedy; both still suppress the override instruction and positive model claims (so the #857 behaviour cannot regress); and the two messages stay distinguishable from each other.

The existing #857 test — which asserts both states suppress positive claims — passes unchanged.

Checklist

• Scope stays focused and avoids unrelated cleanup. — two files, one behavioural branch split, no drive-by changes.
• Docs or release notes were updated when needed. — no user-facing document describes this message; the unknown wording is deliberately aligned with the existing ocx doctor text so the two surfaces agree. Happy to add a note if you'd like one.
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. — no secrets or auth touched. The change is strictly less assertive: it removes a false claim while keeping the conservative suppression, so no path becomes more permissive.

Notes for review

• not_running and fresh are untouched.
• I kept "do not set … overrides" in the unknown text so the instruction itself is unchanged; only the justification and the remedy differ.
• Wording is lifted from doctor.ts on purpose — if you'd rather the two surfaces diverge, or prefer different phrasing, happy to adjust.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

• All CI tests are green on my local testing.

• I pushed my PR to the latest dev commit.

• I resolved all correct Codex and CodeRabbit findings.

• My PR is ready for review.

Summary by CodeRabbit

• Bug Fixes

  • Improved guidance when catalog freshness cannot be verified, with messaging distinct from known stale catalogs.
  • Prevented potentially misleading model and reasoning-effort recommendations when catalog status is unknown.

• Tests

  • Added regression coverage for stale and unverifiable catalog states.

[github-actions]

✅ Deterministic PR hygiene checks passed.

[coderabbitai]

📝 Walkthrough

Walkthrough

The v2 multi-agent guidance now handles stale and unknown catalog states with different messages. Regression coverage verifies state-specific text, override suppression, and continued suppression of positive model claims.

Changes

Catalog Guidance

Layer / File(s)	Summary
State-specific guidance and regression coverage src/server/responses/collaboration.ts, tests/multi-agent-compat.test.ts	stale retains catalog-change and restart guidance. unknown reports that catalog freshness could not be verified and suppresses model and reasoning-effort overrides. Tests cover both states and their distinct output.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

• #1395 — The change distinguishes unknown from stale in multiAgentGuidanceText(), matching the issue’s described no-override guidance behavior.

Suggested reviewers: wibias, lidge-jun, ingwannu

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes satisfy #1354 by separating unknown and stale guidance while preserving override suppression and adding regression coverage.
Out of Scope Changes check	✅ Passed	The changes are limited to collaboration guidance and regression tests required to address #1354.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: separating unknown catalog state guidance from catalog-change reporting.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ READY

• all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

• ✅ All CI tests are green on my local testing.
• ✅ I pushed my PR to the latest dev commit.
• ✅ I resolved all correct Codex and CodeRabbit findings.
• ✅ My PR is ready for review.

✅ 4/4 boxes ticked.

This pull request has been marked Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently. If no CodeRabbit review appears, comment @coderabbitai review to request one.
Maintainers notified: @lidge-jun @Ingwannu @Wibias

Hygiene

✅ Deterministic PR hygiene checks passed.

[Ingwannu]

The wording split is directionally useful, but the current head still preserves the underlying request-scoping bug confirmed in #1395, so I am requesting changes.

collectCodexAppServerCatalogState() folds every current-user app-server into one global state, and the inbound request carries no sender PID or catalog fingerprint. A known-stale process A can therefore make a fresh process B receive this PR's do not set model or reasoning_effort instruction. Rewording only unknown does not fix that case, and the same identity limitation applies to both stale and unknown.

For either state, suppress OpenCodex's disk-derived preferred-model, roster, fallback, and custom guidance, but do not prohibit options advertised by the active spawn_agent tool. The minimal safe behavior is to return no OCX-authored v2 guidance for stale or unknown; keep fresh and not_running unchanged.

Please update the regression so a mixed stale/fresh process set can still produce the global stale observation without producing a blanket no-override instruction for an unidentified request. Also update the catalog/subagent behavior documentation, because this changes the user-visible guidance contract. Once that is done on current dev and exact-head CI is green, this can cover both #1354 and #1395 without a duplicate PR.

[coderabbitai]

@github-actions[bot]: The pull request is ready for review. I will review the changes.

⚠️ Action not completed

No files to review.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[Wibias]

Requesting changes. The wording split is useful, but the current head is not merge-ready.

Merge blockers

1. Global stale/unknown state still blocks valid overrides for unrelated fresh sessions

collectCodexAppServerCatalogState() folds every detected current-user app-server into one global state. A stale process A therefore makes the global result stale, even if the current request came from a fresh process B. multiAgentGuidanceText() has no sender PID/catalog identity, yet this PR still injects do not set model or reasoning_effort overrides for that global state.

That preserves the confirmed #1395 request-scoping bug. For stale and unknown, suppress OpenCodex-authored disk/catalog guidance, but do not prohibit model/reasoning options exposed by the active spawn_agent tool. The minimal safe behavior is to emit no OCX-authored v2 model guidance for those states, or otherwise make the active tool schema authoritative.

2. The new regression test locks in that incorrect contract

The added test explicitly requires both stale and unknown to contain do not set, so it codifies the #1395 bug instead of protecting the intended boundary.

Please add a mixed stale/fresh regression: one process starts before the catalog, another after it, collector resolves globally to stale, and an unidentified current v2 request must not receive a blanket no-override instruction.

Additional correctness gap

The new unknown message says (app-server start time or catalog timestamp unreadable), but unknown can also result from process-enumeration failure. That still asserts a cause which is not established. If any unknown guidance remains, keep it cause-neutral, e.g. Could not verify whether Codex's model catalog is current.

Docs / readiness

Because #1407 is now the selected overlapping fix path for #1354 and #1395, update the catalog/subagent guidance documentation to describe the actual stale/unknown contract.

The reviewed head is also behind current dev, and exact-head React Doctor / Cross-platform CI have not executed jobs yet (action_required, zero jobs). Rebase or merge current dev and get exact-head CI green before merge.

Security

I found no new exploitable security vulnerability in this diff. The blocking issues are correctness, request scoping, and regression coverage.

[coderabbitai]

@github-actions: The pull request is ready for review. I will review the current changes.

⚠️ Action not completed

No files to review.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.