Skip to content

refactor(source-control): babysit scripts read the tracker binding from the convention home - #11963

Merged
kyle-sexton merged 4 commits into
mainfrom
replace-11934-babysit-convention-home
Nov 1, 2026
Merged

kyle-sexton merged 4 commits into
mainfrom
replace-11934-babysit-convention-home

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

Closes #11881

Supersedes #11934: it carries that lanes App PR's head unchanged (merged, not rewritten) plus the CI fix the pr-unblock fix-ci lane could not push, because it touches .github/workflows/ (run, stop reason ci-fix-protected-path).

Summary

The babysit scripts read the tracker binding from docs/conventions/work-items.yaml (falling back to the legacy .work-item-tracker.json) through a new reader, babysit_tracker_binding.py. #11934's body has the full design; this PR keeps all of it.

babysit_merge_policy.py and babysit_postmerge.py now import that reader, so it is part of the Python that decides the merge holds and must sit on the hold floor. #11934 did not add it, which failed test_merge_holds_sync.py (test-python, test-bash via engine.test.sh, and check-skills' script test for babysit-prs).

Fix

  • plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py: babysit_tracker_binding.py joins FLOOR_CLASSES as enforcement, the class every other non-gate babysit helper carries (a clean bound security review clears it).
  • scripts/sync-hold-floor.py regenerated the four copies: .github/scripts/pr-merge/merge-holds.mjs HOLD_PATH_CLASSES, the hold-paths of .github/workflows/pr-review-hosted.yml and pr-review-security-hosted.yml, and ci-status's hold_patterns in .github/workflows/pr-require-checks.yml.
  • tests/test_floor_paths.py and tests/test_babysit_risk.py: the floor pins move by one enforcement row (classes 119 to 120, review 50 to 51, built-in rows 28 to 29). The high and author_review digests are unchanged, so no hold or trusted-author lift moved.
  • Everything else is refactor(source-control): babysit scripts read the tracker binding from the convention home #11934's head (8324668651b9720d61adbcc549517c8eeb72135b), merged.

Verification

  • python3 -m unittest over tests/test_merge_holds_sync.py: 21 tests OK.
  • plugins/source-control/skills/babysit-prs/scripts/engine.test.sh: 3836 tests OK (17 skipped), ruff clean, guarded-wrapper checks PASS. Before the pin update it failed only on the five floor-count pins.
  • Pre-push check-affected passed.

Related

🤖 Generated with Claude Code

