fix(core): validate-adapter.sh's --no-import-syntax flag is a no-op #115

New Issue

Claude · 2026-08-30T14:57:42Z

Claude commented

2026-08-30 14:57:42 +00:00

plugins/core/.apm/skills/provider-adapter-author/scripts/validate-adapter.sh accepts --no-import-syntax, documents it in usage(), and a skill Gotcha instructs the agent to pass it for providers without an import mechanism. The flag changes no outcome.

The bug

validate-adapter.sh:98-101:

if no_import_syntax:
    has_reference = "AGENTS.md" in adapter_content
else:
    has_reference = bool(import_lines) or "AGENTS.md" in adapter_content

import_lines is derived by filtering lines that contain AGENTS.md, so it is a strict subset of the condition it is OR'd with. bool(import_lines) can only be true when "AGENTS.md" in adapter_content is already true. Both branches therefore reduce to the same expression, and the flag is inert.

Confirmed empirically during #99 wave 3, on a .mdc fixture carrying a plain-text pointer and no @AGENTS.md line: exit 0 both with and without the flag.

Why it matters

The flag exists to express a real distinction — Cursor and Copilot have no @-import mechanism, so an adapter that points at AGENTS.md in prose is correct for them and should not be judged against an import line. That distinction is currently unenforced in either direction:

Passing the flag defends nothing.
Omitting it also costs nothing, so the failure is silent. Nobody discovers the flag is dead by using it.

The check is weaker than intended in the other direction too: because has_reference is satisfied by the bare string AGENTS.md appearing anywhere in the file, an adapter that merely mentions AGENTS.md in a comment passes the reference check without deferring to it at all.

How it surfaced

A wave-3 retrofit rewrote the skill's Gotcha from a plain instruction into an assertion about script behaviour:

scripts/validate-adapter.sh fails a correct adapter for such a provider unless you pass --no-import-syntax.

That sentence is false, and it was carrying the rhetorical weight of the surrounding rule. The Gotcha is being reverted to the old instruction form in the retrofit; this issue tracks the underlying script defect, which predates it.

Suggested fix

Decide what the flag should mean, then make the branches differ. The likely intent:

With --no-import-syntax: require a prose pointer, and do not require an @AGENTS.md line.
Without it: require an actual import line, not merely the string AGENTS.md somewhere in the file.

That makes the default branch stricter, so check the existing adapters in this repo before tightening it — the change could turn up pre-existing failures, which is arguably the point.

Worth adding a test that asserts the two branches disagree on at least one fixture. A flag whose branches are provably identical is the kind of thing a test would have caught at authoring time.

Files

plugins/core/.apm/skills/provider-adapter-author/scripts/validate-adapter.sh:98-101 — the dead branch
plugins/core/.apm/skills/provider-adapter-author/tests/ — where the disagreement test belongs

Found during #99 wave 3. Distinct from #110 (BOUNDARY_ARROW regex) and #111 (provenance escape hatch).

`plugins/core/.apm/skills/provider-adapter-author/scripts/validate-adapter.sh` accepts `--no-import-syntax`, documents it in `usage()`, and a skill Gotcha instructs the agent to pass it for providers without an import mechanism. The flag changes no outcome. ## The bug `validate-adapter.sh:98-101`: ```python if no_import_syntax: has_reference = "AGENTS.md" in adapter_content else: has_reference = bool(import_lines) or "AGENTS.md" in adapter_content ``` `import_lines` is derived by filtering lines that contain `AGENTS.md`, so it is a strict subset of the condition it is OR'd with. `bool(import_lines)` can only be true when `"AGENTS.md" in adapter_content` is already true. Both branches therefore reduce to the same expression, and the flag is inert. Confirmed empirically during #99 wave 3, on a `.mdc` fixture carrying a plain-text pointer and no `@AGENTS.md` line: exit 0 both with and without the flag. ## Why it matters The flag exists to express a real distinction — Cursor and Copilot have no `@`-import mechanism, so an adapter that points at AGENTS.md in prose is correct for them and should not be judged against an import line. That distinction is currently unenforced in either direction: - Passing the flag defends nothing. - **Omitting it also costs nothing**, so the failure is silent. Nobody discovers the flag is dead by using it. The check is weaker than intended in the other direction too: because `has_reference` is satisfied by the bare string `AGENTS.md` appearing anywhere in the file, an adapter that merely *mentions* AGENTS.md in a comment passes the reference check without deferring to it at all. ## How it surfaced A wave-3 retrofit rewrote the skill's Gotcha from a plain instruction into an assertion about script behaviour: > `scripts/validate-adapter.sh` fails a correct adapter for such a provider unless you pass `--no-import-syntax`. That sentence is false, and it was carrying the rhetorical weight of the surrounding rule. The Gotcha is being reverted to the old instruction form in the retrofit; this issue tracks the underlying script defect, which predates it. ## Suggested fix Decide what the flag should mean, then make the branches differ. The likely intent: - **With** `--no-import-syntax`: require a prose pointer, and do **not** require an `@AGENTS.md` line. - **Without** it: require an actual import line, not merely the string `AGENTS.md` somewhere in the file. That makes the default branch stricter, so check the existing adapters in this repo before tightening it — the change could turn up pre-existing failures, which is arguably the point. Worth adding a test that asserts the two branches disagree on at least one fixture. A flag whose branches are provably identical is the kind of thing a test would have caught at authoring time. ## Files - `plugins/core/.apm/skills/provider-adapter-author/scripts/validate-adapter.sh:98-101` — the dead branch - `plugins/core/.apm/skills/provider-adapter-author/tests/` — where the disagreement test belongs ## Related Found during #99 wave 3. Distinct from #110 (`BOUNDARY_ARROW` regex) and #111 (provenance escape hatch).

