Feature/Fix: Add conservative lineage safety and promotion controls#2157
Feature/Fix: Add conservative lineage safety and promotion controls#2157iam-brain wants to merge 19 commits into
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 14, 2026, 9:36 PM ET / July 15, 2026, 01:36 UTC. Summary Reproducibility: not applicable. as a PR reproduction result. The underlying fork-overcount problem has source and corpus evidence in related work, but this PR still lacks its own exact-head real-corpus demonstration. Review metrics: 3 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Choose one canonical accounting strategy, land the smallest proven correction for the current overcount first, and keep lineage shadow-only until the complete stack has exact-head frozen-corpus evidence for cold/warm scans, authorization fallback, promotion, cancellation, cache invalidation, and whole-scan rollback. Do we have a high-confidence way to reproduce the issue? Not applicable as a PR reproduction result. The underlying fork-overcount problem has source and corpus evidence in related work, but this PR still lacks its own exact-head real-corpus demonstration. Is this the best way to solve the issue? Unclear. The implementation is internally coherent and well tested, but maintainers have not established that this 18-file lineage architecture is preferable to the narrower proof-positive correction in #2118. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against c61e01e774c4. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (19 earlier review cycles; latest 8 shown)
|
08c97bb to
f681044
Compare
f681044 to
0e3033f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2de15085fb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let accountingProducerKey = Self.codexAccountingProducerKey(mode: options.codexLineageAccountingMode) | ||
| var cache = CostUsageCacheIO.load( | ||
| provider: .codex, | ||
| cacheRoot: options.cacheRoot, | ||
| producerKey: accountingProducerKey) |
There was a problem hiding this comment.
Re-check authorization before returning cached lineage totals
When .lineage is requested without codexLineagePromotionAuthorization after an earlier authorized scan, this still loads the lineage-specific cache; if plan.shouldRefresh is false, applyCodexLineageAccounting is skipped and buildCodexReportFromCache returns the cached lineage totals even though the selector would have fallen back to legacy. This also delays newly granted authorization until the next refresh. Key the unauthorised path to the legacy authority or force the selection/authorization gate before serving a mode cache.
Useful? React with 👍 / 👎.
| init(eventID: String?, fingerprint: Fingerprint) { | ||
| self = eventID.map(Self.event) ?? .fingerprint(fingerprint) | ||
| } |
There was a problem hiding this comment.
Do not use turn IDs as sole observation identities
For token_count rows that carry the same turn ID/ordinal in different rollout files but have different last/total values, this maps both to the same .event key and the later billed observation is discarded. Turn IDs are only per-turn hints and are not event-cardinal across forks, so the fingerprint needs to remain part of the identity unless the ID is a proven per-event ID.
Useful? React with 👍 / 👎.
| if let owner = CostUsageScanner.codexRolloutOwnerID(fileURL: fileURL) { | ||
| self.filesByIdentity[ | ||
| .init(scopeID: scopeID, sessionID: CodexLineageTwoPassDiscovery.canonical(owner)), | ||
| default: [], | ||
| ].insert(fileURL) |
There was a problem hiding this comment.
Index parents by parsed session IDs, not only filename owners
When a child’s forked_from_id names the parent session_meta.id but the parent file name does not end in that same UUID, this index never records the parent, so discovery marks the ancestor unresolved and reconciles the child without its copied-prefix context. The existing scanner index parses session metadata for exactly this lookup; the lineage locator should also index the descriptor/summary session ID instead of relying only on codexRolloutOwnerID.
Useful? React with 👍 / 👎.
Stack dependency
This is PR 3 of 4. It follows PR #2156; review and merge that PR first.
Summary
Adds the conservative safety and promotion layer needed before lineage accounting can become authoritative.
.lineagecan become authoritativeThis remains reversible and does not remove legacy accounting.
Validation
make checkThe prior Linux x64 failure was in unrelated Claude platform-gating tests; the same lineage head passed Linux arm64 and macOS, and the corresponding x64 jobs passed on the other three PRs.
Related investigation: #2037.