Skip to content

test(rendered-views): fail a view lane's skill text that lists medium values - #13076

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

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

Conversation

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

Closes #13036

Summary

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

Adds scripts/check-view-medium-lists.mjs, which scripts/check-changed-skills.sh now runs on every invocation, before its no-changed-skills exit. It fails when a view lane's *.md or evals.json holds a line (or, in evals.json, one JSON string) with two or more distinct backticked medium ids. It reads the medium ids (MEDIUMS keys) and lane ids (lane(...) entries) from lib/view-registry.mjs as text, so the --merge-base lane form never executes the checked tree, and it holds no medium or lane literal. Lanes map plugin:skill to plugins/<plugin>/skills/<skill>/ and a bare id to plugins/<id>/. It skips CHANGELOG.md, README.md and every lib/rendered-view.md copy that scripts/shared-copies.txt lists, all as the recorded decision says. One deviation from that decision: the exemption list holds 7 lanes, not its 9. On the current tree codebase-health:audit (#12677) and visualization:present no longer hold any backticked medium list (a grep of their files finds at most one id per line). The decision's own rule fails a row for a clean lane, so listing them would turn the gate red. The decision's governing rule was 'exactly the lanes that fail on today's tree', and that is the rule applied here. Each row cites its open #12646 slice, and a row for a clean or unregistered lane, a row with no #<issue>, or a duplicate row fails.

