fix: scope GraphQL variable numbering to one compilation

What does this MR do and why?

GraphQL variable names came from a process-global counter that every compile cleared:

lazy_static! {
    static ref COUNTERS: Mutex<HashMap<String, usize>> = Mutex::new(HashMap::new());
}

pub fn reset_unique_id_counters() { COUNTERS.lock().unwrap().clear(); }

The mutex made each increment safe, but the counter's lifetime was wrong: it was shared across compilations that should be independent, and the reset is how one stomps another. Two compilations running at once in a process interleave — one call's reset lands between another's allocations and restarts its numbering mid-document:

query GLQL($epicId4: WorkItemID!, $epicId1: WorkItemID!, $epicId1: WorkItemID!, $epicId1: WorkItemID!, ...) {
  workItems(parentIds: [$epicId4, $epicId1, $epicId1, $epicId1], ...)

That is either an invalid document (UniqueVariableNames) or, if it validated, a multi-value epic / parent filter bound to the wrong ids.

What this does and does not affect

Today's consumers are insulated, and the issue's original framing overstated the reach — worth correcting here so the record is right:

• Ruby extension: compile is exposed through a plain magnus function! (glql_rb/ext/gitlab_query_language/src/lib.rs) which holds the GVL throughout and never releases it, so two Puma threads in one process cannot run glql::compile concurrently.
• Puma workers are separate processes with separate globals.
• WASM is single-threaded.

What it did break is our own corpus check. DUMP_GRAPHQL=1 cargo test && npm run test:graphql had to be run with -- --test-threads=1, because the suite's parallel compiles tripped the race and produced two invalid documents. CI never saw it: cargo nextest run is process-per-test.

So this is about removing fragile shared state and restoring a check we had to work around — not about a live incident.

The fix

Context already carries variables, and every caller registers the name it is given before asking for the next one — the state the counter duplicated is already there, scoped to exactly one compilation:

pub fn unique_variable_key(&self, prefix: &str) -> String {
    let highest = self
        .variables
        .iter()
        .filter_map(|v| v.key.strip_prefix(prefix))
        .filter_map(|suffix| suffix.parse::<usize>().ok())
        .max();

    format!("{prefix}{}", highest.map_or(1, |n| n + 1))
}

That removes the global, the reset, and the ordering hazard between them. lazy_static existed only for that counter and is dropped from the dependencies.

Numbering per prefix is preserved — epicId and parentId still count independently, and unrelated variables (before, after, limit) are ignored. max + 1 rather than a count, so gaps cannot collide; checked against a 60-reference filter, which yields epicId1 … epicId60, all distinct.

Two notes for review

This removes two public functions. utils is pub mod, so glql::utils::common::unique_id and reset_unique_id_counters were part of the crate's API surface. Nothing in this repo calls them, and the Ruby extension only uses compile, transform and glql_schema — but it pins glql by git tag (v0.38.0), so the next tag bump is where any external consumer would meet the removal.

Reusing one Context across two compile_glql calls now continues the numbering instead of restarting at 1, since the reset is gone. That is the safer direction — continuing cannot collide, restarting could — and no shipped path does it: compile builds a fresh context per call.

How this is verified

The regression tests compile the multi-value epic / parent shapes on 16 threads, 25 rounds each, and assert no document declares a name twice and that concurrent compiles agree with a serial one. Checked in both directions by reverting only the source change:

	test_variable_names_are_unique…	test_concurrent_compiles_are_deterministic
Without the fix	fails 5/5 runs	fails 5/5 runs
With the fix	passes	passes

They loop rather than compile once each: a single round can get lucky, and a regression test that passes on the broken code is worthless.

The corpus check now runs clean in parallel, without the workaround:

corpus: 1653 documents (default parallel run, no --test-threads=1)
[success] All documents are valid

How to set up and validate locally

1. Check out and build:

   git checkout bug/217-unique-id-race
   cargo build

2. Run the regression tests:

   cargo test --test concurrent_compile_tests
   cargo test --lib unique_variable_key

3. Confirm they actually catch the bug — revert only the source and watch them fail:

   git checkout origin/main -- src/ Cargo.toml
   cargo test --test concurrent_compile_tests    # expect: 2 failed
   git checkout HEAD -- src/ Cargo.toml Cargo.lock

4. Whole suite and lints:

   cargo test
   cargo clippy --all-targets -- -D warnings
   cargo fmt --check

5. The corpus check, now without the workaround:

   DUMP_GRAPHQL=1 cargo test && npm run test:graphql

Related

• Closes #217 (closed)
• Surfaced while working on !519 (merged), whose validation steps had to recommend --test-threads=1 because of this race