Preserve Goose tool diagnostics and unified session evidence by pelikhan · Pull Request #67349 · github/gh-aw

pelikhan · 2026-10-10T05:31:37Z

Why

Existing Goose smoke artifacts expose MCP server identity and command exit codes that were retained only in duplicated native envelopes, then lost from the essential unified session. The adapter also replaced null tool arguments with {}, could reorder streamed text around tool/reasoning observations, and discarded prior accounting when a later terminal error arrived.

Changes

Map native MCP identity, explicit isError flags, and structured result exit codes into standard tool lifecycle fields; nonzero observed exit codes control failure while contradictory native flags remain preserved.
Preserve falsy arguments and valid source metadata; invalid IDs/timestamps fall back to valid message evidence or are omitted with collection warnings. Merge only adjacent text deltas with equivalent content-free metadata, regardless of object key ordering.
Reconcile cumulative accounting without adding snapshots, retain prior accounting on terminal errors, and report malformed tool observations explicitly.
Emit standard mcp.event startup diagnostics and session.error maximum-turn diagnostics while retaining compatibility with older canonical Goose extensions.
Add sanitized CI-derived fixtures, canonical serialization/collection/schema regressions, and concise Goose documentation.

The PR includes the coordinated shared session projection/type/schema/spec prerequisite as intact cherry-pick 57fc6d19e2 (original b39ff126e9). This wires mcp.event.serverName and standard session errors through publication and rendering instead of leaving startup diagnostics dependent on a separate PR. Goose-specific review fixes are isolated in 3bee66dd97.

Evidence and limits

Compared native agent-stdio.log, persisted agent-session.jsonl, and usage/aw_session.jsonl from existing runs 37865761051, 37552916488, 37266019863, 37264898160, and 37248341114. Verified exact text, tool correlation/outcomes, and available accounting. Two older canonical artifacts use legacy records and fail the current schema; prior artifacts are not rewritten.

37829720724 had no retrievable jobs, artifacts, or logs. No workflows were triggered. Reasoning, refusal, maximum-turn, conflicting exit flags, and snapshot edge cases are synthetic coverage rather than observed CI evidence.

Validation

474 focused JavaScript tests across Goose, custom delegation, bootstrap, shared session parsing/rendering/collection, and generated schemas passed, including 54 Goose tests.
npm --prefix actions/setup/js run typecheck --silent
PATH="/opt/homebrew/bin:$PATH" make fmt
PATH="/opt/homebrew/bin:$PATH" make lint: passed with existing repository warnings. Initial Go-linter runner contention was resolved with its supported serial-runner mode, followed by a successful full target retry.
go test ./pkg/cli -run '^TestGooseUnifiedSessionReconstruction$' -count=1
go test ./actions/setup -count=1
PATH="/opt/homebrew/bin:$PATH" make agent-report-progress: build, scoped lint, schema freshness, and impacted tests passed (202 tests). Installed Bash 5 avoids macOS Bash 3's missing mapfile.

All five in-scope review threads were replied to and resolved after publishing fixes. The initial CI snapshot had 61 successful checks, 9 skipped, and no failures on the prior HEAD. CI on the new agent-pushed HEAD remains unverified until a maintainer re-triggers it.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-10-10T05:33:01Z

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

No ADR enforcement needed for PR #67349: the PR does not carry the 'implementation' label (has_implementation_label=false) and has 0 new lines in default business logic directories (threshold 100, no custom .design-gate.yml). 4 files changed.

🏗️ ADR gate enforced by Design Decision Gate 🏗️

github-actions · 2026-10-10T05:33:01Z

✅ Ponytail Reviewer completed successfully!

Lean already. Ship.

Generated by Ponytail Reviewer for #67349

github-actions · 2026-10-10T05:33:12Z

✅ Test Quality Sentinel completed test quality analysis.

Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff

🧪 Test quality analysis by Test Quality Sentinel

github-actions · 2026-10-10T05:33:12Z

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

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

github-actions · 2026-10-10T05:34:09Z

✅ PR Code Quality Reviewer completed the code quality review.

