fix(ci): gate Windows guard setups on the index response, not list_projects stats - #1958
Conversation
…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>
|
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. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
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. |
This comment has been minimized.
This comment has been minimized.
|
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 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 What makes this the right fix rather than a workaround, in this project's terms:
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. |
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. |
Summary
The Windows guards intermittently red the
test-windows-guardsjob withSETUP FAIL: ASCII baseline did not index ... 'nodes': None(and, in one whole-harness episode, the sibling signaturesindex 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_counttakes node/edge counts exclusively fromlist_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 whileindex_repository's own response reports"nodes":12,"edges":21and definition queries against the same session return 5 definitions. The guard then declarednodes: 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 theindex_repositoryresponse summary — the synchronous, authoritative source.list_projectsremains only as a polled fallback (wait_projects_with_statsinmcp_stdio.py, 90 s bound) for payloads without counts, and the rich no-project diagnostics move unchanged into ano_project_errorhelper.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
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.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.