From 1c4365ce2ac9f8eb01f265d220e05e733a301f3e Mon Sep 17 00:00:00 2001 From: nesquena-hermes <[email protected]> Date: Mon, 1 Jun 2026 17:27:02 +0000 Subject: [PATCH] fix(artifacts): guard malformed tool_calls entries in collectSessionArtifacts (#3329) Codex regression-gate finding: the OpenAI tool_calls loop dereferenced tc.function with no null/type guard, so a persisted message.tool_calls array containing a null or non-object entry would throw and abort artifact collection. Mirror the existing tool_use-block guard. Adds a node-driven regression test. Co-authored-by: mysoul12138 --- static/workspace.js | 3 +- ..._issue3329_artifact_tool_metadata_guard.py | 101 ++++++++++++++++++ 2 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 tests/test_issue3329_artifact_tool_metadata_guard.py diff --git a/static/workspace.js b/static/workspace.js index 3813d623..5cc5378d 100644 --- a/static/workspace.js +++ b/static/workspace.js @@ -285,7 +285,8 @@ function collectSessionArtifacts(){ // Structured tool_calls array (OpenAI format: {function:{name,arguments}}). if(Array.isArray(msg.tool_calls)){ for(const tc of msg.tool_calls){ - const fn = tc.function || tc; + if(!tc || typeof tc !== 'object') continue; + const fn = (tc.function && typeof tc.function === 'object') ? tc.function : tc; const name = fn.name || tc.name || ''; let args = fn.arguments || tc.arguments || tc.args || tc.input || {}; if(typeof args === 'string'){ try{ args = JSON.parse(args); }catch(_){} } diff --git a/tests/test_issue3329_artifact_tool_metadata_guard.py b/tests/test_issue3329_artifact_tool_metadata_guard.py new file mode 100644 index 00000000..7368e6b4 --- /dev/null +++ b/tests/test_issue3329_artifact_tool_metadata_guard.py @@ -0,0 +1,101 @@ +"""Regression: collectSessionArtifacts() tolerates malformed tool_calls entries (#3329). + +#3329 taught the Artifacts tab to read structured tool metadata from messages +(OpenAI `tool_calls` + Anthropic `tool_use` content blocks) in addition to +text-mined diff fences. The pre-release Codex regression gate flagged that the +OpenAI `tool_calls` loop dereferenced ``tc.function`` with no null/type guard, +so a session whose persisted ``message.tool_calls`` array contained a null or +non-object entry (truncated/corrupt sidecar, partial stream) would throw +"Cannot read properties of null", aborting artifact collection for the whole +session. The Anthropic `tool_use` loop already had a ``if(!block ...) continue`` +guard; this test pins the symmetric guard on the OpenAI loop. + +Drives the ACTUAL collectSessionArtifacts() from static/workspace.js via node +(with its private helpers + a stubbed ``S``) so it cannot drift from a mirror. +""" + +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +REPO = Path(__file__).resolve().parent.parent +WORKSPACE_JS = (REPO / "static" / "workspace.js").read_text(encoding="utf-8") +NODE = shutil.which("node") + +pytestmark = pytest.mark.skipif(NODE is None, reason="node not on PATH") + + +def _extract_fn(name: str) -> str: + start = WORKSPACE_JS.index(f"function {name}(") + brace = WORKSPACE_JS.index("{", start) + depth = 0 + for i in range(brace, len(WORKSPACE_JS)): + c = WORKSPACE_JS[i] + if c == "{": + depth += 1 + elif c == "}": + depth -= 1 + if depth == 0: + return WORKSPACE_JS[start:i + 1] + raise AssertionError(f"function {name} did not close") + + +def _collect_via_node(messages): + # Pull in the function under test plus the helpers it transitively calls. + import re + consts = [] + for name in ("ARTIFACT_IGNORE_RE", "ARTIFACT_MUTATION_TOOLS"): + m = re.search(rf"const {name} = .*?;", WORKSPACE_JS) + assert m, f"{name} not found" + consts.append(m.group(0)) + fns = "\n".join(consts) + "\n" + "\n".join( + _extract_fn(n) + for n in ( + "_normalizeArtifactPath", + "_artifactCandidatesFromText", + "_artifactCandidatesFromToolCall", + "collectSessionArtifacts", + ) + ) + driver = ( + "const S = { toolCalls: [], messages: JSON.parse(process.argv[1]), " + "session: { workspace: '/ws' } };\n" + + fns + "\n" + + "const out = collectSessionArtifacts();\n" + + "process.stdout.write(JSON.stringify(out.map(x => x.path)));\n" + ) + r = subprocess.run( + [NODE, "-e", driver, json.dumps(messages)], + capture_output=True, text=True, timeout=15, + ) + assert r.returncode == 0, f"node threw (the #3329 guard regressed): {r.stderr}" + return json.loads(r.stdout) + + +def test_malformed_tool_calls_entries_do_not_throw(): + """A null / non-object tool_calls entry must be skipped, not throw.""" + messages = [ + {"tool_calls": [ + None, + "not-an-object", + 42, + {"function": {"name": "edit_file", "arguments": json.dumps({"path": "real.py"})}}, + ]}, + ] + # The assertion that matters is that node returns 0 (no throw). The valid + # entry's path extraction depends on _artifactCandidatesFromToolCall's + # argument heuristics, which we don't pin here — only the no-throw guarantee. + paths = _collect_via_node(messages) + assert isinstance(paths, list) + + +def test_guard_present_in_source(): + """Pin the explicit guard so it can't be silently removed.""" + fn = _extract_fn("collectSessionArtifacts") + assert "if(!tc || typeof tc !== 'object') continue;" in fn, ( + "OpenAI tool_calls loop must guard malformed entries before " + "dereferencing tc.function (#3329 Codex regression-gate finding)" + )