🔎 Code quality review by PR Code Quality Reviewer

Copilot

🟡 Changes recommended

Shell failures can be marked successful, and invalid explicit metadata can produce schema-invalid session artifacts.

2 open findings

What changed in this PR

Improves Goose log parsing so canonical sessions retain tool diagnostics, ordering, metadata, accounting, and terminal failures.

Changes:

Preserves MCP identities, shell results, streaming order, and cumulative usage.
Adds CI-derived regression fixtures and round-trip validation.
Documents Goose session behavior.

File	Description
`docs/src/content/docs/reference/engines.md`	Documents parser guarantees.
`actions/setup/js/parse_goose_log.cjs`	Enhances Goose event normalization.
`actions/setup/js/parse_goose_log.test.cjs`	Adds parser and collection regressions.
`actions/setup/js/fixtures/goose_ci_sessions.cjs`	Adds sanitized CI fixtures.

🧠 Review effort: Balanced

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

github-actions · 2026-10-10T05:40:59Z

Comment Memory

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

reviewed_at: 2026-10-10T05:36:17.986+00:00
review_event: REQUEST_CHANGES
top_themes:
  - unified-session diagnostic regression
files_reviewed:
  - actions/setup/js/parse_goose_log.cjs
  - actions/setup/js/parse_goose_log.test.cjs
  - actions/setup/js/agent_session.cjs
  - actions/setup/js/unified_session_payload.cjs
  - actions/setup/js/unified_session.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 · 44.7 AIC · ⌖ 5.76 AIC · ⊞ 19.5K · ◷
_{Comment /review to run again}

github-actions

Request changes

The Goose parser change regresses startup-failure diagnostics: the new mcp.event path currently drops serverName during unified-session normalization, so persisted artifacts no longer say which extension failed to start.

Blocking theme

Please wire the new diagnostic shape all the way through normalizeUnifiedSessionEvent()/schema coverage before merging. The current tests only round-trip the parser output, so this loss does not get caught once the event is collected into a unified session.

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 44.7 AIC · ⌖ 5.76 AIC · ⊞ 19.5K
_{Comment /review to run again}

github-actions

Skills-Based Review 🧠

Applied /diagnosing-bugs and /tdd (via pr-triage: bug_fix). This PR fixes real data-loss bugs in the Goose adapter — MCP identity, shell exit codes, falsy tool arguments, streaming text ordering, and cumulative usage reconciliation — each backed by CI-derived fixtures and targeted regression tests. Root causes are addressed directly in parse_goose_log.cjs, not papered over.

📋 Key Themes & Highlights

Key Themes

Two moderate-impact design concerns flagged inline in parse_goose_log.cjs:
1. exitCode extraction gates on toolName === "shell", coupling the parser to one hardcoded tool name instead of a shape check.
2. The new streaming-text dedup key does a broad JSON.stringify of the whole event per delta, which is costly and fragile to get right.

Positive Highlights

✅ Excellent regression coverage: sanitized CI fixtures (goose_ci_sessions.cjs) for success/failed-tool/terminal-error/startup-failure paths, each round-tripped through canonical serialization.
✅ Root-cause fixes, not symptom patches: falsy argument preservation, isMaxTurnsError correctly distinguishes turn limits from credit-limit errors, cumulative usage reconciliation via reconcileSessionUsage instead of overwriting.
✅ Backward compatibility explicitly tested (goose.max_turns/goose.mcp_failure legacy events still handled).
✅ Docs updated to describe the new preserved semantics.

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

github-actions

Impeccable Review — `harden` + `audit` (bug_fix)

Reviewed the Goose adapter diff (parse_goose_log.cjs, new goose_ci_sessions.cjs fixtures, expanded parse_goose_log.test.cjs, doc update). This is a non-UI log-parsing fix, so applied harden (missing/edge-state handling) and audit (correctness/quality) in place of the UI-focused modes.

Verification performed (vitest unavailable offline, so logic was exercised directly with node -e against the real modules):

