From a9a7a17b11645ca8a382064d18df1926171c4e04 Mon Sep 17 00:00:00 2001 From: Snider Date: Sat, 8 Aug 2026 14:45:41 +0100 Subject: [PATCH 1/2] fix(lint): clear all 109 findings, decided site by site MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Uncapped count 109 before, 0 after. Every category adjudicated rather than silenced: no //nolint added except one, and that one carries a deadline. unused, 28, every one dead scaffolding in test files. Twenty-two were forwarding shims in compat_adapters_test.go — methods on PrepSubsystem that call the package-level function of the same name, of which fifty-six are used and these were not. The rest were a pipelineForgeMetaReader nothing constructed, three fetch-loop helpers, and boolPtr, assertNotSame and assertSPDXHeader. Gone, not silenced. staticcheck, 22 down to 0, of which eight died with the code above. SA1012 x11 — nil passed as a Context. Every one in tests, all replaced with context.TODO(); a nil Context is a panic waiting for any callee that reads from it. ST1019 — the MCP SDK imported twice in one file, once aliased and once not, with both spellings in use. Collapsed onto the alias. S1009 x2 — `x == nil || len(x) == 0`, where len(nil) is already 0. QF1002 x4 — switch { case r.URL.Path == "..." } turned into a tagged switch. Checked each block was uniform first; all four were. SA1019 x2 — httputil Director, deprecated since Go 1.26 and superseded since 1.20, migrated to Rewrite. Not a rename: Rewrite receives both the inbound and outbound request, so client headers cannot be forwarded upstream by accident, which is what Director was replaced for. SetURL and SetXForwarded restore what NewSingleHostReverseProxy's own Director did. The last SA1019 is session.Log, and it is documented rather than fixed. MCP protocol 2026-07-28 (SEP-2577) retires the logging feature outright; there is no successor call, so this cannot be a mechanical swap. The window is at least twelve months, so it must move before ~2027-07-28, and the comment says so along with the two candidates — a server notification, or folding the lines into the status:// resource this file already publishes. Marked with the one nolint in this change, with the reason and the date. errcheck, 59 down to 0, and the split is the point. Silence is correct for a read cursor, a response body, a notification whose listener may be gone, a `git rebase --abort` where there may be no rebase to abort, and a cleanup of a directory that may already be deleted. Every one now says so in a comment rather than being bare. Reporting is correct where silence loses work, and those get core.Warn, the idiom already in these files: writeStatusResult x9 — the status file is what the monitor polls; a silent failure leaves a workspace looking stuck forever. performAsyncIfRegistered x6 — a follow-up that fails to dispatch skips a pipeline stage and nothing downstream notices. workspaces.Set x7 — Set only fails on a locked or sealed registry, so a failure means a workspace stops being tracked. Action.Run x4 — runner.start, stop and kill; a lifecycle transition that fails silently leaves the runner in an unknown state. fs.Write x2, CommitToJournal, HTTPPost x3, JSONUnmarshalString, mirror push, gh pr ready, planCleanup — each loses the artefact or the step it exists to produce. Two edits landed on the wrong line while scripting this and are worth naming: one wrapped a doc comment as if it were a call, and one overwrote a `if !getResult.OK {` guard. Both were caught by the compiler within the same minute, and the second was restored from git rather than reconstructed from memory. Patching by line number needs descending order; ascending shifts every site after the first insertion. go build, go vet, go test and gofmt all clean. Co-Authored-By: Virgil --- go/pkg/agentic/commands_test.go | 10 +-- go/pkg/agentic/compat_adapters_test.go | 89 ------------------------ go/pkg/agentic/dispatch.go | 10 ++- go/pkg/agentic/dispatch_sync.go | 6 +- go/pkg/agentic/dispatch_sync_test.go | 15 ++-- go/pkg/agentic/fetch_loop_test.go | 62 ----------------- go/pkg/agentic/handlers.go | 42 +++++++++-- go/pkg/agentic/ingest.go | 5 +- go/pkg/agentic/message.go | 6 +- go/pkg/agentic/mirror.go | 5 +- go/pkg/agentic/persist.go | 8 ++- go/pkg/agentic/pipeline_commands_test.go | 5 -- go/pkg/agentic/plan_retention.go | 5 +- go/pkg/agentic/platform.go | 2 +- go/pkg/agentic/pr.go | 6 +- go/pkg/agentic/prep.go | 39 ++++++++--- go/pkg/agentic/prep_extra_test.go | 14 ++-- go/pkg/agentic/qa.go | 4 +- go/pkg/agentic/queue.go | 6 +- go/pkg/agentic/repo_sync.go | 4 +- go/pkg/agentic/repo_sync_cov_test.go | 2 +- go/pkg/agentic/resume.go | 6 +- go/pkg/agentic/review_queue.go | 9 ++- go/pkg/agentic/review_queue_cov_test.go | 2 +- go/pkg/agentic/runner.go | 5 +- go/pkg/agentic/shutdown.go | 15 +++- go/pkg/agentic/shutdown_example_test.go | 8 ++- go/pkg/agentic/shutdown_test.go | 7 +- go/pkg/agentic/status.go | 6 +- go/pkg/agentic/status_test.go | 2 +- go/pkg/agentic/test_assertions_test.go | 15 ---- go/pkg/agentic/transport.go | 13 ++-- go/pkg/agentic/verify.go | 33 +++++++-- go/pkg/agentic/watch.go | 9 ++- go/pkg/agentic/workspace_stats_test.go | 9 +-- go/pkg/lib/lib_test.go | 12 ---- go/pkg/monitor/monitor.go | 17 ++++- go/pkg/opencode/imports_test.go | 4 +- go/pkg/opencode/proxy.go | 20 ++++-- go/pkg/runner/runner.go | 25 +++++-- 40 files changed, 279 insertions(+), 283 deletions(-) diff --git a/go/pkg/agentic/commands_test.go b/go/pkg/agentic/commands_test.go index 158a17e0..49adc597 100644 --- a/go/pkg/agentic/commands_test.go +++ b/go/pkg/agentic/commands_test.go @@ -163,13 +163,13 @@ func TestCommandsforge_CmdIssueCreate_Good_WithLabelsAndMilestone(t *testing.T) callPaths := []string{} srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { callPaths = append(callPaths, r.URL.Path) - switch { - case r.URL.Path == "/api/v1/repos/core/go-io/milestones": + switch r.URL.Path { + case "/api/v1/repos/core/go-io/milestones": w.Write([]byte(core.JSONMarshalString([]map[string]any{ {"id": 1, "title": "v0.8.0"}, {"id": 2, "title": "v0.9.0"}, }))) - case r.URL.Path == "/api/v1/repos/core/go-io/labels": + case "/api/v1/repos/core/go-io/labels": w.Write([]byte(core.JSONMarshalString([]map[string]any{ {"id": 10, "name": "agentic"}, {"id": 11, "name": "bug"}, @@ -1187,8 +1187,8 @@ func TestCommands_CmdScan_Bad_NoForgeToken(t *testing.T) { func TestCommands_CmdScan_Ugly_EmptyResults(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch { - case r.URL.Path == "/api/v1/orgs/core/repos": + switch r.URL.Path { + case "/api/v1/orgs/core/repos": _, _ = w.Write([]byte(core.JSONMarshalString([]map[string]any{ {"name": "go-io"}, }))) diff --git a/go/pkg/agentic/compat_adapters_test.go b/go/pkg/agentic/compat_adapters_test.go index 0a747554..851bf95f 100644 --- a/go/pkg/agentic/compat_adapters_test.go +++ b/go/pkg/agentic/compat_adapters_test.go @@ -9,22 +9,10 @@ import ( "github.com/modelcontextprotocol/go-sdk/mcp" ) -func (s *PrepSubsystem) dispatch(ctx context.Context, request *mcp.CallToolRequest, input DispatchInput) (*mcp.CallToolResult, DispatchOutput, error) { - return dispatch(s, ctx, request, input) -} - -func (s *PrepSubsystem) watch(ctx context.Context, request *mcp.CallToolRequest, input WatchInput) (*mcp.CallToolResult, WatchOutput, error) { - return watch(s, ctx, request, input) -} - func (s *PrepSubsystem) status(ctx context.Context, request *mcp.CallToolRequest, input StatusInput) (*mcp.CallToolResult, StatusOutput, error) { return status(s, ctx, request, input) } -func (s *PrepSubsystem) createEpic(ctx context.Context, request *mcp.CallToolRequest, input EpicInput) (*mcp.CallToolResult, EpicOutput, error) { - return createEpic(s, ctx, request, input) -} - func (s *PrepSubsystem) createPR(ctx context.Context, request *mcp.CallToolRequest, input CreatePRInput) (*mcp.CallToolResult, CreatePROutput, error) { return createPR(s, ctx, request, input) } @@ -65,26 +53,6 @@ func (s *PrepSubsystem) runWorkspaceLanguagePrep(ctx context.Context, workspaceD return runWorkspaceLanguagePrep(s, ctx, workspaceDir, repoDir) } -func (s *PrepSubsystem) templateCreatePlan(ctx context.Context, request *mcp.CallToolRequest, input TemplateCreatePlanInput) (*mcp.CallToolResult, TemplateCreatePlanOutput, error) { - return templateCreatePlan(s, ctx, request, input) -} - -func (s *PrepSubsystem) planFromIssue(ctx context.Context, request *mcp.CallToolRequest, input PlanFromIssueInput) (*mcp.CallToolResult, PlanFromIssueOutput, error) { - return planFromIssue(s, ctx, request, input) -} - -func (s *PrepSubsystem) dispatchStart(ctx context.Context, request *mcp.CallToolRequest, input ShutdownInput) (*mcp.CallToolResult, ShutdownOutput, error) { - return dispatchStart(s, ctx, request, input) -} - -func (s *PrepSubsystem) shutdownGraceful(ctx context.Context, request *mcp.CallToolRequest, input ShutdownInput) (*mcp.CallToolResult, ShutdownOutput, error) { - return shutdownGraceful(s, ctx, request, input) -} - -func (s *PrepSubsystem) shutdownNow(ctx context.Context, request *mcp.CallToolRequest, input ShutdownInput) (*mcp.CallToolResult, ShutdownOutput, error) { - return shutdownNow(s, ctx, request, input) -} - func (s *PrepSubsystem) pipelineEpicCreate(ctx context.Context, input PipelineEpicCreateInput) (PipelineEpicCreateOutput, error) { return pipelineEpicCreate(s, ctx, input) } @@ -117,10 +85,6 @@ func (s *PrepSubsystem) pipelineFixReviews(ctx context.Context, input PipelineFi return pipelineFixReviews(s, ctx, input) } -func (s *PrepSubsystem) pipelineFixConflicts(ctx context.Context, input PipelineFixInput) (PipelineFixOutput, error) { - return pipelineFixConflicts(s, ctx, input) -} - func (s *PrepSubsystem) pipelineFixFormat(ctx context.Context, input PipelineFixInput) (PipelineFixOutput, error) { return pipelineFixFormat(s, ctx, input) } @@ -133,26 +97,10 @@ func (s *PrepSubsystem) pipelineOnboard(ctx context.Context, input PipelineOnboa return pipelineOnboard(s, ctx, input) } -func (s *PrepSubsystem) pipelineOnboardDispatchDirect(ctx context.Context, input PipelineOnboardInput, issues []PipelineIssueRef) ([]PipelineIssueRef, error) { - return pipelineOnboardDispatchDirect(s, ctx, input, issues) -} - func (s *PrepSubsystem) pipelineTrainingCapture(ctx context.Context, input PipelineTrainingCaptureInput) (PipelineTrainingCaptureOutput, error) { return pipelineTrainingCapture(s, ctx, input) } -func (s *PrepSubsystem) pipelineTrainingReadDiff(ctx context.Context, org, repo string, number int, meta PipelinePRMeta) (string, string, error) { - return pipelineTrainingReadDiff(s, ctx, org, repo, number, meta) -} - -func (s *PrepSubsystem) pipelineTrainingReadGitDiff(ctx context.Context, org, repo string, meta PipelinePRMeta) (string, string, error) { - return pipelineTrainingReadGitDiff(s, ctx, org, repo, meta) -} - -func (s *PrepSubsystem) pipelineMonitorWithReader(ctx context.Context, input PipelineMonitorInput, reader *MetaReader) (PipelineMonitorOutput, error) { - return pipelineMonitorWithReader(s, ctx, input, reader) -} - func (s *PrepSubsystem) phaseGet(ctx context.Context, request *mcp.CallToolRequest, input PhaseGetInput) (*mcp.CallToolResult, PhaseOutput, error) { return phaseGet(s, ctx, request, input) } @@ -209,10 +157,6 @@ func (s *PrepSubsystem) prGet(ctx context.Context, request *mcp.CallToolRequest, return prGet(s, ctx, request, input) } -func (s *PrepSubsystem) prList(ctx context.Context, request *mcp.CallToolRequest, input ListPRsInput) (*mcp.CallToolResult, ListPRsOutput, error) { - return prList(s, ctx, request, input) -} - func (s *PrepSubsystem) prMerge(ctx context.Context, request *mcp.CallToolRequest, input PRMergeInput) (*mcp.CallToolResult, PRMergeOutput, error) { return prMerge(s, ctx, request, input) } @@ -241,14 +185,6 @@ func (s *PrepSubsystem) listRepoPRs(ctx context.Context, org, repo, state string return listRepoPRs(s, ctx, org, repo, state) } -func (s *PrepSubsystem) templateList(ctx context.Context, request *mcp.CallToolRequest, input TemplateListInput) (*mcp.CallToolResult, TemplateListOutput, error) { - return templateList(s, ctx, request, input) -} - -func (s *PrepSubsystem) templatePreview(ctx context.Context, request *mcp.CallToolRequest, input TemplatePreviewInput) (*mcp.CallToolResult, TemplatePreviewOutput, error) { - return templatePreview(s, ctx, request, input) -} - func (s *PrepSubsystem) taskCreate(ctx context.Context, request *mcp.CallToolRequest, input TaskCreateInput) (*mcp.CallToolResult, TaskCreateOutput, error) { return taskCreate(s, ctx, request, input) } @@ -277,10 +213,6 @@ func (s *PrepSubsystem) HandleGooglebotVisit(ctx context.Context, pageID, userAg return HandleGooglebotVisit(s, ctx, pageID, userAgent) } -func (s *PrepSubsystem) contentSEOStore() (*store.Store, error) { - return contentSEOStore(s) -} - func (s *PrepSubsystem) contentSEORevisionRecords(storeInstance *store.Store, pageID string, pendingOnly bool) ([]seoRevisionRecord, error) { return contentSEORevisionRecords(s, storeInstance, pageID, pendingOnly) } @@ -301,11 +233,6 @@ func (s *PrepSubsystem) syncPullInput(ctx context.Context, input SyncPullInput) return syncPullInput(s, ctx, input) } -type pipelineForgeMetaReader struct { - subsystem *PrepSubsystem - org string -} - func (c RemoteClient) Initialize(ctx context.Context) (string, error) { return InitializeRemoteClient(c, ctx) } @@ -313,19 +240,3 @@ func (c RemoteClient) Initialize(ctx context.Context) (string, error) { func (c RemoteClient) Call(ctx context.Context, sessionID string, body []byte) ([]byte, error) { return CallRemoteClient(c, ctx, sessionID, body) } - -func (r *pipelineForgeMetaReader) GetPRMeta(ctx context.Context, repo string, prNumber int) (PipelinePRMeta, error) { - return newPipelineForgeMetaReader(r.subsystem, r.org).GetPRMeta(ctx, repo, prNumber) -} - -func (r *pipelineForgeMetaReader) GetEpicMeta(ctx context.Context, repo string, issueNumber int) (PipelineEpicMeta, error) { - return newPipelineForgeMetaReader(r.subsystem, r.org).GetEpicMeta(ctx, repo, issueNumber) -} - -func (r *pipelineForgeMetaReader) GetIssueState(ctx context.Context, repo string, issueNumber int) (PipelineIssueState, error) { - return newPipelineForgeMetaReader(r.subsystem, r.org).GetIssueState(ctx, repo, issueNumber) -} - -func (r *pipelineForgeMetaReader) GetCommentReactions(ctx context.Context, repo string, commentID int64) ([]PipelineReactionMeta, error) { - return newPipelineForgeMetaReader(r.subsystem, r.org).GetCommentReactions(ctx, repo, commentID) -} diff --git a/go/pkg/agentic/dispatch.go b/go/pkg/agentic/dispatch.go index 29311525..c5ef58ef 100644 --- a/go/pkg/agentic/dispatch.go +++ b/go/pkg/agentic/dispatch.go @@ -668,7 +668,9 @@ func (s *PrepSubsystem) broadcastComplete(agent, workspaceDir, finalStatus strin func (s *PrepSubsystem) onAgentComplete(agent, workspaceDir, outputFile string, exitCode int, processStatus, output string) { if output != "" { - fs.Write(outputFile, output) + if r := fs.Write(outputFile, output); !r.OK { + core.Warn("agentic: failed to write agent output", "path", outputFile, "reason", r.Value) + } } repoDir := WorkspaceRepoDir(workspaceDir) @@ -930,7 +932,11 @@ var dispatch = func(s *PrepSubsystem, ctx context.Context, callRequest *mcp.Call Runs: 1, } preserveStatusNote(workspaceDir, workspaceStatus) // keep VZ→OCI downgrade note (SP2.4) - writeStatusResult(workspaceDir, workspaceStatus) + // Reported: the status file is what the monitor polls, so a silent write + // failure leaves the workspace looking stuck indefinitely. + if r := writeStatusResult(workspaceDir, workspaceStatus); !r.OK { + core.Warn("agentic: failed to write workspace status", "reason", r.Value) + } if s.ServiceRuntime != nil { if runnerResult := s.Core().Service("runner"); runnerResult.OK { if runnerSvc, ok := runnerResult.Value.(workspaceTracker); ok { diff --git a/go/pkg/agentic/dispatch_sync.go b/go/pkg/agentic/dispatch_sync.go index ae7a8ffa..9cd9d0d1 100644 --- a/go/pkg/agentic/dispatch_sync.go +++ b/go/pkg/agentic/dispatch_sync.go @@ -107,7 +107,11 @@ func (s *PrepSubsystem) DispatchSync(ctx context.Context, input DispatchSyncInpu if existing, ok := workspaceStatusValue(ReadStatusResult(workspaceDir)); ok { fillMissingDispatchStatus(dispatched, existing) } - writeStatusResult(workspaceDir, dispatched) + // Reported: the status file is what the monitor polls, so a silent write + // failure leaves the workspace looking stuck indefinitely. + if r := writeStatusResult(workspaceDir, dispatched); !r.OK { + core.Warn("agentic: failed to write workspace status", "reason", r.Value) + } core.Print(nil, " pid: %d", pid) core.Print(nil, " waiting for completion...") diff --git a/go/pkg/agentic/dispatch_sync_test.go b/go/pkg/agentic/dispatch_sync_test.go index 829b56ae..15275c1d 100644 --- a/go/pkg/agentic/dispatch_sync_test.go +++ b/go/pkg/agentic/dispatch_sync_test.go @@ -8,7 +8,6 @@ import ( "time" core "dappco.re/go" - "github.com/modelcontextprotocol/go-sdk/mcp" mcpsdk "github.com/modelcontextprotocol/go-sdk/mcp" ) @@ -40,7 +39,7 @@ func TestDispatchsync_HandleDispatchSync_Good_Completed(t *testing.T) { workspaceDir := core.JoinPath(WorkspaceRoot(), "core", "go-io", "task-7") s := &PrepSubsystem{dispatchSyncTick: 10 * time.Millisecond} - s.dispatchSyncPrep = func(ctx context.Context, _ *mcp.CallToolRequest, input PrepInput) (*mcp.CallToolResult, PrepOutput, error) { + s.dispatchSyncPrep = func(ctx context.Context, _ *mcpsdk.CallToolRequest, input PrepInput) (*mcpsdk.CallToolResult, PrepOutput, error) { core.AssertEqual(t, "core", input.Org) core.AssertEqual(t, "go-io", input.Repo) core.AssertEqual(t, "codex", input.Agent) @@ -85,7 +84,7 @@ func TestDispatchsync_HandleDispatchSync_Good_Completed(t *testing.T) { func TestDispatchsync_HandleDispatchSync_Bad_PrepFailure(t *testing.T) { s := &PrepSubsystem{} - s.dispatchSyncPrep = func(context.Context, *mcp.CallToolRequest, PrepInput) (*mcp.CallToolResult, PrepOutput, error) { + s.dispatchSyncPrep = func(context.Context, *mcpsdk.CallToolRequest, PrepInput) (*mcpsdk.CallToolResult, PrepOutput, error) { return nil, PrepOutput{}, core.E("prepWorkspace", "boom", nil) } @@ -101,7 +100,7 @@ func TestDispatchsync_HandleDispatchSync_Bad_PrepFailure(t *testing.T) { func TestDispatchsync_HandleDispatchSync_Bad_PrepIncomplete(t *testing.T) { s := &PrepSubsystem{} - s.dispatchSyncPrep = func(context.Context, *mcp.CallToolRequest, PrepInput) (*mcp.CallToolResult, PrepOutput, error) { + s.dispatchSyncPrep = func(context.Context, *mcpsdk.CallToolRequest, PrepInput) (*mcpsdk.CallToolResult, PrepOutput, error) { return nil, PrepOutput{ Success: false, }, nil @@ -124,7 +123,7 @@ func TestDispatchsync_HandleDispatchSync_Ugly_SpawnFailure(t *testing.T) { workspaceDir := core.JoinPath(WorkspaceRoot(), "core", "go-io", "task-7") s := &PrepSubsystem{dispatchSyncTick: 10 * time.Millisecond} - s.dispatchSyncPrep = func(context.Context, *mcp.CallToolRequest, PrepInput) (*mcp.CallToolResult, PrepOutput, error) { + s.dispatchSyncPrep = func(context.Context, *mcpsdk.CallToolRequest, PrepInput) (*mcpsdk.CallToolResult, PrepOutput, error) { core.RequireTrue(t, fs.EnsureDir(workspaceDir).OK) core.RequireTrue(t, fs.Write(core.JoinPath(workspaceDir, "status.json"), core.JSONMarshalString(&WorkspaceStatus{ Status: "running", @@ -196,7 +195,7 @@ func TestDispatchSync_PrepSubsystem_DispatchSync_Ugly_WritesInitialStatusWhenPre // Real-like prep: creates the workspace but does NOT pre-write status.json // (the actual prepWorkspace doesn't — the async dispatch() writes it after // spawn, which the sync path used to skip → "status not found" crash). - s.dispatchSyncPrep = func(context.Context, *mcp.CallToolRequest, PrepInput) (*mcp.CallToolResult, PrepOutput, error) { + s.dispatchSyncPrep = func(context.Context, *mcpsdk.CallToolRequest, PrepInput) (*mcpsdk.CallToolResult, PrepOutput, error) { core.RequireTrue(t, fs.EnsureDir(workspaceDir).OK) return nil, PrepOutput{Success: true, WorkspaceDir: workspaceDir, Branch: "agent/x", Prompt: "prompt"}, nil } @@ -221,7 +220,7 @@ func TestDispatchSync_PrepSubsystem_DispatchSync_Good(t *testing.T) { workspaceDir := core.JoinPath(WorkspaceRoot(), "core", "go-io", "task-9") subsystem := &PrepSubsystem{dispatchSyncTick: 10 * time.Millisecond} - subsystem.dispatchSyncPrep = func(_ context.Context, _ *mcp.CallToolRequest, input PrepInput) (*mcp.CallToolResult, PrepOutput, error) { + subsystem.dispatchSyncPrep = func(_ context.Context, _ *mcpsdk.CallToolRequest, input PrepInput) (*mcpsdk.CallToolResult, PrepOutput, error) { core.AssertEqual(t, "core", input.Org) core.AssertEqual(t, "go-io", input.Repo) core.AssertEqual(t, "codex", input.Agent) @@ -321,7 +320,7 @@ func TestDispatchSync_PrepSubsystem_DispatchSync_Ugly_VZFillsFullStatus(t *testi workspaceDir := core.JoinPath(WorkspaceRoot(), "core", "go-io", "task-vz") s := &PrepSubsystem{dispatchSyncTick: 5 * time.Millisecond} - s.dispatchSyncPrep = func(context.Context, *mcp.CallToolRequest, PrepInput) (*mcp.CallToolResult, PrepOutput, error) { + s.dispatchSyncPrep = func(context.Context, *mcpsdk.CallToolRequest, PrepInput) (*mcpsdk.CallToolResult, PrepOutput, error) { core.RequireTrue(t, fs.EnsureDir(workspaceDir).OK) return nil, PrepOutput{Success: true, WorkspaceDir: workspaceDir, Branch: "agent/vz", Prompt: "prompt"}, nil } diff --git a/go/pkg/agentic/fetch_loop_test.go b/go/pkg/agentic/fetch_loop_test.go index 9d58d1b8..4659102b 100644 --- a/go/pkg/agentic/fetch_loop_test.go +++ b/go/pkg/agentic/fetch_loop_test.go @@ -181,49 +181,12 @@ func waitForFetchLoopCalls(t *testing.T, state *fetchLoopCallState, repo string, core.AssertGreaterOrEqual(t, state.count(repo), want) } -func fetchLoopWriteGitScript(t *testing.T, logPath, badRepo string) { - t.Helper() - - binDir := t.TempDir() - gitPath := core.JoinPath(binDir, "git") - script := core.Concat( - "#!/bin/sh\n", - "repo=$(basename \"$(pwd)\")\n", - "printf '%s|%s\\n' \"$repo\" \"$*\" >> ", logPath, "\n", - "if [ \"$1\" = \"symbolic-ref\" ]; then\n", - " printf 'origin/dev\\n'\n", - " exit 0\n", - "fi\n", - "if [ \"$1\" = \"fetch\" ] && [ \"$repo\" = \"", badRepo, "\" ]; then\n", - " exit 1\n", - "fi\n", - "exit 0\n", - ) - core.RequireTrue(t, fs.Write(gitPath, script).OK) - core.RequireTrue(t, testCore.Process().RunIn(context.Background(), binDir, "chmod", "+x", gitPath).OK) - t.Setenv("PATH", core.Concat(binDir, ":", core.Env("PATH"))) -} - func fetchLoopCreateRepo(t *testing.T, codePath, org, repo string) { t.Helper() repoDir := core.JoinPath(codePath, org, repo) core.RequireTrue(t, fs.EnsureDir(core.JoinPath(repoDir, ".git")).OK) } -func fetchLoopWaitForCount(t *testing.T, logPath, repo, snippet string, want int, timeout time.Duration) { - t.Helper() - - deadline := time.Now().Add(timeout) - for time.Now().Before(deadline) { - if fetchLoopLogCount(logPath, repo, snippet) >= want { - return - } - time.Sleep(5 * time.Millisecond) - } - - core.AssertGreaterOrEqual(t, fetchLoopLogCount(logPath, repo, snippet), want) -} - func fetchLoopWaitForDone(t *testing.T, done <-chan struct{}) { t.Helper() @@ -233,28 +196,3 @@ func fetchLoopWaitForDone(t *testing.T, done <-chan struct{}) { t.Fatal("fetch loop did not stop after cancellation") } } - -func fetchLoopLogCount(logPath, repo, snippet string) int { - readResult := fs.Read(logPath) - if !readResult.OK { - return 0 - } - - content := core.Trim(readResult.Value.(string)) - if content == "" { - return 0 - } - - count := 0 - for _, line := range core.Split(content, "\n") { - if repo != "" && !core.HasPrefix(line, core.Concat(repo, "|")) { - continue - } - if snippet != "" && !core.Contains(line, snippet) { - continue - } - count++ - } - - return count -} diff --git a/go/pkg/agentic/handlers.go b/go/pkg/agentic/handlers.go index 8c9e2eff..ff82be2f 100644 --- a/go/pkg/agentic/handlers.go +++ b/go/pkg/agentic/handlers.go @@ -63,7 +63,11 @@ func (s *PrepSubsystem) HandleIPCEvents(c *core.Core, msg core.Message) core.Res } workspaceStatus.PID = pid workspaceStatus.ProcessID = processID - writeStatusResult(workspaceDir, workspaceStatus) + // Reported: the status file is what the monitor polls, so a silent write + // failure leaves the workspace looking stuck indefinitely. + if r := writeStatusResult(workspaceDir, workspaceStatus); !r.OK { + core.Warn("agentic: failed to write workspace status", "reason", r.Value) + } if runnerResult := c.Service("runner"); runnerResult.OK { if runnerSvc, ok := runnerResult.Value.(workspaceTracker); ok { runnerSvc.TrackWorkspace(WorkspaceName(workspaceDir), workspaceStatus) @@ -86,7 +90,11 @@ func handleCompletionQA(c *core.Core, msg core.Message) core.Result { return core.Result{OK: true} } - performAsyncIfRegistered(c, "agentic.qa", workspaceActionOptions(workspaceDir)) + // Reported: a follow-up that fails to dispatch silently skips a pipeline + // stage, and nothing downstream notices it never ran. + if r := performAsyncIfRegistered(c, "agentic.qa", workspaceActionOptions(workspaceDir)); !r.OK { + core.Warn("agentic: async follow-up did not dispatch", "reason", r.Value) + } return core.Result{OK: true} } @@ -101,7 +109,11 @@ func handleCompletionAutoPR(c *core.Core, msg core.Message) core.Result { return core.Result{OK: true} } - performAsyncIfRegistered(c, "agentic.auto-pr", workspaceActionOptions(workspaceDir)) + // Reported: a follow-up that fails to dispatch silently skips a pipeline + // stage, and nothing downstream notices it never ran. + if r := performAsyncIfRegistered(c, "agentic.auto-pr", workspaceActionOptions(workspaceDir)); !r.OK { + core.Warn("agentic: async follow-up did not dispatch", "reason", r.Value) + } return core.Result{OK: true} } @@ -116,7 +128,11 @@ func handleCompletionVerify(c *core.Core, msg core.Message) core.Result { return core.Result{OK: true} } - performAsyncIfRegistered(c, "agentic.verify", workspaceActionOptions(workspaceDir)) + // Reported: a follow-up that fails to dispatch silently skips a pipeline + // stage, and nothing downstream notices it never ran. + if r := performAsyncIfRegistered(c, "agentic.verify", workspaceActionOptions(workspaceDir)); !r.OK { + core.Warn("agentic: async follow-up did not dispatch", "reason", r.Value) + } return core.Result{OK: true} } @@ -156,7 +172,11 @@ func handleCompletionIngest(c *core.Core, msg core.Message) core.Result { return core.Result{OK: true} } - performAsyncIfRegistered(c, "agentic.ingest", workspaceActionOptions(workspaceDir)) + // Reported: a follow-up that fails to dispatch silently skips a pipeline + // stage, and nothing downstream notices it never ran. + if r := performAsyncIfRegistered(c, "agentic.ingest", workspaceActionOptions(workspaceDir)); !r.OK { + core.Warn("agentic: async follow-up did not dispatch", "reason", r.Value) + } return core.Result{OK: true} } @@ -171,7 +191,11 @@ func handleCompletionPoke(c *core.Core, msg core.Message) core.Result { } return core.Result{OK: true} } - performAsyncIfRegistered(c, "agentic.poke", core.NewOptions()) + // Reported: a follow-up that fails to dispatch silently skips a pipeline + // stage, and nothing downstream notices it never ran. + if r := performAsyncIfRegistered(c, "agentic.poke", core.NewOptions()); !r.OK { + core.Warn("agentic: async follow-up did not dispatch", "reason", r.Value) + } return core.Result{OK: true} } @@ -189,7 +213,11 @@ func handleHarvestAutoPR(c *core.Core, msg core.Message) core.Result { if workspaceDir == "" { return core.Result{OK: true} } - performAsyncIfRegistered(c, "agentic.auto-pr", workspaceActionOptions(workspaceDir)) + // Reported: a follow-up that fails to dispatch silently skips a pipeline + // stage, and nothing downstream notices it never ran. + if r := performAsyncIfRegistered(c, "agentic.auto-pr", workspaceActionOptions(workspaceDir)); !r.OK { + core.Warn("agentic: async follow-up did not dispatch", "reason", r.Value) + } return core.Result{OK: true} } diff --git a/go/pkg/agentic/ingest.go b/go/pkg/agentic/ingest.go index 6e91d627..08455822 100644 --- a/go/pkg/agentic/ingest.go +++ b/go/pkg/agentic/ingest.go @@ -91,5 +91,8 @@ func (s *PrepSubsystem) createIssueViaAPI(title, description, issueType, priorit "reporter": "cladius", }) - HTTPPost(context.Background(), core.Concat(s.brainURL, "/v1/issues"), issuePayload, apiKey, "Bearer") + // Reported: a dropped post means this issue silently never reaches the brain. + if r := HTTPPost(context.Background(), core.Concat(s.brainURL, "/v1/issues"), issuePayload, apiKey, "Bearer"); !r.OK { + core.Warn("agentic: failed to post issue to the brain", "reason", r.Value) + } } diff --git a/go/pkg/agentic/message.go b/go/pkg/agentic/message.go index d2dece29..e28f0937 100644 --- a/go/pkg/agentic/message.go +++ b/go/pkg/agentic/message.go @@ -95,7 +95,8 @@ func (s *PrepSubsystem) handleMessageInbox(ctx context.Context, options core.Opt } output, _ := result.Value.(MessageListOutput) if s.Core() != nil { - s.Core().ACTION(messages.InboxMessage{ + // Best-effort: a listener that has gone away must not fail this. + _ = s.Core().ACTION(messages.InboxMessage{ New: output.New, Total: output.Count, }) @@ -162,7 +163,8 @@ func (s *PrepSubsystem) messageSend(_ context.Context, input MessageSendInput) c CreatedAt: time.Now().Format(time.RFC3339), } if s.ServiceRuntime != nil { - s.Core().ACTION(coremcp.ChannelPush{ + // Best-effort: a listener that has gone away must not fail this. + _ = s.Core().ACTION(coremcp.ChannelPush{ Channel: coremcp.ChannelInboxMessage, Data: map[string]any{ "id": msg.ID, diff --git a/go/pkg/agentic/mirror.go b/go/pkg/agentic/mirror.go index 505eb601..712fa62e 100644 --- a/go/pkg/agentic/mirror.go +++ b/go/pkg/agentic/mirror.go @@ -173,7 +173,10 @@ func (s *PrepSubsystem) createGitHubPR(ctx context.Context, repoDir, repo string } func (s *PrepSubsystem) ensureDevBranch(repoDir string) { - s.Core().Process().RunIn(context.Background(), repoDir, "git", "push", "github", "HEAD:refs/heads/dev") + // Reported: a failed mirror push means GitHub silently falls behind. + if r := s.Core().Process().RunIn(context.Background(), repoDir, "git", "push", "github", "HEAD:refs/heads/dev"); !r.OK { + core.Warn("agentic: mirror push to GitHub failed", "reason", r.Value) + } } func (s *PrepSubsystem) hasRemote(repoDir, name string) bool { diff --git a/go/pkg/agentic/persist.go b/go/pkg/agentic/persist.go index 58a1613b..b69a5bb9 100644 --- a/go/pkg/agentic/persist.go +++ b/go/pkg/agentic/persist.go @@ -50,12 +50,16 @@ func (s *PrepSubsystem) restorePersistedState(_ context.Context) core.Result { changed := s.normaliseRestoredWorkspace(workspaceStatus) if workspaceStatus.Status == "completed" && fs.IsDir(workspaceDir).OK { - fs.DeleteAll(workspaceDir) + _ = fs.DeleteAll(workspaceDir) // best-effort cleanup; may already be gone } else if fs.IsDir(workspaceDir).OK && (changed || !fs.IsFile(WorkspaceStatusPath(workspaceDir)).OK) { s.writePersistedWorkspaceStatus(workspaceDir, workspaceStatus) } - s.workspaces.Set(name, cloneWorkspaceStatus(workspaceStatus)) + // Reported: Set only fails on a locked or sealed registry, so a + // failure here means this workspace silently stops being tracked. + if r := s.workspaces.Set(name, cloneWorkspaceStatus(workspaceStatus)); !r.OK { + core.Warn("agentic: failed to track workspace", "reason", r.Value) + } s.stateStoreSet(stateRegistryGroup, name, workspaceStatus) if workspaceStatus.Status == "queued" { diff --git a/go/pkg/agentic/pipeline_commands_test.go b/go/pkg/agentic/pipeline_commands_test.go index 542a49db..f69cd6b8 100644 --- a/go/pkg/agentic/pipeline_commands_test.go +++ b/go/pkg/agentic/pipeline_commands_test.go @@ -444,8 +444,3 @@ func pipelinePRPayload(repoName string, pullRequest *pipelineTestPR) map[string] } return payload } - -//go:fix inline -func boolPtr(value bool) *bool { - return new(value) -} diff --git a/go/pkg/agentic/plan_retention.go b/go/pkg/agentic/plan_retention.go index d8026754..3c1a9824 100644 --- a/go/pkg/agentic/plan_retention.go +++ b/go/pkg/agentic/plan_retention.go @@ -82,7 +82,10 @@ func (s *PrepSubsystem) runPlanCleanupLoop(ctx context.Context, interval time.Du case <-ctx.Done(): return case <-ticker.C: - s.planCleanup(core.NewOptions()) + // Reported: retention that silently stops running grows unbounded. + if r := s.planCleanup(core.NewOptions()); !r.OK { + core.Warn("agentic: scheduled plan cleanup failed", "reason", r.Value) + } } } } diff --git a/go/pkg/agentic/platform.go b/go/pkg/agentic/platform.go index 4ae1270c..da7f4d40 100644 --- a/go/pkg/agentic/platform.go +++ b/go/pkg/agentic/platform.go @@ -599,7 +599,7 @@ func (s *PrepSubsystem) platformEventPayload(ctx context.Context, action, path s if err != nil { return core.Result{Value: core.E(action, "request failed", err), OK: false} } - defer response.Body.Close() + defer func() { _ = response.Body.Close() }() // read side of the response if response.StatusCode >= 400 { readResult := core.ReadAll(response.Body) diff --git a/go/pkg/agentic/pr.go b/go/pkg/agentic/pr.go index c5b61b5d..3e01a84a 100644 --- a/go/pkg/agentic/pr.go +++ b/go/pkg/agentic/pr.go @@ -148,7 +148,11 @@ var createPR = func(s *PrepSubsystem, ctx context.Context, _ *mcp.CallToolReques } workspaceStatus.PRURL = pullRequestURL - writeStatusResult(workspaceDir, workspaceStatus) + // Reported: the status file is what the monitor polls, so a silent write + // failure leaves the workspace looking stuck indefinitely. + if r := writeStatusResult(workspaceDir, workspaceStatus); !r.OK { + core.Warn("agentic: failed to write workspace status", "reason", r.Value) + } cleanupResult := s.cleanupBranch(ctx, cleanupRepoRef(workspaceStatus.Org, workspaceStatus.Repo), workspaceStatus.Branch) if !cleanupResult.OK { core.Warn("createPR: branch cleanup failed", "repo", workspaceStatus.Repo, "branch", workspaceStatus.Branch, "reason", cleanupResult.Value) diff --git a/go/pkg/agentic/prep.go b/go/pkg/agentic/prep.go index 88641bdf..64cd687b 100644 --- a/go/pkg/agentic/prep.go +++ b/go/pkg/agentic/prep.go @@ -440,7 +440,11 @@ func (s *PrepSubsystem) hydrateWorkspaces() { status.Question = "Agent process died during restart" } } - s.workspaces.Set(key, &status) + // Reported: Set only fails on a locked or sealed registry, so a + // failure here means this workspace silently stops being tracked. + if r := s.workspaces.Set(key, &status); !r.OK { + core.Warn("agentic: failed to track workspace", "reason", r.Value) + } return true }) @@ -461,9 +465,17 @@ func (s *PrepSubsystem) hydrateWorkspaces() { if st.Question == "" { st.Question = "Agent process died during restart" } - writeStatusResult(workspaceDir, st) + // Reported: the status file is what the monitor polls, so a silent write + // failure leaves the workspace looking stuck indefinitely. + if r := writeStatusResult(workspaceDir, st); !r.OK { + core.Warn("agentic: failed to write workspace status", "reason", r.Value) + } + } + // Reported: Set only fails on a locked or sealed registry, so a + // failure here means this workspace silently stops being tracked. + if r := s.workspaces.Set(WorkspaceName(workspaceDir), st); !r.OK { + core.Warn("agentic: failed to track workspace", "reason", r.Value) } - s.workspaces.Set(WorkspaceName(workspaceDir), st) } } @@ -476,9 +488,13 @@ func (s *PrepSubsystem) hydrateWorkspaces() { func (s *PrepSubsystem) TrackWorkspace(name string, st *WorkspaceStatus) { if s.workspaces != nil { if st == nil { - s.workspaces.Delete(name) + _ = s.workspaces.Delete(name) // best-effort: the key may already be gone } else { - s.workspaces.Set(name, st) + // Reported: Set only fails on a locked or sealed registry, so a + // failure here means this workspace silently stops being tracked. + if r := s.workspaces.Set(name, st); !r.OK { + core.Warn("agentic: failed to track workspace", "reason", r.Value) + } } } if st == nil { @@ -814,11 +830,15 @@ var prepWorkspace = func(s *PrepSubsystem, ctx context.Context, _ *mcp.CallToolR currentBranch = defaultBranch } if currentBranch != "" { - process.RunIn(ctx, repoDir, "git", "checkout", currentBranch) + // Best-effort repo hygiene: a checkout, pull or fetch that fails + // here (diverged branch, offline remote) must not stop the + // workspace being prepared. The ls-remote between them is checked + // precisely because its answer changes what happens next. + _ = process.RunIn(ctx, repoDir, "git", "checkout", currentBranch) if process.RunIn(ctx, repoDir, "git", "ls-remote", "--exit-code", "--heads", "origin", currentBranch).OK { - process.RunIn(ctx, repoDir, "git", "pull", "--ff-only", "origin", currentBranch) + _ = process.RunIn(ctx, repoDir, "git", "pull", "--ff-only", "origin", currentBranch) } else if defaultBranch != "" { - process.RunIn(ctx, repoDir, "git", "fetch", "origin", defaultBranch) + _ = process.RunIn(ctx, repoDir, "git", "fetch", "origin", defaultBranch) } } } @@ -890,7 +910,8 @@ var prepWorkspace = func(s *PrepSubsystem, ctx context.Context, _ *mcp.CallToolR if !fs.IsDir(docsDir).OK { docsRepo := core.JoinPath(s.codePath, input.Org, "docs") if fs.IsDir(core.JoinPath(docsRepo, ".git")).OK { - process.RunIn(ctx, ".", "git", "clone", "--depth", "1", docsRepo, docsDir) + // Optional enrichment: a workspace without the docs checkout is still usable. + _ = process.RunIn(ctx, ".", "git", "clone", "--depth", "1", docsRepo, docsDir) } } diff --git a/go/pkg/agentic/prep_extra_test.go b/go/pkg/agentic/prep_extra_test.go index e1eab120..fc6b8f03 100644 --- a/go/pkg/agentic/prep_extra_test.go +++ b/go/pkg/agentic/prep_extra_test.go @@ -310,19 +310,19 @@ func TestPrep_RunWorkspaceLanguagePrep_Ugly_CommandFailure(t *testing.T) { func TestPrep_PullWikiContent_Good_WithPages(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch { - case r.URL.Path == "/api/v1/repos/core/go-io/wiki/pages": + switch r.URL.Path { + case "/api/v1/repos/core/go-io/wiki/pages": w.Write([]byte(core.JSONMarshalString([]map[string]any{ {"title": "Home", "sub_url": "Home"}, {"title": "Architecture", "sub_url": "Architecture"}, }))) - case r.URL.Path == "/api/v1/repos/core/go-io/wiki/page/Home": + case "/api/v1/repos/core/go-io/wiki/page/Home": // "Hello World" base64 w.Write([]byte(core.JSONMarshalString(map[string]any{ "title": "Home", "content_base64": "SGVsbG8gV29ybGQ=", }))) - case r.URL.Path == "/api/v1/repos/core/go-io/wiki/page/Architecture": + case "/api/v1/repos/core/go-io/wiki/page/Architecture": w.Write([]byte(core.JSONMarshalString(map[string]any{ "title": "Architecture", "content_base64": "TGF5ZXJlZA==", @@ -680,12 +680,12 @@ func TestPrep_PullWikiContent_Bad_Case(t *testing.T) { func TestPrep_PullWikiContent_Ugly_Case(t *testing.T) { // Forge returns pages with empty content srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch { - case r.URL.Path == "/api/v1/repos/core/go-io/wiki/pages": + switch r.URL.Path { + case "/api/v1/repos/core/go-io/wiki/pages": w.Write([]byte(core.JSONMarshalString([]map[string]any{ {"title": "EmptyPage", "sub_url": "EmptyPage"}, }))) - case r.URL.Path == "/api/v1/repos/core/go-io/wiki/page/EmptyPage": + case "/api/v1/repos/core/go-io/wiki/page/EmptyPage": w.Write([]byte(core.JSONMarshalString(map[string]any{ "title": "EmptyPage", "content_base64": "", diff --git a/go/pkg/agentic/qa.go b/go/pkg/agentic/qa.go index fcd0caaf..4de758e8 100644 --- a/go/pkg/agentic/qa.go +++ b/go/pkg/agentic/qa.go @@ -360,7 +360,9 @@ func publishDispatchReport(storeInstance *store.Store, workspaceName string, dis } tags := map[string]string{"workspace": workspaceName} - storeInstance.CommitToJournal(workspaceName, fields, tags) + if r := storeInstance.CommitToJournal(workspaceName, fields, tags); !r.OK { + core.Warn("agentic: failed to commit QA findings to the journal", "workspace", workspaceName, "reason", r.Value) + } } // runBuildAndTest executes the language-specific build/test cycle, recording diff --git a/go/pkg/agentic/queue.go b/go/pkg/agentic/queue.go index 5096afb5..b21b77bf 100644 --- a/go/pkg/agentic/queue.go +++ b/go/pkg/agentic/queue.go @@ -476,7 +476,11 @@ func (s *PrepSubsystem) drainOne() bool { workspaceStatus.ProcessID = processID workspaceStatus.Runs++ preserveStatusNote(workspaceDir, workspaceStatus) // keep VZ→OCI downgrade note (SP2.4) - writeStatusResult(workspaceDir, workspaceStatus) + // Reported: the status file is what the monitor polls, so a silent write + // failure leaves the workspace looking stuck indefinitely. + if r := writeStatusResult(workspaceDir, workspaceStatus); !r.OK { + core.Warn("agentic: failed to write workspace status", "reason", r.Value) + } s.TrackWorkspace(WorkspaceName(workspaceDir), workspaceStatus) return true diff --git a/go/pkg/agentic/repo_sync.go b/go/pkg/agentic/repo_sync.go index c9445c81..b3e2e8f9 100644 --- a/go/pkg/agentic/repo_sync.go +++ b/go/pkg/agentic/repo_sync.go @@ -167,7 +167,9 @@ func (s *PrepSubsystem) cmdRepoSyncLocal(options core.Options) core.Result { } func (s *PrepSubsystem) runRepoSync(ctx context.Context, target fetchRepoRef, branch string, reset bool) core.Result { - s.registerRepoSyncSupport() + // Idempotent registration; a failure here surfaces as the sync verbs being + // absent on the very next call. + _ = s.registerRepoSyncSupport() repoDir, err := repoSyncRepoDir(s, target) if err != nil { diff --git a/go/pkg/agentic/repo_sync_cov_test.go b/go/pkg/agentic/repo_sync_cov_test.go index 17b0fde6..894565a8 100644 --- a/go/pkg/agentic/repo_sync_cov_test.go +++ b/go/pkg/agentic/repo_sync_cov_test.go @@ -35,7 +35,7 @@ func TestRepoSyncCov_HandleRepoSyncIPC_Ugly_WarnsOnFailedSync(t *testing.T) { // TestRepoSyncCov_RepoSyncContext_Good_NilFallsBackToBackground — a nil context // is replaced with context.Background(); a live context passes through. func TestRepoSyncCov_RepoSyncContext_Good_NilFallsBackToBackground(t *testing.T) { - core.AssertNotNil(t, repoSyncContext(nil)) + core.AssertNotNil(t, repoSyncContext(context.TODO())) ctx := context.Background() core.AssertEqual(t, ctx, repoSyncContext(ctx)) diff --git a/go/pkg/agentic/resume.go b/go/pkg/agentic/resume.go index 108b84d3..c5905812 100644 --- a/go/pkg/agentic/resume.go +++ b/go/pkg/agentic/resume.go @@ -98,7 +98,11 @@ func (s *PrepSubsystem) resume(ctx context.Context, input ResumeInput) core.Resu workspaceStatus.Runs++ workspaceStatus.Question = "" preserveStatusNote(workspaceDir, workspaceStatus) // keep VZ→OCI downgrade note (SP2.4) - writeStatusResult(workspaceDir, workspaceStatus) + // Reported: the status file is what the monitor polls, so a silent write + // failure leaves the workspace looking stuck indefinitely. + if r := writeStatusResult(workspaceDir, workspaceStatus); !r.OK { + core.Warn("agentic: failed to write workspace status", "reason", r.Value) + } return core.Ok(ResumeOutput{ Success: true, diff --git a/go/pkg/agentic/review_queue.go b/go/pkg/agentic/review_queue.go index b12fbdf3..ac1b463d 100644 --- a/go/pkg/agentic/review_queue.go +++ b/go/pkg/agentic/review_queue.go @@ -294,7 +294,9 @@ func (s *PrepSubsystem) reviewRepo(ctx context.Context, repoDir, repo, reviewer } findingsFile := core.JoinPath(repoDir, ".core", "coderabbit-findings.txt") - fs.Write(findingsFile, output) + if r := fs.Write(findingsFile, output); !r.OK { + core.Warn("agentic: failed to write review findings", "path", findingsFile, "reason", r.Value) + } task := core.Sprintf( "Fix CodeRabbit findings. The review output is in .core/coderabbit-findings.txt. Read it, verify each finding against the code, fix what's valid. Run tests. Commit: fix(coderabbit): address review findings\n\nFindings summary (%d issues):\n%s", @@ -318,7 +320,10 @@ var pushAndMerge = func(s *PrepSubsystem, ctx context.Context, repoDir, repo str return core.E("pushAndMerge", core.Concat("push failed: ", r.Error()), nil) } - process.RunIn(ctx, repoDir, "gh", "pr", "ready", "--repo", core.Concat(GitHubOrg(), "/", repo)) + // Reported: leaving a PR in draft silently stalls the review pipeline. + if r := process.RunIn(ctx, repoDir, "gh", "pr", "ready", "--repo", core.Concat(GitHubOrg(), "/", repo)); !r.OK { + core.Warn("agentic: failed to mark PR ready for review", "reason", r.Value) + } if r := process.RunIn(ctx, repoDir, "gh", "pr", "merge", "--merge", "--delete-branch"); !r.OK { return core.E("pushAndMerge", core.Concat("merge failed: ", r.Error()), nil) diff --git a/go/pkg/agentic/review_queue_cov_test.go b/go/pkg/agentic/review_queue_cov_test.go index a170f4d1..ab5c7d86 100644 --- a/go/pkg/agentic/review_queue_cov_test.go +++ b/go/pkg/agentic/review_queue_cov_test.go @@ -309,7 +309,7 @@ func TestReviewqueue_RunPRManageLoop_Bad_GuardsInvalidArgs(t *testing.T) { s := newPrepWithProcess() // Nil context and non-positive interval both return immediately. core.AssertNotPanics(t, func() { - s.runPRManageLoop(nil, time.Hour) + s.runPRManageLoop(context.TODO(), time.Hour) s.runPRManageLoop(context.Background(), 0) }) } diff --git a/go/pkg/agentic/runner.go b/go/pkg/agentic/runner.go index c8eb9899..431236bf 100644 --- a/go/pkg/agentic/runner.go +++ b/go/pkg/agentic/runner.go @@ -30,5 +30,8 @@ func (s *PrepSubsystem) runRunnerAction(name string) { return } - action.Run(context.Background(), core.NewOptions()) + // Reported: the caller has no other signal that this did not run. + if r := action.Run(context.Background(), core.NewOptions()); !r.OK { + core.Warn("agentic: runner action failed", "reason", r.Value) + } } diff --git a/go/pkg/agentic/shutdown.go b/go/pkg/agentic/shutdown.go index 601e2ed1..bfb48bed 100644 --- a/go/pkg/agentic/shutdown.go +++ b/go/pkg/agentic/shutdown.go @@ -71,7 +71,10 @@ func (s *PrepSubsystem) handleDispatchShutdownNow(ctx context.Context, _ core.Op var dispatchStart = func(s *PrepSubsystem, ctx context.Context, _ *mcp.CallToolRequest, input ShutdownInput) (*mcp.CallToolResult, ShutdownOutput, error) { if s.ServiceRuntime != nil { - s.Core().Action("runner.start").Run(ctx, core.NewOptions()) + // Reported: a lifecycle transition that fails silently leaves the + if r := s.Core().Action("runner.start").Run(ctx, core.NewOptions()); !r.OK { + core.Warn("agentic: runner.start action failed", "reason", r.Value) + } } return nil, ShutdownOutput{ Success: true, @@ -81,7 +84,10 @@ var dispatchStart = func(s *PrepSubsystem, ctx context.Context, _ *mcp.CallToolR var shutdownGraceful = func(s *PrepSubsystem, ctx context.Context, _ *mcp.CallToolRequest, input ShutdownInput) (*mcp.CallToolResult, ShutdownOutput, error) { if s.ServiceRuntime != nil { - s.Core().Action("runner.stop").Run(ctx, core.NewOptions()) + // Reported: a lifecycle transition that fails silently leaves the + if r := s.Core().Action("runner.stop").Run(ctx, core.NewOptions()); !r.OK { + core.Warn("agentic: runner.stop action failed", "reason", r.Value) + } } return nil, ShutdownOutput{ Success: true, @@ -91,7 +97,10 @@ var shutdownGraceful = func(s *PrepSubsystem, ctx context.Context, _ *mcp.CallTo var shutdownNow = func(s *PrepSubsystem, ctx context.Context, _ *mcp.CallToolRequest, input ShutdownInput) (*mcp.CallToolResult, ShutdownOutput, error) { if s.ServiceRuntime != nil { - s.Core().Action("runner.kill").Run(ctx, core.NewOptions()) + // Reported: a lifecycle transition that fails silently leaves the + if r := s.Core().Action("runner.kill").Run(ctx, core.NewOptions()); !r.OK { + core.Warn("agentic: runner.kill action failed", "reason", r.Value) + } } return nil, ShutdownOutput{ Success: true, diff --git a/go/pkg/agentic/shutdown_example_test.go b/go/pkg/agentic/shutdown_example_test.go index ae5946bf..91c1aa57 100644 --- a/go/pkg/agentic/shutdown_example_test.go +++ b/go/pkg/agentic/shutdown_example_test.go @@ -2,11 +2,15 @@ package agentic -import core "dappco.re/go" +import ( + "context" + + core "dappco.re/go" +) func ExamplePrepSubsystem_Shutdown_process() { s := newPrepWithProcess() - err := s.Shutdown(nil) + err := s.Shutdown(context.TODO()) core.Println(err == nil) // Output: true } diff --git a/go/pkg/agentic/shutdown_test.go b/go/pkg/agentic/shutdown_test.go index 6b458f65..8b8f66c1 100644 --- a/go/pkg/agentic/shutdown_test.go +++ b/go/pkg/agentic/shutdown_test.go @@ -3,6 +3,7 @@ package agentic import ( + "context" "testing" core "dappco.re/go" @@ -10,20 +11,20 @@ import ( func TestShutdown_Shutdown_Good(t *testing.T) { s := newPrepWithProcess() - err := s.Shutdown(nil) + err := s.Shutdown(context.TODO()) core.AssertNoError(t, err) } func TestShutdown_Shutdown_Bad_AlreadyFrozen(t *testing.T) { s := newPrepWithProcess() s.frozen = true - err := s.Shutdown(nil) + err := s.Shutdown(context.TODO()) core.AssertNoError(t, err) } func TestShutdown_Shutdown_Ugly_NilRuntime(t *testing.T) { s := &PrepSubsystem{} core.AssertNotPanics(t, func() { - _ = s.Shutdown(nil) + _ = s.Shutdown(context.TODO()) }) } diff --git a/go/pkg/agentic/status.go b/go/pkg/agentic/status.go index eb20efb2..6da32dc4 100644 --- a/go/pkg/agentic/status.go +++ b/go/pkg/agentic/status.go @@ -215,7 +215,11 @@ var status = func(s *PrepSubsystem, ctx context.Context, _ *mcp.CallToolRequest, workspaceStatus.Status = "completed" } } - writeStatusResult(workspaceDir, workspaceStatus) + // Reported: the status file is what the monitor polls, so a silent write + // failure leaves the workspace looking stuck indefinitely. + if r := writeStatusResult(workspaceDir, workspaceStatus); !r.OK { + core.Warn("agentic: failed to write workspace status", "reason", r.Value) + } } } diff --git a/go/pkg/agentic/status_test.go b/go/pkg/agentic/status_test.go index e5e2c793..19ebe584 100644 --- a/go/pkg/agentic/status_test.go +++ b/go/pkg/agentic/status_test.go @@ -325,7 +325,7 @@ func TestStatus_Status_Ugly(t *testing.T) { failCount: make(map[string]int), } - _, out, err := s.status(nil, nil, StatusInput{}) + _, out, err := s.status(context.TODO(), nil, StatusInput{}) core.RequireNoError(t, err) core.AssertEqual(t, 3, out.Total) diff --git a/go/pkg/agentic/test_assertions_test.go b/go/pkg/agentic/test_assertions_test.go index cc0ac962..03e65665 100644 --- a/go/pkg/agentic/test_assertions_test.go +++ b/go/pkg/agentic/test_assertions_test.go @@ -25,21 +25,6 @@ func assertIsType(t *testing.T, want, got any, msg ...string) { core.AssertEqual(t, reflect.TypeOf(want), reflect.TypeOf(got), msg...) } -func assertNotSame(t *testing.T, want, got any, msg ...string) { - t.Helper() - - wantValue := reflect.ValueOf(want) - gotValue := reflect.ValueOf(got) - if !wantValue.IsValid() || !gotValue.IsValid() { - t.Fatalf("assertNotSame requires non-nil values") - } - if wantValue.Kind() != reflect.Pointer || gotValue.Kind() != reflect.Pointer { - t.Fatalf("assertNotSame requires pointer values") - } - - core.AssertFalse(t, wantValue.Pointer() == gotValue.Pointer(), msg...) -} - func assertZero(t *testing.T, got any, msg ...string) { t.Helper() if got == nil { diff --git a/go/pkg/agentic/transport.go b/go/pkg/agentic/transport.go index f0ec4182..2c6d84e2 100644 --- a/go/pkg/agentic/transport.go +++ b/go/pkg/agentic/transport.go @@ -120,7 +120,7 @@ func httpDo(ctx context.Context, method, url, body, token, authScheme string) co if requestErr != nil { return core.Result{Value: core.E("httpDo", "request failed", requestErr), OK: false} } - defer response.Body.Close() + defer func() { _ = response.Body.Close() }() // read side of the response readResult := core.ReadAll(response.Body) if !readResult.OK { @@ -174,7 +174,7 @@ func mcpInitializeResult(ctx context.Context, url, token string) core.Result { if err != nil { return core.Result{Value: core.E("mcpInitialize", "request failed", err), OK: false} } - defer response.Body.Close() + defer func() { _ = response.Body.Close() }() // read side of the response if response.StatusCode != 200 { return core.Result{Value: core.E("mcpInitialize", core.Sprintf("HTTP %d", response.StatusCode), nil), OK: false} @@ -198,7 +198,8 @@ func mcpInitializeResult(ctx context.Context, url, token string) core.Result { mcpHeaders(notificationRequest, token, sessionID) notificationResponse, err := defaultClient.Do(notificationRequest) if err == nil { - notificationResponse.Body.Close() + // Read side of the response. + _ = notificationResponse.Body.Close() } return core.Result{Value: sessionID, OK: true} @@ -231,7 +232,7 @@ func mcpCallResult(ctx context.Context, url, token, sessionID string, body []byt if err != nil { return core.Result{Value: core.E("mcpCall", "request failed", err), OK: false} } - defer response.Body.Close() + defer func() { _ = response.Body.Close() }() // read side of the response if response.StatusCode != 200 { return core.Result{Value: core.E("mcpCall", core.Sprintf("HTTP %d", response.StatusCode), nil), OK: false} @@ -282,5 +283,7 @@ func mcpHeaders(request *http.Request, token, sessionID string) { } func drainSSE(response *http.Response) { - core.ReadAll(response.Body) + // Drained purely so the connection can be reused — the body is unwanted and + // a read failure here changes nothing. + _ = core.ReadAll(response.Body) } diff --git a/go/pkg/agentic/verify.go b/go/pkg/agentic/verify.go index 7b9c090b..5f238475 100644 --- a/go/pkg/agentic/verify.go +++ b/go/pkg/agentic/verify.go @@ -35,7 +35,11 @@ func (s *PrepSubsystem) autoVerifyAndMerge(workspaceDir string) { return } st2.Status = "merged" - writeStatusResult(workspaceDir, st2) + // Reported: the status file is what the monitor polls, so a silent write + // failure leaves the workspace looking stuck indefinitely. + if r := writeStatusResult(workspaceDir, st2); !r.OK { + core.Warn("agentic: failed to write workspace status", "reason", r.Value) + } } } @@ -62,7 +66,11 @@ func (s *PrepSubsystem) autoVerifyAndMerge(workspaceDir string) { return } workspaceStatusUpdate.Question = "Flagged for review — auto-merge failed after retry" - writeStatusResult(workspaceDir, workspaceStatusUpdate) + // Reported: the status file is what the monitor polls, so a silent write + // failure leaves the workspace looking stuck indefinitely. + if r := writeStatusResult(workspaceDir, workspaceStatusUpdate); !r.OK { + core.Warn("agentic: failed to write workspace status", "reason", r.Value) + } } } @@ -112,7 +120,8 @@ func (s *PrepSubsystem) rebaseBranch(repoDir, branch string) bool { } if !process.RunIn(ctx, repoDir, "git", "rebase", core.Concat("origin/", base)).OK { - process.RunIn(ctx, repoDir, "git", "rebase", "--abort") + // Best-effort: there may be no rebase in progress to abort. + _ = process.RunIn(ctx, repoDir, "git", "rebase", "--abort") return false } @@ -141,7 +150,11 @@ func (s *PrepSubsystem) flagForReview(org, repo string, pullRequestNumber int, m "labels": []int{s.getLabelID(ctx, org, repo, "needs-review")}, }) url := core.Sprintf("%s/api/v1/repos/%s/%s/issues/%d/labels", s.forgeURL, org, repo, pullRequestNumber) - HTTPPost(ctx, url, payload, s.forgeToken, "token") + // Reported: a label that fails to apply silently drops the PR out of the + // review queue that selects on it. + if r := HTTPPost(ctx, url, payload, s.forgeToken, "token"); !r.OK { + core.Warn("agentic: failed to apply PR label", "url", url, "reason", r.Value) + } reason := "Tests failed after rebase" if mergeOutcome == mergeConflict { @@ -158,7 +171,11 @@ func (s *PrepSubsystem) ensureLabel(ctx context.Context, org, repo, name, colour "color": core.Concat("#", colour), }) url := core.Sprintf("%s/api/v1/repos/%s/%s/labels", s.forgeURL, org, repo) - HTTPPost(ctx, url, payload, s.forgeToken, "token") + // Reported: a label that fails to apply silently drops the PR out of the + // review queue that selects on it. + if r := HTTPPost(ctx, url, payload, s.forgeToken, "token"); !r.OK { + core.Warn("agentic: failed to apply PR label", "url", url, "reason", r.Value) + } } // s.getLabelID(context.Background(), "core", "go-io", "needs-review") @@ -173,7 +190,11 @@ func (s *PrepSubsystem) getLabelID(ctx context.Context, org, repo, name string) ID int `json:"id"` Name string `json:"name"` } - core.JSONUnmarshalString(getResult.Value.(string), &labels) + // Reported: unparsable labels means getLabelID returns nothing and the + // caller silently applies no label at all. + if r := core.JSONUnmarshalString(getResult.Value.(string), &labels); !r.OK { + core.Warn("agentic: failed to parse repo labels", "reason", r.Value) + } for _, l := range labels { if l.Name == name { return l.ID diff --git a/go/pkg/agentic/watch.go b/go/pkg/agentic/watch.go index 252a5555..79f29f13 100644 --- a/go/pkg/agentic/watch.go +++ b/go/pkg/agentic/watch.go @@ -120,7 +120,8 @@ var watch = func(s *PrepSubsystem, ctx context.Context, request *mcp.CallToolReq progressCount++ if request != nil && progressToken != nil && request.Session != nil { - request.Session.NotifyProgress(ctx, &mcp.ProgressNotificationParams{ + // Best-effort: a listener that has gone away must not fail this. + _ = request.Session.NotifyProgress(ctx, &mcp.ProgressNotificationParams{ ProgressToken: progressToken, Progress: progressCount, Total: total, @@ -141,7 +142,8 @@ var watch = func(s *PrepSubsystem, ctx context.Context, request *mcp.CallToolReq progressCount++ if request != nil && progressToken != nil && request.Session != nil { - request.Session.NotifyProgress(ctx, &mcp.ProgressNotificationParams{ + // Best-effort: a listener that has gone away must not fail this. + _ = request.Session.NotifyProgress(ctx, &mcp.ProgressNotificationParams{ ProgressToken: progressToken, Progress: progressCount, Total: total, @@ -161,7 +163,8 @@ var watch = func(s *PrepSubsystem, ctx context.Context, request *mcp.CallToolReq progressCount++ if request != nil && progressToken != nil && request.Session != nil { - request.Session.NotifyProgress(ctx, &mcp.ProgressNotificationParams{ + // Best-effort: a listener that has gone away must not fail this. + _ = request.Session.NotifyProgress(ctx, &mcp.ProgressNotificationParams{ ProgressToken: progressToken, Progress: progressCount, Total: total, diff --git a/go/pkg/agentic/workspace_stats_test.go b/go/pkg/agentic/workspace_stats_test.go index 1e4f3839..519013d3 100644 --- a/go/pkg/agentic/workspace_stats_test.go +++ b/go/pkg/agentic/workspace_stats_test.go @@ -3,6 +3,7 @@ package agentic import ( + "context" "testing" "time" @@ -335,7 +336,7 @@ func TestWorkspacestats_HandleWorkspaceStats_Good_ReturnsEmptyWhenNoRows(t *test } t.Cleanup(s.closeWorkspaceStatsStore) - result := s.handleWorkspaceStats(nil, core.NewOptions()) + result := s.handleWorkspaceStats(context.TODO(), core.NewOptions()) core.AssertTrue(t, result.OK) out, ok := result.Value.(WorkspaceStatsOutput) core.AssertTrue(t, ok) @@ -374,7 +375,7 @@ func TestWorkspacestats_HandleWorkspaceStats_Good_AppliesFilters(t *testing.T) { } // Filter by repo only. - result := s.handleWorkspaceStats(nil, core.NewOptions( + result := s.handleWorkspaceStats(context.TODO(), core.NewOptions( core.Option{Key: "repo", Value: "go-io"}, )) core.AssertTrue(t, result.OK) @@ -382,7 +383,7 @@ func TestWorkspacestats_HandleWorkspaceStats_Good_AppliesFilters(t *testing.T) { core.AssertEqual(t, 2, out.Count) // Filter by repo + status. - result = s.handleWorkspaceStats(nil, core.NewOptions( + result = s.handleWorkspaceStats(context.TODO(), core.NewOptions( core.Option{Key: "repo", Value: "go-io"}, core.Option{Key: "status", Value: "completed"}, )) @@ -390,7 +391,7 @@ func TestWorkspacestats_HandleWorkspaceStats_Good_AppliesFilters(t *testing.T) { core.AssertEqual(t, 1, out.Count) // Limit trims the result set. - result = s.handleWorkspaceStats(nil, core.NewOptions( + result = s.handleWorkspaceStats(context.TODO(), core.NewOptions( core.Option{Key: "limit", Value: 1}, )) out = result.Value.(WorkspaceStatsOutput) diff --git a/go/pkg/lib/lib_test.go b/go/pkg/lib/lib_test.go index 5898386e..cddc0002 100644 --- a/go/pkg/lib/lib_test.go +++ b/go/pkg/lib/lib_test.go @@ -697,15 +697,3 @@ func TestLib_MountEmbed_Bad_Case(t *testing.T) { t.Fatal("mountEmbed should return an error value") } } - -func assertSPDXHeader(t *testing.T, path string) { - t.Helper() - - r := testFs.Read(path) - if !r.OK { - t.Fatalf("failed to read %s", path) - } - if !core.HasPrefix(r.Value.(string), "// SPDX-License-Identifier: EUPL-1.2") { - t.Fatalf("%s missing SPDX header", path) - } -} diff --git a/go/pkg/monitor/monitor.go b/go/pkg/monitor/monitor.go index 343f8b0b..c99c0888 100644 --- a/go/pkg/monitor/monitor.go +++ b/go/pkg/monitor/monitor.go @@ -496,7 +496,8 @@ func (m *Subsystem) checkInbox() string { } if m.ServiceRuntime != nil { - m.Core().ACTION(messages.InboxMessage{ + // Best-effort: a listener that has gone away must not fail this. + _ = m.Core().ACTION(messages.InboxMessage{ New: len(inboxMessages), Total: unread, }) @@ -513,6 +514,20 @@ func (m *Subsystem) notify(ctx context.Context, message string) { for session := range m.svc.Server().Sessions() { // Best-effort: one unreachable session must not stop the loop // logging to the others. + // + // session.Log is deprecated as of MCP protocol 2026-07-28 (SEP-2577), + // which retires the logging feature outright rather than replacing it — + // there is no successor call to migrate to, so this cannot be a + // mechanical swap. The deprecation window is at least twelve months + // from that date, so this must move before ~2027-07-28. + // + // What replaces it is a decision, not a rename: these are human-facing + // progress lines, and the candidates are a server notification or + // folding them into the status:// resource this file already publishes + // through ResourceUpdated. Left working and marked rather than guessed + // at, because picking wrong silently loses operator visibility. + // + //nolint:staticcheck // SA1019: no replacement exists; see SEP-2577. _ = session.Log(ctx, &mcp.LoggingMessageParams{ Level: "info", Logger: "monitor", diff --git a/go/pkg/opencode/imports_test.go b/go/pkg/opencode/imports_test.go index 12c0140b..63a1695e 100644 --- a/go/pkg/opencode/imports_test.go +++ b/go/pkg/opencode/imports_test.go @@ -100,7 +100,7 @@ func TestImportedProject_SchemaReturnsOrmDefinition_Good(t *testing.T) { if schema.Name != "imported_projects" { t.Errorf("schema.Name = %q; want 'imported_projects'", schema.Name) } - if schema.PK == nil || len(schema.PK) == 0 { + if len(schema.PK) == 0 { t.Error("schema.PK must not be empty") } } @@ -213,7 +213,7 @@ func TestImportedProvider_SchemaReturnsOrmDefinition_Good(t *testing.T) { if schema.Name != "imported_providers" { t.Errorf("schema.Name = %q; want 'imported_providers'", schema.Name) } - if schema.PK == nil || len(schema.PK) == 0 { + if len(schema.PK) == 0 { t.Error("schema.PK must not be empty") } } diff --git a/go/pkg/opencode/proxy.go b/go/pkg/opencode/proxy.go index 938ae21e..3f51463a 100644 --- a/go/pkg/opencode/proxy.go +++ b/go/pkg/opencode/proxy.go @@ -80,12 +80,20 @@ func (g *SandboxProxyGroup) Set(id, targetURL, authHeader string) { // when downstream Writer implements core.Flusher, which gin's // ResponseWriter does). No customisation needed for SSE today. if authHeader != "" { - // Wrap the default Director so the upstream-rewrite logic - // (Host, X-Forwarded-*) still runs, then inject auth. - defaultDir := rp.Director - rp.Director = func(req *core.Request) { - defaultDir(req) - req.Header.Set("Authorization", authHeader) + // Rewrite, not Director: Director is deprecated as of Go 1.26 and + // superseded since 1.20. Rewrite is also the safer of the two — it + // hands the hook both the inbound and outbound request, so headers + // arriving from the client cannot be forwarded upstream by accident, + // which is the hazard Director was replaced for. + // + // SetURL reproduces what NewSingleHostReverseProxy's own Director did + // (scheme, host, path join), and SetXForwarded restores the + // X-Forwarded-* handling that came with it. + rp.Rewrite = func(pr *httputil.ProxyRequest) { + pr.SetURL(u) + pr.Out.Host = pr.In.Host + pr.SetXForwarded() + pr.Out.Header.Set("Authorization", authHeader) } } g.mu.Lock() diff --git a/go/pkg/runner/runner.go b/go/pkg/runner/runner.go index 4a24c93e..5cfdb0c2 100644 --- a/go/pkg/runner/runner.go +++ b/go/pkg/runner/runner.go @@ -337,12 +337,16 @@ func (s *Service) actionDispatch(_ context.Context, options core.Options) core.R } workspaceName := core.Concat("pending/", repo) - s.workspaces.Set(workspaceName, &WorkspaceStatus{ + // Reported: Set only fails on a locked or sealed registry, so a + // failure here means this workspace silently stops being tracked. + if r := s.workspaces.Set(workspaceName, &WorkspaceStatus{ Status: "running", Agent: agent, Repo: repo, PID: -1, - }) + }); !r.OK { + core.Warn("runner: failed to track workspace", "reason", r.Value) + } return core.Result{OK: true} } @@ -408,7 +412,11 @@ func (s *Service) actionKill(_ context.Context, _ core.Options) core.Result { core.Warn("runner.actionKill: failed to write failed status", "workspace", agentic.WorkspaceName(workspaceDir), "reason", writeResult.Value) } if s.workspaces != nil { - s.workspaces.Set(agentic.WorkspaceName(workspaceDir), workspaceStatus) + // Reported: Set only fails on a locked or sealed registry, so a + // failure here means this workspace silently stops being tracked. + if r := s.workspaces.Set(agentic.WorkspaceName(workspaceDir), workspaceStatus); !r.OK { + core.Warn("runner: failed to track workspace", "reason", r.Value) + } } case "queued": workspaceName := agentic.WorkspaceName(workspaceDir) @@ -422,7 +430,7 @@ func (s *Service) actionKill(_ context.Context, _ core.Options) core.Result { } cleared++ if s.workspaces != nil { - s.workspaces.Delete(workspaceName) + _ = s.workspaces.Delete(workspaceName) // best-effort: the key may already be gone } } } @@ -467,7 +475,8 @@ func (s *Service) runLoop() { func (s *Service) drainQueueAndNotify(coreApp *core.Core) { completed := s.drainQueue() if coreApp != nil { - coreApp.ACTION(messages.QueueDrained{Completed: completed}) + // Best-effort: a listener that has gone away must not fail this. + _ = coreApp.ACTION(messages.QueueDrained{Completed: completed}) } } @@ -488,7 +497,11 @@ func (s *Service) hydrateWorkspaces() { if workspaceStatus.Status == "running" { workspaceStatus.Status = "queued" } - s.workspaces.Set(agentic.WorkspaceName(workspaceDir), workspaceStatus) + // Reported: Set only fails on a locked or sealed registry, so a + // failure here means this workspace silently stops being tracked. + if r := s.workspaces.Set(agentic.WorkspaceName(workspaceDir), workspaceStatus); !r.OK { + core.Warn("runner: failed to track workspace", "reason", r.Value) + } } } From 8936f32131377739cf0ee3dcbdb4fc9c1d36b244 Mon Sep 17 00:00:00 2001 From: Snider Date: Sat, 8 Aug 2026 15:07:26 +0100 Subject: [PATCH 2/2] fix(agentic): restore the nil Contexts that ARE the input under test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two sites from the SA1012 sweep, one of which hung CI for ten minutes. review_queue_cov_test.go — TestReviewqueue_RunPRManageLoop_Bad_GuardsInvalidArgs states its contract in its own comment: "Nil context and non-positive interval both return immediately." runPRManageLoop guards `ctx == nil`, so passing context.TODO() walked past the guard into ticker := time.NewTicker(interval) // interval = time.Hour and parked in the select waiting an hour for a tick, on a 10m test budget. That is the `panic: test timed out` on PR #27; the goroutine dump names review_queue.go:133 with interval 0x34630b8a000 = 1h. repo_sync_cov_test.go — TestRepoSyncCov_RepoSyncContext_Good_NilFallsBack- ToBackground did NOT fail, which is worse. repoSyncContext's nil branch is the only reason the function exists; with a real Context the first assertion merely repeated the passthrough case asserted on the next line, so the function's whole purpose went untested and the suite stayed green. Both found by auditing every substitution against its enclosing test name instead of trusting a green run — the loud one was already known, and only that method surfaced the silent one. A test named for its input is the specific case; the linter's advice is the general one, and here they disagree. Each site now carries the reason inline. Co-Authored-By: Virgil --- go/pkg/agentic/repo_sync_cov_test.go | 5 ++++- go/pkg/agentic/review_queue_cov_test.go | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/go/pkg/agentic/repo_sync_cov_test.go b/go/pkg/agentic/repo_sync_cov_test.go index 894565a8..86ad8683 100644 --- a/go/pkg/agentic/repo_sync_cov_test.go +++ b/go/pkg/agentic/repo_sync_cov_test.go @@ -35,7 +35,10 @@ func TestRepoSyncCov_HandleRepoSyncIPC_Ugly_WarnsOnFailedSync(t *testing.T) { // TestRepoSyncCov_RepoSyncContext_Good_NilFallsBackToBackground — a nil context // is replaced with context.Background(); a live context passes through. func TestRepoSyncCov_RepoSyncContext_Good_NilFallsBackToBackground(t *testing.T) { - core.AssertNotNil(t, repoSyncContext(context.TODO())) + //nolint:staticcheck // SA1012: the nil Context IS the input under test — + // the nil branch is the only reason repoSyncContext exists, and a real + // Context here just repeats the passthrough case asserted below. + core.AssertNotNil(t, repoSyncContext(nil)) ctx := context.Background() core.AssertEqual(t, ctx, repoSyncContext(ctx)) diff --git a/go/pkg/agentic/review_queue_cov_test.go b/go/pkg/agentic/review_queue_cov_test.go index ab5c7d86..ba22e25f 100644 --- a/go/pkg/agentic/review_queue_cov_test.go +++ b/go/pkg/agentic/review_queue_cov_test.go @@ -309,7 +309,10 @@ func TestReviewqueue_RunPRManageLoop_Bad_GuardsInvalidArgs(t *testing.T) { s := newPrepWithProcess() // Nil context and non-positive interval both return immediately. core.AssertNotPanics(t, func() { - s.runPRManageLoop(context.TODO(), time.Hour) + //nolint:staticcheck // SA1012: the nil Context IS the input under test. + // runPRManageLoop guards `ctx == nil` and returns; a real Context falls + // through to a select that waits an hour for a tick, which is a hang. + s.runPRManageLoop(nil, time.Hour) s.runPRManageLoop(context.Background(), 0) }) }