From 6ebff95ce5c212d8e81f65e108ce807b1b05f259 Mon Sep 17 00:00:00 2001 From: pelikhan Date: Sun, 8 Nov 2026 19:12:16 -0800 Subject: [PATCH] fix: retain structured tool results in unified sessions Preserve separately supplied structuredContent through typed canonical and unified session schemas and publication redaction. Cover sanitized Goose tool results, nested JSON and falsy values without restoring provider envelopes or RPC bodies. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../patch-unified-tool-structured-content.md | 8 +++ actions/setup/js/parse_goose_log.test.cjs | 6 +++ actions/setup/js/session_artifact.cjs | 2 +- actions/setup/js/types/agent_session.d.ts | 1 + actions/setup/js/types/unified_session.d.ts | 19 ++++++- actions/setup/js/unified_session_payload.cjs | 1 + .../setup/js/unified_session_payload.test.cjs | 53 +++++++++++++++++++ docs/public/schemas/agent-session.schema.json | 1 + .../schemas/unified-session.schema.json | 1 + .../unified-agent-session-specification.md | 8 ++- 10 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 .changeset/patch-unified-tool-structured-content.md diff --git a/.changeset/patch-unified-tool-structured-content.md b/.changeset/patch-unified-tool-structured-content.md new file mode 100644 index 00000000000..08f15439aa3 --- /dev/null +++ b/.changeset/patch-unified-tool-structured-content.md @@ -0,0 +1,8 @@ +--- +"gh-aw": patch +--- + +Preserve separately supplied structured tool results in full unified sessions, +including Goose shell stdout, stderr, and exit codes. Type and validate the +payload, retain nested JSON values, and apply existing secret redaction without +restoring provider envelopes or RPC response bodies. diff --git a/actions/setup/js/parse_goose_log.test.cjs b/actions/setup/js/parse_goose_log.test.cjs index 083eeec00d8..ee1e3b1d4be 100644 --- a/actions/setup/js/parse_goose_log.test.cjs +++ b/actions/setup/js/parse_goose_log.test.cjs @@ -131,6 +131,7 @@ describe("Goose unified session parser", () => { success: !native.isError, isError: native.isError, output: native.content, + structuredContent: native.structuredContent, exitCode: native.structuredContent.exit_code, }); expect(parsed.logEntries.at(-1).data.status).toBe("completed"); @@ -345,6 +346,11 @@ describe("Goose unified session parser", () => { expect(validateSession(serializeSessionArtifact(collected.events), "unified")).toBe(collected.events.length); const unified = collected.events.filter(event => event.type === "tool.execution_complete"); expect(unified.map(event => event.data)).toEqual(parsed.filter(event => event.type === "tool.execution_complete").map(event => normalizeUnifiedSessionEvent(event).data)); + for (const event of unified) { + const nativeResult = fixture.flatMap(record => record.message?.content ?? []).find(item => item.type === "toolResponse" && item.id === event.data.toolCallId).toolResult.value; + if (Object.hasOwn(nativeResult, "structuredContent")) expect(event.data.structuredContent).toEqual(nativeResult.structuredContent); + else expect(event.data).not.toHaveProperty("structuredContent"); + } const nativeText = fixture .flatMap(event => event.message?.content ?? []) .filter(item => item.type === "text") diff --git a/actions/setup/js/session_artifact.cjs b/actions/setup/js/session_artifact.cjs index b6632a63552..4bbb6352df8 100644 --- a/actions/setup/js/session_artifact.cjs +++ b/actions/setup/js/session_artifact.cjs @@ -63,7 +63,7 @@ const sessionPayloadFields = new Map([ ["assistant.reasoning", ["content"]], ["assistant.refusal", ["content", "explanation"]], ["tool.execution_start", ["input", "parameters", "command"]], - ["tool.execution_complete", ["output", "result", "error", "fileEdits"]], + ["tool.execution_complete", ["output", "structuredContent", "result", "error", "fileEdits"]], ]); /** @param {unknown} original @param {unknown} redacted @param {string[]} maskedValues @param {string} eventType @param {string[]} [fields] */ diff --git a/actions/setup/js/types/agent_session.d.ts b/actions/setup/js/types/agent_session.d.ts index 62dda769216..e2e1ac4c3f1 100644 --- a/actions/setup/js/types/agent_session.d.ts +++ b/actions/setup/js/types/agent_session.d.ts @@ -84,6 +84,7 @@ export interface ToolExecutionCompleteData { workflowRunId?: string; success?: boolean; output?: JsonValue; + structuredContent?: JsonValue; result?: JsonValue; error?: JsonValue; fileEdits?: JsonValue; diff --git a/actions/setup/js/types/unified_session.d.ts b/actions/setup/js/types/unified_session.d.ts index f3937585c89..5655c6ce6e3 100644 --- a/actions/setup/js/types/unified_session.d.ts +++ b/actions/setup/js/types/unified_session.d.ts @@ -852,7 +852,24 @@ export interface UnifiedSessionEventDataMap { } & UnifiedSessionContextData; "tool.execution_complete": Pick< ToolExecutionCompleteData, - "toolCallId" | "stepIndex" | "toolName" | "success" | "output" | "result" | "error" | "fileEdits" | "durationMs" | "exitCode" | "status" | "mcpServerName" | "isError" | "taskId" | "taskType" | "workflowName" | "workflowRunId" + | "toolCallId" + | "stepIndex" + | "toolName" + | "success" + | "output" + | "structuredContent" + | "result" + | "error" + | "fileEdits" + | "durationMs" + | "exitCode" + | "status" + | "mcpServerName" + | "isError" + | "taskId" + | "taskType" + | "workflowName" + | "workflowRunId" > & UnifiedSessionContextData; "dynamicWorkflows.task_started": DynamicWorkflowObservationData; diff --git a/actions/setup/js/unified_session_payload.cjs b/actions/setup/js/unified_session_payload.cjs index d4df2a00b75..d37e67559e9 100644 --- a/actions/setup/js/unified_session_payload.cjs +++ b/actions/setup/js/unified_session_payload.cjs @@ -255,6 +255,7 @@ const EVENT_FIELDS = { ...TOOL_FIELDS, success: ["success"], output: ["output"], + structuredContent: ["structuredContent"], result: ["result"], error: ["error"], fileEdits: ["fileEdits"], diff --git a/actions/setup/js/unified_session_payload.test.cjs b/actions/setup/js/unified_session_payload.test.cjs index d6d6ef764b2..03cf1648cea 100644 --- a/actions/setup/js/unified_session_payload.test.cjs +++ b/actions/setup/js/unified_session_payload.test.cjs @@ -21,6 +21,59 @@ import { failure as claudeFailures } from "./fixtures/claude_ci_sessions.cjs"; import { createSessionLookups } from "./grader_session.cjs"; describe("essential unified session payloads", () => { + it.each([null, false, 0, "", [], {}, { stdout: 'line\n"quoted"\\path \u2603', stderr: "", exit_code: 0, nested: [false, 0, null, ""] }])( + "retains explicitly supplied structured tool content %j without restoring provider envelopes", + structuredContent => { + const event = { + type: "tool.execution_complete", + id: "completion", + data: { + toolCallId: "shell", + success: false, + output: [], + structuredContent, + nativeEnvelope: { omitted: true }, + }, + }; + const original = structuredClone(event); + const projected = normalizeUnifiedSessionEvent(event); + expect(projected).toEqual({ type: event.type, id: event.id, data: { toolCallId: "shell", success: false, output: [], structuredContent } }); + expect(normalizeUnifiedSessionEvent(projected)).toEqual(projected); + expect(event).toEqual(original); + const [unified] = mergeSessionSources([{ component: "agent", phase: "agent", path: "agent-session.jsonl", events: [event] }]); + const published = JSON.parse(serializeSessionArtifact([unified]).trim()); + expect(published.data.structuredContent).toEqual(structuredContent); + const validate = createSessionValidator("unified").event; + expect(validate(published), JSON.stringify(validate.errors)).toBe(true); + } + ); + + it("does not invent structured tool content or retain it in compact RPC observations", () => { + expect(normalizeUnifiedSessionEvent({ type: "tool.execution_complete", data: { output: [] } }).data).toEqual({ output: [] }); + const rpc = normalizeUnifiedSessionEvent({ + type: "mcp.rpc.response", + data: { id: 1, result: { structuredContent: { omitted: true }, content: [{ type: "text", text: "omitted" }] } }, + }); + expect(JSON.stringify(rpc)).not.toContain("structuredContent"); + expect(JSON.stringify(rpc)).not.toContain("omitted"); + }); + + it("redacts structured tool payload values and keys without rejecting them as structural metadata", () => { + const event = { + type: "tool.execution_complete", + data: { + toolCallId: "shell", + structuredContent: { stdout: "SANITIZED_MASK\n", stderr: "", exit_code: 0, nested: { SANITIZED_MASK: "SANITIZED_MASK" } }, + }, + }; + for (const events of [[event], mergeSessionSources([{ component: "agent", phase: "agent", path: "agent-session.jsonl", events: [event] }])]) { + const serialized = serializeSessionArtifact(events, ["SANITIZED_MASK"], { rejectStructuralRedaction: true }); + expect(serialized).not.toContain("SANITIZED_MASK"); + expect(JSON.parse(serialized.trim()).data.structuredContent).toEqual({ stdout: "***\n", stderr: "", exit_code: 0, nested: { "***": "***" } }); + } + expect(() => serializeSessionArtifact([{ ...event, id: "SANITIZED_MASK" }], ["SANITIZED_MASK"], { rejectStructuralRedaction: true })).toThrow("Session redaction changed structural fields"); + }); + it("preserves observed Codex reconnect progress separately from harness attempts", () => { const native = parseEngineSession("Reconnecting... 2/5 (connection interrupted)\n", "codex").find(event => event.type === "session.result"); expect(native).toMatchObject({ reconnectAttempt: 2, maxReconnects: 5 }); diff --git a/docs/public/schemas/agent-session.schema.json b/docs/public/schemas/agent-session.schema.json index 89f7a9769bb..67eb75beee8 100644 --- a/docs/public/schemas/agent-session.schema.json +++ b/docs/public/schemas/agent-session.schema.json @@ -140,6 +140,7 @@ "workflowRunId": { "type": "string" }, "success": { "type": "boolean" }, "output": { "$ref": "#/definitions/JsonValue" }, + "structuredContent": { "$ref": "#/definitions/JsonValue" }, "result": { "$ref": "#/definitions/JsonValue" }, "error": { "$ref": "#/definitions/JsonValue" }, "fileEdits": { "$ref": "#/definitions/JsonValue" }, diff --git a/docs/public/schemas/unified-session.schema.json b/docs/public/schemas/unified-session.schema.json index f33de7b8a19..02922eecda3 100644 --- a/docs/public/schemas/unified-session.schema.json +++ b/docs/public/schemas/unified-session.schema.json @@ -327,6 +327,7 @@ "toolName": { "type": "string" }, "success": { "type": "boolean" }, "output": { "$ref": "#/definitions/JsonValue" }, + "structuredContent": { "$ref": "#/definitions/JsonValue" }, "result": { "$ref": "#/definitions/JsonValue" }, "error": { "$ref": "#/definitions/JsonValue" }, "fileEdits": { "$ref": "#/definitions/JsonValue" }, diff --git a/docs/src/content/docs/specs/unified-agent-session-specification.md b/docs/src/content/docs/specs/unified-agent-session-specification.md index ace27be517a..7012878e124 100644 --- a/docs/src/content/docs/specs/unified-agent-session-specification.md +++ b/docs/src/content/docs/specs/unified-agent-session-specification.md @@ -243,7 +243,7 @@ opaque because its essential fields are not defined by this specification. | Agent messages and reasoning | Exact `content`, without duplicate text blocks or the original message envelope. | | Agent session errors | Observed diagnostic fields, including `errorType`, `code`, and `statusCode`; absent status codes remain absent, and unrelated native error context is omitted. | | Agent policy refusals | Structured `reason`, exact available `content`, `policyCategory`, `explanation`, and streaming `partial` flag; no duplicated provider envelope. | -| Agent tool lifecycle | Correlation IDs, tool/server names, one `input` or `output` field, command, outcome/error signals, duration, and exit code. Background tool updates retain observed `taskId` and `taskType`; progress and task notifications do not imply a tool completion. | +| Agent tool lifecycle | Correlation IDs, tool/server names, one `input` or `output` field, separately supplied `structuredContent`, command, outcome/error signals, duration, and exit code. Background tool updates retain observed `taskId` and `taskType`; progress and task notifications do not imply a tool completion. | | Agent accounting | Turns, duration, cost, observed terminal `status` and `sourceType`, normalized `usage` including reported reasoning tokens, errors, and permission denials. | | Agent execution diagnostics | Unique harness/compiler `categories`, native `errorCodes` and `errorTypes`, and the observed final process `exitCode`; no diagnostic messages or duplicated engine envelopes. | | MCP | Server, direction, RPC/call/request IDs, method, tool name, duration, sizes, status, reason, and error code/message. Failed calls reporting HTTP 429 also retain `httpStatus: 429` and the sanitized `retryHint`, including retry durations or `Retry-After` seconds/HTTP-date. No RPC arguments, response bodies, echoed queries/URLs in rate-limit diagnostics, or error context. | @@ -765,12 +765,18 @@ Native `parameters` remains a reader alias for `input`. If both are present, rea | `toolName` | Source-dependent native name, or a name recovered from an exact matching start. | | `success` | Boolean when the source completion establishes an outcome; omitted when unknown. | | `output` | Source-dependent JSON-compatible output, not restricted to text. | +| `structuredContent` | Optional separately supplied JSON-compatible tool result, retained alongside `output` without flattening nested fields or replacing explicit empty or falsy values. | | `durationMs` | Source-dependent finite nonnegative duration in milliseconds, including zero. | | `error` | Optional native error string, object, or other supported error value. | | `result` | Optional native result value, including Copilot `result.content` arrays or JSON content blocks. | `output` and `result` can coexist. A reader uses an explicitly present `output` for the primary output view, otherwise the native `result`; neither is destroyed. Source-specific result envelopes remain available to readers. +The full unified artifact retains `structuredContent` independently of the primary +output view, including shell `stdout`, `stderr`, and `exit_code` fields. It uses +the same registered-secret redaction as other tool payloads. This does not retain +duplicated provider envelopes or change the compact MCP RPC projection. + **T-UAS-014 — Explicit failures.** A mapping MUST represent an explicit failed status, error outcome, `is_error: true`, `isError: true`, or nonzero command exit code as a failure. Absence of an error is not, by itself, evidence of completion or success. A source protocol's documented successful completion or default non-error tool-result semantics MAY establish `success: true`; a start alone MUST NOT. If preserved native signals conflict, readers MUST treat an explicit failure as controlling and retain the contradictory source evidence. ### 4.6 `session.result`