Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 72 additions & 7 deletions actions/setup/js/agent_session.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
input_tokens: "inputTokens",
output_tokens: "outputTokens",
total_tokens: "totalTokens",
reasoning_output_tokens: "reasoningOutputTokens",
cache_creation_input_tokens: "cacheCreationInputTokens",
cache_read_input_tokens: "cacheReadInputTokens",
};
Expand All @@ -32,6 +33,65 @@
return typeof value === "number" && Number.isFinite(value) && value >= 0;
}

/** @param {any} event @returns {boolean} */
function isNestedSessionEvent(event) {
if (event == null) return false;
return [event.agentId, event.data?.agentId, event.parent_tool_use_id, event.parentToolUseId, event.parentToolCallId, event.data?.parentToolUseId, event.data?.parent_tool_use_id, event.data?.parentToolCallId].some(
value => typeof value === "string" && value.length > 0
);
}

/**
* An agent instance is authoritative; deprecated parent-tool markers are only
* the identity fallback. Child initialization must not change the root session.
* @param {Array<any>} events
*/
function sessionEventContexts(events) {
const sessions = new Map();
const agents = new Map();
const spawningTools = new Map();
return events.map(event => {
const data = event.data ?? {};
const sourceKey = JSON.stringify([event.provenance?.component, event.provenance?.phase, event.provenance?.path]);
const rootKey = JSON.stringify([sourceKey]);
const rootSessionId = sessions.get(rootKey);
let agentId = event.agentId || data.agentId || undefined;
let parentToolCallId = event.parent_tool_use_id || event.parentToolUseId || event.parentToolCallId || data.parentToolUseId || data.parent_tool_use_id || data.parentToolCallId || undefined;
if (event.type === "subagent.started" && agentId !== undefined) {
const agentKey = JSON.stringify([sourceKey, rootSessionId, agentId]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/codebase-design] sessionEventContexts is a single ~45-line function juggling four Maps and five derived JSON-string keys (sourceKey, rootKey, agentKey, toolKey, sessionKey) to resolve agent identity, session, and scope in one pass. The interface is deep (simple call, rich logic) but the body reuses the word "key" for five different concepts, which makes it hard to trace without re-reading line by line.

💡 Suggested refactor

Consider extracting the agent-identity resolution (lines ~58–72: subagent registration, orphan agentId inference from spawningTools) into a named helper, e.g. resolveAgentIdentity(event, sourceKey, rootSessionId, sessions, agents, spawningTools), so sessionEventContexts reads as: resolve identity → resolve session → build scope. This keeps the rich behavior but gives each phase a name that matches the domain vocabulary already used in comments ("agent instance is authoritative", "deprecated parent-tool markers are only the identity fallback").

Also worth a few focused unit tests directly against sessionEventContexts (not just via copilot_nested_sessions.test.cjs end-to-end flows) for: orphan parentToolCallId with 0, 1, and >1 candidate spawningTools instances — the instances?.size === 1 branch silently no-ops for the >1 case, which is a plausible real-world input (concurrent subagent calls from one tool).

@copilot please address this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored identity resolution into named helpers with distinct identity terminology, and added tests for zero, one, and ambiguous lifecycle candidates plus child-before-root ordering. Verified in commit 446f52f.

agents.set(agentKey, { parentToolCallId: data.toolCallId, parentAgentId: data.parentId });
if (data.toolCallId !== undefined) {
const toolKey = JSON.stringify([sourceKey, rootSessionId, data.toolCallId]);
const instances = spawningTools.get(toolKey) ?? new Set();
instances.add(agentId);
spawningTools.set(toolKey, instances);
}
}
if (agentId === undefined && parentToolCallId !== undefined) {
const instances = spawningTools.get(JSON.stringify([sourceKey, rootSessionId, parentToolCallId]));
if (instances?.size === 1) agentId = instances.values().next().value;
}
const agent = agents.get(JSON.stringify([sourceKey, rootSessionId, agentId]));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This keys child state off rootSessionId before the current event can establish that root ID, so a subagent.started that arrives before session.start is stored under undefined and every later lookup misses it.

💡 Why this blocks the merge

The new correlation logic assumes the root session has already been seen, but merged/untimed streams can delay lifecycle records. In that ordering, the child is inserted under [sourceKey, undefined, agentId], then later events are read from [sourceKey, "root", agentId] and lose both ancestry and tool correlation. That splits one child conversation into multiple scopes and corrupts turn/accounting projections.

Please avoid baking rootSessionId into the agent/tool map keys until it is known, or re-key pending child entries when the root session.start / session.init arrives.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Root session IDs are resolved across the event stream before agent identity registration, so child starts preceding root initialization correlate with later child events. Covered by a regression test in commit 446f52f.

parentToolCallId ??= agent?.parentToolCallId;
const parentAgentId = data.parentAgentId ?? agent?.parentAgentId;
const ownerKey = agentId !== undefined ? ["agent", agentId] : parentToolCallId !== undefined ? ["tool", parentToolCallId] : [];
const sessionKey = JSON.stringify([sourceKey, ...ownerKey]);
const explicitSessionId = event.session_id ?? event.sessionId ?? data.sessionId ?? data.session_id;
if (["session.start", "session.init"].includes(event.type) && explicitSessionId !== undefined) sessions.set(sessionKey, explicitSessionId);
const sessionId = explicitSessionId ?? sessions.get(sessionKey) ?? rootSessionId;
return {
sourceKey,
rootSessionId: sessions.get(rootKey),
sessionId,
agentId,
parentAgentId,
parentToolCallId,
nested: agentId !== undefined || parentToolCallId !== undefined,
scope: JSON.stringify([sourceKey, sessionId, ...ownerKey]),
};
});
}

