fix(tracker): reconcile the v4 capture-owned columns, and stop wiping undeclared evidence - #1373
Open
joeharris76 wants to merge 1 commit into
Open
fix(tracker): reconcile the v4 capture-owned columns, and stop wiping undeclared evidence#1373joeharris76 wants to merge 1 commit into
joeharris76 wants to merge 1 commit into
Conversation
… undeclared evidence Two defects in what phase 5 shipped. First: there was no update path for the v4 columns on an already-landed record. related_paths and suggested_sweep sit outside _CONTENT_FIELDS and the reconcile covered only evidence, so re-syncing reported 'skipped' and left both columns NULL forever -- the state every imported legacy record was in. Reconcile now covers them plus the extra finding_sections rows, each gated on being DECLARED so a draft that never mentioned a key cannot null what an import put there. Second, found by running the fix against the live evidence-tree record: gating at the differs level was not enough. Once reconcile is triggered by some OTHER capture-owned field, the unconditional evidence replace wiped curated rows the draft had no opinion about -- reintroducing by a different route exactly the destructive behaviour the evidence_declared guard exists to prevent. Evidence is now only touched when the draft asserts an evidence list, with a regression test. Event renamed resync_evidence -> resync_capture_owned and its detail now names what changed.
joeharris76
enabled auto-merge (squash)
July 31, 2026 02:22
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Owner
Author
|
@codex review |
![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: 7e81a105db
ℹ️ 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
+813
to
+814
| if _evidence_differs(existing, fields) or _sections_differ(existing, fields): | ||
| return True |
There was a problem hiding this comment.
When an existing finding already has an extra ## section and a same-ID draft edits or removes it, this condition routes the draft into _reconcile_capture_owned, which deletes/replaces finding_sections and marks the draft synced. Extra sections are body prose, so this bypasses the existing same-ID/different-content conflict contract and can silently erase imported audit text; reconciliation should only backfill missing v4 section rows, while differing nonempty sections should remain a conflict.
Useful? React with 👍 / 👎.
joeharris76
added a commit
that referenced
this pull request
Aug 1, 2026
…hema decision Writes down the six decisions the cutover turns on, the runbook, the TODO_DB_RO_AUTH_TOKEN rotation and the rollback path. Fills the ADR gap left by #1377. The load-bearing decision is D2: SCHEMA_VERSION stays 4 and the freeze stays advisory against stale clones. Bumping it would be real enforcement but reproduces the #1347 total-tracker-outage (the version check runs before every subcommand, including read-only ones) that todo-cli-schema-bump-must-ship-with-db-migration-v2 exists to prevent. D3 is the consequence: the freeze is necessary but NOT sufficient, so the runbook pairs it with independently verified quiescence. Also records why the cutover is not authorized today: the tracker is provably not quiescent, its quiescence probe is vacuous, and two findings-domain fixes (#1359, #1373) are still unmerged.
github-actions Bot
pushed a commit
that referenced
this pull request
Aug 1, 2026
) * fix(tracker): make the write freeze fail closed and cover migrate Follow-up defects in the leased write freeze (#1377). Each was reproduced against the merged commit before being fixed; CI was green throughout, so none of these were caught by the suite as shipped. - A non-finite `--ttl` was accepted through the documented flag and then wedged the tracker permanently: `timedelta(hours=inf)` raised inside every mutating command, `--status`, `--release` AND `--release --force`, leaving raw SQL as the only recovery. That is precisely the stuck lock the leased design exists to prevent. `--ttl` is now validated finite, positive and bounded by MAX_FREEZE_TTL_HOURS (168h), and the read path coerces a tampered `meta` row instead of raising, so an already-wedged database can be released. - A malformed freeze stamp escaped as a raw traceback: get_freeze guarded the TTL `float()` but not `_lease_expired`'s `strptime`, and `_freeze_refusal` runs outside main()'s try/except. It now exits 2 cleanly. - A torn freeze (holder row written, stamp row not) read as expired and let every other actor write straight through -- a safety gate whose degraded state was "unlocked". Freeze liveness now fails CLOSED, deliberately opposite to claim leases, which must fail open so items can be reclaimed. The escape hatch is unaffected: `freeze` is exempt from the gate, so a torn freeze is still liftable. - `migrate` bypassed the gate entirely when migrations were pending: it must run before connect_backend(), which refuses an outdated schema, so the normal gate was unreachable on exactly the databases with pending migrations. A schema change could land mid-cutover through a live freeze. It now runs its own check against a raw connection; the freeze holder can still migrate, since it runs the cutover. - set_freeze/clear_freeze did check-then-act across two transactions. Both now read the incumbent inside `_write_txn`, matching claim_item. Also fixes a pre-existing codespell typo ("createable") in the same file, which blocks pre-commit for anyone touching it. Verified: gate mutation-tested (neutering each fix fails a test), 1281 unit tests pass, ruff clean, timing policy 26214/26550. * docs(decisions): record the tracker cutover runbook and the freeze/schema decision Writes down the six decisions the cutover turns on, the runbook, the TODO_DB_RO_AUTH_TOKEN rotation and the rollback path. Fills the ADR gap left by #1377. The load-bearing decision is D2: SCHEMA_VERSION stays 4 and the freeze stays advisory against stale clones. Bumping it would be real enforcement but reproduces the #1347 total-tracker-outage (the version check runs before every subcommand, including read-only ones) that todo-cli-schema-bump-must-ship-with-db-migration-v2 exists to prevent. D3 is the consequence: the freeze is necessary but NOT sufficient, so the runbook pairs it with independently verified quiescence. Also records why the cutover is not authorized today: the tracker is provably not quiescent, its quiescence probe is vacuous, and two findings-domain fixes (#1359, #1373) are still unmerged.
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.
Two defects in what phase 5 shipped.
First: there was no update path for the v4 columns on an already-landed record. related_paths and suggested_sweep sit outside _CONTENT_FIELDS and the reconcile covered only evidence, so re-syncing reported 'skipped' and left both columns NULL forever -- the state every imported legacy record was in. Reconcile now covers them plus the extra finding_sections rows, each gated on being DECLARED so a draft that never mentioned a key cannot null what an import put there.
Second, found by running the fix against the live evidence-tree record: gating at the differs level was not enough. Once reconcile is triggered by some OTHER capture-owned field, the unconditional evidence replace wiped curated rows the draft had no opinion about -- reintroducing by a different route exactly the destructive behaviour the evidence_declared guard exists to prevent. Evidence is now only touched when the draft asserts an evidence list, with a regression test.
Event renamed resync_evidence -> resync_capture_owned and its detail now names what changed.