docs(devlog): record the Wave 3/4 second pass, audit corrections and deferrals - #1865
Conversation
…deferrals Documents what the continuation established, including the corrections two independent read-only audits forced on pre-written plans. - 090 (#1524): the plan's two-mechanism split was unnecessary -- policy fallback reaches comboFailureDecision through shouldHopPolicyCandidate, so one change covers both paths. Modality filtering already worked; the plan's image tests would have passed before any change. Records the remaining optimization. - 101 (#1049): corrected plan after audit. The original was unreachable (withCodexWriteLock refuses routed residue before inject.ts can adopt) and reintroduced the crash window the archived contract removes. Deferred with the corrected five-file shape recorded. - 102 (#1798): a true baseline/injected/current merge is not implementable from today's journal, which stores only a hash of the injected state. Records what shipped instead and why the catalog claim in the plan was false. - 120: continuation outcome, per-head remote suite numbers against the dev baseline, and the ablation lesson -- disabling the structured-code arm alone still passes because a message.includes fallback catches it, so only the ordering ablation proves the mechanism. - 130 (#1795): new. The undeclared-tool guard is a deliberate fail-closed contract, so the request is a safety-boundary loosening whose scope is a product decision. NEEDS_HUMAN with the evidence gap named.
|
✅ Deterministic PR hygiene checks passed. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR adds Wave 3/4 closeout records for capability admission, legacy adoption, configuration restoration, undeclared tools, deferred work, and verification results. ChangesWave 3/4 closeout records
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested labels: Suggested reviewers: ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57e07ba74a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| (`src/server/responses/policy-fallback.ts:72`) delegates to it, which the plan missed | ||
| when it proposed re-evaluating candidates inside `policy-fallback.ts:153`. | ||
|
|
||
| So the context-window half of #1524 is closed by one change rather than two. |
There was a problem hiding this comment.
Retract the claim that context fallback is closed
With the real local 413 response, the message contains "context window", so classifyError returns context_length_exceeded and comboFailureDecision stops at src/combos/failover.ts:124 before reaching the input_admission_refused check at line 132. The newly added 120_outcome.md:100-105 records this same ordering defect and leaves PR #1864 open, so this section incorrectly describes both fallback paths as fixed; mark the first change incomplete until the admission check is ordered before the generic stop list.
Useful? React with 👍 / 👎.
| a `capability-unsatisfied` exclusion. `rankPolicyFallbackCandidates` only considers | ||
| candidates with `eligible === true` and zero exclusions | ||
| (`policy-fallback.ts:32`), so an image request can never hop onto a text-only candidate. | ||
| The plan's proposed image tests would have passed before any change. |
There was a problem hiding this comment.
Keep combo modality filtering in the remaining work
This evidence path applies only to policy profiles. Combo selection still uses payloadEligible in src/server/responses/core.ts:1298-1299, which checks only encrypted-task decryptability; tests/server-combo-failover-e2e.test.ts:1440-1470 even demonstrates an image request invoking a noVisionModels target rather than skipping it. Therefore the proposed test that a combo skips a text-only candidate would not have passed before this change, so qualify this conclusion as policy-only and retain the combo modality preflight as an unresolved case.
Useful? React with 👍 / 👎.
Summary
Devlog-only. Records what the Wave 3/4 continuation established, including the corrections two independent read-only audits forced on plans that had been written against a stale snapshot.
090([Bug]: preflight fallback candidates for context-window and input-modality compatibility #1524) — the plan's two-mechanism split was unnecessary: policy fallback reachescomboFailureDecisionthroughshouldHopPolicyCandidate, so one change covers both paths. Modality filtering already worked, so the plan's proposed image tests would have passed before any change. Records the remaining optimization (request context size is still unknown at initial evaluation).101(Adopt pre-substrate Codex homes into the write coordinator #1049) — corrected plan. The original was unreachable (withCodexWriteLockrefuses routed residue beforeinject.tscan adopt anything) and its "pending row before publishing" step reintroduces the crash window the archived contract exists to remove. Deferred, with the corrected five-file shape recorded.102(ocx stop/restore leave unmarked openai_base_url + proxy models_cache behind when Codex app rewrote config after injection #1798) — a true baseline/injected/current merge is not implementable from today's journal, which stores only a hash of the injected state. Records what shipped instead, and why the plan's catalog claim was false for the reportedmodels_cache.jsoncase.120— continuation outcome, per-head remote suite numbers against thedevbaseline, and the ablation lesson from this pass.130([Bug]: sensenova hallucinates undeclared exec tool in subagent requests causing 502 stream disconnected #1795) — new. The undeclared-tool guard is a deliberate fail-closed contract, so the request is a safety-boundary loosening whose scope is a product decision. NEEDS_HUMAN with the evidence gap named.Verification
Nothing in the build, typecheck, or test path reads from
devlog/, so no code gate is affected.privacy:scandoes read it, and these documents contain no credentials, tokens, account identifiers, or home paths.Remote Linux suite numbers recorded in
120, taken at each PR head:798ecbfb7(earlier baseline)acfedae0a(#1861)6cd5b04b3(#1862)The 16 failures are identical across all three and are
bun-not-on-PATH harness cases. No regression.Checklist
devsrc/,tests/, orgui/changes#1795note describes a shipped, publicly visible guard)devlog/is maintainer-facingSummary by CodeRabbit