Skip to content

Commit 82fdf95

Browse files
wesmclaude
andcommitted
fix: address code review findings across 2 reviews
- install.sh: gate SHA256SUMS download+verify behind AGENTSVIEW_SKIP_CHECKSUM so skip flag bypasses download failure - codex.go: use structural emptiness checks (len(arg.Map())==0) instead of raw string comparison for empty args detection - Add tests: empty-args fallback to input, write_stdin formatting, export legacy Codex markers (parallel, view_image, etc.) - go fmt formatting fixes in config_test.go, analytics_test.go Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 9f257b8 commit 82fdf95

7 files changed

Lines changed: 139 additions & 11 deletions

File tree

internal/config/config_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,4 +245,3 @@ func TestResolveDataDir_DefaultAndEnvOverride(t *testing.T) {
245245
t.Errorf("ResolveDataDir = %q, want %q", dir, custom)
246246
}
247247
}
248-

internal/db/analytics_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1916,12 +1916,12 @@ func TestTimeFilter(t *testing.T) {
19161916
Message{
19171917
SessionID: "tf1", Ordinal: 0, Role: "user",
19181918
Timestamp: "2024-06-01T09:05:00Z",
1919-
Content: "hello", ContentLength: 5,
1919+
Content: "hello", ContentLength: 5,
19201920
},
19211921
Message{
19221922
SessionID: "tf1", Ordinal: 1, Role: "assistant",
19231923
Timestamp: "2024-06-01T09:10:00Z",
1924-
Content: "hi", ContentLength: 2,
1924+
Content: "hi", ContentLength: 2,
19251925
},
19261926
)
19271927

