Skip to content

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

Closed
melodic-automation-lanes[bot] wants to merge 1 commit into
mainfrom
work-resolve-issue/issue-11881-41313113933-1
Closed

melodic-automation-lanes[bot] wants to merge 1 commit into
mainfrom
work-resolve-issue/issue-11881-41313113933-1

Conversation

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

Closes #11881

Summary

Opened by the work-resolve-issue lane. The run's own summary:

Added one Python reader, babysit_tracker_binding.py. It reads the tracker mapping of docs/conventions/work-items.yaml, falls back to the legacy root .work-item-tracker.json, and parses the YAML through lib/yaml-subset.awk records with no PyYAML. The merge policy's admission label, the post-merge human-gated and standards-sync role labels (and through them babysit_merge's human_gated_label) all read through it, and babysit_merge_skip MAIN_PATHS keys on both binding files. Design choice: the reader uses the fixed default home docs/conventions rather than resolving the AGENTS.md pointer region over the API, because these scripts already read pr-pipeline.yaml and source-control.yaml at that fixed home and MAIN_PATHS needs fixed paths. It also follows the seam's binding.sh semantics: a YAML file with no tracker key falls back to the JSON file, while a YAML file that does not parse, or one whose tracker is a scalar, is an error and never a fallback.

The run went past its turn budget (turns=121 max-turns=113): the work may be too big for one run.

Not verified

  • The babysit Python test suites (test_babysit_tracker_binding.py, test_babysit_postmerge.py, test_babysit_merge_policy.py, test_babysit_merge_skip.py, test_babysit_risk.py and the rest under scripts/run-test-file.sh) were not run: this lane runs only the named wrappers, not a test runner. Left to the
  • Tidy skipped: the change touches 15 files, over the 10-file tidy cutoff.
  • Build/lint of the Python files (ruff) not run here; check-affected ran its static checks and passed, and line lengths were kept at 88 or under by hand.
  • docs/conventions/pr-pipeline/README.md (around lines 400, 1552, 2625, 2739) still says the babysit role labels resolve from .work-item-tracker.json. This is outside plugins/source-control, which is the issue's scope and the scope of its grep criterion, so it was not edited.
  • The reader does not resolve a non-default convention home from the AGENTS.md pointer region over the API; it reads the default home docs/conventions, as the babysit scripts already do for pr-pipeline.yaml. A YAML binding whose only tracker content is an empty flow sequence (key: []) prints no reco
  • The .github/ lane scripts (pr-merge, pr-unblock, work-resolve-issue workflows and .mjs) still read .work-item-tracker.json directly. They are outside this issue's babysit-script scope and under protected/shared paths.

