Skip to content

fix(ci): gate Windows guard setups on the index response, not list_projects stats - #1958

Merged
DeusData merged 1 commit into
DeusData:mainfrom
ilyabrykau-orca:fix/windows-guards-setup-retry
Sep 1, 2026
Merged

fix(ci): gate Windows guard setups on the index response, not list_projects stats#1958
DeusData merged 1 commit into
DeusData:mainfrom
ilyabrykau-orca:fix/windows-guards-setup-retry

Conversation

@ilyabrykau-orca

Copy link
Copy Markdown
Contributor

Summary

The Windows guards intermittently red the test-windows-guards job with SETUP FAIL: ASCII baseline did not index ... 'nodes': None (and, in one whole-harness episode, the sibling signatures index did not run, ASCII control did not index via CLI, permanent daemon did not start). #1952 tracks five such runs across unrelated PRs; every log shows the same shape: the index succeeded, but the harness read its verdict from the wrong place.

Root cause, reproduced deterministically (macOS, main binary): index_and_count takes node/edge counts exclusively from list_projects, but that tool publishes its stats columns asynchronously — and in some environments never within a one-shot MCP session. On the reproducing machine the project row comes back with no stats fields at all while index_repository's own response reports "nodes":12,"edges":21 and definition queries against the same session return 5 definitions. The guard then declared nodes: None → precondition skip → the all-skip gate ("every guard skipped — nothing was actually verified") turned an environmental stats lag into a red job.

Fix

  • test_non_ascii_path.py: take counts from the index_repository response summary — the synchronous, authoritative source. list_projects remains only as a polled fallback (wait_projects_with_stats in mcp_stdio.py, 90 s bound) for payloads without counts, and the rich no-project diagnostics move unchanged into a no_project_error helper.
  • Setup steps now retry once against a fresh cache before declaring a precondition skip: the ASCII baseline, the CLI ASCII control (test_cli_non_ascii_arg.py), the hook-augment index, and its permanent-daemon start (test_hook_augment.py). These are setup, not the surfaces under test; a cold runner's coordination-daemon startup latency should cost one retry, not the job.

No product code is touched; the guard contracts (exit 0/1/2, count-equality invariant, diagnostics-on-failure) are unchanged.

RED / GREEN

  • RED (before): driving the unpatched index_and_count + baseline gate against a real binary on the reproducing machine returns exactly the CI signature: {'name': ..., 'nodes': None, 'edges': None, 'definition_nodes': 5}SETUP FAIL: ASCII baseline did not index.
  • GREEN (after): same machine, same binary: baseline nodes=12 edges=21 definitions=5, and a non-ASCII variant (café_repo) matches the baseline exactly — the guard's invariant holds end-to-end through the patched path.

Example flaking runs (all pass on retrigger or on sibling jobs of the same commit): see the run list in #1952.

Fixes #1952.

…ojects stats

The guards read node/edge counts exclusively from list_projects, but that
tool publishes its stats columns asynchronously — on some venues never
within a one-shot MCP session. index_repository's own response carries the
synchronous, authoritative counts, so a healthy index was misread as
'SETUP FAIL: ASCII baseline did not index ... nodes: None' (issue DeusData#1952
carries five such runs; reproduced deterministically on macOS where
list_projects returns the project row with no stats fields at all while
the index response reports nodes=12/edges=21 and definition queries work).

- test_non_ascii_path: take counts from the index_repository summary;
  keep list_projects only as a polled fallback (wait_projects_with_stats
  in mcp_stdio) and for the no-project diagnostics, now in a helper.
- Retry setup steps once (fresh cache) before declaring a precondition
  skip: the ASCII baseline, the CLI ASCII control (test_cli_non_ascii_arg),
  the hook-augment index and its permanent-daemon start — these are setup,
  not the surfaces under test, and a cold runner's daemon startup latency
  must not red the job through the all-skip gate.

Signed-off-by: Ilya Brykau <ilya.brykau@orca.security>
@github-actions

Copy link
Copy Markdown

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

  • It will not be closed for inactivity. No stale bot touches pull requests here.
  • It may still sit a while before a human reads it. That is on us, not on you.
  • Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

  • Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
  • Get CI green, or say which failures you believe are pre-existing.
  • Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
  • Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

@adfjadfj16-a11y

This comment has been minimized.

@DeusData

Copy link
Copy Markdown
Owner

Reviewed, cleared, and queued to merge as soon as CI settles. Taking #1961 straight after, since it carries this commit.

You were told on #1944 to stop force-pushing and let us own #1952. You went and fixed it instead — and the diagnosis is right.

I checked the claim in production rather than taking the description, and it holds with one refinement worth recording. In list_projects, the counts are emitted only under the detail flag:

if (include_details) {
    yyjson_mut_obj_add_int(doc, p, "nodes", nodes);
    yyjson_mut_obj_add_int(doc, p, "edges", edges);
    yyjson_mut_obj_add_int(doc, p, "size_bytes", size_bytes);
}

So a response without it carries no nodes key whatsoever — which is exactly the 'nodes': None in every one of those logs. Whether a given run misses the counts because the stats lag or because the field was never emitted, your conclusion is the same and it is the correct one: list_projects is not a sound source for this verdict, and index_repository's own response is.

What makes this the right fix rather than a workaround, in this project's terms:

  • You attributed before you touched anything. The index had succeeded; the harness read the verdict from the wrong place. That is a harness defect, not a product race and not a test to retry — and getting that distinction right is the whole ladder here.
  • The replacement is authoritative, not merely luckier. Reading the synchronous response is deterministic by construction. Demoting list_projects to a bounded 90-second polled fallback keeps the old path for payloads that genuinely lack counts without letting it decide anything.
  • RED and GREEN on a real binary, with the exact CI signature reproduced on the reproducing machine and the non-ASCII variant matching the baseline afterwards. Not "passes now".
  • No product code touched, contracts unchanged — exit 0/1/2, the count-equality invariant, diagnostics-on-failure.

On the setup retries: retrying an assertion would be a problem, but these are preconditions — a cold runner's daemon-start latency — and you drew that line explicitly rather than quietly widening a retry budget. That reads correctly.

Thank you. Fixing the infrastructure that was blocking your own PR, after being told it was ours to fix, is considerably more than was asked.

@DeusData
DeusData merged commit 58fb945 into DeusData:main Sep 1, 2026
33 checks passed
@DeusData

DeusData commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Maintainer notice: please disregard comments from @adfjadfj16-a11y on this thread

@adfjadfj16-a11y is not a maintainer of this project and does not speak for it. That account has posted replies on 17 threads here written in the project's voice — promising merges, announcing that a case has been "escalated to the development team", asking to close issues, and in some threads replying as though it were the author of someone else's pull request. None of those were maintainer decisions, and none of them carried any weight.

@DeusData is the only account that gives a maintainer response on this repository. If a comment about the fate of your issue or pull request did not come from @DeusData, it is not a decision, however official it reads.

If you were waiting on something because of one of those comments — a promised merge, a review "immediately", a request to close your ticket — I am sorry. That was noise you had no way to identify as noise, and it should not have been on your thread. Your issue or PR is judged on its own merits, and I will answer it here myself.

Nothing in this notice reflects on your contribution. Thank you for your patience, and thank you for the work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ci: test-windows-guards intermittently dies in setup — ASCII baseline did not index, nodes:None

3 participants