/** @param {any} usage @returns {Record<string, any>|undefined} */
function normalizeSessionUsage(usage) {
// Usage is a named-field object; arrays cannot represent its token fields.
Expand Down Expand Up @@ -173,7 +233,7 @@
if (typeof input === "string") {
try {
input = JSON.parse(input);
} catch {

Check warning on line 236 in actions/setup/js/agent_session.cjs

View workflow job for this annotation

GitHub Actions / lint-js

Empty catch block silently swallows the error. Log it (e.g. core.debug/core.warning), assign a fallback value, or add an explicit comment explaining why the error is intentionally ignored

Check warning on line 236 in actions/setup/js/agent_session.cjs

View workflow job for this annotation

GitHub Actions / lint-js

Empty catch block silently swallows the error. Log it (e.g. core.debug/core.warning), assign a fallback value, or add an explicit comment explaining why the error is intentionally ignored
// Preserve malformed provider arguments verbatim.
}
}
Expand All @@ -196,7 +256,7 @@
if (typeof input === "string") {
try {
input = JSON.parse(input);
} catch {

Check warning on line 259 in actions/setup/js/agent_session.cjs

View workflow job for this annotation

GitHub Actions / lint-js

Empty catch block silently swallows the error. Log it (e.g. core.debug/core.warning), assign a fallback value, or add an explicit comment explaining why the error is intentionally ignored

Check warning on line 259 in actions/setup/js/agent_session.cjs

View workflow job for this annotation

GitHub Actions / lint-js

Empty catch block silently swallows the error. Log it (e.g. core.debug/core.warning), assign a fallback value, or add an explicit comment explaining why the error is intentionally ignored
// Preserve malformed provider arguments verbatim.
}
}
Expand Down Expand Up @@ -331,13 +391,14 @@
/**
* Select snapshots field by field, never sum snapshots or erase prior known values.
* @param {Array<any>} events
* @param {{includeNested?: boolean}} [options]
* @returns {Record<string, any>|undefined}
*/
function selectSessionResult(events) {
function selectSessionResult(events, { includeNested = false } = {}) {
const normalized = normalizeAgentSession(events);
const claudeSessions = new Map();
for (const event of normalized) {
if (event.type !== "session.result" || event.data.sourceEngine !== "claude" || event.parent_tool_use_id || typeof event.session_id !== "string") continue;
if (event.type !== "session.result" || event.data.sourceEngine !== "claude" || isNestedSessionEvent(event) || typeof event.session_id !== "string") continue;
claudeSessions.set(event.session_id, [...(claudeSessions.get(event.session_id) ?? []), { ...event, session_id: undefined }]);
}
if (claudeSessions.size > 1) {
Expand All @@ -354,7 +415,7 @@
}
}
for (const event of normalized) {
if (event.type !== "session.result" || event.data.sourceEngine !== "claude" || event.parent_tool_use_id || typeof event.session_id === "string") continue;
if (event.type !== "session.result" || event.data.sourceEngine !== "claude" || isNestedSessionEvent(event) || typeof event.session_id === "string") continue;
// Unassigned diagnostics are valid evidence, but their usage may overlap a named session.
for (const key of ["errors", "permissionDenials"]) {
if (Array.isArray(event.data[key])) aggregate[key] = [...(aggregate[key] ?? []), ...structuredClone(event.data[key])];
Expand All @@ -367,7 +428,7 @@
let result;
for (const event of normalized) {
if (event.type !== "session.result") continue;
if (event.data.sourceEngine === "claude" && event.parent_tool_use_id) continue;
if (!includeNested && isNestedSessionEvent(event)) continue;
result ??= {};
const data = event.data;
for (const [key, value] of Object.entries(data)) {
Expand All @@ -384,9 +445,9 @@
return result;
}

/** @param {Array<any>} events @returns {any|undefined} */
function projectSessionResult(events) {
const data = selectSessionResult(events);
/** @param {Array<any>} events @param {{includeNested?: boolean}} [options] @returns {any|undefined} */
function projectSessionResult(events, options) {
const data = selectSessionResult(events, options);
if (!data) return undefined;
return {
type: "result",
Expand All @@ -404,6 +465,7 @@
let data;
for (const event of normalizeAgentSession(events)) {
if (event.type !== "session.init" && event.type !== "session.start") continue;
if (isNestedSessionEvent(event)) continue;
data ??= {};
for (const [key, value] of Object.entries(event.data)) {
if (value !== undefined) data = { ...data, [key]: structuredClone(value) };
Expand Down Expand Up @@ -535,6 +597,7 @@
function observedSessionModel(events) {
let model;
for (const event of events) {
if (isNestedSessionEvent(event)) continue;
const value = event?.type === "system" && event.subtype === "init" ? event.model : ["session.init", "session.start", "pi.message_snapshot"].includes(event?.type) ? event.data?.model : undefined;
if (typeof value === "string" && value.length) model = value;
}
Expand All @@ -545,6 +608,8 @@
isSessionEvent,
isTokenCount,
isMetric,
isNestedSessionEvent,
sessionEventContexts,
normalizeSessionUsage,
accumulateSessionUsage,
reconcileSessionUsage,
Expand Down
22 changes: 16 additions & 6 deletions actions/setup/js/agent_session_render.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ const { collectArtifactSecretValues, redactManifestValue } = require("./safe_out
// Redacted identities can coincide; correlate using private source envelopes.
const publicationSources = new WeakMap();

/** @param {any} event @returns {any} */
function sessionPublicationSource(event) {
return publicationSources.get(event) ?? event;
}

/** @param {any} source @returns {string} */
function messageSourceScope(source) {
return JSON.stringify([
Expand All @@ -16,7 +21,7 @@ function messageSourceScope(source) {
source.provenance?.path,
source.session_id ?? source.sessionId ?? source.data?.sessionId ?? source.data?.session_id,
source.agentId ?? source.data?.agentId,
source.parent_tool_use_id ?? source.data?.parentToolUseId ?? source.data?.parent_tool_use_id,
(source.agentId ?? source.data?.agentId) === undefined ? (source.parent_tool_use_id ?? source.data?.parentToolUseId ?? source.data?.parent_tool_use_id ?? source.data?.parentToolCallId) : undefined,
source.channel ?? source.data?.channel,
]);
}
Expand All @@ -37,7 +42,7 @@ function collapseStreamedMessages(events) {
/** @type {{key: string, entry: any, legacy: boolean, field: string}|undefined} */
let previous;
for (const entry of events) {
const source = publicationSources.get(entry) ?? entry;
const source = sessionPublicationSource(entry);
const scope = messageSourceScope(source);
const native = source.event ?? source.data?.event;
if (source.type === "claude.stream_event" && native && ["message_start", "content_block_start", "content_block_delta", "content_block_stop", "message_delta", "message_stop"].includes(native.type)) {
Expand All @@ -48,7 +53,12 @@ function collapseStreamedMessages(events) {
output.push(entry);
continue;
}
if (source.type === "assistant.message_delta" || source.type === "claude.assistant_snapshot" || (source.type === "gemini.message_snapshot" && source.data?.role !== "user" && source.data?.type !== "user")) {
if (
source.type === "assistant.message_delta" ||
source.type === "assistant.reasoning_delta" ||
source.type === "claude.assistant_snapshot" ||
(source.type === "gemini.message_snapshot" && source.data?.role !== "user" && source.data?.type !== "user")
) {
if (source.type === "gemini.message_snapshot") geminiSnapshot = { scope, id: source.data?.messageId ?? source.data?.message_id ?? source.messageId ?? source.message_id, content: entry.data?.content };
output.push(entry);
continue;
Expand All @@ -58,13 +68,13 @@ function collapseStreamedMessages(events) {
const type = legacy ? (block?.type === "text" ? "assistant.message" : block?.type === "thinking" ? "assistant.reasoning" : undefined) : source.type;
const field = block?.type === "thinking" ? "thinking" : "text";
const text = legacy ? block?.[field] : entry.data?.content;
const messageId = source.data?.messageId ?? source.data?.message_id ?? source.messageId ?? source.message_id ?? source.message?.id;
const messageId = (type === "assistant.reasoning" ? source.data?.reasoningId : undefined) ?? source.data?.messageId ?? source.data?.message_id ?? source.messageId ?? source.message_id ?? source.message?.id;
const claude = claudeMessages.get(scope);
const snapshotBlocks = source.message?.content;
const matchingBlocks = Array.isArray(snapshotBlocks) ? snapshotBlocks.map((value, index) => ({ value, index })).filter(({ value }) => value?.type === (type === "assistant.reasoning" ? "thinking" : "text")) : [];
const claudeContinuation = claude?.id !== undefined && messageId === claude.id && matchingBlocks.length === 1;
const position = native?.index ?? source.data?.contentIndex ?? (claudeContinuation ? matchingBlocks[0].index : undefined);
const delta = source.delta === true || source.data?.delta === true || source.copilotProjection === "assistant.message_delta";
const delta = source.delta === true || source.data?.delta === true || ["assistant.message_delta", "assistant.reasoning_delta"].includes(source.copilotProjection);
const partial = source.partial === true || source.data?.partial === true;
const key = JSON.stringify([scope, type, messageId ?? claude?.id, claude?.sequence, position]);
const previousText = previous?.legacy ? previous.entry.message.content[0][previous.field] : previous?.entry.data.content;
Expand Down Expand Up @@ -218,4 +228,4 @@ function redactSessionForPublication(events, redact) {
}
}

module.exports = { collapseStreamedMessages, escapeSummaryText, toolInventoryName, displayArgument, renderInitializationLines, toolOutcome, boundSummaryLines, redactSessionForPublication };
module.exports = { collapseStreamedMessages, escapeSummaryText, toolInventoryName, displayArgument, renderInitializationLines, toolOutcome, boundSummaryLines, redactSessionForPublication, sessionPublicationSource };
25 changes: 22 additions & 3 deletions actions/setup/js/codex_session.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,31 @@ function normalizeCodexSession(records, model = null) {
if (!Object.hasOwn(report, "cache_creation_input_tokens") && Object.hasOwn(report, "cache_write_input_tokens")) contribution.cache_creation_input_tokens = report.cache_write_input_tokens;
accumulateSessionUsage(usage, contribution);
for (const [field, value] of Object.entries(report)) {
if (["cached_input_tokens", "cache_write_input_tokens", "reasoning_output_tokens", "total_tokens"].includes(field)) {
if (["cached_input_tokens", "cache_write_input_tokens", "total_tokens"].includes(field)) {
if (isTokenCount(value)) usage[field] = (usage[field] ?? 0) + value;
} else if (!["input_tokens", "output_tokens", "cache_read_input_tokens", "cache_creation_input_tokens", "inputTokens", "outputTokens", "cacheReadInputTokens", "cacheCreationInputTokens"].includes(field))
} else if (
![
"input_tokens",
"output_tokens",
"reasoning_output_tokens",
"cache_read_input_tokens",
"cache_creation_input_tokens",
"inputTokens",
"outputTokens",
"reasoningOutputTokens",
"cacheReadInputTokens",
"cacheCreationInputTokens",
].includes(field)
)
usage[field] = structuredClone(value);
}
for (const [alias, field] of Object.entries({ inputTokens: "input_tokens", outputTokens: "output_tokens", cacheReadInputTokens: "cache_read_input_tokens", cacheCreationInputTokens: "cache_creation_input_tokens" })) {
for (const [alias, field] of Object.entries({
inputTokens: "input_tokens",
outputTokens: "output_tokens",
reasoningOutputTokens: "reasoning_output_tokens",
cacheReadInputTokens: "cache_read_input_tokens",
cacheCreationInputTokens: "cache_creation_input_tokens",
})) {
if ((Object.hasOwn(report, alias) || Object.hasOwn(usage, alias)) && isTokenCount(usage[field])) usage[alias] = usage[field];
}
}
Expand Down
Loading
Loading