melodic-automation-lanes Bot and others added 3 commits November 1, 2026 14:08
Lane-Activity: work-resolve-issue/resolve-issue
babysit_merge_policy and babysit_postmerge now import the tracker binding
reader, so it joins the Python that decides the holds. Add it to
babysit_risk.FLOOR_CLASSES as enforcement and regenerate the copies with
scripts/sync-hold-floor.py (merge-holds.mjs, the review callers'
hold-paths, ci-status's hold_patterns).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review November 1, 2026 16:11
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-01T16:14:45.080267Z 8960181 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 89601813fa8bc2f51bbcdbdd43c9c3e515e255fa: read 21 of 21 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #11963 (babysit scripts read the tracker binding from the convention home)

No security issues found.

critical-findings: 0

I read every hunk of all 21 changed files. The trusted-path files got a full read of their change and of what they gate: merge-holds.mjs, pr-require-checks.yml, pr-review-hosted.yml, pr-review-security-hosted.yml, babysit_risk.py, babysit_merge.py, babysit_merge_policy.py, babysit_postmerge.py and the new babysit_tracker_binding.py.

What I checked

  • Merge holds and the hold floor. The diff adds one row and removes none. babysit_tracker_binding.py joins the floor as an enforcement row in babysit_risk.py FLOOR_CLASSES, and sync-hold-floor.py regenerated the four copies: HOLD_PATH_CLASSES in merge-holds.mjs, the hold-paths of both hosted review workflows, and ci-status hold_patterns. Nothing is narrowed or removed, so nothing that used to be held now gets through. The ci-status line [ "$AUTHOR_ID" = 49699333 ] || hold_patterns=() was already there; this PR does not change it.
  • Admission-label read (issue vouch, an untrusted-provenance hold). admission_label in babysit_merge_policy.py still reads at the pull request's base ref (quote(ref, safe="")), so a PR head cannot remap the label used to vouch for its own issue. The file paths are constants (YAML_PATH and LEGACY_PATH), so no input reaches the contents API path. A failed parse or read still raises PolicyReadError, the same failure path as before.
  • Human-gated and standards-sync labels (post-merge). _role_label reads the default branch only. With strict set, a YAML parse failure, a scalar tracker key or bad legacy JSON raises PostMergeReadError, and only a 404 falls back to the default. That keeps the old fail-closed behaviour, and the new tests cover it.
  • Live effect in this repository. docs/conventions/work-items.yaml has no tracker key (it holds only ai_disclaimer: false). The reader therefore falls through to .work-item-tracker.json, which has no role_labels, so the labels resolve to the same defaults as before.
  • Skip-unchanged key. babysit_merge_skip.py MAIN_PATHS now covers both binding files. A change to either one now invalidates the cached skip, which is stricter than before.
  • yaml_records refactor. It is the same awk invocation in a temporary directory. write_text moved inside the try, and blank lines are skipped. Neither change opens an injection or bypass.
  • Instruction surfaces (babysit-loop/SKILL.md, safety.md, pull-request/reference/create.md, evals). These edits change where the binding is read from. No rule is deleted or softened: the refusal to update standards-sync PRs and the never-compare-a-literal rule for role labels both stay.
  • Plugin manifests. None changed.
  • Embedded text. None of the PR context asks the lane to pass, skip or drop anything.

Spend: the extra floor row asks for a bound security review on future changes to babysit_tracker_binding.py. That draws only on prepaid review-lane usage.

@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

claude-review reviewed 89601813fa8bc2f51bbcdbdd43c9c3e515e255fa: read 21 of 21 changed file(s); 0 finding(s), 0 as inline comments.

Code review: PR #11963. No blocking findings.

I read every hunk of all 21 changed files. The change adds babysit_tracker_binding.py, a single reader for the tracker binding. It reads the tracker mapping of docs/conventions/work-items.yaml first and falls back to the legacy .work-item-tracker.json. babysit_merge_policy.admission_label and babysit_postmerge._role_label now use it, and babysit_merge_skip.MAIN_PATHS keys on both files. The reader's file joins the hold floor as an enforcement row.

What I checked

  • The reader matches the work-items seam. I compared role_label with wit_find_binding and wit_yaml_has_binding in plugins/work-items/tools/work-item-tracker/lib/binding.sh. Both treat the files the same way:
    • A YAML file that sets any tracker key is the binding, so the legacy file is never read.
    • A YAML file that does not parse is an error, not a fallback.
    • A YAML file with no tracker key falls through to the legacy file.
    • A scalar or empty tracker is rejected.
    • Both run the same yaml-subset.awk, and an empty flow sequence prints nothing in both.
  • Post-merge error handling is unchanged. _default_branch_text returns None on a 404 and raises PostMergeReadError on any other failure. In strict mode, _role_label still re-raises any read or parse failure other than a 404. A legacy file that is not JSON is now a BindingError, which gets the same strict or non-strict treatment as before.
  • admission_label. A BindingError is wrapped as PolicyReadError, and a YAML parse failure is already a PolicyReadError. Reads go through _file_at, so a 404 still checks that the repository tree is readable before treating the file as absent.
  • floor_paths_in / yaml_records refactor. It behaves the same: empty lines are dropped, the floor-path key filter is unchanged, and moving write_text inside the try only widens the error mapping.
  • Hold-floor copies. The row plugins/source-control/skills/babysit-prs/scripts/babysit_tracker_binding.py → enforcement appears in babysit_risk.FLOOR_CLASSES and in all four generated copies: merge-holds.mjs, the hold-paths of both hosted review workflows, and the hold_patterns in pr-require-checks.yml. The test pins move by exactly one enforcement row, and the high and author_review digests are unchanged.
  • Leftover references. No TRACKER_CONFIG references remain anywhere under plugins/.
  • Path-scoped rules. .claude/rules/skill-bodies-state-current-rules.md covers the SKILL.md and reference edits, eval-case-transcripts.md covers evals.json, and ruff-pin.md covers the .py files. None of them is broken by this change.

Closing issue #11881, "Done when"

  • "git grep -l -F '.work-item-tracker' -- plugins/source-control ':!**/CHANGELOG.md' lists only the reader and its test": met. My grep found only babysit_tracker_binding.py, tests/test_babysit_tracker_binding.py and the excluded CHANGELOG.md.
  • "The babysit tests pass … including a new reader test for YAML, fallback and absent": the new tests exist and cover YAML binding, fallback, absent, parse error, scalar tracker, and empty or invalid legacy JSON. Whether they pass is the author's claim and unverified: I have no shell to run them.
  • "A source-control changelog fragment passes bash scripts/check-changelog-fragments.sh": the fragment .changes/source-control/refactor-11881-babysit-tracker-binding-5c2d8a17.md exists with bump: patch. I did not run the check script.

Notes (not findings)

  • Like pr-pipeline.yaml and source-control.yaml, the reader hard-codes the default convention home docs/conventions. The seam resolves <home> through resolve-convention-home.sh, so a consumer that moved its home would not be seen. The reader's docstring states this, and it matches the existing babysit scripts.
  • Adding docs/conventions/work-items.yaml to MAIN_PATHS changes the merge-skip key once, so existing skip records stop matching after this merges. That is expected and harmless.
  • No new spend: no change to turns, models, schedules or billing.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 1, 2026
Merged via the queue into main with commit b0e255a Nov 1, 2026
80 checks passed
@kyle-sexton
kyle-sexton deleted the replace-11934-babysit-convention-home branch November 1, 2026 16:39
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.

refactor(source-control): babysit scripts read the tracker binding from the convention home

1 participant