fix(checkpoints): the two that never ran, and the scripts nobody tested - #164
Merged
Conversation
The new validate-checkpoints.sh reports GW-15 and GW-16 as rejected by the assessment runner. Neither has ever executed, and nothing said so — a rejected checkpoint is skipped silently and the report stays quiet. GW-16 is expressible without command substitution: `! git ls-files -- … | grep -q .` reaches the same verdict, and the runner strips a leading `!` before checking the base command. GW-15 is not. Counting commits since the last tag needs `<tag>..HEAD` and `$( )`, and the allowlist rejects `..` as path traversal and `$(` as chaining. The checkpoint is removed and the rule moves to verify-git-workflow.sh, where a full shell is available. GW-21 declares `# mechanical-counterpart: GW-17`; GW-30 declares `none (<reason>)` — its commands fetch a staging lockfile for a judgement and decide nothing on their own. A third abort in verify-git-workflow.sh, same family as the two fixed in #163: `git rev-parse origin/<branch>` echoes the ref NAME on stdout when it does not resolve, so the non-empty guard passed, the rev-list failed on the literal string, and `set -e` killed the script three sections early — on every unpushed branch, which is every branch before its first push. Tests for the three shipped gates that had none: merge-gate.sh (denies BLOCKED, denies UNSTABLE, denies an unresolved thread even when CLEAN, passes unrelated commands), conflict-marker-gate.py (denies staged markers, fails open outside a repo), spec-cleanup-guard.sh (reports artifacts and — its stated invariant — modifies nothing). test_checkpoint_patterns.sh asserts every `type: command` pattern is runnable: single line, no chaining metacharacters, no `..`, base command on the allowlist. The rule is mirrored rather than imported, because automated-assessment is not a dependency of this repo and a test that needs an absent checkout does not run. tests.yml is now a thin caller of the skill-repo-skill reusable. Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
|
Contributor
Dependency ReviewThe following issues were found:
License Issues.github/workflows/tests.yml
OpenSSF Scorecard
Scanned Files
|
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.
validate-checkpoints.sh(netresearch/automated-assessment-skill#59) reports GW-15 and GW-16 as rejected by the assessment runner. Neither has ever executed. A rejected checkpoint is skipped silently and the assessment report stays quiet about it, so nothing ever said so.! git ls-files -- … | grep -q .reaches the same verdict, and the runner strips a leading!before checking the base command.<tag>..HEADand$( ); the allowlist rejects..as path traversal and$(as chaining. The checkpoint is removed and the rule moves toverify-git-workflow.sh, where a full shell is available.# mechanical-counterpart: GW-17. GW-30 declaresnone (<reason>)— its commands fetch a staging lockfile for a judgement and decide nothing on their own.A third silent abort in verify-git-workflow.sh
Same family as the two fixed in #163.
git rev-parse origin/<branch>echoes the ref name on stdout when it does not resolve, so the non-empty guard passed, therev-listfailed on the literal string, andset -ekilled the script three sections early — on every unpushed branch, which is every branch before its first push. The test fixture in #163 had no remote at all, which is why it did not catch this; the new case adds one.Tests for the three gates that had none
merge-gate.sh— denies BLOCKED, denies UNSTABLE (a red non-required check is still red), denies an unresolved thread even when CLEAN, and passes unrelatedghcommands through. Driven with a stubbedghthat answerspr viewandapi graphqldifferently; a single canned payload makes every blocking case look like a pass, which is worth knowing before trusting a hook test.conflict-marker-gate.py— denies staged conflict markers, passes non-commit commands, fails open outside a repository.spec-cleanup-guard.sh— reports artifacts and, per its stated invariant, modifies and stages nothing.Keeping it fixed
test_checkpoint_patterns.shasserts everytype: commandpattern is actually runnable: single line, no chaining metacharacters, no.., base command on the allowlist. The rule is mirrored rather than imported — automated-assessment is not a dependency of this repo, and a test that needs an absent checkout is a test that does not run. Every fix above was verified by re-injecting the defect and watching the specific assertion fail.tests.ymlbecomes a thin caller of the reusable from netresearch/skill-repo-skill#211.