Skip to content

Feature/Fix: Add conservative lineage safety and promotion controls#2157

Open
iam-brain wants to merge 19 commits into
steipete:mainfrom
iam-brain:iam-brain/lineage-safety-and-promotion
Open

Feature/Fix: Add conservative lineage safety and promotion controls#2157
iam-brain wants to merge 19 commits into
steipete:mainfrom
iam-brain:iam-brain/lineage-safety-and-promotion

Conversation

@iam-brain

@iam-brain iam-brain commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

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.

  • classifies residuals without assigning unavailable history speculatively
  • scopes identities and exceptional discovery to one normalized Codex home
  • contains malformed, incomplete, unresolved, and ambiguous families
  • bounds reconciliation to one family and streams families through two-pass discovery
  • rejects files mutated during discovery
  • isolates caches by accounting mode and keeps legacy as the default
  • requires an opaque authorization produced only by a successful promotion evaluation before .lineage can become authoritative
  • preserves a whole-scan rollback path

This remains reversible and does not remove legacy accounting.

Validation

  • 48 focused engine, discovery, ledger, promotion, and selector tests passed after the review fixes
  • explicit no-authorization fallback coverage
  • make check
  • full suite: 650 selections across 55 groups; no failures, retries, or timeouts
  • repository review gate: no material findings after fixes

The 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.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. labels Jul 14, 2026
@clawsweeper

clawsweeper Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed July 14, 2026, 9:36 PM ET / July 15, 2026, 01:36 UTC.

Summary
The branch adds lineage discovery, reconciliation, residual classification, containment, promotion authorization, selectable accounting modes, scanner integration, and eight focused test suites for Codex token accounting.

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.

  • Patch surface: 18 files, +4,446/-21. The branch introduces a large new accounting subsystem and modifies the central scanner and generated parser fingerprint.
  • Focused test surface: 8 test suites added. Coverage is broad across selector, discovery, engine, ledger, promotion, residual, shadow, and two-pass behavior.
  • Authority modes: 3 modes; legacy remains default. Compatibility depends on the authorization and cache gates reliably preventing unintended lineage authority.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #2037
Summary: The open issue is the canonical fork-overcount report; this PR stack and the narrower cache/identity PR are competing or complementary candidate fixes requiring one maintainer-selected landing path.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add redacted exact-head frozen-corpus cold and warm scan output, including aggregate/model comparisons.
  • Demonstrate unauthorized fallback, authorized promotion, cancellation containment, and legacy rollback on the same head.
  • Link the evidence in the PR body and clarify how it compares with Fix Codex fork parent cache invalidation #2118.

Proof guidance:

  • [P1] Needs real behavior proof before merge: Tests and green CI are supplemental only; add redacted exact-head frozen-corpus terminal output or logs for cold/warm scans and fallback/rollback, then update the PR body to trigger review or ask a maintainer for @clawsweeper re-review.

Risk before merge

  • [P1] Merging the full stack can change authoritative usage and cost totals when lineage mode is promoted; an authorization, identity, containment, or cache mistake could silently undercount or overcount existing users until rollback.
  • [P1] The scanner now performs exceptional parent discovery, content hashing, two-pass traversal, and family reconciliation; unit tests and CI do not establish acceptable cold/warm latency, memory use, and cancellation behavior on a large real corpus.
  • [P1] This PR should not land independently while its two stated predecessors remain unmerged, and the final validation slice has not yet supplied inspectable exact-head proof here.

Maintainer options:

  1. Require stack and corpus proof (recommended)
    Merge dependencies first and add redacted exact-head cold, warm, fallback, promotion, cancellation, and rollback output from a frozen real corpus before landing this PR.
  2. Limit this stage to shadow accounting
    Defer authoritative lineage selection and promotion so existing users remain entirely on the established legacy path while evidence accumulates.
  3. Accept the full architecture
    A maintainer may explicitly sponsor the compatibility and scanner-performance risk after reviewing the entire stack and its final validation evidence.

Next step before merge

  • [P1] A maintainer must select the canonical accounting architecture and require contributor-owned exact-head corpus proof; there is no remaining narrow mechanical defect for an automated repair worker.

