From 32ec7cd096ffbefedbf9135ddf986fdb6de62916 Mon Sep 17 00:00:00 2001 From: packetloss404 Date: Sat, 5 Sep 2026 10:39:40 -0500 Subject: [PATCH 1/6] build: migrate the golangci-lint config to the v2 schema The lint job has not linted anything for at least the last three runs on main. It was not failing on findings, it was failing to start: can't load config: unsupported version of the configuration: "" CI pins golangci-lint v2.9.0, which requires a version key and a restructured file, while .golangci.yml was still v1 format. So the repo has had a lint job, a Makefile target and no lint coverage. This is the output of `golangci-lint migrate` run by v2.9.0 itself rather than a hand conversion. It keeps all eight linters: errcheck, govet, ineffassign, misspell, staticcheck and unused stay linters, and gofmt and goimports move to the new top-level formatters section. The exclusion presets it adds reproduce v1's default exclusions, which were on by default before and would otherwise silently switch off. The one key it drops is run.timeout: 5m. That is correct rather than lossy: v1 defaulted to a 1m timeout and 5m was raising it, while v2 disables the timeout by default, so dropping it preserves the intent instead of reimposing a limit. This commit only makes the linter run. It reports 54 pre-existing findings that nothing has ever enforced; the commits that follow clear them. Co-Authored-By: Claude Opus 5 --- .golangci.yml | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 5543644..ac9abd8 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,17 +1,28 @@ +version: "2" linters: enable: - - govet - - errcheck - - staticcheck - - unused - - ineffassign - misspell + settings: + errcheck: + check-type-assertions: true + exclusions: + generated: lax + presets: + - comments + - common-false-positives + - legacy + - std-error-handling + paths: + - third_party$ + - builtin$ + - examples$ +formatters: + enable: - gofmt - goimports - -linters-settings: - errcheck: - check-type-assertions: true - -run: - timeout: 5m + exclusions: + generated: lax + paths: + - third_party$ + - builtin$ + - examples$ From 5acc272a94131d906d8b6024c921b3bfd6a9e097 Mon Sep 17 00:00:00 2001 From: packetloss404 Date: Sat, 5 Sep 2026 10:48:59 -0500 Subject: [PATCH 2/6] all: group imports the way the repository already does The formatters now run, and goimports had findings in nineteen files where a packetcode import had drifted into the stdlib block or sat against the third-party block with no separator. goimports on its own pulls a stray local import out of stdlib but leaves it in a group of its own, ahead of third-party, which is not what any of these files were reaching for: each already had a packetcode group at the bottom. Setting goimports local-prefixes teaches the formatter that packetcode is local, so it sorts those imports into the trailing group instead, and the six files with a stranded single import have it merged into the group that was already there. The result is the convention the files were already using -- stdlib, then third-party, then packetcode -- and running the formatter again changes nothing. Co-Authored-By: Claude Opus 5 --- .golangci.yml | 4 ++++ cmd/packetcode/main.go | 2 +- cmd/packetcode/runtime.go | 2 +- cmd/packetcode/sugar_login_test.go | 5 +++-- internal/computers/registry.go | 2 +- internal/computers/ssh_backend.go | 5 +++-- internal/jobs/subsession.go | 2 +- internal/jobs/worker.go | 2 +- internal/mcp/death_reason_test.go | 3 ++- internal/mcp/manager_test.go | 3 ++- internal/mcp/tool_test.go | 3 ++- internal/provider/codex/codex.go | 1 + internal/provider/openai/openai.go | 1 + internal/provider/sugar/runtime_test.go | 3 ++- internal/provider/sugar/sugar_test.go | 3 ++- internal/session/persistence_test.go | 3 ++- internal/tools/execute_command_test.go | 3 ++- internal/tools/runtime_backend_test.go | 3 ++- internal/ui/components/approval/approval.go | 2 +- 19 files changed, 34 insertions(+), 18 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index ac9abd8..ef8b0b9 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -20,6 +20,10 @@ formatters: enable: - gofmt - goimports + settings: + goimports: + local-prefixes: + - github.com/packetcode/packetcode exclusions: generated: lax paths: diff --git a/cmd/packetcode/main.go b/cmd/packetcode/main.go index 14f2379..60e6258 100644 --- a/cmd/packetcode/main.go +++ b/cmd/packetcode/main.go @@ -11,7 +11,6 @@ import ( "crypto/sha256" "flag" "fmt" - "github.com/packetcode/packetcode/internal/provider" "io" "os" "time" @@ -26,6 +25,7 @@ import ( "github.com/packetcode/packetcode/internal/git" "github.com/packetcode/packetcode/internal/jobs" "github.com/packetcode/packetcode/internal/permissions" + "github.com/packetcode/packetcode/internal/provider" "github.com/packetcode/packetcode/internal/tools" "github.com/packetcode/packetcode/internal/ui/theme" "github.com/packetcode/packetcode/internal/workflow" diff --git a/cmd/packetcode/runtime.go b/cmd/packetcode/runtime.go index 33db291..5e9f33e 100644 --- a/cmd/packetcode/runtime.go +++ b/cmd/packetcode/runtime.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "github.com/packetcode/packetcode/internal/toolout" "io" "sync" "time" @@ -20,6 +19,7 @@ import ( "github.com/packetcode/packetcode/internal/provider" "github.com/packetcode/packetcode/internal/session" "github.com/packetcode/packetcode/internal/skills" + "github.com/packetcode/packetcode/internal/toolout" "github.com/packetcode/packetcode/internal/tools" ) diff --git a/cmd/packetcode/sugar_login_test.go b/cmd/packetcode/sugar_login_test.go index a7a14ac..e4ebaa6 100644 --- a/cmd/packetcode/sugar_login_test.go +++ b/cmd/packetcode/sugar_login_test.go @@ -12,10 +12,11 @@ import ( "testing" "time" - "github.com/packetcode/packetcode/internal/config" - "github.com/packetcode/packetcode/internal/provider/sugar" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/packetcode/packetcode/internal/config" + "github.com/packetcode/packetcode/internal/provider/sugar" ) func TestSugarLoginDiscoversLiveModelsAndPersistsProvider(t *testing.T) { diff --git a/internal/computers/registry.go b/internal/computers/registry.go index 937d57e..af81b21 100644 --- a/internal/computers/registry.go +++ b/internal/computers/registry.go @@ -4,7 +4,6 @@ import ( "crypto/sha256" "encoding/json" "fmt" - "github.com/packetcode/packetcode/internal/atomicfile" "os" "path" "path/filepath" @@ -13,6 +12,7 @@ import ( "sync" "time" + "github.com/packetcode/packetcode/internal/atomicfile" "github.com/packetcode/packetcode/internal/compat" ) diff --git a/internal/computers/ssh_backend.go b/internal/computers/ssh_backend.go index 07fa602..73728fb 100644 --- a/internal/computers/ssh_backend.go +++ b/internal/computers/ssh_backend.go @@ -14,11 +14,12 @@ import ( "sync" "time" - "github.com/packetcode/packetcode/internal/diaglog" - "github.com/packetcode/packetcode/internal/procrun" "github.com/pkg/sftp" "golang.org/x/crypto/ssh" "golang.org/x/crypto/ssh/agent" + + "github.com/packetcode/packetcode/internal/diaglog" + "github.com/packetcode/packetcode/internal/procrun" ) const ( diff --git a/internal/jobs/subsession.go b/internal/jobs/subsession.go index a4daab5..aeae464 100644 --- a/internal/jobs/subsession.go +++ b/internal/jobs/subsession.go @@ -3,11 +3,11 @@ package jobs import ( "encoding/json" "fmt" - "github.com/packetcode/packetcode/internal/atomicfile" "os" "path/filepath" "time" + "github.com/packetcode/packetcode/internal/atomicfile" "github.com/packetcode/packetcode/internal/provider" "github.com/packetcode/packetcode/internal/session" ) diff --git a/internal/jobs/worker.go b/internal/jobs/worker.go index 50d7b8d..c35749b 100644 --- a/internal/jobs/worker.go +++ b/internal/jobs/worker.go @@ -3,7 +3,6 @@ package jobs import ( "context" "fmt" - "github.com/packetcode/packetcode/internal/toolout" "runtime/debug" "strings" "time" @@ -12,6 +11,7 @@ import ( "github.com/packetcode/packetcode/internal/computers" "github.com/packetcode/packetcode/internal/provider" "github.com/packetcode/packetcode/internal/session" + "github.com/packetcode/packetcode/internal/toolout" "github.com/packetcode/packetcode/internal/tools" ) diff --git a/internal/mcp/death_reason_test.go b/internal/mcp/death_reason_test.go index 8dfae44..e3d3342 100644 --- a/internal/mcp/death_reason_test.go +++ b/internal/mcp/death_reason_test.go @@ -9,9 +9,10 @@ import ( "testing" "time" - "github.com/packetcode/packetcode/internal/testwait" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/packetcode/packetcode/internal/testwait" ) // exitingChild starts a real process that exits with the given status and diff --git a/internal/mcp/manager_test.go b/internal/mcp/manager_test.go index 812024f..96f0370 100644 --- a/internal/mcp/manager_test.go +++ b/internal/mcp/manager_test.go @@ -12,9 +12,10 @@ import ( "testing" "time" - "github.com/packetcode/packetcode/internal/testwait" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/packetcode/packetcode/internal/testwait" ) // stubBinaryPath is set by TestMain after compiling internal/mcp/cmd/stub. diff --git a/internal/mcp/tool_test.go b/internal/mcp/tool_test.go index 1fe69b9..60f52dc 100644 --- a/internal/mcp/tool_test.go +++ b/internal/mcp/tool_test.go @@ -8,9 +8,10 @@ import ( "testing" "time" - "github.com/packetcode/packetcode/internal/tools" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/packetcode/packetcode/internal/tools" ) // TestMcpTool_AdaptsNameAsProviderSafe asserts the adapter exposes diff --git a/internal/provider/codex/codex.go b/internal/provider/codex/codex.go index f327b6c..edd49fe 100644 --- a/internal/provider/codex/codex.go +++ b/internal/provider/codex/codex.go @@ -15,6 +15,7 @@ import ( "sync" "github.com/charmbracelet/lipgloss" + "github.com/packetcode/packetcode/internal/provider" "github.com/packetcode/packetcode/internal/provider/codexauth" "github.com/packetcode/packetcode/internal/provider/responses" diff --git a/internal/provider/openai/openai.go b/internal/provider/openai/openai.go index 249aafa..f9b138e 100644 --- a/internal/provider/openai/openai.go +++ b/internal/provider/openai/openai.go @@ -13,6 +13,7 @@ import ( "strings" "github.com/charmbracelet/lipgloss" + "github.com/packetcode/packetcode/internal/provider" "github.com/packetcode/packetcode/internal/provider/openaicompat" "github.com/packetcode/packetcode/internal/provider/responses" diff --git a/internal/provider/sugar/runtime_test.go b/internal/provider/sugar/runtime_test.go index 7f717a9..8fccea2 100644 --- a/internal/provider/sugar/runtime_test.go +++ b/internal/provider/sugar/runtime_test.go @@ -9,9 +9,10 @@ import ( "sync/atomic" "testing" - "github.com/packetcode/packetcode/internal/provider" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/packetcode/packetcode/internal/provider" ) func TestRuntimeClientIsInertUntilExplicitlyEnabled(t *testing.T) { diff --git a/internal/provider/sugar/sugar_test.go b/internal/provider/sugar/sugar_test.go index 4523c71..4870e9f 100644 --- a/internal/provider/sugar/sugar_test.go +++ b/internal/provider/sugar/sugar_test.go @@ -8,9 +8,10 @@ import ( "net/http/httptest" "testing" - "github.com/packetcode/packetcode/internal/provider" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/packetcode/packetcode/internal/provider" ) func TestListModelsUsesLiveSugarCatalogAndPrioritizesConduit(t *testing.T) { diff --git a/internal/session/persistence_test.go b/internal/session/persistence_test.go index c607eb3..a91e639 100644 --- a/internal/session/persistence_test.go +++ b/internal/session/persistence_test.go @@ -6,9 +6,10 @@ import ( "strings" "testing" - "github.com/packetcode/packetcode/internal/provider" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/packetcode/packetcode/internal/provider" ) // New and Load used to hand out the manager's own *Session, so a caller could diff --git a/internal/tools/execute_command_test.go b/internal/tools/execute_command_test.go index 9274edf..c0d80aa 100644 --- a/internal/tools/execute_command_test.go +++ b/internal/tools/execute_command_test.go @@ -12,9 +12,10 @@ import ( "testing" "time" - "github.com/packetcode/packetcode/internal/procrun" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/packetcode/packetcode/internal/procrun" ) // shellEcho emits a portable echo invocation for the test command. diff --git a/internal/tools/runtime_backend_test.go b/internal/tools/runtime_backend_test.go index cdb85a0..ed62dc5 100644 --- a/internal/tools/runtime_backend_test.go +++ b/internal/tools/runtime_backend_test.go @@ -9,9 +9,10 @@ import ( "path" "testing" - "github.com/packetcode/packetcode/internal/computers" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/packetcode/packetcode/internal/computers" ) type memoryRuntimeBackend struct { diff --git a/internal/ui/components/approval/approval.go b/internal/ui/components/approval/approval.go index b886eeb..c8148fd 100644 --- a/internal/ui/components/approval/approval.go +++ b/internal/ui/components/approval/approval.go @@ -9,7 +9,6 @@ package approval import ( "encoding/json" "fmt" - "github.com/packetcode/packetcode/internal/ui/terminaltext" "strings" tea "github.com/charmbracelet/bubbletea" @@ -17,6 +16,7 @@ import ( "github.com/packetcode/packetcode/internal/provider" "github.com/packetcode/packetcode/internal/tools" + "github.com/packetcode/packetcode/internal/ui/terminaltext" "github.com/packetcode/packetcode/internal/ui/theme" ) From 3fa96701ad50567ee29bd077de0821055ed670ac Mon Sep 17 00:00:00 2001 From: packetloss404 Date: Sat, 5 Sep 2026 11:06:38 -0500 Subject: [PATCH 3/6] all: check the errors errcheck found, and scope type assertions to shipping code Sixty-five errcheck findings, none of which anything had ever reported. The real ones are fixed. cmd/packetcode/main.go registered a cleanup that called jobsMgr.Shutdown and then returned nil unconditionally, so a job manager that failed to stop cleanly reported success on the way out; it now returns what Shutdown says. Deferred Shutdown and CloseHandle calls in tests discard the result explicitly instead of silently. The setup writes in mentions_test.go are asserted, because a test whose fixture failed to write is not testing what it claims to. Two production type assertions in internal/procrun were unchecked. trackedJobs only ever holds a windows.Handle so neither can fail, but a value of the wrong type now falls through to the same outcome as "no job tracked" rather than panicking, which is the honest reading of an impossible state. The remaining forty are type assertions in tests, and those are excluded by a scoped rule rather than rewritten. check-type-assertions stays on so that shipping code is held to it, but in a test a panicking assertion already fails the test and names the offending type, while turning forty inline assertions like assert.Equal(t, root, tool.(*ReadFileTool).Root) into two-statement checks would cost real readability for no safety. The rule matches only errcheck's unnamed message, so unchecked errors stay enforced in tests as well. That last part is a policy choice rather than a fix, and it is one line to reverse if the project would rather rewrite the assertions. Co-Authored-By: Claude Opus 5 --- .golangci.yml | 13 +++++++++++ cmd/packetcode/main.go | 3 +-- internal/app/app_jobs_e2e_test.go | 2 +- internal/app/app_lifecycle_parity_test.go | 6 ++--- internal/app/app_slashcmd_test.go | 10 ++++----- internal/app/mentions_test.go | 22 +++++++++--------- internal/mcp/manager_test.go | 10 ++++----- internal/procrun/process_windows.go | 27 ++++++++++++++--------- internal/procrun/process_windows_test.go | 2 +- internal/tools/fetch_test.go | 4 ++-- 10 files changed, 59 insertions(+), 40 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index ef8b0b9..7e89abe 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -7,6 +7,19 @@ linters: check-type-assertions: true exclusions: generated: lax + rules: + # errcheck's check-type-assertions is on so that an unchecked + # assertion in shipping code is a finding rather than a panic in + # front of a user. In a test a panicking assertion already fails the + # test with the offending type in the message, and rewriting forty + # inline assertions into two-statement checks would cost readability + # for no safety. This exempts type assertions only: errcheck reports + # those without a function name, so unchecked *errors* stay enforced + # in tests too. + - path: _test\.go + linters: + - errcheck + text: ^Error return value is not checked$ presets: - comments - common-false-positives diff --git a/cmd/packetcode/main.go b/cmd/packetcode/main.go index 60e6258..d74f436 100644 --- a/cmd/packetcode/main.go +++ b/cmd/packetcode/main.go @@ -455,8 +455,7 @@ func run(providerOverride, modelOverride, resumeID string, trust bool, permissio return tools.NewCollectAgentResultsTool(jobsMgr.AsToolsSpawner(), parentJobID, parentDepth) }) runtime.AddCleanup(func() error { - jobsMgr.Shutdown(5 * time.Second) - return nil + return jobsMgr.Shutdown(5 * time.Second) }) toolReg.Register(tools.NewSpawnAgentTool(jobsMgr.AsToolsSpawner(), "", 0)) diff --git a/internal/app/app_jobs_e2e_test.go b/internal/app/app_jobs_e2e_test.go index 00a6a5c..4ea579b 100644 --- a/internal/app/app_jobs_e2e_test.go +++ b/internal/app/app_jobs_e2e_test.go @@ -148,7 +148,7 @@ func TestE2E_SpawnAgentToolViaSlashCommand(t *testing.T) { if err != nil { t.Fatalf("jobs.NewManager: %v", err) } - defer mgr.Shutdown(2 * time.Second) + defer func() { _ = mgr.Shutdown(2 * time.Second) }() // Build a minimal App by hand so we don't have to stand up every // Deps field. The fields we need for this test are: deps.Jobs, diff --git a/internal/app/app_lifecycle_parity_test.go b/internal/app/app_lifecycle_parity_test.go index b8aa721..cac92d2 100644 --- a/internal/app/app_lifecycle_parity_test.go +++ b/internal/app/app_lifecycle_parity_test.go @@ -28,7 +28,7 @@ func TestFirstVisibleProgressStopsThinkingSpinner(t *testing.T) { func TestLeftArrowOpensAgentsOnlyFromEmptyIdleInput(t *testing.T) { r := newTestApp(t) mgr := wireJobsManagerForSlashTest(t, r) - t.Cleanup(func() { mgr.Shutdown(2 * time.Second) }) + t.Cleanup(func() { _ = mgr.Shutdown(2 * time.Second) }) r.app.input.Reset() r.app.handleKey(tea.KeyMsg{Type: tea.KeyLeft}) if !r.app.agentView.Visible() { @@ -46,7 +46,7 @@ func TestLeftArrowOpensAgentsOnlyFromEmptyIdleInput(t *testing.T) { func TestAgentWorkspaceTaskPromptCanClearReturnAndSpawn(t *testing.T) { r := newTestApp(t) mgr := wireJobsManagerForSlashTest(t, r) - t.Cleanup(func() { mgr.Shutdown(2 * time.Second) }) + t.Cleanup(func() { _ = mgr.Shutdown(2 * time.Second) }) r.app.showAgentView() r.app.handleKey(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("n")}) @@ -84,7 +84,7 @@ func TestAgentWorkspaceTaskPromptCanClearReturnAndSpawn(t *testing.T) { func TestAgentWorkspaceListActionsAreNotSwallowedByTaskInput(t *testing.T) { r := newTestApp(t) mgr := wireJobsManagerForSlashTest(t, r) - t.Cleanup(func() { mgr.Shutdown(2 * time.Second) }) + t.Cleanup(func() { _ = mgr.Shutdown(2 * time.Second) }) _, _ = r.app.handleSpawnCommand([]string{"inspect the renderer"}) r.app.showAgentView() diff --git a/internal/app/app_slashcmd_test.go b/internal/app/app_slashcmd_test.go index 4bfafea..bda975f 100644 --- a/internal/app/app_slashcmd_test.go +++ b/internal/app/app_slashcmd_test.go @@ -1446,7 +1446,7 @@ func TestApp_Queue_ListDropAndClear(t *testing.T) { func TestApp_Agents_ListUsesBackgroundJobs(t *testing.T) { r := newTestApp(t) mgr := wireJobsManagerForSlashTest(t, r) - defer mgr.Shutdown(2 * time.Second) + defer func() { _ = mgr.Shutdown(2 * time.Second) }() snap, spawnErr := mgr.Spawn(jobs.SpawnRequest{ Prompt: "audit fixtures", @@ -1470,7 +1470,7 @@ func TestApp_Agents_ListUsesBackgroundJobs(t *testing.T) { func TestApp_Agents_DetailOpensJobsPanel(t *testing.T) { r := newTestApp(t) mgr := wireJobsManagerForSlashTest(t, r) - defer mgr.Shutdown(2 * time.Second) + defer func() { _ = mgr.Shutdown(2 * time.Second) }() snap, spawnErr := mgr.Spawn(jobs.SpawnRequest{ Prompt: "inspect flaky test", @@ -1490,7 +1490,7 @@ func TestApp_Agents_DetailOpensJobsPanel(t *testing.T) { func TestApp_Agents_NotFoundUsesAgentLabel(t *testing.T) { r := newTestApp(t) mgr := wireJobsManagerForSlashTest(t, r) - defer mgr.Shutdown(2 * time.Second) + defer func() { _ = mgr.Shutdown(2 * time.Second) }() r.app.handleSlashCommand("agents", []string{"missing"}, "/agents missing") convContains(t, r.app, "[agent:missing not found]") @@ -1499,7 +1499,7 @@ func TestApp_Agents_NotFoundUsesAgentLabel(t *testing.T) { func TestApp_Agents_ViewDoesNotOverrideJobsPanelOverlay(t *testing.T) { r := newTestApp(t) mgr := wireJobsManagerForSlashTest(t, r) - defer mgr.Shutdown(2 * time.Second) + defer func() { _ = mgr.Shutdown(2 * time.Second) }() snap, spawnErr := mgr.Spawn(jobs.SpawnRequest{ Prompt: "inspect overlay", @@ -1633,7 +1633,7 @@ func TestApp_ResizeRecomposesBuiltInStatusLine(t *testing.T) { func TestApp_HandleJobUpdate_IgnoresStaleSeqAndDedupesTerminal(t *testing.T) { r := newTestApp(t) mgr := wireJobsManagerForSlashTest(t, r) - defer mgr.Shutdown(2 * time.Second) + defer func() { _ = mgr.Shutdown(2 * time.Second) }() snap, spawnErr := mgr.Spawn(jobs.SpawnRequest{ Prompt: "finish once", diff --git a/internal/app/mentions_test.go b/internal/app/mentions_test.go index 08ce2bc..9cc45bc 100644 --- a/internal/app/mentions_test.go +++ b/internal/app/mentions_test.go @@ -5,13 +5,15 @@ import ( "path/filepath" "strings" "testing" + + "github.com/stretchr/testify/require" ) func TestExpandFileMentions(t *testing.T) { root := t.TempDir() - os.WriteFile(filepath.Join(root, "main.go"), []byte("package main\nfunc main(){}"), 0o644) - os.MkdirAll(filepath.Join(root, "internal"), 0o755) - os.WriteFile(filepath.Join(root, "internal", "util.go"), []byte("package internal"), 0o644) + require.NoError(t, os.WriteFile(filepath.Join(root, "main.go"), []byte("package main\nfunc main(){}"), 0o644)) + require.NoError(t, os.MkdirAll(filepath.Join(root, "internal"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(root, "internal", "util.go"), []byte("package internal"), 0o644)) prompt := "explain @main.go and @internal/util.go please" expanded, attached := expandFileMentions(prompt, root) @@ -32,7 +34,7 @@ func TestExpandFileMentions(t *testing.T) { func TestExpandFileMentions_TrailingPunctuationAndDedup(t *testing.T) { root := t.TempDir() - os.WriteFile(filepath.Join(root, "a.go"), []byte("AAA"), 0o644) + require.NoError(t, os.WriteFile(filepath.Join(root, "a.go"), []byte("AAA"), 0o644)) // Same file twice + trailing period. _, attached := expandFileMentions("look at @a.go. and again @a.go", root) if len(attached) != 1 || attached[0] != "a.go" { @@ -42,7 +44,7 @@ func TestExpandFileMentions_TrailingPunctuationAndDedup(t *testing.T) { func TestExpandFileMentions_IgnoresMissingAndEscapes(t *testing.T) { root := t.TempDir() - os.WriteFile(filepath.Join(root, "real.go"), []byte("X"), 0o644) + require.NoError(t, os.WriteFile(filepath.Join(root, "real.go"), []byte("X"), 0o644)) prompt := "email me @someone and read @nope.go and @../../etc/passwd but also @real.go" expanded, attached := expandFileMentions(prompt, root) if len(attached) != 1 || attached[0] != "real.go" { @@ -63,7 +65,7 @@ func TestExpandFileMentions_NoMentions(t *testing.T) { func TestExpandFileMentions_SkipsBinary(t *testing.T) { root := t.TempDir() - os.WriteFile(filepath.Join(root, "bin.dat"), []byte{1, 2, 0, 3, 4}, 0o644) + require.NoError(t, os.WriteFile(filepath.Join(root, "bin.dat"), []byte{1, 2, 0, 3, 4}, 0o644)) _, attached := expandFileMentions("check @bin.dat", root) if len(attached) != 0 { t.Fatalf("binary file should not be attached: %v", attached) @@ -72,8 +74,8 @@ func TestExpandFileMentions_SkipsBinary(t *testing.T) { func TestExpandFileMentions_RefusesDotEnv(t *testing.T) { root := t.TempDir() - os.WriteFile(filepath.Join(root, ".env"), []byte("PACKETCODE_OPENAI_API_KEY=sk-secret"), 0o600) - os.WriteFile(filepath.Join(root, ".env.example"), []byte("PACKETCODE_OPENAI_API_KEY="), 0o600) + require.NoError(t, os.WriteFile(filepath.Join(root, ".env"), []byte("PACKETCODE_OPENAI_API_KEY=sk-secret"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(root, ".env.example"), []byte("PACKETCODE_OPENAI_API_KEY="), 0o600)) expanded, attached := expandFileMentions("keys in @.env and @.env.example", root) if len(attached) != 1 || attached[0] != ".env.example" { t.Fatalf("attached = %v, want only .env.example", attached) @@ -87,7 +89,7 @@ func TestExpandFileMentions_RefusesSymlinkEscape(t *testing.T) { outside := t.TempDir() root := t.TempDir() secret := filepath.Join(outside, "id_rsa") - os.WriteFile(secret, []byte("PRIVATE KEY MATERIAL"), 0o600) + require.NoError(t, os.WriteFile(secret, []byte("PRIVATE KEY MATERIAL"), 0o600)) link := filepath.Join(root, "notes.txt") if err := os.Symlink(secret, link); err != nil { t.Skipf("symlinks unavailable here: %v", err) @@ -103,7 +105,7 @@ func TestExpandFileMentions_RefusesSymlinkEscape(t *testing.T) { func TestExpandFileMentions_AllowsSymlinkInsideRoot(t *testing.T) { root := t.TempDir() - os.WriteFile(filepath.Join(root, "real.go"), []byte("package real"), 0o644) + require.NoError(t, os.WriteFile(filepath.Join(root, "real.go"), []byte("package real"), 0o644)) if err := os.Symlink(filepath.Join(root, "real.go"), filepath.Join(root, "alias.go")); err != nil { t.Skipf("symlinks unavailable here: %v", err) } diff --git a/internal/mcp/manager_test.go b/internal/mcp/manager_test.go index 96f0370..3b3e1d8 100644 --- a/internal/mcp/manager_test.go +++ b/internal/mcp/manager_test.go @@ -100,7 +100,7 @@ func TestClient_DeathReason_PreservesNonZeroExit(t *testing.T) { LogDir: t.TempDir(), ClientInfo: ClientInfo{Name: "packetcode-test", Version: "0.0.0"}, }) - defer mgr.Shutdown(2 * time.Second) + defer func() { _ = mgr.Shutdown(2 * time.Second) }() reports := mgr.Start(context.Background()) require.Len(t, reports, 1) require.Equal(t, "running", reports[0].Status, reports[0].Err) @@ -157,7 +157,7 @@ func TestManager_Start_MixedStatuses(t *testing.T) { LogDir: logDir, ClientInfo: ClientInfo{Name: "packetcode-test", Version: "0.0.0"}, }) - defer mgr.Shutdown(2 * time.Second) + defer func() { _ = mgr.Shutdown(2 * time.Second) }() reports := mgr.Start(context.Background()) require.Len(t, reports, 3) @@ -194,7 +194,7 @@ func TestManager_StartAgainClosesPreviousClients(t *testing.T) { LogDir: logDir, ClientInfo: ClientInfo{Name: "packetcode-test", Version: "0.0.0"}, }) - defer mgr.Shutdown(2 * time.Second) + defer func() { _ = mgr.Shutdown(2 * time.Second) }() reports := mgr.Start(context.Background()) require.Equal(t, "running", reports[0].Status, reports[0].Err) @@ -220,7 +220,7 @@ func TestManager_Restart_ReplacesOnlyNamedClient(t *testing.T) { LogDir: t.TempDir(), ClientInfo: ClientInfo{Name: "packetcode-test", Version: "0.0.0"}, }) - defer mgr.Shutdown(2 * time.Second) + defer func() { _ = mgr.Shutdown(2 * time.Second) }() reports := mgr.Start(context.Background()) require.Equal(t, "running", reports[0].Status, reports[0].Err) require.Equal(t, "running", reports[1].Status, reports[1].Err) @@ -273,7 +273,7 @@ func TestManager_Start_ParallelSpawn(t *testing.T) { LogDir: logDir, ClientInfo: ClientInfo{Name: "packetcode-test", Version: "0.0.0"}, }) - defer mgr.Shutdown(2 * time.Second) + defer func() { _ = mgr.Shutdown(2 * time.Second) }() start := time.Now() reports := mgr.Start(context.Background()) diff --git a/internal/procrun/process_windows.go b/internal/procrun/process_windows.go index 18c49ef..c055f3b 100644 --- a/internal/procrun/process_windows.go +++ b/internal/procrun/process_windows.go @@ -31,13 +31,14 @@ func killTree(cmd *exec.Cmd) (KillOutcome, error) { } pid := uint32(cmd.Process.Pid) if value, ok := trackedJobs.LoadAndDelete(cmd); ok { - job := value.(windows.Handle) - err := windows.TerminateJobObject(job, 1) - _ = windows.CloseHandle(job) - if err == nil { - // The job contains the tree, so terminating it is proof rather - // than a best effort. This is the only Confirmed path here. - return KillOutcome{Method: KillMethodJobObject, Confirmed: true}, nil + if job, isHandle := value.(windows.Handle); isHandle { + err := windows.TerminateJobObject(job, 1) + _ = windows.CloseHandle(job) + if err == nil { + // The job contains the tree, so terminating it is proof rather + // than a best effort. This is the only Confirmed path here. + return KillOutcome{Method: KillMethodJobObject, Confirmed: true}, nil + } } } out := KillOutcome{Method: KillMethodTreeWalk} @@ -91,7 +92,7 @@ func trackTree(cmd *exec.Cmd) error { _ = windows.CloseHandle(job) return fmt.Errorf("open process for job: %w", err) } - defer windows.CloseHandle(processHandle) + defer func() { _ = windows.CloseHandle(processHandle) }() if err := windows.AssignProcessToJobObject(job, processHandle); err != nil { _ = windows.CloseHandle(job) return fmt.Errorf("assign process to job: %w", err) @@ -112,7 +113,11 @@ func releaseTree(cmd *exec.Cmd) (KillOutcome, error) { if !ok { return KillOutcome{Method: KillMethodNone, Confirmed: true}, nil } - if err := windows.CloseHandle(value.(windows.Handle)); err != nil { + handle, isHandle := value.(windows.Handle) + if !isHandle { + return KillOutcome{Method: KillMethodNone, Confirmed: true}, nil + } + if err := windows.CloseHandle(handle); err != nil { return KillOutcome{Method: KillMethodJobObject, Reason: err.Error()}, err } return KillOutcome{Method: KillMethodJobObject, Confirmed: true}, nil @@ -145,7 +150,7 @@ func processChildren() (map[uint32][]uint32, error) { if err != nil { return nil, err } - defer windows.CloseHandle(snap) + defer func() { _ = windows.CloseHandle(snap) }() children := map[uint32][]uint32{} var pe windows.ProcessEntry32 @@ -192,7 +197,7 @@ func terminateProcess(pid uint32) (alive bool, err error) { } return false, fmt.Errorf("open process %d: %w", pid, err) } - defer windows.CloseHandle(h) + defer func() { _ = windows.CloseHandle(h) }() if err := windows.TerminateProcess(h, 1); err != nil { return true, fmt.Errorf("terminate process %d: %w", pid, err) } diff --git a/internal/procrun/process_windows_test.go b/internal/procrun/process_windows_test.go index 28ae7bf..cf4f03f 100644 --- a/internal/procrun/process_windows_test.go +++ b/internal/procrun/process_windows_test.go @@ -57,7 +57,7 @@ func windowsProcessAlive(pid uint32) bool { if err != nil { return false } - defer windows.CloseHandle(handle) + defer func() { _ = windows.CloseHandle(handle) }() result, err := windows.WaitForSingleObject(handle, 0) return err == nil && result == uint32(windows.WAIT_TIMEOUT) } diff --git a/internal/tools/fetch_test.go b/internal/tools/fetch_test.go index c6a8b21..d338ae1 100644 --- a/internal/tools/fetch_test.go +++ b/internal/tools/fetch_test.go @@ -281,7 +281,7 @@ func TestFetch_StripsTerminalControlSequences(t *testing.T) { func TestFetch_RefusesBinaryContentTypes(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "image/png") - w.Write([]byte{0x89, 'P', 'N', 'G', 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x01}) + _, _ = w.Write([]byte{0x89, 'P', 'N', 'G', 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x01}) })) defer srv.Close() @@ -295,7 +295,7 @@ func TestFetch_RefusesBinaryContentTypes(t *testing.T) { func TestFetch_RefusesBinaryBodyLabelledAsText(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "text/plain") - w.Write([]byte{'a', 0x00, 0xff, 0xfe, 'b'}) + _, _ = w.Write([]byte{'a', 0x00, 0xff, 0xfe, 'b'}) })) defer srv.Close() From efb6402879df70113c38e8602ae95fff3db9e705 Mon Sep 17 00:00:00 2001 From: packetloss404 Date: Sat, 5 Sep 2026 11:23:26 -0500 Subject: [PATCH 4/6] all: delete the code nothing uses Eighteen unused findings, all of them genuinely dead rather than false positives from build tags or reflection. Three test helpers had outlived their tests: approvalProvider, a full provider stub with nine methods; scriptedAlias, which existed to give the scripted provider a second slug; and bytesReader, a hand-rolled io.Reader whose comment explains it avoided importing bytes. Nothing refers to any of them. Two pieces of production code go with them. App.startTurnDisplaying was the entry point for a turn whose transcript line differs from what the model receives, but every caller reaches startTurnWith directly, so the wrapper was never on a live path. The App.err field was never read. Registry.loadDir is a two-argument wrapper around loadDirWith that nothing calls; loadScopeDir is the live one. Deleting rather than annotating is deliberate: this is the same call the audit made when it removed internal/tools/atomic.go. Code that no longer has a caller is easier to re-add from history than to keep explaining. Co-Authored-By: Claude Opus 5 --- internal/app/app.go | 7 ------- internal/app/app_cancel_test.go | 29 ----------------------------- internal/jobs/testhelpers_test.go | 9 --------- internal/mcp/testing_test.go | 18 ------------------ internal/skills/skills.go | 4 ---- 5 files changed, 67 deletions(-) diff --git a/internal/app/app.go b/internal/app/app.go index 71a61e8..2268a89 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -269,7 +269,6 @@ type App struct { width int height int streaming bool - err string // Coalesced live tool-output streaming. Chunks (EventToolOutputChunk) // land in toolOutputPending keyed by the running call id; a single @@ -1574,12 +1573,6 @@ func (a *App) startTurn(text string, emitUser bool) (tea.Model, tea.Cmd) { return a.startTurnWith(turnOptions{display: text, text: text, emitUser: emitUser}) } -// startTurnDisplaying starts a turn whose transcript line is not the text the -// model receives, for text the user typed. -func (a *App) startTurnDisplaying(display, text string, emitUser bool) (tea.Model, tea.Cmd) { - return a.startTurnWith(turnOptions{display: display, text: text, emitUser: emitUser}) -} - // turnOptions describes one turn's two texts and where its text came from. type turnOptions struct { // display is the transcript line; text is what the model receives. diff --git a/internal/app/app_cancel_test.go b/internal/app/app_cancel_test.go index 65c1472..ae888f5 100644 --- a/internal/app/app_cancel_test.go +++ b/internal/app/app_cancel_test.go @@ -87,35 +87,6 @@ func (r *releaseProvider) ChatCompletion(ctx context.Context, _ provider.ChatReq return ch, nil } -// approvalProvider emits a write_file tool call on turn 0 — used to -// drive the App into a state where the approval modal is visible. -type approvalProvider struct { - turnIdx int32 -} - -func (approvalProvider) Name() string { return "appr" } -func (approvalProvider) Slug() string { return "appr" } -func (approvalProvider) BrandColor() lipgloss.Color { return lipgloss.Color("#000000") } -func (approvalProvider) ValidateKey(context.Context, string) error { return nil } -func (approvalProvider) ListModels(context.Context) ([]provider.Model, error) { return nil, nil } -func (approvalProvider) Pricing(string) (float64, float64) { return 0, 0 } -func (approvalProvider) ContextWindow(string) int { return 100_000 } -func (approvalProvider) SupportsTools(string) bool { return true } - -func (a *approvalProvider) ChatCompletion(ctx context.Context, _ provider.ChatRequest) (<-chan provider.StreamEvent, error) { - atomic.AddInt32(&a.turnIdx, 1) - ch := make(chan provider.StreamEvent, 8) - go func() { - defer close(ch) - ch <- provider.StreamEvent{Type: provider.EventToolCallStart, ToolCall: &provider.ToolCallDelta{Index: 0, ID: "c1", Name: "fake_write"}} - ch <- provider.StreamEvent{Type: provider.EventToolCallDelta, ToolCall: &provider.ToolCallDelta{Index: 0, ArgumentsDelta: `{}`}} - ch <- provider.StreamEvent{Type: provider.EventToolCallEnd, ToolCall: &provider.ToolCallDelta{Index: 0}} - ch <- provider.StreamEvent{Type: provider.EventDone, Usage: &provider.Usage{InputTokens: 1, OutputTokens: 1}} - <-ctx.Done() - }() - return ch, nil -} - // fakeWriteTool requires approval so the agent's approver.Approve path // fires. Execute blocks on ctx so the test can verify the approval // modal's Hide on cancel without the tool ever running. diff --git a/internal/jobs/testhelpers_test.go b/internal/jobs/testhelpers_test.go index fe59975..9798f41 100644 --- a/internal/jobs/testhelpers_test.go +++ b/internal/jobs/testhelpers_test.go @@ -102,15 +102,6 @@ func (s *scriptedProvider) snapshotRequests() []provider.ChatRequest { return out } -// scriptedAlias registers the scripted provider under a custom slug. -// Useful when tests need multiple providers with distinct slugs. -type scriptedAlias struct { - *scriptedProvider - slug string -} - -func (a *scriptedAlias) Slug() string { return a.slug } - // fakeApprover records every approval call so tests can assert on // the prefixed tool-call name. type fakeApprover struct { diff --git a/internal/mcp/testing_test.go b/internal/mcp/testing_test.go index 3103fef..72581d3 100644 --- a/internal/mcp/testing_test.go +++ b/internal/mcp/testing_test.go @@ -245,21 +245,3 @@ func NewClientWithStub(name string, stub *StubServer, info ClientInfo, timeoutSe info, ) } - -// bytesReader is a tiny io.Reader over a []byte. We avoid importing -// bytes here only to keep the test helper import set minimal. -type bytesReader struct { - buf []byte - off int -} - -func newBytesReader(b []byte) *bytesReader { return &bytesReader{buf: b} } - -func (r *bytesReader) Read(p []byte) (int, error) { - if r.off >= len(r.buf) { - return 0, io.EOF - } - n := copy(p, r.buf[r.off:]) - r.off += n - return n, nil -} diff --git a/internal/skills/skills.go b/internal/skills/skills.go index b85f63d..c18408d 100644 --- a/internal/skills/skills.go +++ b/internal/skills/skills.go @@ -540,10 +540,6 @@ func (r *Registry) loadScopeDir(d skillScope) { r.loadDirWith(d.path, d.source, d.origin, d.foreignProject) } -func (r *Registry) loadDir(dir, source string) { - r.loadDirWith(dir, source, OriginNative, false) -} - func (r *Registry) loadDirWith(dir, source, origin string, foreignProject bool) { entries, err := os.ReadDir(dir) if err != nil { From f36af64ec7409e287b2c9825282405910eb638fd Mon Sep 17 00:00:00 2001 From: packetloss404 Date: Sat, 5 Sep 2026 11:26:32 -0500 Subject: [PATCH 5/6] all: address the staticcheck findings Twenty-nine findings, and the largest group needed care rather than the obvious fix. Fifteen ST1005 hits are error strings beginning with a capital, but every one of them begins with a product name: Conduit, Sugar, Packet Computers. Lowercasing the word would have been wrong, so the messages lead with the condition instead -- "Conduit run idempotency key is invalid" becomes "invalid Conduit run idempotency key". Only two of these strings are asserted anywhere in the tests, and both keep the asserted substring. Four SA1019 hits are bubbles viewport methods that were renamed: LineDown/LineUp/HalfViewDown/HalfViewUp become ScrollDown/ScrollUp/HalfPageDown/HalfPageUp, which are the same behaviour under the current names. The rest are simplifications that say what the code already meant: two guards around TrimSuffix/TrimPrefix that those functions already perform, a conditional assignment folded into its declaration, a De Morgan negation, a struct literal that is a conversion, and two if/else chains over a single value that are switches. Two were dead scaffolding with comments admitting it: an `if false` block "to silence unused-result lint", and a closure whose critical section locked and unlocked around nothing before being discarded with `_ = track // silence unused`. One ST1018 hit is a test string holding real ESC bytes. It now spells them \x1b, which is the same bytes and readable in a diff; the comment above it explaining why the escapes must be literal still holds. golangci-lint run now exits 0. Co-Authored-By: Claude Opus 5 --- cmd/packetcode/main.go | 4 ++-- cmd/packetcode/runtime.go | 2 +- cmd/packetcode/sugar_login.go | 2 +- internal/app/app_jobs_e2e_test.go | 4 ---- internal/app/picker_items.go | 5 +++-- internal/app/setup_test.go | 2 +- internal/app/tui_fixture.go | 7 +++--- internal/jobs/manager_test.go | 7 ------ internal/provider/cache.go | 6 +---- internal/provider/sugar/runtime.go | 22 +++++++++---------- internal/provider/sugar/sugar.go | 4 +--- internal/tools/code_intelligence.go | 5 +---- internal/tools/search_codebase.go | 4 +--- .../ui/components/agentview/agentview_test.go | 2 +- internal/ui/components/jobs/jobs.go | 8 +++---- 15 files changed, 32 insertions(+), 52 deletions(-) diff --git a/cmd/packetcode/main.go b/cmd/packetcode/main.go index d74f436..d52d6fe 100644 --- a/cmd/packetcode/main.go +++ b/cmd/packetcode/main.go @@ -215,7 +215,7 @@ func run(providerOverride, modelOverride, resumeID string, trust bool, permissio var runtimeBackend computers.RuntimeBackend var activeComputer *computers.Computer if !cfg.PacketComputers.IsEnabled() && computerName != "" { - return fmt.Errorf("Packet Computers integration is disabled; enable [packet_computers].enabled or set PACKETCODE_PACKET_COMPUTERS_ENABLED=true") + return fmt.Errorf("the Packet Computers integration is disabled; enable [packet_computers].enabled or set PACKETCODE_PACKET_COMPUTERS_ENABLED=true") } if computerName != "" { computersDir, dirErr := config.ComputersDir() @@ -328,7 +328,7 @@ func run(providerOverride, modelOverride, resumeID string, trust bool, permissio } } disabledComputersError := func() error { - return fmt.Errorf("Packet Computers integration is disabled; enable [packet_computers].enabled or set PACKETCODE_PACKET_COMPUTERS_ENABLED=true") + return fmt.Errorf("the Packet Computers integration is disabled; enable [packet_computers].enabled or set PACKETCODE_PACKET_COMPUTERS_ENABLED=true") } var resolveWorkspace jobs.WorkspaceResolver = func(string) (jobs.Workspace, error) { return jobs.Workspace{}, disabledComputersError() diff --git a/cmd/packetcode/runtime.go b/cmd/packetcode/runtime.go index 5e9f33e..9661325 100644 --- a/cmd/packetcode/runtime.go +++ b/cmd/packetcode/runtime.go @@ -179,7 +179,7 @@ func buildPacketRuntime(ctx context.Context, opts packetRuntimeConfig) (_ *packe activeModel = opts.ModelOverride } if activeProvider == "sugar" && !opts.Config.SugarIsEnabled() && !opts.Config.SugarUsesCustomProvider() { - return nil, fmt.Errorf("Sugar integration is disabled; enable [sugar].enabled or set PACKETCODE_SUGAR_ENABLED=true") + return nil, fmt.Errorf("the Sugar integration is disabled; enable [sugar].enabled or set PACKETCODE_SUGAR_ENABLED=true") } if activeProvider == "" { return nil, fmt.Errorf("no default provider is configured; configure PacketCode before creating a session") diff --git a/cmd/packetcode/sugar_login.go b/cmd/packetcode/sugar_login.go index b258e7d..b4e6d2a 100644 --- a/cmd/packetcode/sugar_login.go +++ b/cmd/packetcode/sugar_login.go @@ -405,7 +405,7 @@ func validateSugarLoginURL(raw string) error { if parsed.Scheme == "http" && (host == "localhost" || host == "127.0.0.1" || host == "::1") { return nil } - return fmt.Errorf("Sugar server must use HTTPS unless it is localhost") + return fmt.Errorf("the Sugar server must use HTTPS unless it is localhost") } func validateSugarVerificationURL(baseURL, verificationURL string) error { diff --git a/internal/app/app_jobs_e2e_test.go b/internal/app/app_jobs_e2e_test.go index 4ea579b..13f94c3 100644 --- a/internal/app/app_jobs_e2e_test.go +++ b/internal/app/app_jobs_e2e_test.go @@ -192,10 +192,6 @@ func TestE2E_SpawnAgentToolViaSlashCommand(t *testing.T) { appMu.Lock() _, _ = app.handleSlashCommand(cmd, args, "/spawn hi") appMu.Unlock() - if false { - // silence unused-result lint; return values are meaningful to - // the Bubble Tea loop but not to us here - } // (1) Conversation got the queued echo. appMu.Lock() diff --git a/internal/app/picker_items.go b/internal/app/picker_items.go index 2386181..73a8694 100644 --- a/internal/app/picker_items.go +++ b/internal/app/picker_items.go @@ -41,9 +41,10 @@ func providerItems(regs []provider.Provider, cfg *config.Config, activeSlug stri slug := p.Slug() defModel := "" keyStatus := "(no key)" - if slug == "ollama" { + switch slug { + case "ollama": keyStatus = "local" - } else if slug == "codex" { + case "codex": keyStatus = "ChatGPT login" } if cfg != nil { diff --git a/internal/app/setup_test.go b/internal/app/setup_test.go index 686fec5..a8d4485 100644 --- a/internal/app/setup_test.go +++ b/internal/app/setup_test.go @@ -58,7 +58,7 @@ func TestPromptProvider_UsesCanonicalDisplayOrder(t *testing.T) { openAI := strings.Index(text, "openai") anthropic := strings.Index(text, "anthropic") custom := strings.Index(text, "aa-custom") - if openAI < 0 || anthropic < 0 || custom < 0 || !(openAI < anthropic && anthropic < custom) { + if openAI < 0 || anthropic < 0 || custom < 0 || openAI >= anthropic || anthropic >= custom { t.Fatalf("unexpected provider order:\n%s", text) } } diff --git a/internal/app/tui_fixture.go b/internal/app/tui_fixture.go index 6ab0fc0..7552744 100644 --- a/internal/app/tui_fixture.go +++ b/internal/app/tui_fixture.go @@ -135,12 +135,13 @@ func (m *tuiFixtureModel) View() string { } status := m.topbar.View() + "\n " + renderPermModeHint(m.mode) in := m.input.View() - if m.state == "approval" { + switch m.state { + case "approval": in = m.input.ViewBlurred() - } else if m.state == "agents" { + case "agents": in = m.input.ViewWithPlaceholder("press n to dispatch a new agent") status = "" - } else if m.state == "workflows" { + case "workflows": in = "" status = "" } diff --git a/internal/jobs/manager_test.go b/internal/jobs/manager_test.go index a093f8c..231d3bf 100644 --- a/internal/jobs/manager_test.go +++ b/internal/jobs/manager_test.go @@ -76,13 +76,6 @@ func TestManager_ConcurrencyLimit(t *testing.T) { mu sync.Mutex peak int ) - track := func(_ Snapshot) { - mu.Lock() - // peak reads ActiveCount via the manager — but we don't have - // the manager here. Instead, sample on the next OnUpdate. - mu.Unlock() - } - _ = track // silence unused mgr, _ := newTestManager(t, prov, func(c *Config) { c.MaxConcurrent = 2 }) diff --git a/internal/provider/cache.go b/internal/provider/cache.go index 5f874bb..7489b14 100644 --- a/internal/provider/cache.go +++ b/internal/provider/cache.go @@ -25,11 +25,7 @@ func CachePrefixFingerprint(systemPrompt string, tools []ToolDefinition) string canonical := make([]canonicalTool, 0, len(canonicalDefinitions)) for _, tool := range canonicalDefinitions { - canonical = append(canonical, canonicalTool{ - Name: tool.Name, - Description: tool.Description, - Parameters: tool.Parameters, - }) + canonical = append(canonical, canonicalTool(tool)) } payload, err := json.Marshal(struct { diff --git a/internal/provider/sugar/runtime.go b/internal/provider/sugar/runtime.go index 6113e8f..722f143 100644 --- a/internal/provider/sugar/runtime.go +++ b/internal/provider/sugar/runtime.go @@ -179,10 +179,10 @@ func (c *RuntimeClient) StartRun(ctx context.Context, start RuntimeRunStart) (*R return nil, nil } if !validOpaqueID(start.IdempotencyKey, 128) { - return nil, fmt.Errorf("Conduit run idempotency key is invalid") + return nil, fmt.Errorf("invalid Conduit run idempotency key") } if start.Request.Model != DefaultModel { - return nil, fmt.Errorf("Conduit shadow runs require model %q", DefaultModel) + return nil, fmt.Errorf("a Conduit shadow run requires model %q", DefaultModel) } body, err := openaicompat.MarshalChatRequest(start.Request) if err != nil { @@ -240,7 +240,7 @@ func (c *RuntimeClient) Continue(ctx context.Context, runID, idempotencyKey stri return nil, nil } if !validOpaqueID(runID, 128) || !validOpaqueID(idempotencyKey, 128) { - return nil, fmt.Errorf("Conduit continue identifiers are invalid") + return nil, fmt.Errorf("invalid Conduit continue identifiers") } path := "/conduit/runs/" + url.PathEscape(runID) + "/continue" status, responseBody, err := c.postJSON(ctx, path, idempotencyKey, nil) @@ -323,31 +323,31 @@ func runtimeStatusError(operation string, status int, body []byte) error { func validateRuntimeEvent(event RuntimeEvent) error { if !validOpaqueID(event.RunID, 128) || !validOpaqueID(event.IdempotencyKey, 128) { - return fmt.Errorf("Conduit event identifiers are invalid") + return fmt.Errorf("invalid Conduit event identifiers") } if event.Seq < 1 || event.Seq > 1_000_000 { - return fmt.Errorf("Conduit event seq is invalid") + return fmt.Errorf("invalid Conduit event seq") } switch event.Type { case RuntimeToolResult, RuntimeValidation, RuntimeProgress, RuntimeBlocked, RuntimeProvider: default: - return fmt.Errorf("Conduit event type is invalid") + return fmt.Errorf("invalid Conduit event type") } if event.ToolCategory != "" && !validToolCategory(event.ToolCategory) { - return fmt.Errorf("Conduit event tool category is invalid") + return fmt.Errorf("invalid Conduit event tool category") } if event.FailureKind != "" && !validFailureKind(event.FailureKind) { - return fmt.Errorf("Conduit event failure kind is invalid") + return fmt.Errorf("invalid Conduit event failure kind") } providerFailure := event.FailureKind == RuntimeProviderRateLimited || event.FailureKind == RuntimeProviderUnavailable || event.FailureKind == RuntimeProviderAmbiguous if (event.Type == RuntimeProvider) != providerFailure && event.FailureKind != "" { - return fmt.Errorf("Conduit provider failures require event type provider, and provider events accept provider failures only") + return fmt.Errorf("a Conduit provider failure requires event type provider, and provider events accept provider failures only") } if event.FailureFingerprint != "" && !validSHA256Fingerprint(event.FailureFingerprint) { - return fmt.Errorf("Conduit failure fingerprint must be sha256:<64 lowercase hex characters>") + return fmt.Errorf("a Conduit failure fingerprint must be sha256:<64 lowercase hex characters>") } if !validOptionalInt(event.ExitCode, -32_768, 32_767) || !validOptionalInt(event.NewFailures, 0, 100_000) || !validOptionalInt(event.FilesTouched, 0, 100_000) || !validOptionalInt(event.DurationMS, 0, 86_400_000) { - return fmt.Errorf("Conduit event counter is out of range") + return fmt.Errorf("the Conduit event counter is out of range") } if event.Type == RuntimeValidation && event.Success != nil && *event.Success { if event.FailureKind != "" || (event.ExitCode != nil && *event.ExitCode != 0) || (event.NewFailures != nil && *event.NewFailures != 0) { diff --git a/internal/provider/sugar/sugar.go b/internal/provider/sugar/sugar.go index cbfde14..88940fa 100644 --- a/internal/provider/sugar/sugar.go +++ b/internal/provider/sugar/sugar.go @@ -148,9 +148,7 @@ func NormalizeBaseURL(raw string) string { if strings.HasSuffix(base, "/api/v1") { return base } - if strings.HasSuffix(base, "/v1") { - base = strings.TrimSuffix(base, "/v1") - } + base = strings.TrimSuffix(base, "/v1") return base + "/api/v1" } diff --git a/internal/tools/code_intelligence.go b/internal/tools/code_intelligence.go index ae15dd3..bd22a53 100644 --- a/internal/tools/code_intelligence.go +++ b/internal/tools/code_intelligence.go @@ -280,10 +280,7 @@ func (t *FindReferencesTool) Execute(ctx context.Context, raw json.RawMessage) ( return ToolResult{Content: "find_references: " + inferErr.Error(), IsError: true}, nil } limit := boundedInt(p.MaxResults, defaultReferenceLimit, maxReferenceLimit) - includeDeclaration := true - if rawContainsFalse(raw, "include_declaration") { - includeDeclaration = false - } + includeDeclaration := !rawContainsFalse(raw, "include_declaration") matches, truncated, err := collectReferences(ctx, t.Root, p.ScopePath, symbol, p.FileGlob, includeDeclaration, limit) if err != nil { return ToolResult{Content: "find_references: " + err.Error(), IsError: true}, nil diff --git a/internal/tools/search_codebase.go b/internal/tools/search_codebase.go index 233af0d..6b8d82b 100644 --- a/internal/tools/search_codebase.go +++ b/internal/tools/search_codebase.go @@ -126,9 +126,7 @@ func (t *SearchCodebaseTool) searchWithBackend(ctx context.Context, pattern, glo return ctx.Err() } rel := path.Join(dir, entry.Name) - if strings.HasPrefix(rel, "./") { - rel = strings.TrimPrefix(rel, "./") - } + rel = strings.TrimPrefix(rel, "./") if entry.IsDir { if shouldSkipDir(entry.Name) { continue diff --git a/internal/ui/components/agentview/agentview_test.go b/internal/ui/components/agentview/agentview_test.go index 813d7f1..77cd8f9 100644 --- a/internal/ui/components/agentview/agentview_test.go +++ b/internal/ui/components/agentview/agentview_test.go @@ -333,7 +333,7 @@ func TestTruncate_MeasuresDisplayWidthNotRunes(t *testing.T) { // when there is no TTY, so styled output in a test would carry no escapes // at all and the regression could not be expressed. The goldens hit this // path because they are captured through a real PTY. - styled := "a1b2c3d4 running focused tests" + styled := "\x1b[38;5;33ma1b2c3d4\x1b[0m \x1b[38;5;250mrunning focused tests\x1b[0m" width := ansi.StringWidth(styled) if width >= len([]rune(styled)) { t.Fatalf("precondition: styled string should carry invisible escapes, width=%d runes=%d", width, len([]rune(styled))) diff --git a/internal/ui/components/jobs/jobs.go b/internal/ui/components/jobs/jobs.go index 26cf98e..8000504 100644 --- a/internal/ui/components/jobs/jobs.go +++ b/internal/ui/components/jobs/jobs.go @@ -140,10 +140,10 @@ func (m Model) Update(msg tea.Msg) (Model, tea.Cmd) { m.visible = false return m, nil case "j", "down": - m.vp.LineDown(1) + m.vp.ScrollDown(1) return m, nil case "k", "up": - m.vp.LineUp(1) + m.vp.ScrollUp(1) return m, nil case "g": m.vp.GotoTop() @@ -152,10 +152,10 @@ func (m Model) Update(msg tea.Msg) (Model, tea.Cmd) { m.vp.GotoBottom() return m, nil case "pgdown", " ": - m.vp.HalfViewDown() + m.vp.HalfPageDown() return m, nil case "pgup": - m.vp.HalfViewUp() + m.vp.HalfPageUp() return m, nil } } From b5b6351ce123f70b249dacee09752b10ba13b978 Mon Sep 17 00:00:00 2001 From: packetloss404 Date: Sat, 5 Sep 2026 11:35:41 -0500 Subject: [PATCH 6/6] procrun: check the POSIX group handle the way the Windows one is checked releaseTree on POSIX asserted trackedGroups' value to int without checking it, the exact counterpart of the two assertions already fixed in process_windows.go. It was missed because linting on Windows never compiles the !windows files, so the finding only appeared once CI ran on Linux. Same treatment as the Windows side: the map only ever holds a pgid, so this cannot fail, and a value of the wrong type now yields the same outcome as no group being tracked rather than panicking. Verified by running golangci-lint under GOOS=linux, darwin and windows, which is what should have been done before the first push. All three report zero issues. Co-Authored-By: Claude Opus 5 --- internal/procrun/process_posix.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/internal/procrun/process_posix.go b/internal/procrun/process_posix.go index 4b31056..d61ae5a 100644 --- a/internal/procrun/process_posix.go +++ b/internal/procrun/process_posix.go @@ -100,5 +100,12 @@ func releaseTree(cmd *exec.Cmd) (KillOutcome, error) { if !ok { return KillOutcome{Method: KillMethodNone, Confirmed: true}, nil } - return signalGroup(value.(int), true) + // trackedGroups only ever holds a pgid, so this cannot fail. A value + // of the wrong type means nothing usable is tracked, which is the same + // outcome as no group being tracked at all. + pgid, isPGID := value.(int) + if !isPGID { + return KillOutcome{Method: KillMethodNone, Confirmed: true}, nil + } + return signalGroup(pgid, true) }