Repository navigation
Mechanical fixes from third audit pass - #63
Conversation
Mechanical fixes from a third audit pass, each with a test that fails
without it:
- cmd_report marked the digest sent only when fewer notifiers failed
than were configured, but outbox retry failures count as errors. Once
a notifier had a backlog, every run re-sent the same digest and
queued another copy. Failed deliveries are already queued, so the
digest is now marked sent after every attempt.
- The autofix executor gave the fixer {default_branch, description}
while the preview gave it the full snapshot. A plugin reading any
other field could never be applied ("the repo changed since
approval"). It now gets the same snapshot.
- dependabot-config targeted repos whose .github/ion.yml says
`ignore: true` or `enabled: false`; readme-skeleton already skipped
them.
- plan-config listed autofix and follow-up proposals as "no longer a
candidate" under any config, including an identical one.
- `ion propose`, `sync-approvals` and `execute` exited 0 when they
reported errors, including a breaker trip. They now exit 1, as `run`
does.
- Archive evidence said "no other repo references it" (+0.2) even when
some live repos' mirrors were missing, so their references were never
read.
- Split's "refuse to push into a non-empty repo" check read an
`ls-remote` failure as an empty repo. It now refuses.
- A score rise of exactly rescore_delta (0.55 -> 0.7) was still
suppressed by cooldown, and a 0.45 -> 0.6 move did not re-plan,
because of float rounding.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016PpXZJdMN5FExq9THy5VNg
|
The Merge note: open PR #48 already conflicts with
Generated by Claude Code |
|
Independent review by a different agent than the drafter. Verdict: approve. What I checked. Eight small fixes, each in its own file, each with the reason in a comment. The ones with safety weight: the split executor's The rest are correctness fixes: digest marked sent once queued in the outbox, stage commands exit non-zero when a stage reports errors, Dependabot candidates honour repo overrides, plan-config no longer mislabels follow-up proposals, and the rescore comparisons round away float noise. Suite on the branch: 259 passed, against 250 on One behaviour change worth naming: a digest is now marked sent even when a notifier failed, on the grounds that the failure is queued in the outbox and retried. That reverses the previous at-least-one-success rule. The reasoning is sound and documented in the code, and the old behaviour risked duplicate sends. Flagging it here so the merge is a conscious choice. This PR closes none of the open issues: its findings were new in the third audit pass, and it explicitly leaves #68, #69, and #70 open. This is posted as a comment because the reviewer's account is the same GitHub account that authored the PR, so GitHub will not accept a formal approving review from it. Review is requested from the other maintainer accounts. Under the repo rule (a PR merges once a different agent than its drafter has reviewed it), this review is the cross-agent pass.
|
Mechanical fixes from a third audit pass. Each one has a test that fails without it.
Changes
ion/cli.py(cmd_report): the digest was marked sent only whenlen(errs) < len(notifiers). Buterrsfromnotify()includes every failed outbox retry. Once one notifier had a backlog,mark_digest_sentnever ran, so every later run re-sent the same digest to the working notifiers and queued another copy for the failing one. When it recovered, it received all the copies at once. README says "A digest is sent only when something changed." Every failed delivery is already queued and retried, so the digest is now marked sent after every attempt.ion/execute/autofix.py: the executor gave the fixer{"default_branch", "description"}, while the preview (fleet.preview) gave it the repo's full snapshot. A plugin that reads any other field (languages,topics,license,private) previews one diff, gets it approved, and then always fails at execution with "the change differs from the approved diff … the repo changed since approval". Re-proposing never helps. The executor now passes the same snapshot. The built-in fixers only readdescription, so they are unaffected.ion/fleet.py(DependabotConfig.find_repos): it targeted repos whose.github/ion.ymlsaysignore: trueorenabled: false;readme-skeletonalready skips them. It now skips them too.ion/planconfig.py: autofix proposals and merge follow-up archives are never analysis candidates.plan-configlisted them as "no longer a candidate under the new config" for any config, including an identical one. That effect is now reported only for proposals that were candidates under the current config.ion/cli.py(main): standaloneion propose,sync-approvalsandexecutereturned 0 even when they reported errors, includingexecute --applyhitting a breaker trip. They now exit 1, asion runalready did.ion/analyze/lifecycle.py,ion/analyze/__init__.py: archive evidence added +0.2 and "no other repo references it" even when some live repos had no loaded mirror (failed first clone,run --no-mirror), so their references were never read. Withalpha-coredepending onold-toolandalpha-core's mirror missing,old-toolqualified for archiving. The claim and the +0.2 now need every live repo to have been read; otherwise the reason says "references unknown: N other repos were not analysed". This fails safe: while any mirror is missing, archive candidates that relied on the +0.2 do not qualify.ion/execute/split.py(has_refs): the "refuse to push into a non-empty repo" check ranls-remotewithcheck=False. It read a failure (network, auth) as an empty remote. If a pre-existing target with matching visibility had content only on a non-basebranch, the split then pushed its history alongside that content. An unreadable remote now raisesSafetyError. As with a failedcloneorpushin any step, that ends the proposal asfailed.ion/propose/scoring.py: README says "the score moves by 0.15 or more". In floats0.7 - 0.55and0.6 - 0.45are just under 0.15. So a rise of exactlyrescore_deltawas still suppressed by cooldown, and that move did not re-plan. The difference is now rounded before the comparison.Not in this PR
Filed separately because the fix involves a design choice:
edit_issue_bodycap;Validation
uv run pytest -q: 259 passed (250 existing, 9 new). All 9 new tests fail with theion/changes reverted.scripts/check_licenses.py: 0 not allowed.🤖 Generated with Claude Code
https://claude.ai/code/session_016PpXZJdMN5FExq9THy5VNg