missing_docs: keep at most one bookkeeping PR open - #630
Conversation
The PR strategy said a bookkeeping PR "is independently mergeable in any order." That is true against feature PRs, which is the case it was written for. It is false against a second bookkeeping PR: both edit feature_surface_map.md and surface_snapshot.json, and the snapshot is a wholesale regen, so a conflict between two of them cannot be resolved by hand at all -- it has to be regenerated on the merged tree. #614 and #624 are that collision, sitting open right now. Two drift-watch runs a day apart independently triaged the same two /factory routes, reached the same Gate 0 verdict, and each added the same two map lines. Neither looked for the other's PR, and they used different branch-naming schemes, so there was no way to find one from the other. Run #2's ledger eway to find one from the other. Run #2's ledger eway to find one from the other. Run #2's ledger eway to find one from the other. Run #2's ledger eway to find one from the other. Run #2's ledgeran stays rare. Changes: - Scope the independence claim to feature PRs, and add the rule: check for an open bookkeeping PR first, extend that branch, and re-run --commit and --update-snapshot there so the marker and snapshot stay one regen covering every release the PR carries. Wait for the merge instead when it is already approved. - Standardize the title (missing_docs bookkeeping: <version>) and branch (missing-docs/boo (missing-docs/boo (mise search (missing-docs/boo (missing-docs/boo (mise search (missing-docs/boo (miste their inconsistent titles. - Triage now reads the ledger and map as they stand on any open bookkeeping PR, not just on main. A verdict in an unmerged PR is still a verdict. - Carry both into drift-watch step 8 and the scheduled-agent prompt, which is the only copy a cron run is guaranteed to read. Co-Authored-By: Warp <agent@warp.dev>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR updates the missing_docs skill to prevent multiple simultaneous bookkeeping PRs from drifting into unresolvable shared-file conflicts. It scopes the existing independence claim to feature PRs, adds the open-bookkeeping-PR reuse rule, and propagates that guidance into the drift-watch workflow and scheduled-agent prompt.
Concerns
No blocking concerns found. The change is prose-only, introduces no security-sensitive code or configuration, and the attached spec context contains no approved or repository spec commitments to validate against.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /warp-agent-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
… titles The convention I introduced, "missing_docs bookkeeping: <version>", quietly contradicted create_pr's existing rule that docs PR titles carry a prefix. It also invented a third naming shape in a change whose whole point is that two competing shapes are what let #614 and #624 miss each other. "chore(missing_docs): bookkeeping for <version>" matches the shape #614 already used, matches the repo's conventional-commit practice, and still carries both terms the documented search keys on -- verified against #614, whose title has exactly this form and which the search finds. Co-Authored-By: Warp <agent@warp.dev>
The gh pr list --search "missing_docs bookkeeping in:title" query ANDs
the two words anywhere in the title, so it also matches this rule-only
PR's own title ("...keep at most one bookkeeping PR open"). The next
drift-watch run would then check out and extend this PR's branch
instead of a real bookkeeping PR.
Restrict the search to the quoted "bookkeeping for" phrase, which
matches the canonical and legacy bookkeeping-PR titles but not this
PR's title, and add reviewDecision to the output so the already-
approved exception can be applied without a second query. Apply the
same corrected query to the recommended scheduled-agent prompt.
Co-Authored-By: Warp <agent@warp.dev>
…s into rrenk/bookkeeping-pr-rule
There was a problem hiding this comment.
Overview
The rework limits discovery to canonical bookkeeping PR titles, so this rule-only PR cannot be selected as the bookkeeping PR. No findings remain.
Verdict
Checks: CI ✅ · typecheck ✅ · build ✅
Found: 0 critical, 0 important, 0 suggestions
Approve
Prior concerns: none remain
Review run
https://oz.staging.warp.dev/runs/01a03aa0-ddb1-7868-a1ce-2e967a4d55cb
Summary
The PR strategy in
missing_docssaid the audit-bookkeeping PR "is independently mergeable in any order." That holds against feature PRs, which is the case it was written for. It does not hold against a second bookkeeping PR: both editfeature_surface_map.mdandsurface_snapshot.json, and the snapshot is a wholesale regen, so a conflict between two of them can't be resolved by hand — it has to be regenerated on the merged tree.#614 and #624 are that collision, open right now. Two drift-watch runs a day apart independently triaged the same two
/factoryroutes, reached the same Gate 0 verdict, and each added the same two map lines:Neither run looked for the other's PR, and they used different branch-naming schemes, so there was no way to find one from the other. #624's ledger entry even cites "sibling factory routes are mapped internal" as evidence — state that exists only in unmerged #614. It reached the right answer from a premise that wasn't true on
main.A daily schedule mints one bookkeeping PR per release, so this compounds rather than stays rare. This lands the rule before the schedule turns on.
Changes
check_new_release.py --commitand--update-snapshotthere so the marker and snapshot stay a single regen covering every release the PR carries. The one exception is a bookkeeping PR that's already approved — wait for the merge and branch from the result.missing_docs bookkeeping: <version>/missing-docs/bookkeeping-<version>) so the documented search can find it. Without a stable name there's no mechanical way for one run to discover another's PR.main. A verdict recorded in an unmerged PR is still a verdict; re-triaging it burns the run and produces duplicate map entries.Rework changes
gh pr list --search "missing_docs bookkeeping in:title"ANDs the two words anywhere in the title, so it also matches this rule-only PR's own title ("...keep at most one bookkeeping PR open") — the next drift-watch run would check out and extend this PR's branch instead of a real bookkeeping PR. Restricted the query to the quoted"bookkeeping for"phrase, which matches the canonical/legacy bookkeeping-PR titles but not this PR's title, and addedreviewDecisionto the--jsonfields so the already-approved exception can be applied from the same call. Applied the identical corrected query to the recommended scheduled-agent prompt section.Verification
Re-ran the documented search against this repo. The old query self-matches this PR; the corrected quoted-phrase query does not:
(#624, the real open bookkeeping PR used to demonstrate the fix, merged partway through this rework — the corrected query matched it with
reviewDecision: APPROVEDbeforehand, confirming the already-approved exception is checkable from the same call.)Skill prose only — no scripts, doc pages, or tests changed;
style_lint --changedscans no content files for this diff, as expected for a skill-doc-only change. PR CI (8/8) is green after the rework push.Follow-up
Doesn't fix the collision already open. That's handled separately: merge #614 first (it also carries an
audit_docs.pystaleness fix, 29 → 23 findings), then rebase #624, drop the duplicate map lines, and regenerate the snapshot rather than resolving the JSON by hand.Co-Authored-By: Warp agent@warp.dev