Claude referenced this issue

2026-08-30 14:58:30 +00:00

design(bin): research had a fan-out design its allowed-tools never permitted #116

Claude referenced this issue

2026-08-30 15:12:26 +00:00

refactor(skills): retrofit the 39 skills to the ADR-0020 context contract #99

Claude added the Kind/Bug

labels 2026-08-30 15:58:53 +00:00

Claude added this to the Skills & Agents milestone 2026-08-30 15:58:58 +00:00

Claude referenced this issue

2026-08-30 17:05:09 +00:00

refactor(skills): retrofit the 39 skills to the ADR-0020 context contract #99

Claude referenced this issue

2026-08-30 21:15:21 +00:00

refactor(skills): retrofit the 39 skills to the ADR-0020 context contract #99

Claude referenced this issue

2026-08-31 08:08:53 +00:00

design(core): no provider-file audit path — "is my CLAUDE.md OK?" reaches a write-only skill #128

Claude commented

2026-08-31 08:10:21 +00:00

Fixed on `refactor/adr0020-skill-retrofit` (not yet pushed — the PR body will carry the close)

Implemented exactly as the suggested fix specifies. The branches now differ:

if no_import_syntax:
    has_reference = bool(pointer_lines)
else:
    has_reference = bool(import_lines)

import_lines is no longer OR'd with the superset it is derived from, and the default branch now requires an actual @-import line rather than the bare string AGENTS.md appearing anywhere in the file — closing the second, weaker-than-intended direction this issue also flagged. The --no-import-syntax branch requires a prose pointer, so an inert @AGENTS.md line with no prose does not satisfy it either.

The disagreement test you asked for exists. tests/validate-adapter.bats:

the two --no-import-syntax branches disagree: a text-pointer-only adapter fails in default mode

plus the converse guard, with --no-import-syntax, an inert @AGENTS.md line alone is not a prose pointer. A flag whose branches are provably identical now fails the suite.

Also added, beyond the issue's scope: the two malformed --max-lines invocations (flag as final argument with no value; non-numeric value) now report a real error instead of failing silently.

Suite result, run through tests/run-bats.sh: 14 tests, 14 ok.

On the warning about pre-existing failures: it did not materialise. The repo has two adapters — CLAUDE.md (@AGENTS.md) and providers/claude-code/CLAUDE.md (@~/.agents/AGENTS.md, @~/.claude/core/instructions/governance.md). Both carry real @-imports and both still exit 0 under the stricter default. Nothing was tightened into a break.

## Fixed on `refactor/adr0020-skill-retrofit` (not yet pushed — the PR body will carry the close) Implemented exactly as the suggested fix specifies. The branches now differ: ```python if no_import_syntax: has_reference = bool(pointer_lines) else: has_reference = bool(import_lines) ``` `import_lines` is no longer OR'd with the superset it is derived from, and the default branch now requires an actual `@`-import line rather than the bare string `AGENTS.md` appearing anywhere in the file — closing the second, weaker-than-intended direction this issue also flagged. The `--no-import-syntax` branch requires a **prose pointer**, so an inert `@AGENTS.md` line with no prose does not satisfy it either. **The disagreement test you asked for exists.** `tests/validate-adapter.bats`: > `the two --no-import-syntax branches disagree: a text-pointer-only adapter fails in default mode` plus the converse guard, `with --no-import-syntax, an inert @AGENTS.md line alone is not a prose pointer`. A flag whose branches are provably identical now fails the suite. Also added, beyond the issue's scope: the two malformed `--max-lines` invocations (flag as final argument with no value; non-numeric value) now report a real error instead of failing silently. Suite result, run through `tests/run-bats.sh`: **14 tests, 14 ok.** **On the warning about pre-existing failures:** it did not materialise. The repo has two adapters — `CLAUDE.md` (`@AGENTS.md`) and `providers/claude-code/CLAUDE.md` (`@~/.agents/AGENTS.md`, `@~/.claude/core/instructions/governance.md`). Both carry real `@`-imports and both still exit 0 under the stricter default. Nothing was tightened into a break.

Defame1297 referenced this issue from a commit

2026-08-31 09:14:47 +00:00

fix(core): make --no-import-syntax actually change the adapter validation

Claude referenced this issue

2026-08-31 09:15:19 +00:00

refactor(skills): retrofit the corpus to the ADR-0020 context contract #129

Defame1297 referenced this issue from a commit

2026-09-01 13:09:28 +00:00

fix(core): close five ways validate-adapter.sh graded an adapter it had not read

Defame1297 closed this issue

2026-09-01 13:09:28 +00:00

Defame1297 referenced this issue from a commit