test(cli): pin the two metavariable-binding fixes, and warn about inert rules

packages/cli/src/agent/create-sg-rule.txt

@@ -1,4 +1,4 @@
-# Topic: create-sg-rule     (CLI v%(CLI_VERSION)s / topic v3)
+# Topic: create-sg-rule     (CLI v%(CLI_VERSION)s / topic v4)
 
 ## You are here
 This is `create-sg-rule`. It helps you write an ast-grep rule: a check
@@ -218,6 +218,53 @@ whole rule.
    - Only after the user confirms, fetch `%(TASKLESS_CLI)s agent
      create-remote-rule` and follow it. Do not call the service silently.
 
+## Two ways a relational rule matches nothing
+
+Both of these parse, verify, run, and exit 0. Neither reports anything
+ever, which reads exactly like a clean codebase.
+
+**`follows` and `precedes` need `stopBy: end` to cross punctuation.**
+A sibling relation walks the immediate siblings, and in most grammars
+the separators are nodes too. Between two array elements sits a `,`, so
+the previous element is not the previous SIBLING. Measured on
+TypeScript, `[10, 10, 30]` with
+
+```
+rule:
+  kind: number
+  follows: { kind: number }
+```
+
+matches nothing, and the same rule with `stopBy: end` inside `follows`
+matches. If a sibling relation reports zero on input you believe should
+match, add `stopBy: end` before doubting the rest of the rule.
+
+**A `not` containing a bare metavariable excludes everything.** Writing
+"the name is not used in the initializer" as
+
+```
+rule:
+  kind: variable_declarator
+  has: { field: name, pattern: $A }
+  not:
+    has: { pattern: $A, stopBy: end }
+```
+
+matches nothing at all, because `$A` is bound to the name node and that
+node is its own descendant: the `not` finds it inside every declarator
+and rejects all of them. Scope the negation to the part you meant, in
+this case the value:
+
+```
+  not:
+    has:
+      field: value
+      has: { pattern: $A, stopBy: end }
+```
+
+The general shape: when a `not` searches the same subtree that bound the
+metavariable, it always finds it.
+
 ## Important Notes
 
 - Do NOT make any HTTP requests to taskless.io on this path.

packages/cli/test/agent-extensions.test.ts

@@ -453,3 +453,41 @@ describe("the GitHub-owner constraint is guarded twice", () => {
     }
   );
 });
+
+/**
+ * The two shapes that verify clean and report nothing forever. Both cost real
+ * time to diagnose while building the matching-semantics differential, and
+ * neither was written down anywhere.
+ */
+describe("create-sg-rule warns about silently inert relational rules", () => {
+  let cwd: string;
+
+  beforeEach(async () => {
+    cwd = await mkdtemp(join(tmpdir(), "taskless-sg-inert-"));
+  });
+
+  afterEach(async () => {
+    await rm(cwd, { recursive: true, force: true });
+  });
+
+  it("says a sibling relation needs stopBy to cross punctuation", async () => {
+    const result = await runCli(["agent", "create-sg-rule", "-d", cwd]);
+    expect(result.exitCode).toBe(0);
+    expect(result.stdout).toContain("stopBy: end` to cross punctuation");
+    // The reason, not just the fix: the separator is a node too.
+    expect(result.stdout).toContain("the separators are nodes too");
+  });
+
+  it("says a not over the binding subtree excludes everything", async () => {
+    const result = await runCli(["agent", "create-sg-rule", "-d", cwd]);
+    expect(result.stdout).toContain("excludes everything");
+    expect(result.stdout).toContain("its own descendant");
+  });
+
+  it("frames both as reporting nothing rather than erroring", async () => {
+    // The dangerous property: zero findings is what a clean codebase looks
+    // like, so neither failure announces itself.
+    const result = await runCli(["agent", "create-sg-rule", "-d", cwd]);
+    expect(result.stdout).toContain("reads exactly like a clean codebase");
+  });
+});

packages/cli/test/ast-grep-vendor-contract.test.ts

@@ -285,6 +285,46 @@ const arityRule = (body: string) =>
     "",
   ].join("\n");
 