MCP server identity + shell exit-code propagation into tool.execution_start/tool.execution_complete ✅
Falsy tool-argument preservation (null/false/0/""/{}/[]) without defaulting to {} ✅
No text-merge across intervening reasoning/tool observations; distinct streaming metadata kept separate ✅
Envelope metadata (id/timestamp/parentId/vendor) retained instead of overwritten ✅
Cumulative usage reconciliation across multiple complete/error records, preserving prior accounting on a later terminal error ✅
goose.max_turns/goose.mcp_failure legacy canonical events still round-trip; new mcp.event/session.error-based diagnostics work equivalently ✅
Malformed tool request/response records emit session.collection_warning without fabricating lifecycle events ✅
Full CI-fixture round trip through collectUnifiedSession + normalizeUnifiedSessionEvent (schema validation itself skipped — ajv unavailable offline) ✅

No correctness, security, or missing-edge-case issues found. Documentation update in engines.md accurately reflects the new behavior.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · copilot · sonnet50 · 127 AIC · ⌖ 13.1 AIC · ⊞ 8.1K

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Preserve Goose tool diagnostics and session evidence

7fe9d62

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

pelikhan marked this pull request as ready for review October 10, 2026 05:32

Copilot AI balanced review requested due to automatic review settings October 10, 2026 05:32

Copilot started reviewing on behalf of pelikhan October 10, 2026 05:32 View session

Copilot AI reviewed Oct 10, 2026

View reviewed changes

Comment thread actions/setup/js/parse_goose_log.cjs Outdated

Comment thread actions/setup/js/parse_goose_log.cjs Outdated

github-actions Bot mentioned this pull request Oct 10, 2026

[aw] No-Op Runs #65943

Open

github-actions Bot requested changes Oct 10, 2026

View reviewed changes

Comment thread actions/setup/js/parse_goose_log.cjs

github-actions Bot reviewed Oct 10, 2026

View reviewed changes

Comment thread actions/setup/js/parse_goose_log.cjs Outdated

Comment thread actions/setup/js/parse_goose_log.cjs Outdated

github-actions Bot approved these changes Oct 10, 2026

View reviewed changes

github-actions Bot mentioned this pull request Oct 10, 2026

[pr-review] Daily PR Code Quality Review — 38028794652 #67366

Closed

pelikhan and others added 2 commits October 9, 2026 22:58

fix(session): preserve shared engine evidence in unified projections

57fc6d1

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Fix Goose metadata and exit outcome review findings

3bee66d

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

github-actions Bot mentioned this pull request Oct 10, 2026

[pr-sous-chef] PR Sous Chef Report – 2026-10-10T02:09:05Z #67318

Closed

pelikhan merged commit 595e115 into main Oct 10, 2026
51 checks passed

pelikhan deleted the pelikhan-goose-session-audit branch October 10, 2026 10:14

Navigation Menu

Repository navigation

Preserve Goose tool diagnostics and unified session evidence - #67349

Preserve Goose tool diagnostics and unified session evidence#67349
pelikhan merged 3 commits into
mainfrom
pelikhan-goose-session-audit

pelikhan commented Oct 10, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 10, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 10, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 10, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 10, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 10, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Oct 10, 2026

Uh oh!

github-actions Bot left a comment

Uh oh!

Uh oh!

github-actions Bot left a comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

pelikhan commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

Changes

Evidence and limits

Validation

Uh oh!

github-actions Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Oct 10, 2026

Comment Memory

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Request changes

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Skills-Based Review 🧠

Key Themes

Positive Highlights

Uh oh!

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Impeccable Review — harden + audit (bug_fix)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pelikhan commented Oct 10, 2026 •

edited

Loading

github-actions Bot commented Oct 10, 2026 •

edited

Loading

github-actions Bot commented Oct 10, 2026 •

edited

Loading

github-actions Bot commented Oct 10, 2026 •

edited

Loading

github-actions Bot commented Oct 10, 2026 •

edited

Loading

github-actions Bot commented Oct 10, 2026 •

edited

Loading

Impeccable Review — `harden` + `audit` (bug_fix)