Repository navigation
normalize task-handler error codes, add state guard to updateTaskProgress - #73
Merged
Merged
Conversation
…kProgress Two related fixes to server/serve_tasks.go, both closing gaps flagged in #49: - Missing-id errors across claimTask (worker lookup), updateTaskProgress, and markTaskAsFinished now map to 404 via the existing database.ItemNotFoundError, routed through a new statusForDBError() helper in server_util.go instead of the previous mix of 500/400. claimTask's task-lookup branch already did this; the other three call sites now follow the same pattern. Non-ItemNotFoundError failures keep their historical fallback status (500 for DB errors, 400 for FinishTask's state-transition errors) so this only tightens the missing-id case. - updateTaskProgress silently accepted progress updates on terminal tasks (SUCCESS/STOPPED/etc). It now fetches the task first and rejects with 400 when the task isn't RUNNING, matching the FIXME that was already sitting in the handler. Updated TestFinishTask_MissingTask, TestUpdateProgress_MissingTask, and TestClaim_MissingWorker to assert 404. Adjusted TestUpdateProgress_Valid to move the task to RUNNING first (progress updates now require it) and added TestUpdateProgress_WrongState as a regression test for the new guard. Also ran `go fmt` on server_test.go, which had unrelated pre-existing drift. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUXdJfeDXeLerp3W74Rjpm
Owner
Author
|
Code looks good. CI failing with fmt error. Please upgrade your version of go locally to avoid the mismtatch. Feel free to merge when CI is passing. |
turtlemonvh
added a commit
that referenced
this pull request
Sep 3, 2026
- server/server_test.go: gofmt output differs between Go 1.25.9 (CI's pinned version, per Dockerfile) and 1.27 -- re-ran with the correct version. This is the same fix applied to PR #73/#49. - Filed the two follow-up issues requested in review: #78 (reusable NewTestServer test utility) and #79 (Windows CI job). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUXdJfeDXeLerp3W74Rjpm
5 tasks done
turtlemonvh
added a commit
that referenced
this pull request
Sep 3, 2026
Ambient system Go can drift out from under the project (as it did this session: 1.27.0 installed while Dockerfile/scripts/setup.sh pin 1.25.9), and a bare gofmt on PATH then reformats differently than CI expects, breaking `make check-fmt` (#73, #75) with no actual code problem. Per the design discussion on #80, go.mod's `toolchain` directive is more idiomatic than `go install golang.org/dl/goX.Y.Z` for per-project pinning: with GOTOOLCHAIN=auto (the default), any `go` subcommand re-execs into the pinned toolchain regardless of PATH. Also point `make check-fmt` at `$(go env GOROOT)/bin/gofmt` instead of a bare `gofmt` on PATH, since gofmt itself isn't a `go` subcommand and so isn't toolchain-switched on its own — this was the actual mechanism of the incident and needed the explicit fix to close the gap. CI is unaffected: it builds via Docker, and the Dockerfile's pinned GO_VERSION was already correctly aligned with scripts/setup.sh. Documented the three pins that must move together in CLAUDE.md. Claude-Session: https://claude.ai/code/session_01ENu5Xyih32XUfEF85k53ea Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
server/serve_tasks.go:claimTask's worker lookup,updateTaskProgress, andmarkTaskAsFinishednow return 404 viadatabase.ItemNotFoundError(routed through a newstatusForDBErrorhelper inserver_util.go), matching the pattern already used inclaimTask's task-lookup branch. Non-ItemNotFoundError failures keep their previous fallback status (500 for DB errors, 400 forFinishTask's state-transition errors).updateTaskProgress: it now fetches the task first and rejects (400) a progress update on a task that isn'tRUNNING, closing the gap where a terminal (SUCCESS/STOPPED/etc.) task would silently accept a progress update.Test plan
go fmt ./...go test ./server/... -run 'TestUpdateProgress|TestFinishTask|TestClaim' -v— all pass, including newTestUpdateProgress_WrongStatego test ./server/...go test ./...go vet ./...gofmt -lcleanCloses #49
🤖 Generated with Claude Code
https://claude.ai/code/session_01HUXdJfeDXeLerp3W74Rjpm