[Copilot]

gh aw audit only diagnosed model-availability failures from Pi's agent-stdio.log message. It missed fail-closed model.resolution records that the harnesses already write to usage/aw_session.jsonl, such as Copilot sub-agent runs 39499460077, 39499464895 and 39508150750. It now reads the unified session first and keeps the Pi log match only as a fallback.

Session evidence (audit_model_resolution.go, new)

• Reads usage/aw_session.jsonl with the existing bounded readUnifiedSessionLine, requiring the session.format v1 header. Symlinks and non-regular files are rejected.
• Accepts failed model.resolution events only with the recorded provenance (model_resolution / agent / agent/model-resolution.json). Their fields are checked against the T-UAS-074 contract.
• Also collects the agent.execution summary (exit code, categories) and native-agent activity (session.start, assistant.*, tool.* from component agent).

Attribution

Failures are attributed as the startup cause only when all three hold: the run failed, agent.execution exited non-zero, and there is no native agent activity. Router or classifier traffic, the wrapper's state: "started" and an error-summary-only agent-session.jsonl do not count as the agent continuing.

Otherwise audit emits one low-severity advisory: new code model_resolution_failed, titled Model Resolution Failure Recorded. This matters because a repository-agent record is identical to a declared sub-agent record, and the producer was not changed to tell them apart.

When several roles failed, the finding states that the session cannot identify which role stopped startup.

Classification

• Roles are grouped by engine, provider, configured model and cause. A run gets at most one Model Unavailable finding and one Model Availability Unverified finding.
• Each group is checked only against the same run's awf-reflect.json, through checkModelAvailability, which was extracted from the Pi path with its messages unchanged.
• A complete, unambiguous inventory without the model gives model_unavailable (high).
• A missing, incomplete, malformed or ambiguous reflection, or no recorded provider, gives model_availability_unknown.
• Recorded causes other than model_unavailable (e.g. catalog_unavailable) are reported as recorded and never confirmed as unavailable.

Legacy fallback (audit_model_availability.go)

The Pi agent-stdio.log match runs only when the session has no failed resolution events. It cannot double-report a failure or override a recorded cause.

Example finding for 39499460077 (abridged):

Model Unavailable (high): Startup model resolution failed before the agent started inference.
engine copilot role(s) "mini-whoami" configured model "gpt-5-mini" (provider copilot, recorded cause model_unavailable): …
engine copilot role(s) "mini54-whoami" configured model "gpt-5.4-mini" (…): …
Startup evidence: agent execution exited with code 1 (categories: model_not_supported_error) and the unified session contains no native agent session activity. … cannot identify which of these 2 failed roles stopped startup.

Tests and fixtures

• Fixtures in pkg/cli/testdata/model_resolution_startup/ are sanitized from runs 39499460077, 39499464895, 39508150750 and 39508154330. Run 39508154330 supplies only the native-continuation events.
• Derived variants (missing or incomplete reflection, shared model, catalog_unavailable, the continuation replay) are labelled as derived in test comments.
• All ten named acceptance tests from the issue are included.
• …Run39499460077EndToEnd drives AuditWorkflowRun through the fresh and cached paths with a fake gh and checks the JSON output, run_summary.json, audit.json and the console report.

Notes for reviewers

• Please run the live gh aw audit 39499460077 --repo github/gh-aw --json with a build of this PR; it has not been run here.
• Known failures also present on main and not fixed here:
  • lint-go-golangci: Go 1.27 embedlit findings in pkg/cli/add_interactive_workflow.go, pkg/cli/add_workflow_resolution.go and pkg/parser/mcp.go.
  • TestWorkCommandPolicyFromConfigClosedFlags in pkg/cli.
• Follow-up: docs/.../reference/audit.md and .github/aw/debug-playbook.md still describe these findings as Pi-only and omit model_resolution_failed. They are left alone because this PR is limited to pkg/cli.

• Fixes audit: report model resolution startup failures from the unified session #70648

[github-actions]

