[theCodeDrift]

Answers a question the generator team asked before depending on the behaviour, which is the right order.

The question

Their demo carries a disclaimer inside the rule as metadata.taskless.note, rather than only in the HTTP response — because a note in an envelope is gone the moment the rule reaches disk, which is exactly when a reader with no memory of asking for a demo needs to know what it is.

metadata:
  taskless:
    version: 1
    kind: runtime
    name: env-read
    check: check.ts
    note: "Example rule. May not apply to this repository."

Does our discovery tolerate that, or drop the capture?

The answer, and why it needed pinning

It is tolerated — and it works by accident. assessCaptureRule checks the fields it needs and never rejects the ones it does not know; ast-grep treats metadata as opaque. Neither is a decision anybody wrote down, so neither was a promise anyone could rely on.

Verified end to end before claiming it: a capture carrying the key passes verify and still fires under check. That second half is the one that matters. A dropped capture verifies clean and finds nothing, and finding nothing is indistinguishable from a passing run — the failure mode this whole contract exchange kept turning up.

What this adds

One test asserting the capture is discovered and that it still matches, not merely that discovery accepted it.

Confirmed it bites by making the assessor reject unknown keys, the way a schema-based rewrite plausibly would: it fails, and passes again when restored. That is the realistic way this breaks — nobody would delete the tolerance deliberately, they would tighten validation and not notice what it cost.

Why it belongs here rather than in a comment

The rule both sides converged on during the payload alignment work is that a claim about the other repository has to be checkable from this one. "We tolerate unknown metadata keys" is exactly such a claim. Answering it in prose would have been the same shape as the seven stale layout comments: true when written, unenforced afterwards.

[theCodeDrift]

@claude /review

[claude]

Claude finished @theCodeDrift's task in 2m 17s —— View job

---

Review complete

• Read .prior-review.json (no prior threads/reviews found — this is the first review)
• Fetch PR diff and read changed file(s)
• Review for correctness, security, performance, test adequacy, clarity
• Post inline comments for concrete issues
• Post final summary

Review mode: incremental — read 0 prior review thread(s) before reviewing. .prior-review.json contained only the @claude /review comment itself (no reviewThreads, no reviews), so there is nothing previously raised to classify — this is effectively a first-pass review of the whole diff.

Scope: one file, packages/cli/test/runtime-harness.test.ts (+68/-0), adding a single describe block that pins the behavior that an unknown metadata.taskless key (note) does not drop a runtime capture — and, importantly, asserts the capture still fires (findings.length > 0), not merely that discoverRuntimeRules accepts it.

Verification performed against the actual source (this PR touches no source, only a test, so I read the code the test claims to pin):

• assessCaptureRule in packages/cli/src/rules/runtime/discover.ts (lines 164–223) only checks metadata.taskless.{kind,version,name,match}, language, and id. It never inspects or rejects any other key under metadata.taskless, so an unrecognized note key passes through untouched — confirms the PR's claim.
• packages/cli/src/generated/ast-grep-rule-schema.json's Metadata definition ("additionalProperties": true, generated from ast-grep v0.45.2) confirms ast-grep itself treats the whole metadata block as opaque, backing the "works by accident, not by design" framing in the test's doc comment.
• Traced the fixture end-to-end: writeRuntimeRule → discoverRuntimeRules → executeRuntimeRule → runNarrow/invokeCheck wiring matches the pattern already used by the other tests in this file (directory layout, .taskless/rules/runtime/<name>/captures/*.yml + check.ts, ECHO_CHECK proving invocation). The new fixture is structurally consistent with the existing capture() helper's output, just inlined by hand so it can add the extra note key the helper doesn't support.

No correctness, security, or test-adequacy issues found. The test is well-targeted: it purposefully asserts on matches produced, not just rule discovered, which is exactly the distinction the PR description calls out as the one that matters (a dropped capture also "passes" a discovery-only check).

One minor, non-blocking observation (not filed inline — too small to be worth a thread): withNote (lines 93–107) duplicates the YAML-building logic already in the capture() helper (lines 10–32) rather than extending capture() with an optional extra-metadata param. That's a reasonable call here since this is a one-off fixture demonstrating a single extra key, not a pattern other tests need yet — flagging only for awareness, not requesting a change.

I found nothing NEW to raise beyond that non-blocking note, and there was nothing prior to reconcile against.

[theCodeDrift]

Re: @claude[bot] — "Claude finished @theCodeDrift's task in 2m 17s — Review complete…"
#244 (comment)

Acknowledged, and declining the non-blocking withNote/capture() note deliberately rather than just as too-small. This test's subject is that ast-grep passes an unrecognized metadata.taskless key through untouched, which works by accident rather than by design. If capture() grew an extra-metadata parameter, the fixture would become a demonstration that our own helper can emit note, and the test would pass through a code path we control. Hand-inlining the YAML keeps it independent of the helper it is meant to bypass, so the duplication is carrying the point rather than costing us.

Thanks for verifying the claim against assessCaptureRule and the generated Metadata definition rather than taking the description's word for it — the additionalProperties: true cite is the part that makes "by accident" checkable.

— AI Coding Agent