+/** Findings from one semantics rule over one source file, text and message. */
+const semanticsFindings = (body: string, source: string) =>
+  scan(
+    project({
+      rules: { semantics: semanticsRule(body) },
+      sources: { "src/a.ts": source },
+    })
+  )
+    .stdout.split("\n")
+    .filter((line) => line !== "")
+    .map((line) => JSON.parse(line) as { text: string; message: string });
+
+/** The statement `nthChild` selects at `position`, over three siblings. */
+const nthChildAt = (position: number) =>
+  semanticsFindings(
+    [
+      "  kind: expression_statement",
+      "  nthChild:",
+      `    position: ${String(position)}`,
+      "    ofRule: { pattern: $S }",
+    ].join("\n"),
+    "function f() { a(); b(); c(); }\n"
+  ).map((match) => match.text);
+
+/**
+ * A rule whose `message` is a bare metavariable, so a leaked binding is
+ * visible in the rendered output rather than only in the match.
+ */
+const semanticsRule = (body: string) =>
+  [
+    "id: semantics",
+    "language: TypeScript",
+    "severity: error",
+    "message: $A",
+    "note: n",
+    "rule:",
+    body,
+    "",
+  ].join("\n");
+
 /** A Markdown rule whose `rule:` body is given verbatim, already indented. */
 const markdownRule = (body: string) =>
   [
@@ -972,6 +1012,84 @@ withSg("ast-grep vendor contract", () => {
     });
   });
 
+  /**
+   * Two matching-semantics fixes from the 0.41.0 to 0.45.2 upgrade, pinned
+   * because they are the class no local check catches: a valid, unchanged rule
+   * matching a DIFFERENT set of nodes, with no error and no warning.
+   *
+   * Both cases come from the upstream pull requests rather than from guessing
+   * at the constructs. That distinction mattered: hand-written rules over the
+   * same features showed no difference at all, because neither bug fires
+   * unless a metavariable is bound in the specific position the fix changed.
+   */
+  describe("metavariable bindings do not leak", () => {
+    it("counts nthChild positions from 1, not from 0", () => {
+      // Pinned because the vendored schema says otherwise. `NthChildSimple`
+      // describes the plain-number form as "A number indicating the precise
+      // element index" with `minimum: 0`, which reads as 0-indexed and would
+      // make the test below off by one, selecting `c();` while claiming
+      // `b();`.
+      //
+      // Measured against the binary: position 0 matches nothing at all, and
+      // 1, 2, 3 select the first, second and third. The behaviour is CSS-like
+      // and the schema's own description is misleading, so the ordering the
+      // next test depends on is asserted here rather than assumed.
+      expect(nthChildAt(0)).toEqual([]);
+      expect(nthChildAt(1)).toEqual(["a();"]);
+      expect(nthChildAt(2)).toEqual(["b();"]);
+      expect(nthChildAt(3)).toEqual(["c();"]);
+    });
+
+    it("counts every sibling when nthChild's ofRule binds a metavariable", () => {
+      // ast-grep/ast-grep#2677. `ofRule` reused one environment across all
+      // siblings, so the first match committed `$S` and every later sibling
+      // failed the consistency check and went uncounted. The rule then matched
+      // NOTHING at 0.41.0 while looking correct.
+      //
+      // Measured across the upgrade: 0.41.0 found 0, 0.45.2 finds `b();`.
+      // Swapping `pattern: $S` for a non-binding `kind:` was the workaround,
+      // which is what identified binding as the cause.
+      const matches = semanticsFindings(
+        [
+          "  kind: expression_statement",
+          "  nthChild:",
+          "    position: 2",
+          "    ofRule: { pattern: $S }",
+        ].join("\n"),
+        "function f() { a(); b(); c(); }\n"
+      );
+      expect(matches.map((m) => m.text)).toEqual(["b();"]);
+    });
+
+    it("leaves a metavariable unbound when a negated not rejected it", () => {
+      // ast-grep/ast-grep#2676. A `not` must contribute no bindings, since a
+      // successful negation means the inner rule did NOT match. It passed the
+      // live environment to its inner matcher, so a failed candidate left its
+      // bindings behind, and relational rules reuse one environment across
+      // candidates.
+      //
+      // The subtle part, and the reason this is asserted on the MESSAGE: the
+      // finding itself is identical across the upgrade. Same file, same range,
+      // same rule. Only the binding differs, so a differential comparing
+      // locations would report no change. At 0.41.0 `$A` rendered as `foo`,
+      // leaked from `return foo;`; here it must be empty.
+      const matches = semanticsFindings(
+        [
+          "  kind: expression_statement",
+          "  pattern: target;",
+          "  follows:",
+          "    not:",
+          "      pattern: return $A",
+          "    stopBy: end",
+        ].join("\n"),
+        "function f() { bar(); return foo; target; }\n"
+      );
+      expect(matches).toHaveLength(1);
+      expect(matches[0]?.text).toBe("target;");
+      expect(matches[0]?.message.trim()).toBe("");
+    });
+  });
+
   describe("the `sg` alias prints a deprecation banner on stderr", () => {
     // DEPRECATED AT 0.45.0. `AST_GREP_BINARY.binaryNames` puts `ast-grep`
     // first, but the resolver reverses that list at its link-based tiers, so