[aw] Make CI KBE issue search deterministic

[vitek-karas]

Summary

• Route CI failure scanner issue searches through a repository-scoped MCP script that returns only issue numbers and author logins.
• Require every returned candidate to be inspected with issue_read before making semantic duplicate decisions.
• Remove direct built-in issue-search access and strengthen the scanner eval to enforce the wrapper path and fail-closed behavior.

Dependency

This PR depends on dotnet/runtime#133958, which must merge first. The /ci-eval workflow restores evaluator inputs from main before checking out the PR head; without the prerequisite files already present on main, the wrapper, grader, and focused tests are removed during restoration and this PR's evaluator changes are not exercised.

Motivation

The built-in issue search result projection can omit author metadata. When that happens, existing bot-authored KBEs can be hidden from duplicate detection even though prompt guidance requests the author field. This makes the transport deterministic while leaving non-exact query formulation and semantic comparison to the model.

This is narrower than #132619 for issue lookup: pull request searches continue to use the GitHub MCP tools, while issue searches use the dedicated wrapper.

Scope limitation

The eval's direct-search protection is pattern-based and cannot recognize every possible shell spelling or equivalent command. This is an existing limitation of the eval setup and is out of scope for this PR; the workflow's network policy and mandatory wrapper checks remain the enforcement mechanisms for this change.

Validation

• Compiled ci-failure-scan with gh-aw v0.86.2 and actionlint enabled.
• Passed Vally 0.14 strict lint for ci-failure-scan.eval.yaml.
• Passed actionlint v1.7.12 for ci-eval.yml.
• Exercised the wrapper against live KBE searches and confirmed bot-authored candidates include their author logins.
• Passed the focused Node test suite with 10 tests.

Note

This pull request description was generated by GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).
15 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

🟡 Changes recommended

Unresolved issues affect fail-closed search behavior, wrapper enforcement, candidate verification, and workflow policy consistency.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Updates CI KBE issue searches to use a repository-scoped wrapper and require candidate inspection before duplicate decisions.

Changes:

• Adds deterministic issue-search validation.
• Updates prompts, MCP configuration, and evaluation checks.
• Regenerates the compiled workflow.

File summaries

File	Reviewed changes
.github/workflows/shared/create-kbe.instructions.md	Updates candidate inspection and fail-closed guidance.
.github/workflows/evals/search-kbe-issues.cjs	Adds the scoped issue-search wrapper.
.github/workflows/evals/README.md	Documents updated eval transport usage.
.github/workflows/evals/ci-failure-scan.eval.yaml	Enforces wrapper and candidate-read usage.
.github/workflows/ci-failure-scan.md	Configures production tools and the search wrapper.
.github/workflows/ci-failure-scan.lock.yml	Regenerates the compiled workflow artifact.
.github/workflows/ci-eval.yml	Preserves trusted evaluation inputs.

Review details

Suppressed comments (4)

.github/workflows/ci-failure-scan.lock.yml:1425

• This generated-only change switches agent-failure issue expiry from 0 to 168 hours, while the other agentic workflow lock files still use 0 and no source setting in this PR requests the behavior change. That can hide an unresolved workflow failure after seven days; please restore the established value or add an explicit source-level setting and document the intended policy.

          GH_AW_ACTION_FAILURE_ISSUE_EXPIRES_HOURS: "168"

.github/workflows/evals/ci-failure-scan.eval.yaml:124

• The deterministic tool-calls grader only requires one matching issue_read; it does not relate the number of reads to the candidates returned by the two wrapper calls. A run can read one issue, skip other candidate numbers (or fail a read), and still satisfy this grader; the prompt judge is not a deterministic fail-closed check. Add a post-run verifier or stateful harness that requires a successful read for every returned number before accepting output.

            - name: 'issue_read'
              args:
                owner: '^dotnet$'
                repo: '^runtime$'
                method: '^get$'

.github/workflows/evals/search-kbe-issues.cjs:3

• This adds a second implementation of the wrapper: the eval executes search-kbe-issues.cjs, while production executes the separate inline mcp-scripts block in ci-failure-scan.md. A future scope or validation change in only one copy can make the eval pass while production behaves differently. Factor the fetch/validation logic into one checked-in module and have both entry points call it, or generate the eval harness from the same source.

async function searchKbeIssues(query, token, fetchImpl = fetch) {

.github/workflows/shared/create-kbe.instructions.md:196

• These searches now require reading every candidate, but they do not define what happens when a read fails or returns [Filtered]. The fail-closed rule above is scoped to KBE-oriented searches (and variation 5), so an unreadable plain tracker can be treated as a miss and allow a duplicate KBE. Add an explicit stop/skipped: integrity-filtered candidate, needs human review rule here.

Search for a plain tracker, then read every returned candidate:

• Files reviewed: 7/7 changed files
• Comments generated: 4
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Critical grader integration and candidate-read enforcement issues, plus moderate workflow and parity issues, remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (5)

.github/workflows/ci-eval.yml:351

• The eval invocation repeats the same incorrect plugin path: this step's working directory is the repository root, while the trusted grader is restored under .github/workflows/evals/. As a result the scanner eval cannot load kbe-candidate-reads; use the restored path here as well.

            grader_args+=(--grader-plugin ./kbe-candidate-reads-grader.mjs)

.github/workflows/ci-failure-scan.lock.yml:663

• The quoted heredoc writes ${RUNNER_TEMP} literally into tools.json (and the generated server below), rather than expanding it. The server's log path therefore does not match the actual ${RUNNER_TEMP}/gh-aw/mcp-scripts/logs directory consumed by the parse/upload steps, so MCP-script diagnostics will be lost; generate this path from process.env.RUNNER_TEMP or otherwise expand it before regenerating the lock.

            "logDir": "${RUNNER_TEMP}/gh-aw/mcp-scripts/logs",

.github/workflows/ci-failure-scan.md:77

• Appending repo:dotnet/runtime is:issue does not enforce the wrapper's scope: a caller can supply a conflicting or negated qualifier such as -repo:dotnet/runtime or is:pr, producing an empty result while the wrapper reports success. The scanner could then treat an existing KBE as absent and file a duplicate. Reject or normalize conflicting caller qualifiers before invoking gh, and keep the same guard in the eval copy and generated lock implementation.

        `q=${query.trim()} repo:dotnet/runtime is:issue`,

.github/workflows/ci-failure-scan.md:93

• The parity test only checks that this extracted script mentions execFile, search/issues, and GITHUB_TOKEN; it never executes or asserts the incomplete_results and candidate-validation branches here. A future drift in the production wrapper could therefore reintroduce fail-open or malformed-candidate behavior while the trusted tests still pass. Add behavior-level parity coverage for these checks, or share one implementation.

      if (result.incomplete_results !== false || !Array.isArray(result.items)) {
        throw new Error("GitHub issue search returned an invalid response");
      }
      return result.items.map((item) => {

.github/workflows/evals/kbe-search.test.mjs:42

• This parity test only checks three text fragments in the extracted production script. It does not verify the fail-closed incomplete_results check, candidate validation, or the bounded query, so the inline workflow implementation could diverge from search-kbe-issues.cjs while the test remains green. Add behavior-level parity assertions or execute shared code so this gate covers the security-critical behavior it claims to keep in sync.

test("production wrapper uses authenticated gh api transport", async () => {
    const script = await productionScript();
    assert.match(script, /execFile\)\("gh"/);
    assert.match(script, /"api",\s*"search\/issues"/);
    assert.match(script, /GITHUB_TOKEN/);
});

• Files reviewed: 10/10 changed files
• Comments generated: 3
• Review effort level: Lite

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).
15 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

🟡 Changes recommended

The critical eval bootstrap issue and additional grader/test gaps must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (4)

.github/workflows/evals/kbe-candidate-reads-grader.mjs:130

• MCP results can carry { isError: true } while the event's success flag remains true; the existing fixer grader explicitly rejects that shape at ci-failure-fix.eval.yaml:183-190. This check only looks for [Filtered], so it records an error response as a valid read and can let the eval pass without inspecting the candidate. Require strict success and reject result.isError here, with a regression test.

                } else if (call.validScope && Number.isInteger(call.number) &&
                    event.data.success && !resultText(event.data.result).includes("[Filtered]")) {

.github/workflows/evals/kbe-candidate-reads-grader.mjs:43

• parseCandidates scans every line and selects the last line that happens to be a JSON array rather than proving that it is the wrapper's stdout. A command such as node ... query; echo [] (or a failed wrapper followed by an empty array) satisfies isSearchHarnessCall, hides the real candidates, and passes with no issue_read calls. Parse the exact process output/exit status or reject compound/ambiguous transcripts, and add a regression test.

function parseCandidates(result) {
    const text = resultText(result).trim();
    const payloads = [text, ...text.split(/\r?\n/).reverse()];
    let parsed;

.github/workflows/evals/kbe-candidate-reads-grader.mjs:38

• The repository's existing eval fixtures record GitHub MCP calls as github-issue_read (evals/test_ci_failure_fix_candidates.py:211-217), and the fixer grader accepts (?:mcp__)?github[-_.]+issue_read. This predicate only recognizes bare issue_read or names ending in .issue_read, so a compliant trajectory using the established name is ignored and the scanner eval fails despite successful reads. Match the namespaced forms and guard non-string tool names.

function isIssueReadCall(event) {
    return event.type === "tool_call" &&
        (event.data.toolName === "issue_read" || event.data.toolName.endsWith(".issue_read"));
}

.github/workflows/evals/kbe-search.test.mjs:42

• This production-parity test only checks that the inline script contains execFile, the endpoint, and a token name; it never verifies the newly added fail-closed/metadata invariants (incomplete_results === false, repository/PR filtering, and nonempty user.login). Because the production and eval wrappers are duplicated, those paths can drift while this test still passes; execute a shared implementation or assert each invariant in the production path.

test("production wrapper uses authenticated gh api transport", async () => {
    const script = await productionScript();
    assert.match(script, /execFile\)\("gh"/);
    assert.match(script, /"api",\s*"search\/issues"/);
    assert.match(script, /GITHUB_TOKEN/);
});

• Files reviewed: 10/10 changed files
• Comments generated: 1
• Review effort level: Lite

[PureWeen]

Holistic Review

Motivation: Deterministic REST issue search addresses the semantic-search mismatch in KBE lookup.

Approach: The production search wiring looks coherent, but the eval integration has a reproduced plugin-loading failure.

Summary: Needs Changes at 78f5dae0. Please fix the plugin path below. The already-reported canonical github-issue_read name mismatch also remains; I am not filing it again as a new finding.

Detailed Findings

❌ Error — Grader plugin path is relative to the spec, not the repository

In .github/workflows/ci-eval.yml:324,360, --grader-plugin ./.github/workflows/evals/kbe-candidate-reads-grader.mjs is resolved by pinned Vally 0.14 relative to the eval-spec directory. This produces a duplicated .github/workflows/evals/.github/workflows/evals/ path and fails module import. The exact committed lint invocation reproduced exit 1; changing the argument to ./kbe-candidate-reads-grader.mjs passed strict lint for all three specs.

Use that spec-relative path or an absolute path in both invocations. This corrects the earlier advice in comment 3993276213: the full repository-relative path requested there is not correct for this pinned Vally resolver. Because the eval harness is loaded from the trusted base, this affects the eval variants once this harness change lands; it is not a claim that current PR checks have already exercised it.

The checked-in Node tests and mocked production-handler cases pass, but they do not cover this CLI integration failure. No full agent eval or Actions run was performed; CI status and merge readiness were not assessed.

Note

This review was generated by GitHub Copilot.

[vitek-karas]

Addressed in d16a45e2841:

• Both Vally invocations now use the spec-relative grader path ./kbe-candidate-reads-grader.mjs.
• The candidate-read grader now recognizes the canonical github-issue_read tool name, with regression coverage.
• Focused Node tests pass, and strict Vally lint passes for all three eval specs.

PureWeen’s holistic review is a top-level review rather than an inline thread, so there was no separate thread to resolve.

Note

This response was generated by GitHub Copilot.

[Copilot]

🟡 Changes recommended

Unresolved moderate findings remain in workflow path alignment, production/evaluation parity, and fail-closed grader validation.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (6)

.github/workflows/ci-eval.yml:360

• This step has the same root-relative path problem: the trusted grader is restored under .github/workflows/evals/, but this argument looks for it at the repository root. As a result the actual eval cannot load kbe-candidate-reads and the new candidate-read enforcement is not applied. Pass the restored path here as well.

            grader_args+=(--grader-plugin ./kbe-candidate-reads-grader.mjs)

.github/workflows/ci-failure-scan.md:65

• This frontmatter embeds a second implementation of the search wrapper, but the focused tests execute only the standalone helper and only shallowly inspect the production script. A later edit could make the actual MCP script accept incomplete, cross-repository, or PR results while all support tests still pass. Add an executable parity test for the extracted frontmatter script, or generate/use one implementation so the production path cannot drift.

[!NOTE]
This review comment was created by GitHub Copilot.

    script: |
      if (typeof query !== "string" || !query.trim()) {
        throw new Error("query must be a non-empty string");
      }
      const token = process.env.GITHUB_TOKEN;

.github/workflows/evals/ci-failure-scan.eval.yaml:53

• Once #133958 supplies the trusted candidate-read grader, this command form will not match it: the trusted helper check requires node followed by the exact helper path (or KBE_SEARCH_HELPER), while this prompt requires the relative .github/... path. The eval step does not set KBE_SEARCH_HELPER, so the two searches will be ignored and the grader will report that the harness was never called. Set the trusted-helper environment/path consistently in the eval setup and prompt, or make the grader resolve this relative path.

[!NOTE]
This review comment was created by GitHub Copilot.

      3. Before filing, check whether an equivalent KBE already exists so you would not duplicate it. The eval runner does not launch workflow-frontmatter MCP servers, so invoke its CLI harness for the workflow's `search-kbe-issues` wrapper with `node .github/workflows/evals/search-kbe-issues.cjs '<query>'` for `in:title "[ci-scan]"` and for your signature text. For every candidate returned by either query, call the GitHub `issue_read` tool with `owner: dotnet`, `repo: runtime`, and `method: get` before deciding whether it matches. Do not use any other shell issue search or the built-in `search_issues` tool. Assume you would file only if every readable candidate is unrelated; fail closed if a candidate cannot be read.

.github/workflows/evals/ci-failure-scan.eval.yaml:119

• min_count: 2 only proves that the wrapper path appeared in two shell commands; it does not require one query to be in:title "[ci-scan]" and the other to contain the extracted signature. The custom grader also ignores the command text, so an agent can run the same empty query twice, return [], and satisfy both this tool-call check and the candidate-read grader without performing the required signature search. Have the trusted grader capture/validate the two query variants (or add equivalent deterministic checks).

              command: '\.github/workflows/evals/search-kbe-issues\.cjs'
              min_count: 2

.github/workflows/evals/kbe-candidate-reads-grader.mjs:120

• If the harness tool call has no matching tool_result (for example because the trajectory is truncated or the tool times out), this code records no error. With no parsed candidates, harnessCallCount is still nonzero and missing is empty, so the grader passes despite not knowing whether the search succeeded. Track unmatched search calls and fail closed when any lacks a result.

                if (call.kind === "search") {
                    if (!event.data.success) {
                        errors.push("search-kbe-issues call failed");
                        continue;
                    }

.github/workflows/evals/kbe-candidate-reads-grader.mjs:133

• The existing eval grader accepts a tool result such as {isError: true} even when the event's success flag is true, but this condition rejects only [Filtered]. Such a failed issue_read would therefore count as a successful read and allow the fail-closed check to pass. Reject structured error results too and add a regression case.

                } else if (call.validScope && Number.isInteger(call.number) &&
                    event.data.success && !resultText(event.data.result).includes("[Filtered]")) {
                    reads.set(call.number, index);

• Files reviewed: 10/10 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Blocking grader validation and fail-closed enforcement gaps remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 10/10 changed files
• Comments generated: 3
• Review effort level: Lite

[PureWeen]

Holistic Review

Motivation: Deterministic KBE search and candidate-read enforcement are worthwhile, and the two issues from my previous review are fixed.

Approach: The dependent change has not yet incorporated the current trusted-support layout from prerequisite #133958, so the stacked result is not executable as configured.

Summary: Needs Changes at 745c27f7. The plugin path and canonical github-issue_read findings are resolved, but the dependency integration below remains merge-blocking.

Detailed Findings

❌ Error — Reconcile the dependent spec with the prerequisite trusted-helper layout

Current #133958 removes the checkout-relative helper/grader/tests and restores trusted copies under $RUNNER_TEMP. This PR still stages .github/workflows/evals/search-kbe-issues.cjs, instructs the agent to invoke that relative path, and requires that path in the tool rule. Recreating the combined layout makes strict Vally lint fail before agent startup because the staged source no longer exists; using the prerequisite grader also rejects the dependent relative invocation as search-kbe-issues was not called.

The dependent test module has the same integration issue: it reads ../ci-failure-scan.md, but the prerequisite copies the suite to an isolated trusted directory without that parent file. In that exact layout, the suite reports 9 passes and 5 ENOENT failures.

Please incorporate the current prerequisite revision and update the spec source, prompt, required command, grader path, and test inputs together. Keep the trusted test suite self-contained or explicitly stage the production workflow used by the parity tests, then validate the restored combined layout—not only this branch checkout.

This is ordinary startup integration, not an adversarial tampering concern. Local branch tests pass because the old support files and parent workflow are still present there. No full model eval or Actions run was performed; CI status and merge readiness were not assessed.

Note

This review was generated by GitHub Copilot.

[vitek-karas]

Responding to the trusted-support layout concern in the latest review: addressed in e09e4539953 after merging the latest main, including prerequisite #133958.

The trusted evaluator now keeps the helper, grader, and focused tests in the trusted temporary layout, exports KBE_SEARCH_HELPER, uses the trusted grader path, and stages ci-failure-scan.md where the parity test expects it. The scanner eval invokes the trusted helper through node "$KBE_SEARCH_HELPER", so the restored evaluator support and the dependent workflow are exercised together.

Validation on the merged branch:

• Focused Node tests: 14/14 passed.
• Strict Vally lint: all three eval specifications passed.
• The no-new-KBE result Result: No new Known Build Error passed all six static output graders.

The eval also now accepts a valid live run with no actionable new KBE, while still requiring the build fetch, both deterministic searches, and successful candidate reads.

Note

This response was generated by GitHub Copilot.

[PureWeen]

Both integration issues from my previous review are fixed at e09e4539. The spec, prompt, and grader now use the trusted KBE_SEARCH_HELPER path, and the tests find ci-failure-scan.md in the isolated layout. Two small regressions in the eval graders remain:

1. ci-failure-scan.eval.yaml (no-op alternatives, ~L75-100): each pattern uses (?ims) with ^\s*Result:\s*No new Known Build Error\s*$. In multiline mode, ^...$ matches any line, so a malformed KBE artifact that contains that line anywhere, including inside a fenced block, passes all six deterministic format checks. With the installed file-matches grader: an invalid title and labels without the marker passes 0/6, and the same artifact with the marker line inserted passes 6/6. Could the no-op branch require the whole file to be exactly the marker (for example \A\s*Result:\s*No new Known Build Error\s*\z in a non-multiline alternative), while the KBE branch keeps its multiline matching? Mixed-content and fenced-marker negative cases would lock this in.

2. kbe-candidate-reads-grader.mjs commandBoundary (~L108): (?:^|(?:&&|\|\||[;\n])\s*) has no whitespace allowance after ^. A real call such as node "$KBE_SEARCH_HELPER" query fails with "search-kbe-issues was not called", even though the grader on main accepted it (^\s*node…). Could you restore ^\s* and add a positive test for an indented call?

This is separate from the deferred command-suffix discussion. I reproduced both issues with the actual graders; I am not claiming the full eval or LLM judge would accept malformed output.

Note

This review was generated by GitHub Copilot.

[vitek-karas]

Addressed both findings in commit 4c221230b1c:

1. The six no-op output alternatives now use absolute start/end assertions, so Result: No new Known Build Error is accepted only as the complete file. Added regression coverage proving an embedded or fenced marker in malformed output is rejected.
2. The candidate-read grader now permits leading whitespace before the trusted helper invocation, with a regression test for an indented command.

Validation passed:

• 16 focused Node tests
• Strict Vally lint for all three eval specs
• git diff --check

Note

This response was generated by GitHub Copilot.