devlog: record the third Spark projection found during wp2 - #2657
Conversation
020_phase2.md named two server projections carrying the Spark window. The implementation found three: a codexAccountMode "direct" install reports quota through fetchMainAccountInfoSnapshot, which never touches the Codex Auth DTO, so filtering at quotaForPlan alone left it visible. It surfaced because tests/provider-quota.test.ts still PASSED after the first attempt - a test asserting Spark is present, passing when the filter was supposed to have removed it. Recorded so the doc matches what shipped in bf73afe.
|
✅ 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 (1)
📝 WalkthroughWalkthroughThe suppression plan expands quota visibility coverage to Codex Auth, pooled provider, and direct-mode provider projections. It centralizes filtering in ChangesQuota visibility projections
Estimated code review effort: 1 (Trivial) | ~2 minutes 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 |
![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: a62117623e
ℹ️ 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".
| The filter therefore lives in `withSparkVisibility` (exported from `auth-api.ts`) and is | ||
| applied at BOTH `quotaForPlan` and `providerQuotaFromCodexQuota`, the latter being the one | ||
| point every Codex-sourced provider report funnels through. |
There was a problem hiding this comment.
The new description correctly says direct mode bypasses quotaForPlan and requires filtering again in providerQuotaFromCodexQuota, but the unchanged Dependency section at lines 144–148 still concludes that both surfaces funnel through quotaForPlan, calls it the single filtering point, and requires only two test targets. Anyone implementing or auditing this open work unit from that summary could recreate the direct-mode leak this commit documents; update or remove that conclusion so it names both filter boundaries and the direct-mode regression case.
Useful? React with 👍 / 👎.
1 participant
Summary
Documentation correction for the wp2 unit merged in #2649.
020_phase2.mdnamed two server projections carrying the Codex Spark window. The implementation found three: acodexAccountMode: "direct"install reports quota throughfetchMainAccountInfoSnapshot(providers/quota.ts:1121), which never touches the Codex Auth DTO — so filtering atquotaForPlanalone left it visible.It surfaced because
tests/provider-quota.test.tsstill passed after the first attempt: a test asserting Spark is present, passing when the filter was supposed to have removed it. That is the useful kind of test failure, and worth recording rather than quietly fixing.The shipped code already handles all three (
withSparkVisibilityis applied at bothquotaForPlanandproviderQuotaFromCodexQuota). This makes the doc match what shipped.Verification
bun test tests/codex-spark-visibility.test.ts tests/provider-quota.test.ts tests/codex-auth-api.test.ts tests/repo-hygiene.test.ts— receipt taken, 0 faildevlog/.No GUI change, so no screenshot applies.
Checklist
Summary by CodeRabbit