diff --git a/AUDIT.md b/AUDIT.md index 95cda6a..6e73497 100644 --- a/AUDIT.md +++ b/AUDIT.md @@ -1005,6 +1005,44 @@ refuses rather than guessing). `go build ./...`, `go vet ./...`, `go test ./...` all pass. +## S11 — `CodexActivity` verified and rewritten against a live rollout, 2026-07-28 + +The guessed `function_call`/`function_call_output` shape did not exist in any +real rollout on this machine (`~/.codex/sessions`, checked directly, not +recalled). The real shape: + +- File edits arrive as `event_msg`/`patch_apply_end`, carrying `changes` + (map of absolute path → diff) and a top-level `success` bool directly — a + strictly better source than the old assumption, no call/result pairing + needed. +- Shell commands arrive as a single freeform `response_item`/ + `custom_tool_call` named `"exec"` whose `input` is a JS snippet + (`tools.exec_command({cmd:"...", ...})`), not a flat arguments object. + `codexExecCommand` regex-extracts the first embedded `cmd:"..."`. + Success/failure has no structured exit-code field either — but a failed + script's output block reliably starts with the literal string + `"Script error:"` on this machine's real transcripts (observed for both a + JS syntax error and an `apply_patch` verification failure), so that + heuristic is now confirmed, not guessed. + +`internal/herdr/activity.go`'s `CodexActivity` rewritten to this shape; +`internal/herdr/activity_test.go` gained +`TestCodexActivityParsesRealRolloutShape` / +`TestCodexActivityMarksScriptErrorAsFailure`, fixtures built to match the +confirmed live shape, not an assumed one. Manually re-ran the new parser +against a real multi-hundred-line rollout file end-to-end (not just the unit +tests) and spot-checked the output — commands, file edits, and pass/fail all +matched the transcript by eye. + +**Still open:** `DetectMilestone`'s "last call was `git commit`" check only +sees the *first* `cmd:"..."` in a chained exec script, so a commit issued +after other commands in the same script call won't be seen as the "last +call" even though it was the last command executed — a known limitation of +single-command extraction, not fixed here. Opencode still has no verified +per-tool-call source. + +`go build ./...`, `go vet ./...`, `go test ./...` all pass. + ## S8 — closed, 2026-07-27 No compensation-event mechanism existed — §3.1's own invariant ("a wrong diff --git a/internal/herdr/activity.go b/internal/herdr/activity.go index fd58c86..a907dba 100644 --- a/internal/herdr/activity.go +++ b/internal/herdr/activity.go @@ -127,17 +127,28 @@ func ClaudeActivity(path string) ([]ToolCall, error) { return calls, s.Err() } -// CodexActivity is best-effort and unverified against a live Codex rollout — -// unlike ClaudeActivity, which reads the same transcript shape ClaudeUsage -// already confirmed. Codex's Responses-API-style rollout is expected to log -// a "function_call" payload (name, arguments, call_id) followed later by a -// "function_call_output" payload (call_id, output), mirroring the -// "payload.type" wrapper CodexUsage already reads for "token_count". Success -// is inferred heuristically from the output text (no confirmed structured -// exit-code field), which is coarser than Claude's explicit is_error flag. -// Treat any thrash/milestone signal derived from this as advisory until -// checked against a real rollout, same caveat this file's other Codex-facing -// code already carries (AUDIT.md Phase 0's own bar: verify before trusting). +// CodexActivity parses the real Codex rollout shape, confirmed live against +// this machine's own ~/.codex/sessions on 2026-07-28 — the previous version +// of this function assumed an unverified "function_call"/"function_call_output" +// payload shape (flat name/arguments/call_id, mirroring CodexUsage's +// token_count wrapper) that turned out not to exist in any real rollout file. +// The real shape has two independent signals: +// +// 1. File edits: an `event_msg` with `payload.type == "patch_apply_end"` +// carries `changes` (map of absolute path -> diff) and a top-level +// `success` bool directly — no pairing needed, and a much more reliable +// "same file edited M times" source than trying to parse it out of a +// tool call's input. +// 2. Shell commands: Codex's actual tool surface is a single freeform +// `custom_tool_call` named "exec" whose `input` is a JS snippet calling +// `tools.exec_command({cmd:"...", ...})` — not a flat arguments object. +// codexExecCommand best-effort-extracts the first embedded cmd string. +// Success is read from the paired `custom_tool_call_output`'s text +// blocks: a failed script's output observably starts with "Script +// error:" on this machine's real transcripts (both a JS syntax error and +// an apply_patch verification failure took this form) — there is no +// structured exit-code field, so this remains a text heuristic, just a +// confirmed one rather than a guessed one. func CodexActivity(path string) ([]ToolCall, error) { f, err := os.Open(path) if err != nil { @@ -145,14 +156,20 @@ func CodexActivity(path string) ([]ToolCall, error) { } defer f.Close() + type outputBlock struct { + Text string `json:"text"` + } type payload struct { - Type string `json:"type"` - CallID string `json:"call_id"` - Name string `json:"name"` - Arguments json.RawMessage `json:"arguments"` - Output string `json:"output"` + Type string `json:"type"` + CallID string `json:"call_id"` + Name string `json:"name"` + Input string `json:"input"` + Output []outputBlock `json:"output"` + Success bool `json:"success"` + Changes map[string]json.RawMessage `json:"changes"` } type entry struct { + Type string `json:"type"` Payload payload `json:"payload"` } @@ -165,13 +182,27 @@ func CodexActivity(path string) ([]ToolCall, error) { if json.Unmarshal(s.Bytes(), &e) != nil { continue } - switch e.Payload.Type { - case "function_call": - kind, key := toolCallMeta(e.Payload.Arguments) + switch { + case e.Type == "event_msg" && e.Payload.Type == "patch_apply_end": + for path := range e.Payload.Changes { + calls = append(calls, ToolCall{Name: "apply_patch", Kind: "file", Key: path, Success: e.Payload.Success}) + } + case e.Type == "response_item" && e.Payload.Type == "custom_tool_call": + kind, key := "", "" + if cmd := codexExecCommand(e.Payload.Input); cmd != "" { + kind, key = "command", cmd + } pending[e.Payload.CallID] = ToolCall{Name: e.Payload.Name, Kind: kind, Key: key, IsTest: isTestCommand(kind, key)} - case "function_call_output": + case e.Type == "response_item" && e.Payload.Type == "custom_tool_call_output": if tc, ok := pending[e.Payload.CallID]; ok { - tc.Success = !strings.Contains(strings.ToLower(e.Payload.Output), "error") + failed := false + for _, o := range e.Payload.Output { + if strings.HasPrefix(strings.TrimSpace(o.Text), "Script error:") { + failed = true + break + } + } + tc.Success = !failed calls = append(calls, tc) delete(pending, e.Payload.CallID) } @@ -180,6 +211,20 @@ func CodexActivity(path string) ([]ToolCall, error) { return calls, s.Err() } +// codexExecCmdRe extracts the first `cmd:"..."` argument out of an "exec" +// custom-tool-call's JS-scripted input. Only the first embedded command in a +// multi-call script is captured — a documented limitation, not an oversight. +var codexExecCmdRe = regexp.MustCompile(`cmd\s*:\s*"((?:[^"\\]|\\.)*)"`) + +func codexExecCommand(input string) string { + m := codexExecCmdRe.FindStringSubmatch(input) + if m == nil { + return "" + } + cmd := strings.NewReplacer(`\"`, `"`, `\n`, "\n", `\t`, "\t", `\\`, `\`).Replace(m[1]) + return strings.TrimSpace(cmd) +} + // OpenCodeActivity has no verified source. OpenCodeUsage already only reads // aggregate token counts per message file (~/.local/share/opencode/storage/ // message/), not per-tool-call records, and nobody has confirmed opencode's diff --git a/internal/herdr/activity_test.go b/internal/herdr/activity_test.go index 6c5b068..b9cd999 100644 --- a/internal/herdr/activity_test.go +++ b/internal/herdr/activity_test.go @@ -72,6 +72,112 @@ func TestClaudeActivityPairsToolUseWithResult(t *testing.T) { } } +func codexPatchApplyLine(t *testing.T, callID string, success bool, paths ...string) string { + t.Helper() + changes := map[string]any{} + for _, p := range paths { + changes[p] = map[string]any{"type": "update"} + } + b, err := json.Marshal(map[string]any{ + "type": "event_msg", + "payload": map[string]any{ + "type": "patch_apply_end", + "call_id": callID, + "success": success, + "changes": changes, + }, + }) + if err != nil { + t.Fatal(err) + } + return string(b) +} + +func codexExecCallLine(t *testing.T, callID, input string) string { + t.Helper() + b, err := json.Marshal(map[string]any{ + "type": "response_item", + "payload": map[string]any{ + "type": "custom_tool_call", + "call_id": callID, + "name": "exec", + "input": input, + }, + }) + if err != nil { + t.Fatal(err) + } + return string(b) +} + +func codexExecOutputLine(t *testing.T, callID string, texts ...string) string { + t.Helper() + var out []map[string]any + for _, tx := range texts { + out = append(out, map[string]any{"type": "input_text", "text": tx}) + } + b, err := json.Marshal(map[string]any{ + "type": "response_item", + "payload": map[string]any{ + "type": "custom_tool_call_output", + "call_id": callID, + "output": out, + }, + }) + if err != nil { + t.Fatal(err) + } + return string(b) +} + +// Fixture shape confirmed live against this machine's own ~/.codex/sessions +// rollout files on 2026-07-28 (see activity.go's CodexActivity doc comment) — +// not a guess at the schema. +func TestCodexActivityParsesRealRolloutShape(t *testing.T) { + path := writeJSONL(t, []string{ + codexExecCallLine(t, "c1", `const r = await tools.exec_command({cmd:"go test ./...","workdir":"/repo"}); text(r.output)`), + codexExecOutputLine(t, "c1", "Script completed\nWall time 0.1 seconds\nOutput:\n", "FAIL"), + codexExecCallLine(t, "c2", `const r = await tools.exec_command({cmd:"git commit -am x"}); text(r.output)`), + codexExecOutputLine(t, "c2", "Script completed\n"), + codexPatchApplyLine(t, "exec-1", true, "/repo/main.go"), + codexExecCallLine(t, "c3", `const r = await tools.update_plan({plan:[]}); text(r)`), + codexExecOutputLine(t, "c3", "Script error:\nSyntaxError: bad"), + }) + calls, err := CodexActivity(path) + if err != nil { + t.Fatal(err) + } + if len(calls) != 4 { + t.Fatalf("calls=%+v, want 4 (update_plan call resolved with empty kind/key, not dropped)", calls) + } + if calls[3].Kind != "" || calls[3].Key != "" { + t.Fatalf("calls[3]=%+v, want empty kind/key for a call with no embedded cmd:", calls[3]) + } + if calls[0].Kind != "command" || calls[0].Key != "go test ./..." || !calls[0].IsTest || !calls[0].Success { + t.Fatalf("calls[0]=%+v", calls[0]) + } + if calls[1].Kind != "command" || calls[1].Key != "git commit -am x" || !calls[1].Success { + t.Fatalf("calls[1]=%+v", calls[1]) + } + if calls[2].Kind != "file" || calls[2].Key != "/repo/main.go" || !calls[2].Success { + t.Fatalf("calls[2]=%+v", calls[2]) + } +} + +func TestCodexActivityMarksScriptErrorAsFailure(t *testing.T) { + path := writeJSONL(t, []string{ + codexExecCallLine(t, "c1", `const r = await tools.exec_command({cmd:"pytest"}); text(r.output)`), + codexExecOutputLine(t, "c1", "Script error:\napply_patch verification failed"), + }) + calls, err := CodexActivity(path) + if err != nil { + t.Fatal(err) + } + if len(calls) != 1 || calls[0].Success { + t.Fatalf("calls=%+v, want one failed call", calls) + } +} + func TestDetectThrashConsecutiveTestFailures(t *testing.T) { calls := []ToolCall{ {Name: "Bash", Kind: "command", Key: "go test ./...", Success: false, IsTest: true}, diff --git a/progress.md b/progress.md index 476c05b..224df06 100644 --- a/progress.md +++ b/progress.md @@ -398,9 +398,40 @@ Fixed so far: its unverified parser is checked; opencode not until a real per-tool-call source is found). +- **S11 (CodexActivity verified against a live rollout) — closed, 2026-07-28.** + The previously-unverified `function_call`/`function_call_output` shape + turned out not to exist in any real Codex rollout on this machine. Rewrote + `CodexActivity` against the real shape found in `~/.codex/sessions`: + `event_msg`/`patch_apply_end` for file edits (has `changes`+`success` + directly, no pairing needed), and `response_item`/`custom_tool_call` named + `"exec"` (a freeform JS-scripted tool, not flat arguments — commands are + extracted from an embedded `cmd:"..."` via regex) paired with + `custom_tool_call_output`, whose failure signal is the observed literal + prefix `"Script error:"` in the output text. New tests + (`TestCodexActivityParsesRealRolloutShape`, + `TestCodexActivityMarksScriptErrorAsFailure`) use fixtures built from the + confirmed shape; also manually re-ran the parser against a real multi- + hundred-line rollout file and spot-checked the output by eye. See + AUDIT.md's S11 section for the full writeup and the one known remaining + limitation (only the first command in a chained exec script is extracted, + so `DetectMilestone` can miss a commit that isn't the first call in its + script). + +- **Live status check, 2026-07-28** — the stuck task named throughout this + file (`06FT6CKD9Y98AZRX6X8K3QXFZG`) is no longer "stuck" from Orchestra's + own point of view: it now reads `state: "failed"` (retries exhausted, + `MaxAttempts` hit it). But the underlying herdr pane (`wA:p1`, opencode) + is still live and `agent_status: "blocked"` — confirmed via a direct + `agent.get` probe against `192.168.1.105:9245` — meaning the orphaned-pane + prediction in AUDIT.md's B2/B5 sections was correct: the router gave up + and moved on, but nothing ever released or killed the actual agent. Left + untouched deliberately (no `pane.close`/`release_agent` call made) — user + was asked and chose to leave it for now rather than have it cleaned up in + this session. + Not yet started: live verification of Phase 1 occupancy against a real -session, verifying `CodexActivity`'s parser shape against a live rollout, and -the two-machine federation run. See `AUDIT.md` for the full plan. +session, and the two-machine federation run. See `AUDIT.md` for the full +plan. **Phase 0 done (2026-07-27):** this box has live TCP reachability to the real herdr instance at `192.168.1.105:9245` — verified by hand (raw JSON-RPC