docs(devlog): 260806 ten-item disposition sweep + #1090 CRLF external-provider regression test - #1097
Merged
Merged
Conversation
…s inject byte-for-byte
…pen with status comment
… PASS, pushed unmerged
… rebase lane, log.ts overlap guard
github-actions
Bot
added
the
documentation
|
✅ Deterministic PR hygiene checks passed. |
Contributor
📝 WalkthroughWalkthroughThe change adds plans and ledgers for a ten-item disposition sweep. It records GitHub actions, rebases, audits, validation, and closeout status. It also adds a CRLF integration test for external-provider configuration preservation. ChangesDisposition sweep
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@devlog/_plan/260806_disposition_sweep/000_plan.md`:
- Line 44: Confirm whether PR `#1085` is security-pass-only using the
authoritative GitHub evidence, then synchronize the disposition across all
records: update the planned action in
devlog/_plan/260806_disposition_sweep/000_plan.md:44-44, align the matrix in
devlog/_plan/260806_disposition_sweep/001_disposition_matrix.md:23-23, record
the same action and evidence in
devlog/_plan/260806_disposition_sweep/010_github_dispositions.md:31-44, and make
the posted draft consistent in
devlog/_plan/260806_disposition_sweep/011_comment_drafts.md:165-171.
In `@devlog/_plan/260806_disposition_sweep/010_github_dispositions.md`:
- Around line 3-4: Update the ledger evidence rule and corresponding rows in the
disposition plan so they agree: record actual comment IDs for comment/review
actions and explicit close-state evidence for issue or PR closures, including PR
`#1036` and the close rows in the referenced section. If comment IDs are
unavailable, revise the rule to explicitly distinguish comment evidence from
close evidence.
In `@devlog/_plan/260806_disposition_sweep/011_comment_drafts.md`:
- Around line 84-93: Update the disposition entry for `#1084` to either document
item (3) with exact source locations and a concise explanation of the
quota-parsing defect, or remove the quota-parsing item entirely if it cannot be
substantiated. Leave the cooldown no-op and missing pool-routing consumer
findings unchanged.
In `@devlog/_plan/260806_disposition_sweep/030_936_rebase.md`:
- Around line 21-25: Update the “terra security audit” PASS evidence in the
disposition ledger to include exact file paths and line ranges for each of the
four verified hardening claims: Vertex location, Bun provenance, Claude ambient
fail-closed, and the health attestation gate. Preserve the existing audit result
and claim descriptions while adding concrete file:line references that support
them.
In `@devlog/_plan/260806_disposition_sweep/050_closeout.md`:
- Around line 37-38: Update the sentence containing the `#1090` token in the
closeout text so it no longer begins a Markdown heading: keep the token attached
to the preceding sentence or wrap it in backticks, while preserving the existing
meaning.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b0a392fe-de0a-411f-9585-4f7c910e6add
📒 Files selected for processing (9)
devlog/_plan/260806_disposition_sweep/000_plan.mddevlog/_plan/260806_disposition_sweep/001_disposition_matrix.mddevlog/_plan/260806_disposition_sweep/010_github_dispositions.mddevlog/_plan/260806_disposition_sweep/011_comment_drafts.mddevlog/_plan/260806_disposition_sweep/020_1090_regression_test.mddevlog/_plan/260806_disposition_sweep/030_936_rebase.mddevlog/_plan/260806_disposition_sweep/040_1008_rebase.mddevlog/_plan/260806_disposition_sweep/050_closeout.mdtests/codex-inject-integration.test.ts
| | 7 | #1008 (own, usage rollup) | R3 | rebase, triage 29 threads → fix-now vs redesign, implement fix-now, push — no merge | | ||
| | 8 | #1019 (account picker lifecycle, 106 files) | R5-adjacent | comment: split request into reviewable slices; hygiene gate noted; stays open | | ||
| | 9 | agentHits campaign: PRs #1084/#1083/#1081/#1079/#1077 | R6/R1 | close each with tailored, verified defect list + explicit "complete and reopen" guidance (user rule R1; author is active — audit finding 5 noted, tone must be respectful and specific). Linked issues #1062/#1063/#1060/#1058/#1076/#1082 are IN SCOPE as part of item 9: one policy comment each, stay open. Verified defects: #1084 cooldown no-op (`oauth-account-routes.ts:374` → `clearAnthropicAccountCooldown` Anthropic-only `anthropic-routing.ts:117`), #1081/#1079 invalid TS in six locales (bare string after value) | | ||
| | 10 | #1085 + #997 (easy rebases) | R5-adjacent | comment asking authors to rebase; note READY verdict; stay open | |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Synchronize the PR #1085 disposition across all ledgers.
The plan and matrix require a rebase-request comment. The GitHub ledger records a security-pass comment, and the draft says the branch is current against dev. Confirm the intended action, then update every affected record.
devlog/_plan/260806_disposition_sweep/000_plan.md#L44-L44: update the planned action if#1085is security-pass-only.devlog/_plan/260806_disposition_sweep/001_disposition_matrix.md#L23-L23: align the matrix with the selected action.devlog/_plan/260806_disposition_sweep/010_github_dispositions.md#L31-L44: record the same action and evidence in the ledger.devlog/_plan/260806_disposition_sweep/011_comment_drafts.md#L165-L171: keep the posted comment consistent with the plan and ledger.
📍 Affects 4 files
devlog/_plan/260806_disposition_sweep/000_plan.md#L44-L44(this comment)devlog/_plan/260806_disposition_sweep/001_disposition_matrix.md#L23-L23devlog/_plan/260806_disposition_sweep/010_github_dispositions.md#L31-L44devlog/_plan/260806_disposition_sweep/011_comment_drafts.md#L165-L171
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@devlog/_plan/260806_disposition_sweep/000_plan.md` at line 44, Confirm
whether PR `#1085` is security-pass-only using the authoritative GitHub evidence,
then synchronize the disposition across all records: update the planned action
in devlog/_plan/260806_disposition_sweep/000_plan.md:44-44, align the matrix in
devlog/_plan/260806_disposition_sweep/001_disposition_matrix.md:23-23, record
the same action and evidence in
devlog/_plan/260806_disposition_sweep/010_github_dispositions.md:31-44, and make
the posted draft consistent in
devlog/_plan/260806_disposition_sweep/011_comment_drafts.md:165-171.
Comment on lines
+3
to
+4
| All writes are comments/closes/reviews; no code, no merges. Every action | ||
| records its comment id in the ledger table at the bottom. |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Make the ledger evidence rule match the recorded rows.
Line 4 says every action records its comment ID. The PR #1036 row has only a date, and the close rows contain only gh issue close or gh pr close. Record the actual comment or close-state evidence, or revise the rule to distinguish comment IDs from close evidence.
Also applies to: 35-49
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@devlog/_plan/260806_disposition_sweep/010_github_dispositions.md` around
lines 3 - 4, Update the ledger evidence rule and corresponding rows in the
disposition plan so they agree: record actual comment IDs for comment/review
actions and explicit close-state evidence for issue or PR closures, including PR
`#1036` and the close rows in the referenced section. If comment IDs are
unavailable, revise the rule to explicitly distinguish comment evidence from
close evidence.
Comment on lines
+84
to
+93
| > Closing this draft for now — the direction (Antigravity account pool) is | ||
| > wanted, but the current cut implements configuration without the runtime | ||
| > that would use it: (1) no pool-routing consumer reads the added config; | ||
| > (2) the cooldown endpoint accepts `google-antigravity` but calls | ||
| > `clearAnthropicAccountCooldown`, which only clears the Anthropic health | ||
| > map (`src/server/management/oauth-account-routes.ts` → | ||
| > `src/oauth/anthropic-routing.ts`) — a functional no-op for the new | ||
| > provider; (3) quota parsing duplicates existing logic. Please reopen (or | ||
| > open fresh) with a slice that wires a real consumer first — a generic | ||
| > pool-routing path for Google accounts — and we will review it properly. |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repo files around plan:"
git ls-files | rg '(^|/)011_comment_drafts\.md$|audited|matrix|devlog/_plan' | head -200
echo
echo "Target file excerpt:"
sed -n '70,105p' devlog/_plan/260806_disposition_sweep/011_comment_drafts.md | cat -n
echo
echo "Search for 1084 and quota parsing references:"
rg -n "1084|quota parsing|quota-parsing|quota|parse" devlog tests src package.json --glob '!node_modules' --glob '!dist' --glob '!build' | head -300Repository: lidge-jun/opencodex
Length of output: 45781
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Disposition matrix around `#1084`:"
sed -n '1,220p' devlog/_plan/260806_disposition_sweep/001_disposition_matrix.md | cat -n | rg -n "1084|quota|pool|cooldown|Antigravity|google-antigravity" -C 5 || true
echo
echo "Find PR `#1084` identifiers in committed files:"
rg -n "google-antigravity|clearAnthropicAccountCooldown|quarant|quota|accountQuota|AccountQuota|antigravity|AccountPool|pool" devlog/_plan/260806_disposition_sweep src tests --glob '!node_modules' --glob '!dist' --glob '!build' | head -400
echo
echo "List files modified in the current diff/stat (if available):"
git diff --stat -- 2>/dev/null || true
git diff --name-only -- 2>/dev/null | head -200 || trueRepository: lidge-jun/opencodex
Length of output: 50375
Remove or evidence the quota-parsing defect.
The disposition matrix only lists the cooldown no-op and no pool-routing consumer for #1084. Add exact source locations and why the parsing path is a defect, or remove item (3).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@devlog/_plan/260806_disposition_sweep/011_comment_drafts.md` around lines 84
- 93, Update the disposition entry for `#1084` to either document item (3) with
exact source locations and a concise explanation of the quota-parsing defect, or
remove the quota-parsing item entirely if it cannot be substantiated. Leave the
cooldown no-op and missing pool-routing consumer findings unchanged.
Comment on lines
+21
to
+25
| | Step | Evidence | | ||
| |------|----------| | ||
| | Merge dev into branch | `a90981e67` (origin/dev `b3a1d90a8` → `codex/916-trust-boundaries`); conflicts: auth-cors.ts (redactSecretString + effectiveGoogleMode composed), server/index.ts (localAttestationSecret folded into StartServerDeps, CLI caller → object form) | | ||
| | Duplication check | terra: no equivalent hardening landed on dev since branch point `6a7351b4d` — nothing double-applies | | ||
| | terra security audit | FAIL(3 stale test call-sites for old positional secret) → fixed in `4874390dd` → PASS; four hardening claims verified on merged tree with file:line (Vertex location, Bun provenance, Claude ambient fail-closed, health attestation gate) | |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Include the file and line evidence for the PASS claim.
Line 25 says the four hardening claims were verified “with file:line,” but it lists only claim names. Add exact paths and line ranges for the Vertex location, Bun provenance, Claude ambient fail-closed, and health attestation gate. The ledger must support the recorded audit result.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@devlog/_plan/260806_disposition_sweep/030_936_rebase.md` around lines 21 -
25, Update the “terra security audit” PASS evidence in the disposition ledger to
include exact file paths and line ranges for each of the four verified hardening
claims: Vertex location, Bun provenance, Claude ambient fail-closed, and the
health attestation gate. Preserve the existing audit result and claim
descriptions while adding concrete file:line references that support them.
Comment on lines
+37
to
+38
| retracted with ancestry evidence; sweep range = 7 devlog commits + the | ||
| #1090 test commit). |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Prevent #1090 from becoming a malformed heading.
Line 38 starts with #1090 without a space. This triggers the reported MD018 warning. Keep the token on the previous sentence, or wrap it in backticks.
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 38-38: No space after hash on atx style heading
(MD018, no-missing-space-atx)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@devlog/_plan/260806_disposition_sweep/050_closeout.md` around lines 37 - 38,
Update the sentence containing the `#1090` token in the closeout text so it no
longer begins a Markdown heading: keep the token attached to the preceding
sentence or wrap it in backticks, while preserving the existing meaning.
Source: Linters/SAST tools
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99b3b21207
ℹ️ 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".
Comment on lines
+25
to
+27
| | terra security audit | FAIL(3 stale test call-sites for old positional secret) → fixed in `4874390dd` → PASS; four hardening claims verified on merged tree with file:line (Vertex location, Bun provenance, Claude ambient fail-closed, health attestation gate) | | ||
| | Tests | typecheck clean; full suite 9076 pass / 0 fail / 8 skip (579 files, 281s) | | ||
| | Push | `727722cba..4874390dd` on origin; PR #936 OPEN draft — NOT merged (human security review per MAINTAINERS.md still required) | |
There was a problem hiding this comment.
PR #936 is explicitly still open, unmerged, and awaiting human security review, yet this tracked document records secret-handling and attestation hardening details from its audit. That publishes pre-disclosure security work before the fix has shipped; move these details to .tmp/ and commit only the published outcome after the security review and release are complete.
AGENTS.md reference: AGENTS.md:L79-L83
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
devlog/_plan/260806_disposition_sweep/): audited plan, per-item disposition matrix, comment drafts, and a live end-state ledger with comment ids and head SHAs for every action.#1090regression test: a CRLF Windows-style config with an externalmodel_provider = "deepseek"and a coexisting[model_providers.opencodex]table must surviveinjectCodexConfig()byte-for-byte. Red-ablation was performed (guard neutered → test fails; restored → passes).Verification
bun test tests/codex-inject-integration.test.ts— 23 pass / 0 fail (new test included).bun test tests/codex-inject-integration.test.ts tests/codex-inject.test.ts tests/codex-inject-write-lock.test.ts— 58 pass / 0 fail.bun x tsc --noEmit— clean.Checklist
Summary by CodeRabbit
Documentation
Tests