Skip to content

docs(ci): the reconcile job does not loop for the reason I wrote - #87

Merged
stranske merged 1 commit into
mainfrom
claude/reconcile-comment-truth
Aug 24, 2026
Merged

docs(ci): the reconcile job does not loop for the reason I wrote#87
stranske merged 1 commit into
mainfrom
claude/reconcile-comment-truth

Conversation

@stranske

Copy link
Copy Markdown
Owner

Two corrections to #83, both found by watching its first production run rather than by review. Comment and logging only — no behaviour change.

1. The convergence comment was wrong

#83's ci.yml said:

Converges without a loop guard: the commit it pushes re-triggers this workflow, that run finds no drift, writes nothing, and pushes nothing.

It does not re-trigger anything. A push made with the default GITHUB_TOKEN starts no workflow run at all. Measured on the real reconcile commit:

$ gh api "repos/stranske/Orchestrator/actions/runs?head_sha=349cb1c6" --jq .total_count
0

So the loop is impossible for a stronger reason than convergence — but the stated reason was false, and a false reason in a gate comment is worse than none, because it stops the next reader looking. It hid two things that matter:

  • The reconcile commit is never verified by ci.yml. Harmless as written — it changes only the two integers the run just measured — but main now carries commits CI never saw, which is a constraint on how wide that job's write may ever get.
  • Swapping to a PAT or App token would make the loop REAL. That's the dangerous edit, and the old comment told that reader it was already handled.

2. The log line printed a different number than it wrote

floor updated: collected=442 passed=416     ← the log
"collected": 442, "passed": 442             ← the file it just wrote

Both were correct: the file deliberately records passed + skipped, the only machine-invariant figure, while the log printed the bare pass count. But a log that contradicts its own artifact is how a correct write comes to look like a bug — and how a real one hides. It now reads from the same blob it writes, so the two cannot disagree.

Verification

Forced a heal locally — floor lowered to 435 against a reality of 442:

floor updated: collected=442 passed=442     ← log
file: collected=442 passed=442              ← file

verify.py --selftest OK, 442 collected, ruff check . clean, black --check 198 unchanged, ci.yml parses. .verify-floor.json deliberately untouched.

Note on #83's first run, which is the reason this PR exists

It worked, and it found real drift within four minutes of landing: main's floor read 441 while reality was 442, left behind by #81 merging. The job healed it and pushed. That is exactly the case it was built for — so the mechanism is sound; only my explanation of why it is safe was not.

Two corrections to #83, both found by watching its first production run.

1. The convergence comment was WRONG. It said "the commit it pushes re-triggers
   this workflow, that run finds no drift, and pushes nothing". It does not
   re-trigger anything: a push made with the default GITHUB_TOKEN starts no
   workflow run at all. Measured -- the pushed commit 349cb1c has ZERO workflow
   runs. The loop is impossible for a STRONGER reason than convergence, but the
   stated reason was false, and it hid two things worth knowing: the reconcile
   commit is never verified by ci.yml, and swapping this for a PAT or App token
   to get CI on it would make the loop REAL. The old comment would have told
   that reader it was already handled.

2. The log line printed a different number than it wrote. "floor updated: ...
   passed=" reported py["passed"] while the file records passed+skipped
   (machine-invariant, deliberately). The first production run logged
   passed=416 while recording 442. A log that contradicts its own artifact is
   how a correct write comes to look like a bug -- and how a real one hides.
   It now reads from the same blob it writes, so the two cannot disagree.

No behaviour change: comment and logging only.

Verified: forced a heal locally (floor 435 vs reality 442) and the log and the
file now agree -- log "collected=442 passed=442", file collected=442 passed=442.
selftest OK, 442 collected, ruff/black clean, ci.yml parses.
.verify-floor.json deliberately untouched.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@stranske
stranske temporarily deployed to agent-high-privilege August 24, 2026 00:12 — with GitHub Actions Inactive
@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 40 minutes.

View limit details

Limit details: You’ve used the included review currently available. Your 73 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5a3deee7-4422-4ab3-bde9-8eca5d94f30d

📥 Commits

Reviewing files that changed from the base of the PR and between 349cb1c and 543403d.

📒 Files selected for processing (2)
  • .github/workflows/ci.yml
  • verify.py

Comment @coderabbitai help to get the list of available commands.

@stranske-keepalive

Copy link
Copy Markdown

Automated Status Summary

