Repository navigation
Fix the test, tooling, and skill defects that the #98 benchmark found in #96 - #105
Merged
DocksDocks merged 9 commits intoOct 23, 2026
Conversation
The test-contract check accepted a registry row that left out a toolchain the suite needs: repo-plan-cli lists bun-1 because the fake gh runs on Bun, but removing bun-1 still passed, because the check looked only at the runners in scripts/ci.mjs and only asked that each of them be listed. The check now also discovers the #! interpreter of each executable fixture under scripts/tests/fixtures/ that a suite's files name, and it requires each row's toolchains to equal the discovered set: a missing or an extra toolchain fails with the row id and the evidence. Refs #102
The fake gh built every pull request node with fixed fields, whether or not the GraphQL query selected them. A helper query that dropped mergedAt and baseRefName still passed all 66 plan CLI tests, although GitHub would then omit both fields and the helper would reject every closing pull request as malformed. Closers, associated pull requests, and Repository.pullRequest now return only the selected fields, with one node builder for all three. Refs #102
No test reached "plan #<n> changed during the settle wait; run reopen again". Two tests now reach its two triggers without a helper change: a later close by the same pull request during the wait, and a close reason that changes to not planned. Each asserts the refusal, one settle wait, no write, and the stored issue. The commit-close test no longer inspects the request text for a claim mutation; the stored issue and the single write already show that an owned plan gets no claim. Refs #102
The Report section rule gave a shell command as its check, and that command printed one line with the full count, which the rule calls a pass, also when every skill lacked the section. No gate ran it. The skills guard, which the gate runs for each plugin, now fails when a SKILL.md does not end with the text of scripts/config/report-section.md byte for byte, and it names each file and whether the section is missing or differs. The docks skills node and the validator table name the guard as the check. Refs #102
…ecks Code review round 1 of #102 found that no committed test ran the new rejection paths: a change that disabled them would keep CI green and bring back D3 or D7. Author-tooling now runs the registry check on copies that lack bun-1 for repo-plan-cli and that list an unneeded toolchain, and runs the skills guard on a copy of the docks skills with one section removed, one byte changed, and every section removed. The registry check takes `--registry <path>` for those copies; without it, it reads the committed registry as before. Refs #102
Code review round 1 of #102 found that the fake gh treated an alias as a field name, so `{mergedAt: url}` returned the stored mergedAt and an unrequested url, and that no test checked the projection itself. The fake gh now reads `alias: field` at each level, puts the stored value under the response key, and refuses one key for two fields. New tests call it directly and check that pull request nodes, closers, and associated pull requests hold only the selected keys, also with aliases. All six fail against the stub before #102, and the three alias tests fail against the stub before this commit. Refs #102
Code review round 2 of #102 found that the fake gh took a node's selection from the first matching `name{` in the whole query, so two aliased timeline connections gave both closers the first selection and `nodes{number} nodes{url}` lost the second one; and that a pull request's nested repository had no `__typename`, so an inline fragment on it returned nothing. The fake gh now builds each object and keeps only what its own field selects, merging repeated selections of one response key at every level, and the nested repository carries its type. Five new tests cover the cases; all fail against the fake gh before this commit. Refs #102
…evidence Code review round 2 of #102 found that `#!/usr/bin/env -S -u NODE_OPTIONS bun` gave the interpreter NODE_OPTIONS, and that the extra-toolchain diagnostic named no evidence. The shebang parser now skips env options and their operands as GNU and BSD env do, and the diagnostic lists the discovered runners with their sources. The parser is exported and tested in author-tooling, and the registry check runs only when the file is executed. Refs #102
Code review round 3 of #102 found two small gaps: `#!/usr/bin/env -S -- FORCE_COLOR=0 bun` gave the interpreter FORCE_COLOR=0, because the parser stopped at `--` although env still reads assignments after it; and the aliased-timeline test read `issue.updatedAt` without selecting it. The parser now skips assignments after `--`, with a new case, and the test selects `updatedAt`. Refs #102
DocksDocks
deleted the
102-fix-the-test-tooling-and-skill-defects-that-the-98-benchmark-found-in-96
branch
October 23, 2026 20:31
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Yolo run: #102 Fix the test, tooling, and skill defects that the #98 benchmark found in #96
Pull request: #105. Checks: 5 of 5 passed. Reviewed head:
848a2863.What changed
#!interpreter of each executable fixture that a suite runs and requires each row's toolchains to equal the discovered set; a missing or an extra toolchain fails with the row, the toolchain, and the evidence (scripts/tests/test-contracts.mjs, D3).scripts/tests/fixtures/gh-stub/gh,scripts/tests/plan-cli/gh-stub.test.mjs, D5).scripts/tests/plan-cli/reopen.test.mjs, D4 and D6).scripts/config/report-section.md(scripts/skills/guard.mjs, D7).scripts/tests/author-tooling.mjs).Decisions and corrections
Reviews
Plan review passed in round 2 (a delta round of 38 s). Code review passed in round 4: round 1 was a three-lens panel with the self-review running at the same time; rounds 2 and 3 were full panel rounds, because their repairs changed more than half of the diff; round 4 was a delta round of 34 s.
Open findings: none.
Verification
bun scripts/ci.mjs: passed for the repo-wide checks and both plugins at the reviewed head.node scripts/tests/plan-cli.mjs: 79 of 79 passed;plan.mjsline coverage 99.68%.bun scripts/tests/author-tooling.mjsandbun scripts/tests/test-contracts.mjspassed.bun-1removed fromrepo-plan-cli, the registry check fails (before: it passed); withmergedAtandbaseRefNameremoved from the helper's pull request query, 26 plan CLI tests fail (before: all passed); with the settle-wait refusal removed, both new reopen tests fail; with a Report section removed or changed, the guard fails and names the file.Limits and failures
Kept resources
Merge now: none; the repository now deletes a merged branch automatically.Leave pull request open: the linked branch, the open issue, and the pull request.After merge
Closes #102