Not verified

  • scripts/check-view-medium-lists.test.sh (node --test) and scripts/check-changed-skills.test.sh were not executed: this lane runs no node or bash besides the named wrappers. Left to the PR's CI test-bash lane.
  • The new gate itself was never executed in this run (the check-changed-skills wrapper runs the base tree's copy). Its behavior on the current tree was checked by an equivalent Grep and by hand-tracing; CI's changed-skills step runs it for real.
  • Tidy not run: the change sits under scripts/, a listed shared path for the tidy cutoffs.
  • Exemption list differs from the recorded decision's 9 lanes: codebase-health:audit (#12677) and visualization:present (#12669) were left out because the current tree already holds no backticked medium list in them, and the decision's stale-row rule would fail those rows.
  • Untracked editor and shell dotfiles at the checkout root (.bash_profile, .bashrc, .gitconfig, .idea, .mcp.json, .profile, .ripgreprc, .vscode, .zprofile, .zshrc) were present and not created by this run; left untouched.

Fix

  • scripts/check-view-medium-lists.mjs: new gate (exit 0 clean, 1 findings, 2 unreadable registry or usage); reads lib/view-registry.mjs as text, scans each lane's *.md and evals.json, enforces the exemption list's shrink-only rules; the self-run guard compares real paths so a symlinked checkout…
  • scripts/view-medium-list-exemptions.txt: new exemption list: architecture (#12672), education:illustrate and education:teach (#12682), overengineering:audit (#12679), prototype:explore-directions and prototype:pressure-test (#12684), visualization:visualize (#12669)
  • scripts/check-view-medium-lists.test.mjs: new node:test suite: registry text parse equals the imported MEDIUMS/LANES keys; two backticked ids on a line fail; one id, a repeated id, plain-word prose and a fixture prompt naming one medium pass; evals.json is read one JSON string at a time; skips…
  • scripts/check-view-medium-lists.test.sh: discovery wrapper so scripts/run-plugin-tests.sh runs the node suite
  • scripts/check-changed-skills.sh: runs the new check on every invocation when lib/view-registry.mjs exists (exit 2 when the check script is missing or errors), folds its result into the no-changed-skills exit and the final exit; header comment and exit-code text updated
  • scripts/check-changed-skills.test.sh: new scenario: a view lane naming one medium passes, and one listing two media fails the gate with exit 1 although no skill changed

Verification

The runner ran each check from the base copy against 9d9b0c6:

  • scripts/check-changelog-fragments.sh --check: passed
  • scripts/check-changed-skills.sh --merge-base: passed

The run reports:

  • .lane/checks/bin/check-affected: 10 passed, 6 skipped over 6 changed files (shellcheck, shell-portability, exec-bit, changelog-fragments, comment-hygiene, machine-specific-paths and others passed; markdown, ruff, typos, cross-plugin-source-drift, validate-plugins and changed-skills skipped as not…
  • .lane/checks/bin/check-changelog-fragments: all 6 fragments valid; no fragment required for a scripts/-only change
  • .lane/checks/bin/check-changed-skills: passed (no changed skills); this wrapper runs the base tree's copy of the gate, so it did not exercise the new check
  • Grep over plugins/ for lines with two backticked medium ids, with CHANGELOG/README and the four lib/rendered-view.md copies removed: hits in exactly the 7 exempt lanes and no other view lane; codebase-health:audit and visualization:present hold at most one id per line
  • Fresh-context review subagent: no blocking bugs; it traced every new test by hand as passing and confirmed the exemption list matches the tree. Fixed its findings: symlink-safe self-run guard, exit 2 on a missing check script, scan moved before the evals-warrant stale-row exit so it runs on every…
  • The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/41761175633

Left out: the items under Not verified.

Related

#13036

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

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 9d9b0c665bec616a75affee2e2cbd8ed4b86a462: read 6 of 6 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #13076

No security issues found. I read every hunk of all 6 changed files in the diff.

This pull request adds a docs-consistency gate, scripts/check-view-medium-lists.mjs, with its tests and an exemption list. It also wires the gate into scripts/check-changed-skills.sh. None of the changed files matches a trusted-path glob, and none is a plugin manifest, a workflow, an action or a merge-hold module.

What I checked:

  • Executing the checked tree. The gate reads lib/view-registry.mjs as text using regexes and never imports it. Lane ids and medium ids are only used to build directory paths under plugins/ and to match strings. Nothing from the checked tree is evaluated, so no new code from the pull request's tree runs under the --merge-base lane form.
  • Reading files. walk reads only regular *.md and evals.json files. Because it uses Dirent.isFile() and isDirectory(), it does not follow symlinked entries inside a lane directory. Output is limited to path:line plus the medium ids it matched, written to the gate's own stderr. No secrets or tokens are involved.
  • Shell wiring. check-changed-skills.sh runs node "$SCRIPT_DIR/check-view-medium-lists.mjs" --root "$PWD". Both arguments are quoted, nothing is interpolated from the pull request's content, and any exit code above 1 fails closed with exit 2. The gate can only add failures to check-changed-skills.sh. It removes no existing check.
  • Trust controls and instruction surface. Nothing that decides who is trusted, what a token may do, the kill switch, a merge hold, or what untrusted text reaches a lane is touched. No standing instruction is deleted or softened.
  • Embedded text. The pull request context contains no directive aimed at this review.

Spend: none beyond the existing CI test steps, which now also run this gate and one more node test suite.

critical-findings: 0

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

claude-review reviewed 9d9b0c665bec616a75affee2e2cbd8ed4b86a462: read 6 of 6 changed file(s); 0 finding(s), 0 as inline comments.

Code review: PR #13076, view-lane medium-list gate

Result: no blocking or advisory findings.

What I checked: I read every hunk of all 6 changed files. I checked them against the closing issue #13036 and the decision comment on it, and read the code around them: lib/view-registry.mjs, scripts/check-changed-skills.sh, scripts/shared-copies.txt, the check-affected.sh selector and the CI step in pr-require-checks.yml. I ran no tests or scripts. The author says the test suites were not run, and that remains unverified here. They are left to CI's test-bash and test-node lanes.

Closing issue #13036: done-when items

  1. Met. check-changed-skills.sh runs check-view-medium-lists.mjs on every run, before it exits for having no changed skills.
    • The medium ids are the MEDIUMS keys and the lane ids come from the lane(...) entries. Both are read from the registry as text, and the script hard-codes none of them.
    • The regexes match the real registry. The test that compares the text parse with the imported MEDIUMS and LANES keys holds up, because LANES is an object keyed by lane id (lib/view-registry.mjs#L45).
    • CI runs the script on every pull request (pr-require-checks.yml#L3195-L3206).
  2. Met. The tests cover:
    • a list that fails;
    • a one-medium fixture prompt in evals.json that passes;
    • the current tree passing with its exemption list;
    • the check-changed-skills.sh case where the check fails the gate although no skill changed.
  3. Met, with the 7-lane exemption list.
    • Why 7 rows, not the decision's 9: a grep found no line with two backticked medium ids in codebase-health:audit or visualization:present outside CHANGELOG and README. A row for either would fail the gate's own stale-row rule, so leaving them out follows the decision's rule of exempting exactly the lanes that fail today.
    • Every exempt lane still has a hit the gate would flag:
      • architecture: skills/improve/actions/deepening.md
      • education:illustrate: SKILL.md
      • education:teach: context/lessons.md
      • overengineering:audit: context/report-template.md
      • prototype:explore-directions: SKILL.md and reference/medium.md
      • prototype:pressure-test: SKILL.md
      • visualization:visualize: evals.json lines 180 and 184, where each JSON string holds both ids.
    • No other registered lane has a hit. The remaining matches are either shared copies of lib/rendered-view.md, which scripts/shared-copies.txt lines 540–543 list, or sit outside any lane: session-flow:handoff, code-metrics, and discovery/reference/, since discovery:blindspot maps only to skills/blindspot.

Other notes (not counted as findings)

  • Local pre-push gap: scripts/check-affected.sh runs changed-skills only when a plugins/*/skills/* file changed (check-affected.sh#L715-L716). So locally, the new check never runs for edits like these:

    • an edit outside skills/ in a bare-id lane (for example plugins/architecture/reference/*.md);
    • an edit to the exemption list or to the registry.

    CI still catches all of these, because its step runs check-changed-skills.sh on every pull request.

  • Spend: the gate adds one Node run per check-changed-skills.sh invocation. That is negligible and starts no charge.

  • Lane form (--merge-base): the check runs from this tree's copy and only reads CHECK_DATA_ROOT, as the PR description says.

No prior findings (round 0 of 3) and no settled threads. The PR context holds no embedded instructions aimed at this lane.

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

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

pr-merge's gate refuses its verifier verdict (verify:missing)

@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 4, 2026
@melodic-automation-lanes

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

Copy link
Copy Markdown
Contributor Author

The pr-unblock lane stopped fix-ci on this pull request: the lane could not fix every failing check, after one retry a model rung up, so a session builder takes it; its run log names each one and why. 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=13076 -f activity=fix-ci

@kyle-sexton kyle-sexton added the do-not-merge Hard merge gate: do not merge while applied. label Nov 4, 2026
@kyle-sexton

Copy link
Copy Markdown
Member

Hold: the fix lane escalated this PR (ci-fix-escalated); PR Lane is building an owner-login replacement carrying this change with its check-view-medium-lists and check-skills failures fixed. Releases when: the replacement merges, then this PR is closed as superseded.

@kyle-sexton

Copy link
Copy Markdown
Member

Superseded by #13106, which merged this change with the stale exemption rows removed.

@kyle-sexton kyle-sexton closed this Nov 4, 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.

test(rendered-views): fail a view lane's skill text that lists medium values

1 participant