@@ -1934,7 +1934,7 @@ func TestTimeFilter(t *testing.T) {
19341934
insertMessages(t, d, Message{
19351935
SessionID: "tf2", Ordinal: 0, Role: "user",
19361936
Timestamp: "2024-06-01T14:05:00Z",
1937-
Content: "world", ContentLength: 5,
1937+
Content: "world", ContentLength: 5,
19381938
})
19391939

19401940
insertSession(t, d, "tf3", "proj", func(s *Session) {
@@ -1946,7 +1946,7 @@ func TestTimeFilter(t *testing.T) {
19461946
insertMessages(t, d, Message{
19471947
SessionID: "tf3", Ordinal: 0, Role: "user",
19481948
Timestamp: "2024-06-03T09:30:00Z",
1949-
Content: "test", ContentLength: 4,
1949+
Content: "test", ContentLength: 4,
19501950
})
19511951

19521952
f := AnalyticsFilter{

internal/parser/codex.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -210,9 +210,14 @@ func parseCodexFunctionArgs(
210210
}
211211
return gjson.Result{}, s
212212
default:
213-
if arg.IsObject() || arg.IsArray() {
214-
trimmed := strings.TrimSpace(arg.Raw)
215-
if trimmed == "{}" || trimmed == "[]" {
213+
if arg.IsObject() {
214+
if len(arg.Map()) == 0 {
215+
continue
216+
}
217+
return arg, ""
218+
}
219+
if arg.IsArray() {
220+
if len(arg.Array()) == 0 {
216221
continue
217222
}
218223
return arg, ""

internal/parser/parser_test.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1054,6 +1054,78 @@ func TestParseCodexSession(t *testing.T) {
10541054
}
10551055
},
10561056
},
1057+
{
1058+
name: "empty arguments falls through to input",
1059+
content: testjsonl.JoinJSONL(
1060+
testjsonl.CodexSessionMetaJSON(
1061+
"fc-empty-args", "/tmp", "user", tsEarly,
1062+
),
1063+
testjsonl.CodexMsgJSON(
1064+
"user", "run command", tsEarlyS1,
1065+
),
1066+
testjsonl.CodexFunctionCallFieldsJSON(
1067+
"exec_command",
1068+
map[string]any{},
1069+
`{"cmd":"ls -la"}`,
1070+
tsEarlyS5,
1071+
),
1072+
),
1073+
wantID: "codex:fc-empty-args",
1074+
wantMsgs: 2,
1075+
check: func(
1076+
t *testing.T, _ *ParsedSession,
1077+
msgs []ParsedMessage,
1078+
) {
1079+
t.Helper()
1080+
want := "[Bash]\n$ ls -la"
1081+
if msgs[1].Content != want {
1082+
t.Errorf(
1083+
"content = %q, want %q",
1084+
msgs[1].Content, want,
1085+
)
1086+
}
1087+
},
1088+
},
1089+
{
1090+
name: "write_stdin formats with session and chars",
1091+
content: testjsonl.JoinJSONL(
1092+
testjsonl.CodexSessionMetaJSON(
1093+
"fc-stdin", "/tmp", "user", tsEarly,
1094+
),
1095+
testjsonl.CodexMsgJSON(
1096+
"user", "send input", tsEarlyS1,
1097+
),
1098+
testjsonl.CodexFunctionCallArgsJSON(
1099+
"write_stdin",
1100+
map[string]any{
1101+
"session_id": "sess-42",
1102+
"chars": "yes\n",
1103+
},
1104+
tsEarlyS5,
1105+
),
1106+
),
1107+
wantID: "codex:fc-stdin",
1108+
wantMsgs: 2,
1109+
check: func(
1110+
t *testing.T, _ *ParsedSession,
1111+
msgs []ParsedMessage,
1112+
) {
1113+
t.Helper()
1114+
want := "[Bash: stdin -> sess-42]\nyes\\n"
1115+
if msgs[1].Content != want {
1116+
t.Errorf(
1117+
"content = %q, want %q",
1118+
msgs[1].Content, want,
1119+
)
1120+
}
1121+
if msgs[1].ToolCalls[0].Category != "Bash" {
1122+
t.Errorf(
1123+
"category = %q, want Bash",
1124+
msgs[1].ToolCalls[0].Category,
1125+
)
1126+
}
1127+
},
1128+
},
10571129
{
10581130
name: "large message within scanner limit",
10591131
content: testjsonl.JoinJSONL(

internal/server/export_test.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,30 @@ func TestFormatContentForExport_Escaping(t *testing.T) {
244244
[]string{`class="tool-block"`},
245245
nil,
246246
},
247+
{
248+
"LegacyCodexExecCommand",
249+
"[exec_command]\n$ ls -la",
250+
[]string{`class="tool-block"`},
251+
nil,
252+
},
253+
{
254+
"LegacyCodexParallel",
255+
"[parallel]\nrunning tasks",
256+
[]string{`class="tool-block"`},
257+
nil,
258+
},
259+
{
260+
"LegacyCodexViewImage",
261+
"[view_image]\nimage.png",
262+
[]string{`class="tool-block"`},
263+
nil,
264+
},
265+
{
266+
"LegacyCodexUpdatePlan",
267+
"[update_plan]\nnew plan",
268+
[]string{`class="tool-block"`},
269+
nil,
270+
},
247271
{
248272
"EmptyInput",
249273
"",

internal/testjsonl/testjsonl.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,30 @@ func CodexFunctionCallArgsJSON(
154154
return mustMarshal(m)
155155
}
156156

157+
// CodexFunctionCallFieldsJSON returns a Codex function_call
158+
// response_item with explicit arguments and input fields.
159+
func CodexFunctionCallFieldsJSON(
160+
name string, arguments, input any, timestamp string,
161+
) string {
162+
payload := map[string]any{
163+
"type": "function_call",
164+
"name": name,
165+
"call_id": "call_test",
166+
}
167+
if arguments != nil {
168+
payload["arguments"] = arguments
169+
}
170+
if input != nil {
171+
payload["input"] = input
172+
}
173+
m := map[string]any{
174+
"type": "response_item",
175+
"timestamp": timestamp,
176+
"payload": payload,
177+
}
178+
return mustMarshal(m)
179+
}
180+
157181
// JoinJSONL joins JSON lines with newlines and appends a
158182
// trailing newline.
159183
func JoinJSONL(lines ...string) string {

scripts/install.sh

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,10 +126,14 @@ install_from_release() {
126126
return 1
127127
fi
128128

129-
if ! download "${base_url}/SHA256SUMS" "$tmpdir/SHA256SUMS"; then
130-
error "Failed to download SHA256SUMS. Cannot verify binary integrity."
129+
if [ "${AGENTSVIEW_SKIP_CHECKSUM:-0}" != "1" ]; then
130+
if ! download "${base_url}/SHA256SUMS" "$tmpdir/SHA256SUMS"; then
131+
error "Failed to download SHA256SUMS. Cannot verify binary integrity."
132+
fi
133+
verify_checksum "$tmpdir/release.tar.gz" "$tmpdir/SHA256SUMS" "$filename"
134+
else
135+
warn "Checksum verification skipped (AGENTSVIEW_SKIP_CHECKSUM=1)"
131136
fi
132-
verify_checksum "$tmpdir/release.tar.gz" "$tmpdir/SHA256SUMS" "$filename"
133137

134138
info "Extracting..."
135139
tar -xzf "$tmpdir/release.tar.gz" -C "$tmpdir"

0 commit comments

Comments
 (0)