Maintainer decision needed

  • Question: Should CodexBar adopt this four-PR lineage accounting architecture, or address the current fork-overcount problem through the narrower identity and cache fix in Fix Codex fork parent cache invalidation #2118?
  • Rationale: Both paths target Ultra-mode Terra and Sol sessions can overcount forked context #2037, but they have materially different scope, maintenance cost, rollout behavior, and proof maturity; source review cannot determine the intended permanent accounting architecture.
  • Likely owner: steipete — The repository owner is the best available authority for selecting the permanent accounting model and rollout policy.
  • Options:
    • Prefer the narrow proven fix (recommended): Land the bounded identity and cache correction first, then require evidence of remaining failures before adopting lineage authority.
    • Sponsor the lineage stack: Confirm the broader architecture, review and merge its dependencies in order, and require exact-head real-corpus promotion and rollback proof before authority can change.
    • Keep lineage shadow-only: Accept analytical discovery and diagnostics while deferring the authoritative selector and promotion surface until longer-term corpus validation is available.

Security
Cleared: The patch adds no dependency, workflow, secret, permission, download, publishing, or third-party execution changes, and no concrete security or supply-chain concern was found.

Review details

Best 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 changes

Label justifications:

  • P2: This is a substantial correctness improvement with limited immediate blast radius because legacy accounting remains the default.
  • merge-risk: 🚨 compatibility: The patch introduces alternate cache namespaces and an authoritative accounting selector that can change persisted scan behavior and reported totals.
  • merge-risk: 🚨 availability: Exceptional discovery, content hashing, and family reconciliation add scanner work whose real-corpus latency, memory, and cancellation behavior are not yet demonstrated on this exact head.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Tests and green CI are supplemental only; add redacted exact-head frozen-corpus terminal output or logs for cold/warm scans and fallback/rollback, then update the PR body to trigger review or ask a maintainer for @clawsweeper re-review.
Evidence reviewed

What I checked:

Likely related people:

  • steipete: The decision concerns the permanent Codex accounting architecture, rollout policy, and merge ordering across a four-PR stack in the repository they own. (role: repository owner and likely decision owner; confidence: high; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift)
  • xx205: They authored the narrower proof-positive fork identity and cache invalidation candidate and are also connected to earlier merged fork-accounting work referenced in repository release history. (role: fork-accounting contributor; confidence: medium; commits: 8152bcae8f6b; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift, Tests/CodexBarTests/CostUsageScannerBreakdownTests.swift, Tests/CodexBarTests/CostUsageCacheTests.swift)
  • jskoiz: Repository release history credits prior work on fork-aware Codex session accounting, making them a useful reviewer for compatibility with the established scanner behavior. (role: earlier fork-accounting contributor; confidence: low; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (19 earlier review cycles; latest 8 shown)
  • reviewed 2026-07-14T22:34:23.530Z sha db9c7dd :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-14T22:52:07.301Z sha db9c7dd :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-14T23:08:55.740Z sha db9c7dd :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-14T23:24:09.795Z sha db9c7dd :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-14T23:40:51.939Z sha db9c7dd :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-14T23:57:17.692Z sha db9c7dd :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-15T00:13:45.530Z sha db9c7dd :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-15T00:42:28.064Z sha db9c7dd :: needs real behavior proof before merge. :: none

@iam-brain
iam-brain force-pushed the iam-brain/lineage-safety-and-promotion branch from f681044 to 0e3033f Compare July 14, 2026 04:52
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 14, 2026
@iam-brain
iam-brain marked this pull request as ready for review July 14, 2026 15:22

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +2654 to +2658
let accountingProducerKey = Self.codexAccountingProducerKey(mode: options.codexLineageAccountingMode)
var cache = CostUsageCacheIO.load(
provider: .codex,
cacheRoot: options.cacheRoot,
producerKey: accountingProducerKey)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Comment on lines +301 to +303
init(eventID: String?, fingerprint: Fingerprint) {
self = eventID.map(Self.event) ?? .fingerprint(fingerprint)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Comment on lines +195 to +199
if let owner = CostUsageScanner.codexRolloutOwnerID(fileURL: fileURL) {
self.filesByIdentity[
.init(scopeID: scopeID, sessionID: CodexLineageTwoPassDiscovery.canonical(owner)),
default: [],
].insert(fileURL)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant