Skip to content

fix(pr-pipeline)!: reject fable and best profiles and pin fix-ci rung-up behavior with tests - #10377

Merged
melodic-automation-lanes[bot] merged 1 commit into
mainfrom
work-resolve-issue/issue-9022-40407741208-1
Oct 27, 2026
Merged

melodic-automation-lanes[bot] merged 1 commit into
mainfrom
work-resolve-issue/issue-9022-40407741208-1

Conversation

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

Closes #9022

Summary

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

Following the recorded decision on #9022, pr-pipeline.schema.json no longer admits best or fable for a profile's model or subagent-model, so the config reader rejects them before any run until the owner sets a Fable allowance. L1 needs no new mechanism: a new test shows that a rung-up retry's check is the pr-unblock / fix-ci check the dispatcher counts, so the existing per-head cap (limit-stops-per-head, 3) already bounds rung-ups and no code fix was needed. Another new test shows that fix-docs's default patch row never selects rung-up.

Not verified

  • The new and existing node tests (models.test.mjs, apply-patch.test.mjs, fix-ci-dispatch.test.mjs, workflow-gates.test.mjs) were not run: this lane allows no interpreter, so they were checked only by reading and are left to the PR's CI test-node job
  • Changes left uncommitted in the working tree, as the runner instructs; the runner turns them into the patch
  • The CI-only checks check-affected lists (actionlint, line-citations, convention-yaml-schema and the rest) are left to the PR's CI

Fix

  • docs/conventions/pr-pipeline/pr-pipeline.schema.json: dropped best and fable from the profile model and subagent-model enums, and the descriptions say why
  • docs/conventions/pr-pipeline/README.md: Models lists the admitted aliases and says the schema rejects fable/best until a Fable allowance is set; the fix-ci rung-up paragraph says the retry's check counts toward limits.limit-stops-per-head
  • .github/actions/resolve-config/models.test.mjs: schema-rejection cases for a model or subagent-model of fable or best, and a test that default/opus/sonnet x inherit/opus/sonnet profiles still resolve
  • .github/scripts/apply-patch/apply-patch.test.mjs: test that the default row (fix-docs) never selects skip reason rung-up at rung 0 or 1, for an empty patch (patch-empty) or a written one (proceeds)
  • .github/scripts/pr-unblock/fix-ci-dispatch.test.mjs: test that the three fix-ci-rung-up call jobs in pr-unblock.yml run pr-run-activity-read.yml as pr-unblock / fix-ci at model-rung 1, so their check is DEFAULTS.checkName; a head with a rung-0 and a rung-up failure is dispatched once more on a new b
  • .changes/pr-pipeline/fix-9022-reject-fable-profiles-4b7e1c2a.md: new major fragment (breaking schema narrowing, matching the 4.0.0 precedent for schema removals) plus a Fixed entry for the rung-up tests

Verification

The runner ran each check from the base copy against 18b7585:

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

