Skip to content

portability gate: escape-hatch grammar rigor — standalone-comment-only matching + required nonempty reason (pair from PR #609 re-review) #624

Description

@kyle-sexton

Category: bug (latent) / hardening · Source: claude-review (Codex) bot re-review on PR #609 (#531), 2026-07-20 — a matched pair, deferred together per control-tower ruling.

Both findings tighten the portability gate's escape-declaration grammar in scripts/check-skill-portability.sh. That grammar's tokens (portability-ok:, portability-scope:) are the same escapes awaiting operator-convention ratification on #531 (items 1-2), so the rigor fix bundles with that ratification pass rather than a separate review cycle. Neither is blocking: both sit on the escape-hatch logic, the review job passed, and CI is green.

Finding 1 — restrict portability-scope: to a standalone comment (and require a reason)

Thread: #609 (comment)

scan_file gates the whole-file escape with grep -qE 'portability-scope:', which matches the substring anywhere in a scanned file. So ordinary skill prose or a doc example — e.g. a line explaining the `portability-scope: <reason>` annotation — silently exempts the entire file, disabling the gate for any later bare origin/main addition in it.

Expected: recognize the declaration only in its intended standalone-comment form, and require a non-whitespace reason, before treating the file as exempt.

Finding 2 — require a nonempty reason for portability-ok:

Thread: #609 (comment)

is_annotated treats any portability-ok: substring as an exemption, including an empty <!-- portability-ok: -->. A changed skill can therefore keep a bare origin/main and pass this required gate with no reason recorded for a reviewer to assess.

Expected: validate that non-whitespace reason text follows the marker before suppressing the hit.

Why deferred, not fixed inline

Both change the gate's documented escape-declaration contract (header escape list at scripts/check-skill-portability.sh lines 33-43, the violation help text, and new self-test cases), and both operate on the escape-hatch matching logic — not the quoted-path collection fix that PR #609 shipped. They are a coherent grammar-rigor pair and are best ratified alongside the #531 operator-convention items 1-2 they depend on, keeping #609's diff scoped to the quoted-path finding it was opened for.

Related

Found during the post-push Codex re-review on PR #609 (#531). The two threads above are resolved on that PR with a pointer to this issue. Sibling deferral from the same gate: #611 (annotation-carry inline-vs-block comment distinction).

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

    needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.priority: highSignificant impact, or blocks an imminent release; staff this cycle.

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions