Fix empty configured_model for routed and fallback Copilot CLI runs, and isolate selection-persistence errors in the post-step

[Copilot]

Routed Copilot CLI runs, and runs where only the environment fallback supplies the model, wrote a model-selection log with configured_model: "". The harness captured COPILOT_MODEL before routing or fallback had set it. On the runner, persistCopilotModelSelection correctly rejects that record, but it threw inside the same try as routing attribution and token accounting. That skipped model_routing, the token-usage summary, agent_usage.json and the outputs, which caused the R1 smoke failures. SDK mode never creates this record, so it was unaffected.

Harness (copilot_harness.cjs)

• The configured model passed to createCopilotModelObserver is now set after routing and fallback are known:

  Run	configured_model	resolved_model / effective_model
  Routed	auto	router-selected wire model / observed
  Fallback only (COPILOT_MODEL unset)	GH_AW_MODEL_FALLBACK value	after alias resolution / observed
  Explicit model: or alias	unchanged (name before alias resolution)	unchanged

• COPILOT_MODEL is not re-read after resolution, so the requested and resolved names stay separate. applyCopilotWireAPI, endpoint-mismatch records and SDK mode are untouched.

Post-step (parse_token_usage.cjs)

• persistCopilotModelSelection now has its own error boundary. Routing attribution, the token summary and aggregate, outputs, and persistAgentAwInfoCopy always run.
• The original error is still raised as ERR_PARSE: Cannot record Copilot model selection: … through core.setFailed, after that work is done. The step is continue-on-error, so this does not fail the job.
• Invalid selection data is never written. Validation is not loosened, and nothing new is written inside the sandbox.

let selectionError = "";
try { persistCopilotModelSelection(); } catch (error) { selectionError = getErrorMessage(error); }
try { routedAttribution = recordModelRoutingFromArtifacts(); /* usage accounting… */ }
catch (error) { core.setFailed(`${ERR_PARSE}: ${getErrorMessage(error)}`); }
finally {
  persistAgentAwInfoCopy();
  if (selectionError) core.setFailed(`${ERR_PARSE}: ${selectionError}`);
}

Fixtures (actions/setup/js/fixtures/copilot-routed-run-39147795806/)

• Taken from the agent artifact of run 39147795806: copilot-model-selection.json (with the real empty configured_model), aw_info.json, awf-routing-outcome.json, model-routing.jsonl and token-usage.jsonl. JSON files are Prettier-formatted only.
• awf-reflect.json is trimmed to the copilot endpoint and the routing selection. Unconfigured provider endpoints and unrelated top-level fields are omitted.

Tests

• copilot_harness.test.cjs: runs the real harness startup with a stand-in CLI, and stubs /reflect through a --require preload that patches fetchAWFReflect. Cases:
  • routed with COPILOT_MODEL unset produces configured_model: "auto", and persisting it succeeds;
  • "records the fallback configured model when COPILOT_MODEL is unset";
  • an explicit model is kept;
  • SDK mode, routed and unrouted, creates no CLI selection record.
• parse_token_usage.test.cjs: "records routing and usage when Copilot model selection persistence fails", run with the real empty record, an empty file and malformed JSON. It checks the error message, model_routing in all three aw_info.json copies, the summary, agent_usage.json, the outputs, and that model_selection is absent. A second test checks that a corrected selection is saved.
• unified_session.test.cjs: runs the post-step on the fixtures, then checks that the workflow.info event from runtime/aw_info.json has modelRouting and modelSelection with gpt-5.6-luna.

Notes for reviewers

• Fallback through an alias is not covered at harness level, because the alias table lives in a shared file that several test files write to concurrently. The existing alias tests in copilot_model_selection.test.cjs still cover it.
• Hosted acceptance still needs smoke-copilot-routed, -routed-claude, -routed-steering and smoke-copilot-sdk-routed run on this branch. The PR also needs the smoke-routing label.
• TestSessionParserSourcesIncludeLocalDependencies (CGO "Unit tests (S-Z and other)") already fails on main.

• Fixes Copilot CLI model-selection persistence aborts routed attribution and token usage #69965

[github-actions]

✅ Ponytail Reviewer completed successfully!

Ponytail Reviewer

Outcome: reported_no_action

The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible.

Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.

Generated from Ponytail Reviewer · gpt56 · 9.01 AIC · ⌖ 5.39 AIC · ⊞ 14.6K · ◷

Generated by Ponytail Reviewer for #69966

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

🔎 Code quality review by PR Code Quality Reviewer

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

[github-actions]

✅ Test Quality Sentinel completed test quality analysis.

Test Quality Sentinel

Outcome: reported_no_action

The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible.

Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.

Generated from Test Quality Sentinel · haiku45 · 5.19 AIC · ⌖ 7.55 AIC · ◇ 2.5 AIC · ⊞ 9.2K · ◷

🧪 Test quality analysis by Test Quality Sentinel

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.

Design Decision Gate 🏗️

Outcome: reported_no_action

The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible.

Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.

Generated from Design Decision Gate 🏗️ · opus55 · 11.2 AIC · ⌖ 31.9 AIC · ◇ 15.1 AIC · ⊞ 1.9K · ◷

🏗️ ADR gate enforced by Design Decision Gate 🏗️

[github-actions]

Comment Memory

Peek at saved memory (pr-code-quality-reviewer)

reviewed_at: 2026-10-18T18:45:03.599+00:00
review_event: REQUEST_CHANGES
top_themes:
  - requested-model attribution regression on routed explicit-model runs
files_reviewed:
  - actions/setup/js/copilot_harness.cjs
  - actions/setup/js/parse_token_usage.cjs
  - actions/setup/js/copilot_model_selection.cjs
  - actions/setup/js/copilot_harness.test.cjs
  - actions/setup/js/parse_token_usage.test.cjs
  - actions/setup/js/unified_session.test.cjs
comment_count: 1

Note

This comment is managed by comment memory.

Expand the saved memory block to view or edit the persistent context for this thread.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 106.4 AIC · ⌖ 5.88 AIC · ⊞ 12.7K · ◷
_{Comment /review to run again}

[github-actions]

Request changes

This fixes the empty-model case, but it also rewrites every routed CLI run to look like an auto request, which corrupts requested-model attribution for explicitly configured runs.

The blocking theme from this review

• actions/setup/js/copilot_harness.cjs: routed runs now always persist configured_model: "auto", even when the workflow explicitly set COPILOT_MODEL or an alias before routing. That changes the meaning of the evidence record, leaks into requested_model / unified-session metadata, and is not covered by the new tests.

This was a static review of the diff and nearby source only; I did not execute the code or tests.

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 106.4 AIC · ⌖ 5.88 AIC · ⊞ 12.7K
_{Comment /review to run again}

[Copilot]

🟡 Changes recommended

The new session regression test fails when checkout paths contain spaces or non-ASCII characters.

1 open issue

• Use decoded filesystem path instead of URL.pathname · New

What changed in this PR

Addresses #69965 by preserving Copilot CLI model metadata and preventing selection-persistence errors from skipping runner-side accounting.

Changes:

• Capture configured models after routing or fallback, before alias resolution.
• Isolate persistence errors from routing attribution and token accounting.
• Add regression tests, recorded-run fixtures, and a patch changeset.

File	Description
actions/​setup/​js/​unified_session.test.cjs	Tests runner metadata normalization.
actions/​setup/​js/​parse_token_usage.test.cjs	Tests accounting despite invalid selection records.
actions/​setup/​js/​parse_token_usage.cjs	Separates selection-persistence errors from accounting.
actions/​setup/​js/​fixtures/​copilot-routed-run-39147795806/​token-usage.jsonl	Supplies recorded token usage.
actions/​setup/​js/​fixtures/​copilot-routed-run-39147795806/​model-routing.jsonl	Supplies routing and request evidence.
actions/​setup/​js/​fixtures/​copilot-routed-run-39147795806/​copilot-model-selection.json	Reproduces the empty configured model.
actions/​setup/​js/​fixtures/​copilot-routed-run-39147795806/​awf-routing-outcome.json	Supplies the selected route.
actions/​setup/​js/​fixtures/​copilot-routed-run-39147795806/​awf-reflect.json	Supplies provider and routing metadata.
actions/​setup/​js/​fixtures/​copilot-routed-run-39147795806/​aw_info.json	Supplies initial workflow metadata.
actions/​setup/​js/​copilot_harness.test.cjs	Tests routed, fallback, explicit, and SDK startup.
actions/​setup/​js/​copilot_harness.cjs	Preserves requested model identifiers.
.changeset/​fix-copilot-cli-model-selection-persistence.md	Documents the patch release.

_{Review effort: Max}

[github-actions]

Skills-Based Review 🧠

Applied /diagnosing-bugs and /tdd (via pr-triage: change_type=bug_fix). No blocking issues found.

📋 Key Themes & Highlights

Root cause fix

• selectionConfiguredModel in copilot_harness.cjs correctly captures the request identifier (auto for routed runs, the resolved fallback value, or the explicit model) after routing/fallback decide it, rather than reading COPILOT_MODEL too early. This directly fixes the empty configured_model that broke persistCopilotModelSelection validation downstream.
• configuredCopilotModel (pre-routing) is intentionally left untouched for the unrelated applyCopilotWireAPI and recordModelEndpointMismatch call sites — those predate this PR and are out of scope, so the two separate model variables are not a naming inconsistency.

Error isolation in the post-step

• parse_token_usage.cjs now isolates persistCopilotModelSelection() in its own try/catch, so a bad selection record (empty/malformed/invalid) no longer skips model_routing, the token-usage summary, agent_usage.json, outputs, or persistAgentAwInfoCopy() in the finally block. Verified the early return paths (no usage data, checkpoint found) still run finally, so the deferred selectionError is always reported via core.setFailed using the existing ERR_PARSE: convention.
• Validation itself is not loosened — invalid selection data is still rejected and never written to aw_info.json.

Test coverage (/tdd)

• copilot_harness.test.cjs spawns the real harness against a stub CLI with a patched /reflect, covering routed, fallback-only, explicit-model, and SDK-mode (no record) cases — genuine black-box coverage rather than mocking internals.
• parse_token_usage.test.cjs uses test.each over three failure modes (real empty record, empty file, malformed JSON) and asserts routing/usage/outputs still complete, plus a positive case with a corrected selection.
• unified_session.test.cjs adds an end-to-end check that workflow.info surfaces modelRouting and modelSelection correctly from the runner snapshot.
• All fixtures are real, unmodified records (JSON reformatted only) from the failing run (39147795806), which keeps the tests honest about the actual bug shape rather than synthetic data.

Positive highlights

• ✅ Root cause addressed (timing of model capture), not just the downstream symptom
• ✅ Clear code comments explaining why selectionConfiguredModel exists
• ✅ Error isolation preserves existing continue-on-error semantics and error-code conventions
• ✅ Changeset accurately describes the user-visible behavior change

No actionable issues identified in the changed lines. Approving.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 137.8 AIC · ⌖ 14.9 AIC · ⊞ 10.1K
_{Comment /matt to run again}

The requested change was answered with no code change needed: routed workflows never get COPILOT_MODEL (addCopilotModelEnv returns early with engine.model-routing), so configured_model "auto" is the actual routed request, as #69965 specifies (see the resolved thread). The Copilot reviewer's fixture-path finding was fixed in 0b50b48. Verified at c564f55: JS tests pass, and all four routed smokes passed R1-R4 on this branch.