docs(plan): correct pre-step (0) — two callers, two pins, not one - #303
Merged
Conversation
Pre-step (0) told a later operator that re-pinning the sync engine also activates the never-armed watchdog, because #291 delivers both in one commit. The commit does, but standards does not consume it that way: the watchdog is a separate reusable reached through a separate caller carrying its own pin, so re-pinning `sync.yml` activates the arming half and nothing else. That watchdog caller still sits at `43bc8d0` (2026-07-22), which predates both #291's never-armed detection and #234's split-scan reliability fix. Record the deadline the old text implied was already met: the watchdog re-pin must land before `automerge: true` is restored, because the never-armed scan only considers targets the manifest marks `automerge: true` — inert while the window holds all eight at `false`, load-bearing the moment the restore lands. Also note that the armed-but-stuck scan is not automerge-gated, so the stale pin is not wholly dormant today; it is quiet only because no sync PR is currently armed. No phase tag advances. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 25s —— View job Reviewing PR #303
SummaryDocs-only change to Verification performed
FindingsNone. This is an accurate, narrowly-scoped, well-substantiated correction to planning documentation. No code, workflow, or security-relevant files are affected. |
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
The paragraph still opened by stating standards `sync.yml` pins `ac223bb`. standards#293 moved it to `8202e03f`, so the text described a state that no longer exists — and against the watchdog correction in the previous commit the paragraph contradicted itself, reporting the watchdog pin as current while reporting the engine pin as stale. Mark the engine half landed and the watchdog half outstanding, and put the gate rationale in past tense. No phase tag advances; the rationale is kept because the watchdog step still depends on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Summary
Pre-step (0) (merged in #301) states that re-pinning the standards sync engine
to
8202e03falso activates the never-armed watchdog, "so ONE pin satisfiesboth." The commit does carry both halves; standards does not consume it that
way, so the guidance is wrong for the operator who follows it.
Verified against the live repos:
sync.yml's pinstandards-sync-stuck-automerge-alert.yml, a different reusableRe-pinning
sync.ymltherefore activates the arming half only.43bc8d0predates two commits, not one —git log --oneline 43bc8d0..8202e03f -- .github/workflows/standards-sync-stuck-automerge-alert.yml:8202e03fix(standards-sync-stuck-automerge-alert): detect sync PRs that were never armed #291 — never-armed detection42329effix(standards-sync-stuck-automerge-alert): split the stuck-PR scan into a cheap page fetch and a per-candidate merge-state probe with retry #234 — split the stuck-PR scan into a cheap page fetch + per-candidate merge-state probe with retryThe deadline the old text implied was already met. The never-armed scan
selects targets with
jq '[.include[] | select(.automerge) | .repo_name]'andgates candidates on
automergeRepoNames.has(repo), so it is inert while therollout window holds all 8 targets at
automerge: false— and becomesload-bearing at the exact moment
automerge: trueis restored. The watchdogre-pin must land before that restore.
One nuance the correction records, because it cuts against reading the
stale pin as wholly dormant: the armed-but-stuck scan is not
automerge-gated. It loops
repoNames(all 8 targets) at:299and pushesisArmedCandidatematches at:315with no automerge check — only:316'snever-armed branch is gated. It sweeps every target on the hourly cron today,
and is quiet only because no sync PR is currently armed (checked: dotfiles#361,
provisioning#231, github-iac#244, medley#1676 all report
autoMergeRequest: null).No phase tag advances. Scope is the pre-step (0) paragraph only.
Test plan
markdownlint-cli2 docs/topics/claude-review-lanes/PLAN.md→Summary: 0 error(s)git diff -U0 | grep -E "^[+-].*\[(DONE|DOING|TODO)\]"→ no matches (no phase tag touched)on a full clone — the local clone was shallow, which can make
git diff <sha>^ <sha>error in a way that reads as a false positive;git fetch --unshallowwas run and every finding re-derived. All held.Related
No linked issue. Corrects text merged in #301; describes work delivered by #291
and #234. The paired standards-side watchdog re-pin ships as its own PR.