Head SHA: 80e7a8c
Latest Runs: ⏳ pending — Gate
Required: core tests (3.12): ⏳ pending, core tests (3.13): ⏳ pending, docker smoke: ⏳ pending, gate: ⏳ pending

Workflow / Job Result Logs
(no jobs reported) ⏳ pending

Coverage Overview

  • Coverage history entries: 1

Coverage Trend

Metric Value
Current 31.97%
Baseline 0.00%
Delta +31.97%
Minimum 70.00%
Status ❌ Below minimum

Top Coverage Hotspots (lowest coverage)

File Coverage Missing
capability_effectiveness.py 0.0% 153
capability_firing_monitor.py 0.0% 192
capability_matcher_proposals.py 0.0% 111
capability_opportunity.py 0.0% 142
capability_propensity.py 0.0% 1668
ccusage_reconcile.py 0.0% 286
codemod_lane.py 0.0% 351
evidence_acquisition.py 0.0% 102
exploration_collection.py 0.0% 328
feature_scan.py 0.0% 118
frontend_verify.py 0.0% 255
improvement_log.py 0.0% 248
issue_readiness.py 0.0% 504
keepalive_evidence.py 0.0% 377
keepalive_supervisor.py 0.0% 322

Low Coverage Files (<50.0%)

File Coverage Missing
capability_effectiveness.py 0.0% 153
capability_firing_monitor.py 0.0% 192
capability_matcher_proposals.py 0.0% 111
capability_opportunity.py 0.0% 142
capability_propensity.py 0.0% 1668
ccusage_reconcile.py 0.0% 286
codemod_lane.py 0.0% 351
evidence_acquisition.py 0.0% 102
exploration_collection.py 0.0% 328
feature_scan.py 0.0% 118
frontend_verify.py 0.0% 255
improvement_log.py 0.0% 248
issue_readiness.py 0.0% 504
keepalive_evidence.py 0.0% 377
keepalive_supervisor.py 0.0% 322

Updated automatically; will refresh on subsequent CI/Docker completions.


Keepalive checklist

Scope

No scope information available

Tasks

  • No tasks defined

Acceptance criteria

  • No acceptance criteria defined

@github-actions

Copy link
Copy Markdown
Contributor

Workflow state fingerprint for Agents Gate Followups. Do not edit.

@stranske
stranske merged commit 7a4c3b0 into main Aug 24, 2026
35 checks passed
@stranske
stranske deleted the claude/reconcile-comment-truth branch August 24, 2026 00:21
stranske pushed a commit that referenced this pull request Aug 24, 2026
…erode it

FOUND BY DOGFOODING, not by review. Running --pr 91 before merging it, the check
PASSED -- and should not have. While pr-00-gate.yml sat held, every newly merged
PR merged WITHOUT the Gate, so after twelve such merges the Gate's checks no
longer appeared on 75% of the reference window, stopped counting as "normally
reporting", and their absence stopped being flagged. The expected set fell
23 -> 14 names and #91 was pronounced clean by the tool written to catch exactly
that.

A SUSTAINED outage is the case that matters most, and it was the one case the
frequency rule could not see. The erosion test already in this file covered only
PRs that reported NOTHING; a PR reporting some checks but not the Gate's slid
straight through.

config/expected-checks.json is now the high-water mark, seeded from PRs #87/#89
whose Gate demonstrably ran (33 names, 8 of them python-ci). A name that has ever
been expected stays expected until somebody DELETES ITS LINE -- a visible act in
a diff. Same shape as pyproject.toml's mypy exempt ratchet, and the same reason:
an automatic downward move is indistinguishable from the defect.

--update-ratchet raises it and never lowers it.

With the ratchet, PR 91's head correctly reports 19 absences including the whole
python-ci set. PR 89 (Gate ran) reports one, "guard", which is a TRUE positive:
agents-guard.yml was already held by then.

Break -> revert: returning the observed set instead of its union with
ratchet_names() fails test_the_ratchet_is_wired_into_the_expected_set;
byte-identical after.

448 passed, floor 448, 85/85 selftests, 5 of 5 gates, mypy Success, ruff and
black clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
stranske added a commit that referenced this pull request Aug 24, 2026
* feat(ci): an absent check is a red, whatever made it absent

THE DEFECT. main has no branch protection, and `gh pr checks` lists what DID
report -- so a check that never started is not red, it is missing, and a PR
with no Gate reads exactly like a PR whose Gate passed. Silence
indistinguishable from success: this repo's founding defect, twice over.
2026-08-23, five python-ci jobs died at a shared install step and #61/#64/#65
merged with all five red. 2026-08-24, #90's Gate run was held at
`action_required` with ZERO jobs and merged with no lint, no format and no
typecheck, landing six F821s found only because somebody ran ruff by hand.