Fix

  • plugins/source-control/skills/babysit-prs/scripts/babysit_tracker_binding.py: new reader. YAML_PATH, LEGACY_PATH, PATHS, BindingError, and role_label(read, records, role, default).
  • plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_tracker_binding.py: new reader test. Covers a YAML binding, a YAML binding with no role, a YAML file with no tracker key (falls back), the legacy file only, blank and non-string labels, neither file present (default), YAML that doe
  • plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py: new yaml_records(text) built from floor_paths_in's awk run (floor_paths_in now uses it, and a temp-file write failure now raises PolicyReadError). admission_label now reads through the reader, and TRACKER_CONFIG is removed.
  • plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py: _role_label reads through the reader via a new _default_branch_text (raw contents read, None on 404). Strict mode now turns parse failures into PostMergeReadError. TRACKER_CONFIG is removed and the standards_sync_label docstring
  • plugins/source-control/skills/babysit-prs/scripts/babysit_merge_skip.py: MAIN_PATHS takes *tracker_binding.PATHS (the YAML and legacy files).
  • plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py: human_gated_label docstring names the tracker binding.
  • plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_policy.py: GhFake answers an unset work-items.yaml as absent, the same way it answers source-control.yaml, and the tracker fixtures use the reader's LEGACY_PATH.
  • plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge.py: RoleLabelRead serves each file by path, and adds a YAML-mapped label case and a strict-raise case for YAML that does not parse.
  • plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk.py: the low-risk path list uses babysit_tracker_binding.LEGACY_PATH.
  • plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py: a comment names the reader.
  • plugins/source-control/skills/babysit-prs/reference/safety.md: the standards-sync and admission-label rules name scripts/babysit_tracker_binding.py and the YAML-then-legacy order.
  • plugins/source-control/skills/babysit-loop/SKILL.md: escalation role-label resolution names the tracker binding (work-items.yaml, else the legacy JSON).
  • plugins/source-control/skills/babysit-loop/evals/evals.json: the same wording in one expected_output.
  • plugins/source-control/skills/pull-request/reference/create.md: the tracker-bound path names <home>/work-items.yaml or the legacy JSON binding.
  • .changes/source-control/refactor-11881-babysit-tracker-binding-5c2d8a17.md: new patch fragment (### Changed).

Verification

The runner ran each check from the base copy against 8324668:

  • scripts/check-changelog-fragments.sh --check: passed

The run reports:

  • Grep for .work-item-tracker under plugins/source-control excluding CHANGELOG.md: matches only babysit_tracker_binding.py and tests/test_babysit_tracker_binding.py.
  • .lane/checks/bin/check-changelog-fragments: first run failed (fragment had no ### section). Fixed, and the rerun passed: all 119 fragments valid.
  • .lane/checks/bin/check-affected: first run failed only on changelog-fragments. After the fix it reported 12 passed, 6 skipped, 0 failed over 15 changed files; the final run after the review fixes reported the same.
  • Fresh-context review subagent over the whole diff: no bugs. It raised 2 risks and 3 nits. Fixed: scalar tracker key now raises, empty legacy file is the default again, the yaml_records temp write is inside the try, and the fragment/docstring now say the admission label is read at the base branch. Le
  • Diff measured with git-read diff --numstat HEAD plus status: 12 tracked files plus 3 new files changed.
  • The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/41313113933

Left out: the items under Not verified.

Related

#11881

Lane-Activity: work-resolve-issue/resolve-issue
@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 8324668651b9720d61adbcc549517c8eeb72135b: read 15 of 15 changed file(s); 0 finding(s), 0 as inline comments.

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

No security issues found.

critical-findings: 0

I read every hunk of all 15 changed files. Several are trusted paths: babysit_merge_policy.py, babysit_postmerge.py, babysit_merge.py, and the babysit_risk test list. For each one I checked what it gates.

What I checked

  • Admission label (part of the untrusted-provenance vouch hold). admission_label still reads the binding from the PR's base branch (see babysit_merge_policy.py#L2332). A PR's own head therefore can't change which label vouches for an issue.
    • The vouch also still requires that a write-access human applied the label in person, whatever the label is called.
    • The new YAML source is no easier to change than the legacy JSON. docs/conventions/x.md and the legacy file are both already in the LOW-risk merge list in test_babysit_risk.py, so this widens nothing.
    • Read and parse failures still raise PolicyReadError, so this check still fails closed.
  • Human-gated and standards-sync role labels. _role_label reads from the default branch, as before. In strict mode, which human_gated_label and babysit_verify_evidence use:
    • A 404 on both files gives the default, as before.
    • Any other read failure on either file raises.
    • A YAML parse failure, a scalar tracker value, or invalid legacy JSON now raises PostMergeReadError.
    • So this check also fails closed. Non-strict mode returns the default on failure, the same as the base.
  • Effect on this repository. docs/conventions/work-items.yaml has no tracker key, so the reader falls back to .work-item-tracker.json. That file maps no role_labels, so the labels resolved here are the same as before.
  • yaml_records subprocess. The awk call passes the parser path and a temp-file path as argv to bash -c 'LC_ALL=C exec awk -f "$0" "$1"'. No document text reaches the shell, so there's no injection surface. The document comes from a trusted branch anyway.
  • babysit_merge_skip.MAIN_PATHS. This adds the YAML path to the skip key. A skip key with more paths can only make skips less frequent, so this narrows the skip and widens nothing.
  • Instruction-surface lens. The docs and skill edits (babysit-loop/SKILL.md, evals.json, safety.md, create.md) restate where the binding is read from. They remove and soften no rule; the standards-sync no-refresh rule and the admission-label rule keep the same wording.
  • PR context. It contains no embedded instructions to the review lane.

No trust control is widened. Nothing changes the trusted-actor list, token scope, the kill switch, or any merge hold, and no plugin manifest is touched.

@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

claude-review reviewed 8324668651b9720d61adbcc549517c8eeb72135b: read 15 of 15 changed file(s); 1 finding(s), 0 as inline comments.

Code review: babysit scripts read the tracker binding from the convention home

I read every hunk of all 15 changed files. I also read the shared parser (plugins/source-control/lib/yaml-subset.awk), _file_at, _call/_read in post-merge, MAIN_PATHS consumption in babysit_merge_skip.py, the existing RoleLabelRead and gh-fake tests, and FLOOR_CLASSES in babysit_risk.py.

What the change does. It adds one new reader, babysit_tracker_binding.role_label, which reads the tracker mapping in docs/conventions/work-items.yaml first and falls back to the legacy .work-item-tracker.json. The admission label in babysit_merge_policy.py and the human-gated and standards-sync labels in babysit_postmerge.py now read through it. yaml_records is split out of floor_paths_in, and a temp-file write failure now raises PolicyReadError instead of escaping as OSError.

Behaviour I checked against the parser:

  • tracker: with block children never emits a bare tracker record, so only a scalar tracker: value trips the not-a-mapping error. This is correct.
  • Quoted values such as ' human-review ' are unquoted by the parser and then stripped.
  • tracker: {…} is a parse error, so it raises PolicyReadError and never falls back.
  • In post-merge, a 404 on either file still gives the default even in strict mode, and any other failure still raises in strict mode, as before.
  • This repository's own docs/conventions/work-items.yaml has no tracker key, so it falls back to the legacy JSON and behaviour here does not change.
  • The existing gh fakes (test_babysit_postmerge.py ~L393, test_babysit_merge_unattended.py ~L701, and GhFake's UNSET_YAML) answer the new YAML read with a 404.

Finding (1, inline): babysit_tracker_binding.py is a new module that babysit_merge_policy.py imports, and that file is in the trust floor class. The new module is not in FLOOR_CLASSES, while every other babysit_* module the policy imports is.

Closing issue #11881, Done-when:

  • git grep lists only the reader and its test: met. I ran an equivalent search over plugins/source-control (excluding CHANGELOG.md) at this head, and it matched only babysit_tracker_binding.py and tests/test_babysit_tracker_binding.py.
  • Babysit tests pass, including a new reader test for YAML, fallback and absent: the new tests cover YAML, YAML without a tracker key falling back, legacy only, neither file, parse failure, scalar tracker, and a failed read passing through. I did not run them, and the PR says they were not run either. Not verified.
  • The changelog fragment passes check-changelog-fragments.sh: the PR says this passed. Not verified by this review.

Not flagged:

  • Adding a key to MAIN_PATHS invalidates existing skip records once, which only causes one extra gate run per PR.
  • The admission-label check now makes one or two extra API reads in repositories without work-items.yaml (the 404, then the root listing). This uses GitHub API quota, not paid spend.
  • The PR acknowledges that tracker: [] and a bare tracker: fall back to the legacy file; I left this as is.

No plugin manifest changed. I found no instructions embedded in the PR context.

critical-findings: 1

Findings not posted inline (their path, line or side is malformed):

  • plugins/source-control/skills/babysit-prs/scripts/babysit_tracker_binding.py:1: The new gate module is outside the merge-hold floor. babysit_merge_policy.py imports this module (import babysit_tracker_binding as tracker_binding), and that file is a trust row in FLOOR_CLASSES (babysit_risk.py#L337). babysit_postmerge.py (enforcement) imports it too. Every other babysit_* module the policy imports has its own row: babysit_dependabot, babysit_repo_config, babysit_run_cache, babysit_whole_review, babysit_classify, babysit_gh, babysit_risk and babysit_util (L322-L354). This one has none.

This module now decides two gate inputs: which label counts as a human's admission vouch in _vouch_refusal (admission_label), and which label an autonomous merge holds on (human_gated_label).

How it fails: a later PR that touches only this file can change either label without the person hold that the same edit inside babysit_merge_policy.py would get. For example, it could make role_label return the default no matter what the binding says, or read a different path. That PR scores against the plain plugins/**/scripts rule, not the floor.

Fix: add a row ("plugins/source-control/skills/babysit-prs/scripts/babysit_tracker_binding.py", "trust") (or enforcement, to match babysit_postmerge). Then rerun scripts/sync-hold-floor.py so merge-holds.mjs, the review callers' hold-paths and ci-status pick it up, and change the count in test_floor_paths.py (len(risk.FLOOR_CLASSES) == 28, and 119 - 28) to match.

@melodic-automation-lanes

melodic-automation-lanes Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor Author

Unattended merge: native auto-merge leaves this pull request unarmed at head 8324668651b9720d61adbcc549517c8eeb72135b. The lanes clear this reason on their own; the next scheduled run judges it again.

a pr-unblock lane stopped its head (pr-unblock/fix-ci is failure), which ci-status fails in the
  merge queue

@melodic-automation-lanes melodic-automation-lanes Bot added the needs-session A pr-unblock stop a session builder takes: a change the lane cannot make. Not an owner stop. label Nov 1, 2026
@melodic-automation-lanes

melodic-automation-lanes Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor Author

The pr-unblock lane stopped fix-ci on this pull request: the CI fix changes a path the lane does not push: .github/workflows/, CODEOWNERS, .gitmodules or a settings file. The lane labels it needs-session for a session builder, who makes the change this lane cannot, and the run ends in a failing pr-unblock/fix-ci status on the head the lane gated.

When a retry on this head can change the outcome, the lane runs one on its own, up to its per-head cap, and edits this comment if it stops again; a run that does not stop clears the status. When no retry can (a review thread the lane does not take or settle, or a failing check after the retries are spent), a person resolves the threads or gets the checks green, then dispatches the workflow below: that run finds the cause gone and clears the status. On a pull request the lanes App opened, the dispatchers start that run once per stop; on any other, a person does. A new commit to the branch starts clean.

To run it again on this head now, dispatch the pr-unblock workflow:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=11934 -f activity=fix-ci

@kyle-sexton

Copy link
Copy Markdown
Member

Hold: replaced by #11963 (fix-ci needs a protected path). Releases when: never; closes when #11963 merges.

@kyle-sexton

Copy link
Copy Markdown
Member

Superseded by #11963, merged.

@kyle-sexton kyle-sexton closed this Nov 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge Hard merge gate: do not merge while applied. needs-session A pr-unblock stop a session builder takes: a change the lane cannot make. Not an owner stop.

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