The run reports:

  • .lane/checks/bin/check-changelog-fragments: All 9 changelog fragment(s) under .changes/ are valid
  • .lane/checks/bin/check-affected: 9 passed, 5 skipped over 6 changed files (it failed first with MISSING FRAGMENT for docs/conventions/pr-pipeline; adding the fragment fixed it)
  • fresh-context review subagent (opus) over the whole diff: no correctness bugs. It traced each new test against resolve.mjs, apply-patch.mjs/patch-rules.json, fix-ci-dispatch.mjs and pr-unblock.yml and expects all to pass. Its one accuracy note (the usual rung-0 run posts SUCCESS, so the cap is an up
  • read .github/workflows/pr-unblock.yml (fix-ci-rung-up, -subscription, -fallback) and .github/actions/report-check-run/decide.mjs:69: the rung-up check is named <lane> / <activity> = pr-unblock / fix-ci, the name fix-ci-dispatch.mjs counts, so no code fix was needed for the restarts-cap
  • grep across the repo: no pr-pipeline.yaml, fixture or test names fable/best as a profile model or subagent-model (the remaining hits are role-map tests and unrelated plugins)
  • The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/40407741208

Left out: the items under Not verified.

Related

#9022

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

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 18b7585298ce3f60d6c5d70d7b176dcc9d7bd325: 0 finding(s), 0 as inline comments.

No security issues found in this pull request.

Trust controls: The change narrows rather than widens. pr-pipeline.schema.json drops best and fable from the profile model and subagent-model enums. The config reader then rejects those aliases before any run. This reduces what the token broker's config can grant. docs/conventions/pr-pipeline.yaml is not changed, and it names neither alias, so the live config still validates. The diff touches no trusted-actor list, token-scope code, kill switch, merge hold or isolation action.

Tests and docs: The other changes are tests (models.test.mjs, apply-patch.test.mjs, fix-ci-dispatch.test.mjs), README text and a changelog fragment. The tests add no runtime code. The fix-ci-dispatch test only reads pr-unblock.yml as text.

Manifests: No plugin.json or marketplace.json changes, so there are no manifest <field> lines.

Instruction surface: No standing instruction is deleted or softened.

PR context: It contains no embedded directives aimed at the lane.

Cost: None. The only spend touched is prepaid API usage, and the change tightens it by keeping the Fable tier off.

critical-findings: 0

@github-actions

Copy link
Copy Markdown
Contributor

claude-review reviewed 18b7585298ce3f60d6c5d70d7b176dcc9d7bd325: 0 finding(s), 0 as inline comments.

No findings. I read the full diff and the closing issue (#9022), and cross-checked the schema, the config reader, pr-unblock.yml and the dispatcher. I ran no tests (no shell), so the new tests are checked by reading only.

Schema change (L2). model and subagent-model in pr-pipeline.schema.json no longer admit best or fable. Nothing else in .github or docs/conventions/pr-pipeline names either value as a profile model or subagent model. The live pr-pipeline.yaml doesn't name them, and the existing "live config" test still holds. The README and the schema descriptions now match the enum. The new rejection cases in models.test.mjs cover all four combinations. The new "still admits" test covers the 3×3 grid of default/opus/sonnet models against inherit/opus/sonnet subagent models.

Rung-up cap test (L3). I read the three fix-ci-rung-up* jobs at .github/workflows/pr-unblock.yml:2225, :2296 and :2366. Each runs pr-run-activity-read.yml with lane: pr-unblock, activity: fix-ci and model-rung: "1". The test's line-based job-block parsing therefore matches those lines, and pr-unblock / fix-ci is the name the dispatcher counts.

The dispatch assertions follow skipReason and capCount in fix-ci-dispatch.mjs:

  • Two failed runs older than the base tip leave no current-base failures, so the head dispatches once more.
  • A third run makes capCount reach 3 and every run retryable, so the cap warning fires with the exact message the test expects.

fix-docs test. The default-row test matches the issue's description. An empty patch escalates patch-empty and a written patch proceeds. I did not open apply-patch.mjs or patch-rules.json to confirm the select outputs.

Changelog fragment. bump: major fits a schema narrowing that fails existing configs, and the author cites the 4.0.0 precedent.

Unverified. The closing issue's acceptance criteria are truncated in the PR context file after the third item, "A `fix-c…". I could not check the third criterion. The first two are met and have tests that would fail if they broke.

Spend. None. The change only narrows what a run may do.

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

Unattended merge: native auto-merge leaves this pull request unarmed at head 18b7585298ce3f60d6c5d70d7b176dcc9d7bd325. A person must clear this reason, so the pr-merge lane added the needs-human label: fix the reason, then remove the label, and the next scheduled run judges it again.

it changes .github/actions/resolve-config/models.test.mjs, a hold path

@melodic-automation-lanes melodic-automation-lanes Bot added the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 26, 2026

@kyle-sexton kyle-sexton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Owner approval of head 18b7585: hold path resolve-config/models.test.mjs (test-only).

@kyle-sexton kyle-sexton removed the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 27, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The unattended merge path armed native auto-merge at head 18b7585298ce3f60d6c5d70d7b176dcc9d7bd325.

@melodic-automation-lanes
melodic-automation-lanes Bot added this pull request to the merge queue Oct 27, 2026
Merged via the queue into main with commit 8535f1f Oct 27, 2026
71 of 73 checks passed
@melodic-automation-lanes
melodic-automation-lanes Bot deleted the work-resolve-issue/issue-9022-40407741208-1 branch October 27, 2026 17:30
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.

fix(pr-pipeline): harden the fix-ci rung-up retry and reject Fable profiles until an allowance exists

1 participant