Skip to content

fix(blueprint-plugin): the four shellspec hook specs have been dead since #2440 — revive or delete #2703

Description

@laurigates

Filed from the review on #2701, which flagged the ADR spec as contradicting the WARN-only design. Verified — and it is broader than one assertion.

Nothing runs these

blueprint-plugin/hooks/spec/ holds four shellspec files. shellspec appears nowhere in .github/workflows/, the justfile, .pre-commit-config.yaml, or any other runner, and is not installed. So they have never failed, because they have never run.

They describe a validator that #2440 deleted

#2440 replaced the hand-rolled bash field lists with check-schema.py driven by schemas/*.schema.json. The specs still assert the old hook's messages and exit codes. Measured against the live hook today:

Spec assertion (validate_adr_frontmatter_spec.sh) Actual behaviour Verdict
missing id → exit 2, ERROR: Missing required frontmatter field: id exit 2, SEVERITY=ERROR … AT=/frontmatter DETAIL='id' is a required property status ✅, message ❌
bad id format → exit 2, ERROR: Invalid ADR id format exit 2, schema-violation line status ✅, message ❌
bad status → exit 2, ERROR: Invalid ADR status exit 2, 'Pending' is not one of [...] status ✅, message ❌
missing ## Decision → exit 2, ERROR: Missing required section: ## Decision exit 0, STATUS=WARN ❌ both
missing domain → exit 0, WARNING: Missing 'domain' field STATUS=OK, no domain warning at all status ✅, message ❌

Every message assertion is wrong; two of five are wrong on exit status too. validate_prp_frontmatter_spec.sh:90 has the same missing-section shape (ERROR: Missing required section: ## Success Criteria) and prp.schema.json also carries x-blueprint-severity: warning, so it is wrong the same way. validate_prd_frontmatter_spec.sh and check_prp_readiness_spec.sh need the same audit.

One thing the specs get right: the hook writes to stderr, so The stderr should include is the correct stream.

Why this is a question

Two defensible paths, materially different work:

  1. Delete all four. They assert a deleted design, nothing runs them, and scripts/tests/test-check-schema.sh already covers this ground properly — 27 assertions, semantic (it executes the validator), mutation-verified, and actually wired into pre-commit. On this reading the specs are not a test suite, they are stale documentation that looks like one, and the cost of keeping them is that the next reader believes hooks are tested when they are not.
  2. Rewrite them against the structured output and wire shellspec into CI. Recovers genuine hook-level coverage that test-check-schema.sh does not have: these drive the hook's own stdin/stderr/exit-code contract end to end. Costs a new CI dependency, and a rewrite of every assertion.

Option 1 is the smaller, honester change and is what I would lean to given the duplication with test-check-schema.sh. Option 2 is right if hook-contract coverage is wanted as a distinct layer.

What is not defensible is leaving them as they are — a suite that cannot fail cannot regress, and it misrepresents the coverage.

Not done in #2701

Deliberately. The PR's scope was narrowing the ADR section set; deleting or rewriting four test files is a different change. shellspec is also not installable in that session's sandbox (the GitHub release archive returns 403 through the proxy), so any rewrite there would have been unverifiable — editing tests one cannot run is how this situation arose.

Refs #2440, #2701, #2446

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    blueprint-pluginBlueprint plugin relatedbugSomething isn't workingquestionFurther information is requested

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions