Reconcile stale failure ledger projections - #1493
Conversation
Signed-off-by: Chris0Jeky <jeky.tck@gmail.com>
Signed-off-by: Chris0Jeky <jeky.tck@gmail.com>
|
@codex review exact head b3838d0. Please focus on seed-only and issue-less projection semantics, append-only ledger truth, resolution evidence/overclaiming, future appendability of tests, linked-issue safety, and generated-render synchronization. All earlier-head evidence is stale if this head moves. |
Deliberate self-review findings — exact head
|
Self-review fix evidence
Verification after the fixes:
The outstanding |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Review ResultNo findings on exact head
Tooling LimitationI could not independently post the required no-findings review comment or re-read live GitHub comments because this checkout has neither the Testing
Documentation
Outstanding Maintainer ChecklistOpen items remain in The explicit operational residuals also remain: |
Review ResultNo findings at exact head
Testing and Verification
Outstanding Maintainer Items
|
Independent adversarial review — projection/test-correctnessExact head: CRITICALNone. HIGHNone. MEDIUM
Exact negative probe (temporary files only): @'
import sys, tempfile
from pathlib import Path
sys.path.insert(0, str(Path.cwd() / "scripts" / "agent_hooks"))
import render_failure_ledger as r
for name, payload in {"malformed": "{not-json}\n", "non_object": "[]\n"}.items():
with tempfile.TemporaryDirectory() as d:
source, target = Path(d) / "failure_ledger.jsonl", Path(d) / "FAILURE_LEDGER.md"
source.write_text(payload, encoding="utf-8")
target.write_text("sentinel", encoding="utf-8")
old_source, old_target = r.JSONL, r.MD
r.JSONL, r.MD = source, target
try: rc = r.main()
finally: r.JSONL, r.MD = old_source, old_target
output = target.read_text(encoding="utf-8")
body = output.partition("| --- | --- | --- | --- | --- | --- | --- |")[2].partition("## Classification")[0]
rows = [line for line in body.splitlines() if line.startswith("| ")]
print(f"{name}: rc={rc} sentinel_preserved={output == 'sentinel'} rendered_data_rows={len(rows)}")
'@ | py -3 -B -Result: Expected fix: validate every nonblank source line as a JSON object, report the line/error, return nonzero, and leave the existing Markdown untouched; add both malformed and non-object entry-point regressions.
The projection helper tests are useful, but the only @'
import io, sys, unittest
from pathlib import Path
sys.path.insert(0, str(Path.cwd() / "scripts" / "agent_hooks"))
import render_failure_ledger as r
import test_render_failure_ledger as tests
original = r.project_latest_entries
r.project_latest_entries = lambda entries: []
try:
result = unittest.TextTestRunner(stream=io.StringIO(), verbosity=0).run(
unittest.defaultTestLoader.loadTestsFromModule(tests))
finally:
r.project_latest_entries = original
print(f"drop_all_render_rows_mutant_tests_run={result.testsRun} failures={len(result.failures)} errors={len(result.errors)} successful={result.wasSuccessful()}")
'@ | py -3 -B -Result: Expected fix: add a positive mixed-input render assertion proving a normal issue-less row appears and seed does not, plus a future-appendable repository synchronization test that parses the checked-in JSONL and compares LOWNone. Exact-head evidence and comment triage
No out-of-scope finding was discovered. |
Signed-off-by: Chris0Jeky <jeky.tck@gmail.com>
|
Addressed both MEDIUM findings from the changes-requested projection review on new exact head Finding → fix → evidence:
Additional exact-head verification:
Exact-head CI and CI Extended are running as |
|
@codex review Please perform a fresh adversarial review of PR #1493 at exact head |
![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: 75eb802359
ℹ️ 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".
Signed-off-by: Chris0Jeky <jeky.tck@gmail.com>
Review-fix evidence — exact head
|
|
@codex review Please perform a fresh adversarial review of PR #1493 at exact head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0d90457b9
ℹ️ 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".
Signed-off-by: Chris0Jeky <jeky.tck@gmail.com>
Review-fix evidence — exact head
|
|
@codex review Please perform a fresh adversarial review of PR #1493 at exact head |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Chris0Jeky
left a comment
Chris0Jeky
left a comment
There was a problem hiding this comment.
Independent adversarial review — operational truth
Exact head: 9842e945af62416262a52db846f4cf71122346ae.
CRITICAL
None.
HIGH
None.
MEDIUM
- Narrow the stale zero-CI-coverage claim to the remaining smoke/deny-floor gap —
docs/STATUS.md:25.
This PR makes Required Docs Governance execute test_render_failure_ledger.py, but STATUS still says scripts/agent_hooks/ has zero CI coverage. That now contradicts the new invariant added at the top of the same document. Issue #1476 remains valid for the ungated smoke_test.py / configured-handler deny-floor matrix; update the historical residual wording to say that precise remaining gap instead of zero coverage.
LOW
- Fail closed when source metadata/read access is denied on Python 3.14 —
scripts/agent_hooks/render_failure_ledger.py:94.
Path.exists() can return False for a source PermissionError under the documented/local Python 3.14 runtime. The renderer then treats the denied ledger as missing, returns 0, and overwrites an existing Markdown target with an empty projection. Exact probe: Python 3.14 returned rc=0 / target_preserved=False; Python 3.13 returned rc=1 / preserved. Read directly, catch only FileNotFoundError as the valid missing-source case, and add a regression proving other OSError values remain nonzero and preserve the target.
- Refresh the materially changed testing guide date —
docs/TESTING_GUIDE.md:5.
The guide now documents a new fail-before-render required check, but its Last Updated line remains 2026-07-14. Set it to 2026-07-26 so the active document's date matches its new operational contract.
Review state
All current conversation comments, both resolved Codex threads, the exact-head Codex no-major-issues response, project/closing metadata, and hosted checks were swept. The exact head remained pinned during reproduction. These findings block merge and require a fresh exact-head review/CI round after the fix; no finding is waived.
Signed-off-by: Chris0Jeky <jeky.tck@gmail.com>
|
Formal review
Fix evidence:
|
|
@codex review Please perform a fresh adversarial review of exact head |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Chris0Jeky
left a comment
Chris0Jeky
left a comment
There was a problem hiding this comment.
Independent adversarial review — exact-head operational truth
Exact head: 161638cf80a4b7d6116cf08a7b5cd8c61e1ec2e4.
CRITICAL
None.
HIGH
None.
MEDIUM
None.
LOW
- Narrow the live #1476 tracker now that this PR adds partial agent-hook CI coverage — GitHub issue #1476.
The source docs are now accurate, but the durable tracker itself still says scripts/agent_hooks has zero CI coverage, no automated gate whatsoever, and that searching .github/ finds no agent_hooks references. At this exact head, Required Docs Governance runs test_render_failure_ledger.py; only the configured-handler smoke_test.py / deny-floor matrix remains ungated. Leaving #1476 unchanged gives future agents a false current-state premise and can cause them to duplicate the renderer gate while triaging the real smoke/configuration gap.
Expected fix: update #1476's title and opening/current-state text to distinguish the newly gated renderer synchronization suite from the still-ungated configured-handler/deny-floor smoke matrix. Keep #1476 open for that remaining work; no source change is required for this metadata-only correction.
Exact-head evidence
- Read all 26 current conversation items, all five submitted reviews, and both resolved inline threads, including review
4781131779, fix map5082405095, and the exact-head Codex no-major-issues notice. - The three findings in
4781131779are fixed in source: STATUS narrows #1476 correctly; Python 3.14-shaped denied reads return 1 and preserve the sentinel; TESTING_GUIDE now has the exact2026-07-26date. - Python 3.14 and 3.13 focused suites each passed 11/11. Malformed/non-object probes returned 1 and preserved the target. The drop-all mutation failed 2/11 as intended. A temporary stale-Markdown control failed exactly the synchronization assertion.
- Raw JSONL is a byte-prefix append of the 19-row base plus four rows; the 23 raw rows project to 15 (14 resolved / one open), seed stays raw-only, and
invalid_signal / ci/e2e-smokeremains the sole open projection. The checked-in Markdown equals the exact HEAD render. - YAML parsed; Required CI calls the reusable docs workflow. Hosted job
Docs Governance / Docs Governancecompleted successfully with Setup Python → synchronization test → three governance checks in that order. Exact-head CI Extended and Workflow Lint are green. - All three local governance checks and hook smoke passed. Scope is exactly eight files, JSONL numstat is
4/0,git diff --checkpassed, DCO trailers are present 6/6, and local/remote head remained exact and clean.
NOT verified / residual risk
- Required CI run
30191454890was still in progress because its Windows API integration job had not completed at review time; this review does not claim the whole run green. - No backend/frontend/full E2E suite was rerun locally because product runtime code did not change.
- The denied-read regression uses the deliberate Python 3.14-shaped mock seam rather than changing real Windows ACLs.
This LOW finding blocks merge under the repository's all-severity review policy until the existing tracker metadata is corrected and mapped back here.
|
Formal exact-head review
|
Chris0Jeky
left a comment
Chris0Jeky
left a comment
There was a problem hiding this comment.
Independent adversarial review — final exact-head operational/provenance verdict
Exact head: 161638cf80a4b7d6116cf08a7b5cd8c61e1ec2e4.
CRITICAL
None.
HIGH
None.
MEDIUM
None.
LOW
- Live #1476 tracker overstated the remaining CI gap — resolved during this review.
At the start of this exact-head review, #1476 still claimed that scripts/agent_hooks had zero/no CI coverage even though this PR makes Required Docs Governance run the failure-ledger synchronization suite. The accurate residual is the configured-handler smoke_test.py / deny-floor matrix.
Resolution verified: without changing the PR head, #1476 was updated at 2026-07-26T06:50:22Z to “Gate configured-handler hook smoke matrix in Required CI.” Its current body distinguishes the renderer gate delivered here, keeps the remaining smoke/configuration work open, depends on #1487's launcher contract, and contains concrete acceptance criteria. No source change is required and no LOW finding remains open.
Exact-head evidence
- Re-read the complete eight-file diff and the pre-submission live feedback set: 28 merged conversation items, six submitted reviews, and both inline threads resolved. This includes operational review
4781131779, its fix map5082405095, exact-head review4781149104, and resolution map5082426644. - Independently confirmed all three findings from review
4781131779are fixed: STATUS precisely narrows #1476; denied reads fail nonzero while preserving the target; TESTING_GUIDE carries the exact2026-07-26date. - Python 3.14 and 3.13 focused suites each passed 11/11. The real hook smoke matrix passed with the Python 3.14 path prepared. Independent denied-read probes on both runtimes returned 1, preserved the target, and named the inaccessible source. The stale-projection and drop-all-rows controls failed as intended.
- Raw JSONL is a byte-for-byte prefix-preserving append: 19 rows / 10,171 bytes at base to 23 rows / 12,759 bytes at head, with four additions and no deletions. Projection is 15 rows (14 resolved, one open); seed remains raw-only;
invalid_signal / ci/e2e-smokeremains the sole open projection. Each of #1343, #1347, #1348, and #1490 has one latest resolved record. - The checked-in Markdown equals a fresh render; Windows file SHA-256 is
52C4CBC17BC401AC41B3DA50A4CBA9A3AB95AA5AEA52A857DC492FD6EF8E4B7B. - Live provenance matches the ledger: #1343/#1347/#1348/#1490 are closed-completed; #1388/#1410 remain open; PRs #1386/#1360/#1361/#1362/#1491 are merged at the five cited merge SHAs, and each SHA is an ancestor of this exact head.
- Required CI's hosted Docs Governance job passed with
Setup PythonthenValidate failure ledger projection synchronizationthen all three governance checks. Local docs, Golden Principles, and GitHub-operations governance checks, hook smoke, andgit diff --checkpassed. - Scope is exactly eight files, JSONL numstat is
4/0, DCO is 6/6, and the worktree is clean at the remote head. - Live closing metadata contains exactly #1492; #1492 and parent #1291 are both open before merge. The #1492 issue and PR project items are each
Review / Priority II. - Draft PR #1488 remains open at
34c163c30ebf7e2594ac7d29a61f2bcf3c727f95and overlaps five files here; it must absorb current main and regenerate/reverify after this PR lands.
NOT verified / residual risk
- Required CI run
30191454890is still in progress: its Windows API Integration job is the only unfinished job. CI Extended run30191455034, CodeQL, Codex, and all other observed Required-CI jobs are green. This review does not claim the full exact-head CI gate is complete. - Backend, frontend, and full E2E suites were not rerun locally because no product/runtime code changed.
- The committed denied-read regression uses a deterministic Python-3.14-shaped mock seam; independent real entry-point probes covered both installed runtimes without altering Windows ACLs.
- #1476 correctly remains open for the configured-handler smoke matrix, #1388/#1410 remain explicit residuals, and #1291 remains open.
Verdict: no remaining CRITICAL/HIGH/MEDIUM/LOW finding at this exact head. Source, documentation, provenance, metadata, and comment-triage controls are clean. Merge eligibility still requires the pending exact-head Required CI run to finish green.
Exact-head merge gate — PASSHead
All repository merge conditions hold. Merging by merge commit with the branch retained. |
1 participant
Summary
class=seedmetadata from every rendered projection, including seed-only input, without hiding issue-less non-seed rows.Part of #1291; #1291 remains open.
Closes #1492
Implementation Notes
(surface, first tracking issue in future_fix); each resolution row keeps its target issue first.FileNotFoundErroras a valid missing source, and validates all nonblank lines before target creation or write; denied reads return nonzero and preserve the target, while empty and seed-only sources remain valid.actions/setup-python@v7and runs the existing unittest before its governance checks; the testing guide documents the same fail-fast ordering before regeneration.docs/STATUS.mdrecords the enforced current state and narrows Gate configured-handler hook smoke matrix in Required CI #1476 to its still-ungated configured-handler/deny-floor scope;docs/IMPLEMENTATION_MASTERPLAN.mdrecords the delivered gate without changing roadmap priority or ADRs.docs/TESTING_GUIDE.mdnow carries the current2026-07-26update date.Verification
dotnet test backend/Taskdeck.sln -c Release(not run: no backend/product behavior changed)Push-Location frontend/taskdeck-web; npm run typecheck; if ($LASTEXITCODE -ne 0) { Pop-Location; exit $LASTEXITCODE }; npm run build; if ($LASTEXITCODE -ne 0) { Pop-Location; exit $LASTEXITCODE }; npx vitest --run; $code = $LASTEXITCODE; Pop-Location; if ($code -ne 0) { exit $code }(not run: no frontend behavior changed)py -3.14 -B -m unittest discover -s scripts/agent_hooks -p 'test_render_failure_ledger.py' -v— 11/11 passedpy -3.13 -B -m unittest discover -s scripts/agent_hooks -p 'test_render_failure_ledger.py' -v— 11/11 passedactions/setup-python@v7/ Python 3.12 pin and test-before-render ordering assertions passedPermission denied$env:PATH = 'C:\Python314;' + $env:PATH; py -3 -B scripts/agent_hooks/smoke_test.py— hook behavior smoke matrix passed52C4CBC17BC401AC41B3DA50A4CBA9A3AB95AA5AEA52A857DC492FD6EF8E4B7Binvalid_signal / ci/e2e-smoke; raw seed count 1, projected seed count 0node scripts/check-docs-governance.mjsnode scripts/check-golden-principles.mjsnode scripts/check-github-ops-governance.mjsgit diff --check 61f718af941c56c31b8b7595333b9debd6e47da8..HEADDocumentation
docs/agentic/failure_ledger.jsonlappended anddocs/agentic/FAILURE_LEDGER.mdregenerateddocs/STATUS.mdupdated with the Required Docs Governance synchronization invariant and corrected Gate configured-handler hook smoke matrix in Required CI #1476 scopedocs/IMPLEMENTATION_MASTERPLAN.mdupdated with the delivered gate/history; roadmap sequencing and ADRs unchangeddocs/STATUS.md,docs/IMPLEMENTATION_MASTERPLAN.md, anddocs/TESTING_GUIDE.mdLast Updated:lines verified exactly as2026-07-26docs/TESTING_GUIDE.mdupdated with the enforced unittest before renderer regeneration;docs/MANUAL_TEST_CHECKLIST.mdreviewed and unchangedTracking
Closes #1492Review / Priority IICI Workflow Validation
161638cf80a4b7d6116cf08a7b5cd8c61e1ec2e4— run30191454890161638cf80a4b7d6116cf08a7b5cd8c61e1ec2e4— run30191455034Risk Notes