✅ Ponytail Reviewer completed successfully!

Generated by Ponytail Reviewer for #70653

[github-actions]

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

Commit pushed: 1c08c96

🏗️ ADR gate enforced by Design Decision Gate 🏗️

[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]

✅ PR Code Quality Reviewer completed the code quality review.

🔎 Code quality review by PR Code Quality 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 · sonnet50 · 14.2 AIC · ⌖ 17.2 AIC · ◇ 6.6 AIC · ⊞ 12.6K · ◷

🧪 Test quality analysis by Test Quality Sentinel

[github-actions]

Two small opportunities to keep the new session parser aligned with the existing unified-session conventions.

net: -14 lines possible.

Generated by ✂️ Ponytail Reviewer for #70653 · codex · gpt56 · 17.2 AIC · ⌖ 5.33 AIC · ⊞ 14.7K
_{Comment /ponytail to run again}

[github-actions]

Comment Memory

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

reviewed_at: 2026-10-20T23:59:00Z
review_event: REQUEST_CHANGES
top_themes: duplicate execution summaries causing order-dependent attribution; unbounded unified-session parsing/resource exhaustion
files_reviewed: pkg/cli/audit_model_resolution.go, pkg/cli/audit_model_availability.go, pkg/cli/audit_finding_codes.go, pkg/cli/audit_model_resolution_test.go, pkg/cli/testdata/model_resolution_startup/*
comment_count: 2

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 · gpt50 · 2.64 AIC · ⌖ 0.689 AIC · ⊞ 1.6K · ◷
_{Comment /review to run again}

[github-actions]

Verdict: changes requested

The new parser has two correctness/robustness gaps: duplicate execution summaries are order-dependent, and unified-session processing has no total resource bound. Both can produce incorrect audit attribution or prevent audits from completing on untrusted artifacts. See the inline comments for surgical fixes.

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt50 · 2.64 AIC · ⌖ 0.689 AIC · ⊞ 1.6K
_{Comment /review to run again}

[github-actions]

🏗️ Design Decision Gate — ADR Required

This PR adds a lot of core business logic (1,470 new lines under pkg/), but no Architecture Decision Record (ADR) is linked.

The main decision here is to make the unified session (usage/aw_session.jsonl) the primary evidence for audit model-resolution findings. It also adds a conservative three-condition rule for attributing a startup failure and keeps the Pi agent-stdio.log match only as a fallback.

📄 Draft ADR committed: docs/adr/70653-use-unified-session-as-primary-source-for-audit-model-resolution-findings.md. It was generated from the PR diff, so review and complete it before merging.

🔒 This PR can't merge until an ADR is linked in the PR body.

📋 What to do next

1. Review the draft ADR committed to your branch. It was generated from the PR diff and description.
2. Fill in what the draft couldn't infer. Confirm the alternatives (log matching per harness, treating every failed record as fatal, changing the producer to record the role kind) and resolve the [TODO: verify] marker.
3. Commit the finished ADR to docs/adr/ on your branch.
4. Link the ADR in this PR body with a line such as:

   ADR: ADR-70653: Use the Unified Session as the Primary Source for Audit Model-Resolution Findings

Once the ADR is linked in the PR body, this gate will run again and check that the implementation matches the decision.

📋 Michael Nygard ADR Format Reference

An ADR must contain these four sections to be considered complete:

• Context: What is the problem? What forces are at play?
• Decision: What did you decide, and why?
• Alternatives Considered: What else could have been done?
• Consequences: What are the trade-offs, positive and negative?

All ADRs are stored in docs/adr/ as Markdown files numbered by PR number.

🏗️ ADR gate enforced by Design Decision Gate 🏗️ · pi · opus55 · 27.1 AIC · ⌖ 40.4 AIC · ⊞ 1.9K · ◷
_{Comment /review to run again}

[Copilot]

🟡 Changes recommended

Startup attribution can misdiagnose unrelated failures as fatal model-resolution errors.

5 open issues

• Add model resolution failure to the debug playbook · New
• Bump audit cache schema to invalidate stale reports · New
• Avoid attributing unrelated startup failures to model resolution · New
• Support model identifiers with multiple query parameters · New
• Count session.init as native agent activity · New

What changed in this PR

Extends gh aw audit to diagnose model-resolution failures from unified sessions, addressing #70648 beyond the existing Pi-only detection.

Changes:

• Adds session parsing, startup attribution, and role/model grouping.
• Reuses same-run inventory checks while preserving the legacy Pi fallback.
• Adds an advisory code, replay fixtures, and end-to-end coverage.

File	Description
pkg/​cli/​testdata/​model_resolution_startup/​39508154330/​aw_session.jsonl	Native-continuation fixture.
pkg/​cli/​testdata/​model_resolution_startup/​39508150750/​awf-reflect.json	Same-run model inventory.
pkg/​cli/​testdata/​model_resolution_startup/​39508150750/​aw_session.jsonl	Post-merge resolution-failure fixture.
pkg/​cli/​testdata/​model_resolution_startup/​39508150750/​agent-session.jsonl	Error-summary-only fixture.
pkg/​cli/​testdata/​model_resolution_startup/​39508150750/​agent_execution.json	Wrapper-state fixture.
pkg/​cli/​testdata/​model_resolution_startup/​39499464895/​awf-reflect.json	Routed-run model inventory.
pkg/​cli/​testdata/​model_resolution_startup/​39499464895/​aw_session.jsonl	Routed failure with classifier traffic.
pkg/​cli/​testdata/​model_resolution_startup/​39499464895/​agent-session.jsonl	Error-summary-only fixture.
pkg/​cli/​testdata/​model_resolution_startup/​39499464895/​agent_execution.json	Wrapper-state fixture.
pkg/​cli/​testdata/​model_resolution_startup/​39499460077/​awf-reflect.json	Two-role failure inventory.
pkg/​cli/​testdata/​model_resolution_startup/​39499460077/​aw_session.jsonl	Two-role resolution-failure fixture.
pkg/​cli/​testdata/​model_resolution_startup/​39499460077/​agent-session.jsonl	Error-summary-only fixture.
pkg/​cli/​testdata/​model_resolution_startup/​39499460077/​agent_execution.json	Wrapper-state fixture.
pkg/​cli/​audit_model_resolution.go	Parses evidence and generates findings.
pkg/​cli/​audit_model_resolution_test.go	Adds replay and audit-pipeline tests.
pkg/​cli/​audit_model_availability.go	Prioritizes session evidence and shares inventory checks.
pkg/​cli/​audit_finding_codes.go	Registers the advisory and playbook link.

_{Review effort: Max}

[github-actions]

Skills-Based Review 🧠

Applied /tdd and /grill-with-docs (per pr-triage classification: new_feature), with a couple of /codebase-design notes. No correctness-blocking issues found — posting as COMMENT with minor, non-blocking suggestions.

📋 Key Themes & Highlights

Key Themes

• Test coverage gaps (minor): no test for the "malformed session → silent fallback to legacy Pi path" branch in generateModelAvailabilityFindings, and no test for a run producing both a confirmed-unavailable group and an unverified group simultaneously via classifyModelResolutionGroups.
• Doc drift (acknowledged by author): docs/.../reference/audit.md still frames model_unavailable/model_availability_unknown as Pi-only and omits model_resolution_failed. The PR description calls this out as deliberately out-of-scope; flagged inline anyway since it's a real user-facing mismatch risk.
• Readability (optional): modelResolutionContinuationReason's opaque empty/non-empty string convention and the missing explicit no-op case in appendModelAvailabilityRecommendations are both navigability nits, not bugs — tests already confirm current behavior is intentional.

Positive Highlights

• ✅ Conservative, well-reasoned attribution logic (conclusion + exit code + no native-agent-activity) that avoids misattributing optional-agent failures to run-level startup failure
• ✅ Strong security posture: strict regex validation on engine/role/model/provider/cause before quoting into findings, symlink rejection via validateSubagentSessionSource, bounded line reads, and strict resolved_model == null contract enforcement
• ✅ Excellent negative-control tests (TestAuditModelAvailabilityRepositoryAgentFailureOnSuccessIsAdvisory and the 5 invalid-session variants) that pin down edge-case behavior precisely
• ✅ Honest acknowledgment of the ambiguity limitation (multiple failed roles, can't tell which stopped startup) directly in the PR body rather than hiding it

Investigation was bounded to the pre-fetched diff, fixtures, and directly-referenced helper functions (readUnifiedSessionLine, validateUnifiedDetectionHeader, validateSubagentSessionSource); testdata fixture diffs were spot-checked rather than read in full.

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

Comments that could not be inline-anchored

pkg/cli/audit_model_availability.go:38

[/tdd] When readSessionModelResolutionEvidence itself returns a non-nil err (malformed/oversized header, bad JSON line, etc.), this code silently falls through to generateLegacyPiModelAvailabilityFindings with no logged distinction from the normal "no evidence found" case. There is no test asserting this fallback-on-error path.

Content redacted to fit size limit.

@copilot please address this.

pkg/cli/audit_model_resolution.go:298

[/codebase-design] modelResolutionContinuationReason encodes four conjunctive conditions (conclusion==failure, agent.execution present, non-zero exit code, no native agent activity) for attribution. The logic is correct per the tests, but the function returns an opaque reason string used purely as a non-empty/empty signal in the caller, which hides the decision table from readers at the call site.

Content redacted to fit size limit.

@copilot please address this.

pkg/cli/audit_model_availability.go:266

[/codebase-design] appendModelAvailabilityRecommendations has no case for the new AuditFindingModelResolutionFailed code. Tests confirm this is intentional (advisory-only, no recommendation), but a future maintainer scanning this switch for "all finding codes" could mistake the omission for a bug.

Content redacted to fit size limit.

@copilot please address this.

pkg/cli/audit_model_resolution_test.go:1

[/tdd] Good coverage overall (10 named acceptance tests plus 5 invalid-session variants), but I didn't find a case where classifyModelResolutionGroups produces both a confirmed model_unavailable group and an unverified model_availability_unknown group in the same run. The per-group classification loop in audit_model_resolution.go clearly supports this, but it's untested.

Content redacted to fit size limit.

@copilot please address this.

[SivaKesava1]

Review round: fix the valid review threads

CI is green. All 9 open threads are valid, and I'd fix them in one round:

1. Playbook row (audit_finding_codes.go:30): add a model_resolution_failed row to the agentic-workflows debug playbook so make test-guidance passes. Stay within its word budget.
2. Cache schema (audit_model_availability.go:37): bump auditSchemaVersion to 10, make TestLoadCachedAuditDataRejectsOlderSchema reject 9, and add an upgrade regression seeded with an old report.
3. Affirmative startup-stop evidence (audit_model_resolution.go:313): a nonzero exit with no native events is not proof that resolution stopped startup. Require the harness refusal (Refusing to start ...) or a model-specific execution error for that failed role. Otherwise keep the advisory. Add a regression with an optional repository-agent warning followed by an unrelated pre-engine failure.
4. session.init counts as native activity (:187), with an initialization-only test.
5. Model ids with & (:22): accept multi-parameter selectors such as openai/gpt-6.1-sol?effort=high&temperature=0.2, with a regression.
6. Duplicate agent.execution (:181): reject the duplicate or validate exactly one, never last-wins.
7. Bounded reading (:117): add a total size or record limit with an explicit invalid-session result, matching the existing bounded readers.
8. Canonical path only (:80): read usage/aw_session.jsonl; drop the root-level fallback.
9. Shared envelope (:56): add Path to the existing unified-session event envelope instead of duplicating it.

Superseded: its two points (duplicate agent.execution records, unbounded session read) are fixed in d566aff and re-verified; gh aw audit 39499460077 with a build of d566aff reports the expected model_unavailable finding.