THE CAUSE WILL BE DIFFERENT NEXT TIME, so nothing here models holds. A check
can vanish to a hold, a cancellation, a deleted or renamed workflow, a rate
limit, a mistaken path filter or a GitHub incident. All present identically to
whoever is merging. scripts/check_checks_reported.py asks only: did every check
that NORMALLY reports also report here.

TWO SIMPLER DESIGNS WERE TRIED AND REJECTED BY REAL DATA, both recorded in the
file because the next person will reach for them:
  * one reference PR -- failed on the actual incident. #90 had no Gate, and the
    newest merged PR (#93) had no Gate checks either, so #90 was declared
    healthy. The hold had already swallowed the yardstick.
  * the union across recent merges -- caught #90 (21 absent) but reported 25-26
    absences on entirely healthy PRs, sweeping in event-driven checks. A test
    that cries wolf 25 times gets waived.
Frequency (>=75% of 12 merged PRs) discriminates: #90 exit 1 with 10 absent,
be a second copy of the CI topology.

NOT BRANCH PROTECTION, deliberately. A required check that is HELD never
reports, so the PR could never merge -- the clear path blocked by the very
thing the gate measures. On a solo-maintained repo "unverified but movable"
beats "permanently stuck".

pr-00-gate.yml also gains `push: [main]`. A held PR run cannot be fixed from
inside CI, but the silence AFTER the merge can: #90's F821s would have gone red
on main within one run instead of never. Fail toward noise.

docs/ABSENT_CHECK_LANE_WIRING.md carries the one step this repo cannot land --
the closer's pre-merge call and the prerun `--sweep` line, since the lane TOMLs
live outside any repository. Same shape as docs/MIRROR_SYNC_PATCH.md.

Already found a live one: --sweep flags open PR #91 with 10 absent checks,
which I would have merged on a green-looking list.

Verified: 446 passed, 0 failed, 0 skipped, floor 446, 85/85 selftests, 5 of 5
gates, mypy Success, ruff and black clean. Break -> revert: removing the
max(2, ...) threshold floor fails test_the_threshold_never_falls_to_one;
reverted byte-identical.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(ci): degrade the absent-check sweep gracefully on a rate limit

The sweep runs every lane round from handoff-prerun.sh. Dumping GitHub's full
rate-limit paragraph hourly would train the reader to skip the whole section,
which is how a health report stops being read. One line instead, and it says
UNKNOWN rather than implying clean -- a reporter that cannot report must not
read as a clean bill of health.

Found by testing the prerun block while genuinely rate-limited, which is the
degraded path I would otherwise have had to simulate.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(ci): ratchet the expected-check set so a sustained outage cannot erode it

FOUND BY DOGFOODING, not by review. Running --pr 91 before merging it, the check
PASSED -- and should not have. While pr-00-gate.yml sat held, every newly merged
PR merged WITHOUT the Gate, so after twelve such merges the Gate's checks no
longer appeared on 75% of the reference window, stopped counting as "normally
reporting", and their absence stopped being flagged. The expected set fell
23 -> 14 names and #91 was pronounced clean by the tool written to catch exactly
that.

A SUSTAINED outage is the case that matters most, and it was the one case the
frequency rule could not see. The erosion test already in this file covered only
PRs that reported NOTHING; a PR reporting some checks but not the Gate's slid
straight through.

config/expected-checks.json is now the high-water mark, seeded from PRs #87/#89
whose Gate demonstrably ran (33 names, 8 of them python-ci). A name that has ever
been expected stays expected until somebody DELETES ITS LINE -- a visible act in
a diff. Same shape as pyproject.toml's mypy exempt ratchet, and the same reason:
an automatic downward move is indistinguishable from the defect.

--update-ratchet raises it and never lowers it.

With the ratchet, PR 91's head correctly reports 19 absences including the whole
python-ci set. PR 89 (Gate ran) reports one, "guard", which is a TRUE positive:
agents-guard.yml was already held by then.

Break -> revert: returning the observed set instead of its union with
ratchet_names() fails test_the_ratchet_is_wired_into_the_expected_set;
byte-identical after.

448 passed, floor 448, 85/85 selftests, 5 of 5 gates, mypy Success, ruff and
black clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Tim Stranske <tim@stranskemo.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant