fix(cypher): refuse a query the parser did not fully read

cbm_parse built a query and returned success without checking that it
had read every token. The grammar accepts at most one WITH and treats
RETURN as optional, so the parser stopped at the first thing it did not
understand and reported success anyway.

The dropped tail took the filter and the RETURN with it. The engine then
answered from the fragment it had parsed, using its default projection.
It reported success and returned wrong rows, which is worse than a
refusal, because nothing tells the caller to look.

Two shapes hit this:

  MATCH (f:Function) WHERE f.name = 'x' RETURN f.name AS n BANANA SPLIT 99
  -> one row, no error, the trailing words silently dropped

  MATCH (f:Function)
  OPTIONAL MATCH (a)-[:CALLS]->(f)
  WITH f, count(a) AS calls
  OPTIONAL MATCH (b)-[:USAGE]->(f)
  WITH f, calls, count(b) AS usages
  WHERE calls = 0 AND usages = 0
  RETURN f.name AS n
  -> every function, unfiltered, under the default column names

cbm_parse now checks that the cursor sits on the end-of-input token
before it returns success, and names the leftover token when it does not.

The message adds a note about the one-WITH limit, but only when a
standalone WITH really sits in the part that went unread. Appending it
every time pointed the reader at WITH when the problem was a typo -- the
BANANA query above holds no WITH anywhere. The test cannot simply look at
the token the parse stopped on either: on the second-WITH query above it
stops at OPTIONAL rather than at WITH, because parse_post_where consumes
the first WITH and then has no way to take another MATCH stage. So it
scans the unread tail, and skips a WITH straight after STARTS, which is
the STARTS WITH operator rather than a clause.

The check exposed a second defect. parse_post_where parses the branch
after UNION by calling cbm_parse on a slice of the same tokens, using a
separate parser. The outer parser's cursor never moved past the UNION
keyword, so a valid UNION query looked unfinished. That cursor was
already wrong; nothing caught it, because nothing checked where the
cursor ended up. The UNION branch now moves the cursor to the end after
the sub-parse succeeds, which is sound because that sub-parse no longer
returns success with tokens left over.

Three tests cover both directions. Removing only the guard turns the two
rejection tests red with rc == 0 -- the parser reporting success on input
it never finished reading -- while the acceptance test stays green, so
the guard is load-bearing for exactly these two behaviours.

The two rejection tests also pin the message. The BANANA case must name
BANANA and must not mention WITH; that assertion was seen red first, at
tests/test_cypher.c:226. The second-WITH case must still carry the note;
that one passed before and after, and is the control that stops the note
being suppressed everywhere instead of only where it misleads.

Verified on macOS with Apple clang:
  make -f Makefile.cbm test-focused TEST_SUITES=cypher  -> 186 passed
  guard removed, tests kept                             -> 184 passed, 2 failed
  make -f Makefile.cbm cbm                              -> exit 0, no warnings

The full suite reports 7623 passed, 2 failed. Both failures are in
tests/test_cli.c (lines 1748 and 6723) and reproduce on a clean tree with
this change stashed out. They depend on the coding agents installed on
the machine, not on this change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>

Author: CaptainMittens

src/cypher/cypher.c

@@ -1992,6 +1992,13 @@ static int parse_post_where(parser_t *p, cbm_query_t *q, // NOLINT(misc-no-recur
         q->union_next = sub.query;
         sub.query = NULL;
         cbm_parse_free(&sub);
+        /* The branch after UNION was parsed by a SEPARATE parser over a slice
+         * of these tokens, so this parser's cursor never moved past the UNION
+         * keyword. That sub-parse now refuses to succeed with anything left
+         * over, so everything from here to the end is accounted for. Move the
+         * cursor to the end to say so, or cbm_parse's end-of-input check reads
+         * a fully parsed UNION query as unfinished. */
+        p->pos = p->count;
     }
     return 0;
 }
@@ -2055,6 +2062,36 @@ int cbm_parse(const cbm_token_t *tokens, int token_count, // NOLINT(misc-no-recu
         return CBM_NOT_FOUND;
     }
 
+    /* Every token must be consumed. The grammar accepts at most one WITH and
+     * treats RETURN as optional, so a query with a second WITH stage — or any
+     * typo after RETURN — used to stop parsing there and succeed anyway. The
+     * dropped tail took the filter and the RETURN with it, and the engine
+     * answered from the fragment it had parsed, using its default projection.
+     * That reported success and returned wrong rows, which is worse than a
+     * refusal because nothing tells the caller to look. Refuse instead. */
+    if (peek(&p)->type != TOK_EOF) {
+        /* Only mention the one-WITH limit when a standalone WITH really is
+         * sitting in the part we could not read. Saying it every time points
+         * a reader at WITH when the problem is a typo. A WITH straight after
+         * STARTS is the STARTS WITH operator rather than a clause, the same
+         * guard parse_post_where uses. */
+        const char *hint = "";
+        for (int i = p.pos; i < p.count; i++) {
+            if (p.tokens[i].type == TOK_WITH &&
+                (i == 0 || p.tokens[i - SKIP_ONE].type != TOK_STARTS)) {
+                hint = " Note that only one WITH clause is supported.";
+                break;
+            }
+        }
+        snprintf(p.error, sizeof(p.error),
+                 "unexpected input at pos %d ('%s') — the query was not fully "
+                 "parsed.%s",
+                 peek(&p)->pos, peek(&p)->text ? peek(&p)->text : "", hint);
+        out->error = heap_strdup(p.error);
+        cbm_query_free(q);
+        return CBM_NOT_FOUND;
+    }
+
     out->query = q;
     return 0;
 }

tests/test_cypher.c

@@ -209,6 +209,71 @@ TEST(cypher_parse_simple_node) {
     PASS();
 }
 
+/* Trailing input must be an error, never a silent drop. The parser used to
+ * stop at the first thing it did not understand and report success, so the
+ * engine answered from the fragment it had parsed. */
+TEST(cypher_parse_rejects_trailing_tokens) {
+    cbm_query_t *q = NULL;
+    char *err = NULL;
+    int rc = cbm_cypher_parse("MATCH (f:Function) RETURN f.name AS n BANANA SPLIT 99", &q, &err);
+    ASSERT_NEQ(rc, 0);
+    ASSERT_NOT_NULL(err);
+    ASSERT_NULL(q);
+
+    /* The message must name what actually stopped the parse, and must not
+     * mention WITH: this query has no WITH in it anywhere. */
+    ASSERT(strstr(err, "BANANA") != NULL);
+    ASSERT(strstr(err, "WITH") == NULL);
+
+    free(err);
+    PASS();
+}
+
+/* Only one WITH is supported. A second one used to take the rest of the
+ * query with it — the filter and the RETURN both vanished, and every row
+ * came back unfiltered under the default projection. */
+TEST(cypher_parse_rejects_second_with_clause) {
+    cbm_query_t *q = NULL;
+    char *err = NULL;
+    int rc = cbm_cypher_parse("MATCH (f:Function) "
+                              "OPTIONAL MATCH (a)-[:CALLS]->(f) "
+                              "WITH f, count(a) AS calls "
+                              "OPTIONAL MATCH (b)-[:USAGE]->(f) "
+                              "WITH f, calls, count(b) AS usages "
+                              "WHERE calls = 0 AND usages = 0 "
+                              "RETURN f.name AS n",
+                              &q, &err);
+    ASSERT_NEQ(rc, 0);
+    ASSERT_NOT_NULL(err);
+    ASSERT_NULL(q);
+
+    /* Here the note earns its place. The parse stops at OPTIONAL, and the
+     * reason is the second WITH further along, which the reader cannot see
+     * from the stopping point alone. */
+    ASSERT(strstr(err, "only one WITH clause is supported") != NULL);
+
+    free(err);
+    PASS();
+}
+
+/* The guard must not reject a query that is simply finished. One WITH, a
+ * WHERE after it and a RETURN is the shape the grammar does support. */
+TEST(cypher_parse_accepts_single_with_clause) {
+    cbm_query_t *q = NULL;
+    char *err = NULL;
+    int rc = cbm_cypher_parse("MATCH (f:Function) "
+                              "WITH f, f.name AS n "
+                              "WHERE n = 'buildTree' "
+                              "RETURN n",
+                              &q, &err);
+    ASSERT_EQ(rc, 0);
+    ASSERT_NULL(err);
+    ASSERT_NOT_NULL(q);
+
+    cbm_query_free(q);
+    PASS();
+}
+
 TEST(cypher_parse_relationship_outbound) {
     cbm_query_t *q = NULL;
     char *err = NULL;
@@ -4044,6 +4109,9 @@ SUITE(cypher) {
     RUN_TEST(cypher_lex_full_query);
     /* Parser */
     RUN_TEST(cypher_parse_simple_node);
+    RUN_TEST(cypher_parse_rejects_trailing_tokens);
+    RUN_TEST(cypher_parse_rejects_second_with_clause);
+    RUN_TEST(cypher_parse_accepts_single_with_clause);
     RUN_TEST(cypher_parse_relationship_outbound);
     RUN_TEST(cypher_parse_relationship_inbound);
     RUN_TEST(cypher_parse_relationship_any);