diff --git a/docs/superpowers/plans/2026-07-18-agent-atomic-scan-consume.md b/docs/superpowers/plans/2026-07-18-agent-atomic-scan-consume.md new file mode 100644 index 00000000..05e9c755 --- /dev/null +++ b/docs/superpowers/plans/2026-07-18-agent-atomic-scan-consume.md @@ -0,0 +1,871 @@ +# Agent Atomic Scan Consumption Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add a genuinely atomic `POST /scan/consume` endpoint to the Idento hardware agent so a fast second barcode/QR scan can never be silently erased by the existing `GET /scan/last` + `POST /scan/clear` polling protocol (the race CodeRabbit flagged on panel PR #77, branch `panel/p4.1-checkin-loop`). + +**Architecture:** Extract the scan buffer currently held as three local variables inside `func main()` (`scanDataMutex`, `lastScannedCode`, `lastScanTime`) into a small mutex-guarded `scanBuffer` type with a new `Consume()` method that reads and clears the buffer under a single critical section — eliminating the read/clear race by construction rather than by coordination. Extract the three `/scan/*` HTTP handlers out of `main()` into a `registerScanRoutes` function so they're testable via `net/http/httptest` without booting the whole agent. `GET /scan/last` and `POST /scan/clear` keep their exact current behavior (the panel client is not touched in this task — that's a separate follow-up gated on this agent release shipping). + +**Tech Stack:** Go 1.25 (module `idento/agent`), standard library `net/http`/`net/http/httptest`/`sync`/`encoding/json`, existing `go.bug.st/serial`-based scanner package (unaffected). + +## Global Constraints + +- Module: `idento/agent` (agent/go.mod), Go 1.25.4 / toolchain go1.26.5. Run all Go commands from the `agent/` directory. +- Package: everything in this plan lives in `package main` at the agent module root, matching the existing convention (`main.go`, `config_auth_test.go` are all `package main`). +- CI runs `cd agent && go test -race -coverprofile=coverage.out -covermode=atomic ./...` and `golangci-lint run ./...` (v2.12) — new code must pass both. Verify locally with the same commands before considering a task done. +- `GET /scan/last` and `POST /scan/clear` are used today by `panel/src/shared/agent/agentClient.ts` — their request/response shape and behavior must not change. Do not touch any file under `panel/` in this plan. +- The agent has no CHANGELOG file and `openapi.yaml`'s `version: 1.1.0` field has not moved across the last three doc/behavior-sync commits (`4ae2236`, `df93727`, `419d8ea`) — do not invent a version bump or CHANGELOG entry; this repo's convention for the agent is simply keeping `openapi.yaml` and `README.md` in sync with the implementation. +- `mux.HandleFunc` routes registered on the agent's `http.ServeMux` are automatically wrapped by `authorizer.Middleware(mux)` (agent/main.go:1136) — no per-route auth code is needed or written in this plan. +- Every response body for `/scan/*` endpoints is the same JSON shape: `{"code": string, "time": RFC3339 timestamp}` (empty string / zero time `0001-01-01T00:00:00Z` when the buffer is empty), matching the existing `ScanData` schema in `agent/openapi.yaml`. + +--- + +## File Structure + +- Create: `agent/scan_buffer.go` — the `scanBuffer` type (`Set`, `Last`, `Clear`, `Consume`), package main. +- Create: `agent/scan_buffer_test.go` — unit + concurrency tests for `scanBuffer`. +- Create: `agent/scan_routes.go` — `registerScanRoutes(mux *http.ServeMux, buf *scanBuffer)` wiring `/scan/last`, `/scan/clear`, `/scan/consume`. +- Create: `agent/scan_routes_test.go` — HTTP-level tests for the three routes via `httptest`. +- Modify: `agent/main.go` — replace the three local scan vars and three inline handler closures with `scanBuffer` + `registerScanRoutes`. +- Modify: `agent/openapi.yaml` — document `POST /scan/consume`. +- Modify: `agent/README.md` — add the new endpoint to the API overview table. + +--- + +### Task 1: `scanBuffer` type with atomic `Consume` + +**Files:** +- Create: `agent/scan_buffer.go` +- Test: `agent/scan_buffer_test.go` + +**Interfaces:** +- Produces: `type scanBuffer struct { ... }`, `func newScanBuffer() *scanBuffer`, `func (b *scanBuffer) Set(code string)`, `func (b *scanBuffer) Last() (code string, at time.Time)`, `func (b *scanBuffer) Clear()`, `func (b *scanBuffer) Consume() (code string, at time.Time)`. Task 3 (route wiring) and Task 4 (main.go wiring) call these exact names. + +- [ ] **Step 1: Write the failing tests** + +Create `agent/scan_buffer_test.go`: + +```go +package main + +import ( + "fmt" + "sync" + "testing" +) + +func TestScanBuffer_InitiallyEmpty(t *testing.T) { + buf := newScanBuffer() + code, at := buf.Last() + if code != "" || !at.IsZero() { + t.Fatalf("expected empty buffer, got code=%q at=%v", code, at) + } +} + +func TestScanBuffer_SetThenLast_DoesNotClear(t *testing.T) { + buf := newScanBuffer() + buf.Set("A") + + code, at := buf.Last() + if code != "A" || at.IsZero() { + t.Fatalf("expected code=A with non-zero time, got code=%q at=%v", code, at) + } + + // Last() must be read-only: calling it again returns the same value. + code2, at2 := buf.Last() + if code2 != "A" || at2 != at { + t.Fatalf("Last() must not mutate the buffer, got code=%q at=%v (want code=A at=%v)", code2, at2, at) + } +} + +func TestScanBuffer_Clear(t *testing.T) { + buf := newScanBuffer() + buf.Set("A") + buf.Clear() + + code, at := buf.Last() + if code != "" || !at.IsZero() { + t.Fatalf("expected empty buffer after Clear, got code=%q at=%v", code, at) + } +} + +func TestScanBuffer_Consume_ReturnsAndClearsInOneCall(t *testing.T) { + buf := newScanBuffer() + buf.Set("A") + + code, at := buf.Consume() + if code != "A" || at.IsZero() { + t.Fatalf("expected Consume to return code=A with non-zero time, got code=%q at=%v", code, at) + } + + // The buffer must now be empty: Consume cleared exactly what it returned. + code2, at2 := buf.Last() + if code2 != "" || !at2.IsZero() { + t.Fatalf("expected buffer cleared after Consume, got code=%q at=%v", code2, at2) + } +} + +func TestScanBuffer_Consume_EmptyBufferReturnsZeroValue(t *testing.T) { + buf := newScanBuffer() + code, at := buf.Consume() + if code != "" || !at.IsZero() { + t.Fatalf("expected zero value from Consume on empty buffer, got code=%q at=%v", code, at) + } +} + +// TestScanBuffer_Consume_NeverDropsAScanArrivingBeforeConsumption reproduces +// the race CodeRabbit flagged on panel PR #77 (panel/p4.1-checkin-loop): with +// the old GET /scan/last + POST /scan/clear protocol, a second physical scan +// arriving between a poller's read and its later clear call was silently +// erased by that clear. Consume() collapses read+clear into a single +// critical section, so there is no window between them for a second Set() +// to land in — it either lands before this Consume() call (and is what gets +// returned) or after it (and survives untouched for the next call). +func TestScanBuffer_Consume_NeverDropsAScanArrivingBeforeConsumption(t *testing.T) { + buf := newScanBuffer() + buf.Set("A") + + // A poller reads the buffer (as GET /scan/last would) ... + readCode, _ := buf.Last() + if readCode != "A" { + t.Fatalf("expected to read A, got %q", readCode) + } + + // ... and before it gets a chance to clear what it read, a second + // physical scan arrives. + buf.Set("B") + + // The atomic consume must hand back the newer scan B — it was never + // blind to it, unlike an unconditional POST /scan/clear at this point + // would have been. + consumedCode, _ := buf.Consume() + if consumedCode != "B" { + t.Fatalf("expected Consume to return the newer scan B (not silently dropped), got %q", consumedCode) + } + + finalCode, finalAt := buf.Last() + if finalCode != "" || !finalAt.IsZero() { + t.Fatalf("expected buffer cleared after Consume, got code=%q at=%v", finalCode, finalAt) + } +} + +// TestScanBuffer_ConcurrentSetAndConsume_NoTornReads hammers Set and Consume +// from many goroutines at once. Run with `go test -race` (as CI does): the +// race detector catches any unsynchronized access, and the per-call +// assertion catches torn reads (a non-empty code paired with a zero time, +// which could only happen if a read observed the struct mid-write). +func TestScanBuffer_ConcurrentSetAndConsume_NoTornReads(t *testing.T) { + buf := newScanBuffer() + var wg sync.WaitGroup + + for i := 0; i < 50; i++ { + wg.Add(1) + go func(n int) { + defer wg.Done() + buf.Set(fmt.Sprintf("CODE-%d", n)) + }(i) + } + + for i := 0; i < 50; i++ { + wg.Add(1) + go func() { + defer wg.Done() + code, at := buf.Consume() + if code != "" && at.IsZero() { + t.Errorf("Consume returned non-empty code %q with zero time (torn read)", code) + } + }() + } + + wg.Wait() + + // Whatever is left over must itself be internally consistent. + code, at := buf.Last() + if code != "" && at.IsZero() { + t.Fatalf("final buffer state inconsistent: code=%q at=%v", code, at) + } +} +``` + +- [ ] **Step 2: Run the tests to verify they fail to compile** + +Run: `cd agent && go test ./... -run TestScanBuffer -v` +Expected: FAIL — `undefined: newScanBuffer` (scan_buffer.go does not exist yet). + +- [ ] **Step 3: Write the implementation** + +Create `agent/scan_buffer.go`: + +```go +package main + +import ( + "sync" + "time" +) + +// scanBuffer holds the most recently scanned barcode/QR code behind a +// mutex. It is shared between the scanner's listen goroutine (Set) and the +// /scan/* HTTP handlers (Last, Clear, Consume). +type scanBuffer struct { + mu sync.Mutex + code string + at time.Time +} + +func newScanBuffer() *scanBuffer { + return &scanBuffer{} +} + +// Set records a freshly scanned code, overwriting whatever was buffered +// before it and stamping it with the current time. +func (b *scanBuffer) Set(code string) { + b.mu.Lock() + b.code = code + b.at = time.Now() + b.mu.Unlock() +} + +// Last returns the currently buffered code without clearing it. +func (b *scanBuffer) Last() (code string, at time.Time) { + b.mu.Lock() + defer b.mu.Unlock() + return b.code, b.at +} + +// Clear unconditionally empties the buffer. +func (b *scanBuffer) Clear() { + b.mu.Lock() + b.code = "" + b.at = time.Time{} + b.mu.Unlock() +} + +// Consume atomically returns the buffered code and empties the buffer in +// the same critical section. Unlike a separate Last()-then-Clear() pair, +// there is no window between the read and the clear for another Set() to +// slip through unnoticed: a scan that arrives concurrently either happens +// before this call's lock (and is what gets returned) or after it (and +// survives for the next Consume()). +func (b *scanBuffer) Consume() (code string, at time.Time) { + b.mu.Lock() + defer b.mu.Unlock() + code, at = b.code, b.at + b.code = "" + b.at = time.Time{} + return code, at +} +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `cd agent && go test ./... -run TestScanBuffer -v -race` +Expected: PASS (all `TestScanBuffer_*` tests), no data race reported. + +- [ ] **Step 5: Commit** + +```bash +git add agent/scan_buffer.go agent/scan_buffer_test.go +git commit -m "feat(agent): add atomic scanBuffer.Consume to fix scan read/clear race" +``` + +--- + +### Task 2: `registerScanRoutes` — wire `/scan/last`, `/scan/clear`, and the new `/scan/consume` + +**Files:** +- Create: `agent/scan_routes.go` +- Test: `agent/scan_routes_test.go` + +**Interfaces:** +- Consumes: `newScanBuffer() *scanBuffer`, `(*scanBuffer).Set/Last/Clear/Consume` from Task 1. +- Produces: `func registerScanRoutes(mux *http.ServeMux, buf *scanBuffer)`. Task 4 (main.go wiring) calls this exact name/signature. + +- [ ] **Step 1: Write the failing tests** + +Create `agent/scan_routes_test.go`: + +```go +package main + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" +) + +type scanDataResponse struct { + Code string `json:"code"` + Time string `json:"time"` +} + +func newScanTestServer() (*httptest.Server, *scanBuffer) { + buf := newScanBuffer() + mux := http.NewServeMux() + registerScanRoutes(mux, buf) + return httptest.NewServer(mux), buf +} + +func TestScanRoutes_Last_ReflectsBufferWithoutClearing(t *testing.T) { + srv, buf := newScanTestServer() + defer srv.Close() + buf.Set("A") + + resp, err := http.Get(srv.URL + "/scan/last") + if err != nil { + t.Fatalf("GET /scan/last: %v", err) + } + defer resp.Body.Close() + + var got scanDataResponse + if err := json.NewDecoder(resp.Body).Decode(&got); err != nil { + t.Fatalf("decode response: %v", err) + } + if got.Code != "A" { + t.Fatalf("expected code=A, got %q", got.Code) + } + + // /scan/last must not clear the buffer. + code, _ := buf.Last() + if code != "A" { + t.Fatalf("expected buffer to still hold A after GET /scan/last, got %q", code) + } +} + +func TestScanRoutes_Clear_RequiresPOST(t *testing.T) { + srv, _ := newScanTestServer() + defer srv.Close() + + resp, err := http.Get(srv.URL + "/scan/clear") + if err != nil { + t.Fatalf("GET /scan/clear: %v", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusMethodNotAllowed { + t.Fatalf("expected 405 for GET /scan/clear, got %d", resp.StatusCode) + } +} + +func TestScanRoutes_Clear_EmptiesBuffer(t *testing.T) { + srv, buf := newScanTestServer() + defer srv.Close() + buf.Set("A") + + resp, err := http.Post(srv.URL+"/scan/clear", "application/json", nil) + if err != nil { + t.Fatalf("POST /scan/clear: %v", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("expected 200, got %d", resp.StatusCode) + } + + code, _ := buf.Last() + if code != "" { + t.Fatalf("expected buffer cleared, got %q", code) + } +} + +func TestScanRoutes_Consume_RequiresPOST(t *testing.T) { + srv, _ := newScanTestServer() + defer srv.Close() + + resp, err := http.Get(srv.URL + "/scan/consume") + if err != nil { + t.Fatalf("GET /scan/consume: %v", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusMethodNotAllowed { + t.Fatalf("expected 405 for GET /scan/consume, got %d", resp.StatusCode) + } +} + +func TestScanRoutes_Consume_ReturnsAndClearsInOneRequest(t *testing.T) { + srv, buf := newScanTestServer() + defer srv.Close() + buf.Set("A") + + resp, err := http.Post(srv.URL+"/scan/consume", "application/json", nil) + if err != nil { + t.Fatalf("POST /scan/consume: %v", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("expected 200, got %d", resp.StatusCode) + } + + var got scanDataResponse + if err := json.NewDecoder(resp.Body).Decode(&got); err != nil { + t.Fatalf("decode response: %v", err) + } + if got.Code != "A" { + t.Fatalf("expected code=A, got %q", got.Code) + } + + // The buffer must be empty immediately after the response — no separate + // clear call is needed, and none is possible to race against. + code, _ := buf.Last() + if code != "" { + t.Fatalf("expected buffer cleared after /scan/consume, got %q", code) + } +} + +func TestScanRoutes_Consume_EmptyBufferReturnsEmptyCode(t *testing.T) { + srv, _ := newScanTestServer() + defer srv.Close() + + resp, err := http.Post(srv.URL+"/scan/consume", "application/json", nil) + if err != nil { + t.Fatalf("POST /scan/consume: %v", err) + } + defer resp.Body.Close() + + var got scanDataResponse + if err := json.NewDecoder(resp.Body).Decode(&got); err != nil { + t.Fatalf("decode response: %v", err) + } + if got.Code != "" { + t.Fatalf("expected empty code from empty buffer, got %q", got.Code) + } +} +``` + +- [ ] **Step 2: Run the tests to verify they fail to compile** + +Run: `cd agent && go test ./... -run TestScanRoutes -v` +Expected: FAIL — `undefined: registerScanRoutes` (scan_routes.go does not exist yet). + +- [ ] **Step 3: Write the implementation** + +Create `agent/scan_routes.go`: + +```go +package main + +import ( + "encoding/json" + "log" + "net/http" + "time" +) + +// registerScanRoutes wires the /scan/* endpoints onto mux against the +// shared buf. Extracted out of main() so the atomic Consume behavior (the +// fix for the panel PR #77 read/clear race) is testable via httptest +// without booting the full agent (printers, scanners, auth, config). +func registerScanRoutes(mux *http.ServeMux, buf *scanBuffer) { + mux.HandleFunc("/scan/last", func(w http.ResponseWriter, r *http.Request) { + code, at := buf.Last() + writeScanData(w, code, at) + }) + + mux.HandleFunc("/scan/clear", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) + return + } + buf.Clear() + w.WriteHeader(http.StatusOK) + if err := json.NewEncoder(w).Encode(map[string]string{"status": "cleared"}); err != nil { + log.Printf("Failed to encode response: %v", err) + } + }) + + mux.HandleFunc("/scan/consume", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) + return + } + code, at := buf.Consume() + writeScanData(w, code, at) + }) +} + +// writeScanData writes the shared ScanData JSON shape ({"code", "time"}) +// used by /scan/last and /scan/consume. +func writeScanData(w http.ResponseWriter, code string, at time.Time) { + response := map[string]interface{}{ + "code": code, + "time": at, + } + + // Marshal to bytes first to avoid partial writes on error. + data, err := json.Marshal(response) + if err != nil { + log.Printf("Failed to marshal scan response: %v", err) + http.Error(w, "Failed to encode response", http.StatusInternalServerError) + return + } + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + if _, err := w.Write(data); err != nil { + log.Printf("Failed to write scan response: %v", err) + } +} +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `cd agent && go test ./... -run TestScanRoutes -v -race` +Expected: PASS (all `TestScanRoutes_*` tests). + +- [ ] **Step 5: Commit** + +```bash +git add agent/scan_routes.go agent/scan_routes_test.go +git commit -m "feat(agent): extract /scan/* handlers, add POST /scan/consume route" +``` + +--- + +### Task 3: Wire `scanBuffer` + `registerScanRoutes` into `main()`, remove the old inline handlers + +**Files:** +- Modify: `agent/main.go:234-237` (local var declarations) +- Modify: `agent/main.go:291-296` (scanner `OnScan` callback) +- Modify: `agent/main.go:838-878` (inline `/scan/last` and `/scan/clear` handlers, replaced by one `registerScanRoutes` call) + +**Interfaces:** +- Consumes: `newScanBuffer()`, `(*scanBuffer).Set`, `registerScanRoutes(mux, buf)` from Tasks 1–2. + +- [ ] **Step 1: Replace the local scan buffer variables** + +In `agent/main.go`, find (around line 234): + +```go + // For storing scanned data temporarily + var scanDataMutex sync.Mutex + var lastScannedCode string + var lastScanTime time.Time +``` + +Replace with: + +```go + // For storing scanned data temporarily + scanBuf := newScanBuffer() +``` + +- [ ] **Step 2: Update the scanner's `OnScan` callback** + +Find (around line 291): + +```go + s.OnScan(func(data string) { + scanDataMutex.Lock() + lastScannedCode = data + lastScanTime = time.Now() + scanDataMutex.Unlock() + log.Printf("📋 Scan received: %s", data) + }) +``` + +Replace with: + +```go + s.OnScan(func(data string) { + scanBuf.Set(data) + log.Printf("📋 Scan received: %s", data) + }) +``` + +- [ ] **Step 3: Replace the inline `/scan/last` and `/scan/clear` handlers with `registerScanRoutes`** + +Find (around line 838): + +```go + mux.HandleFunc("/scan/last", func(w http.ResponseWriter, r *http.Request) { + scanDataMutex.Lock() + defer scanDataMutex.Unlock() + + // Return last scanned code (for polling) + response := map[string]interface{}{ + "code": lastScannedCode, + "time": lastScanTime, + } + + // Marshal response to bytes first to avoid partial writes on error + data, err := json.Marshal(response) + if err != nil { + log.Printf("Failed to marshal scan response: %v", err) + http.Error(w, "Failed to encode response", http.StatusInternalServerError) + return + } + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + if _, err := w.Write(data); err != nil { + log.Printf("Failed to write scan response: %v", err) + } + }) + + mux.HandleFunc("/scan/clear", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) + return + } + + scanDataMutex.Lock() + lastScannedCode = "" + lastScanTime = time.Time{} + scanDataMutex.Unlock() + + w.WriteHeader(http.StatusOK) + if err := json.NewEncoder(w).Encode(map[string]string{"status": "cleared"}); err != nil { + log.Printf("Failed to encode response: %v", err) + } + }) +``` + +Replace with: + +```go + registerScanRoutes(mux, scanBuf) +``` + +- [ ] **Step 4: Build and verify no leftover references** + +Run: `cd agent && go build ./...` +Expected: builds cleanly. If `sync` or `time` become unused imports in main.go, `go build` will fail with "imported and not used" — check first with `grep -n "sync\.\|time\." agent/main.go`; both packages are still used elsewhere in `main.go` (`configMu sync.RWMutex`, `10 * time.Second` server timeouts, etc.), so no import changes are expected. + +Run: `grep -n "scanDataMutex\|lastScannedCode\|lastScanTime" agent/main.go` +Expected: no output (all references removed). + +- [ ] **Step 5: Run the full agent test suite** + +Run: `cd agent && go test -race ./...` +Expected: PASS, all packages including the new `TestScanBuffer_*` and `TestScanRoutes_*` tests. + +- [ ] **Step 6: Manual smoke test** + +Run: `cd agent && go run . --mock --port 12399 &` then, once it logs `Listening on: http://127.0.0.1:12399`: + +```bash +curl -s -X POST http://127.0.0.1:12399/scan/consume -H 'Content-Type: application/json' +``` + +Expected: `{"code":"","time":"0001-01-01T00:00:00Z"}` (no scanner hardware attached in `--mock` mode, so the buffer is empty — this just confirms the route is wired and returns the right shape). Stop the background process afterward (`kill %1` or the equivalent job-control command in your shell). + +- [ ] **Step 7: Commit** + +```bash +git add agent/main.go +git commit -m "refactor(agent): wire scanBuffer + registerScanRoutes into main()" +``` + +--- + +### Task 4: Document `POST /scan/consume` in `openapi.yaml` + +**Files:** +- Modify: `agent/openapi.yaml` + +**Interfaces:** +- Consumes: nothing code-level — this is documentation matching the route added in Task 2/3. + +- [ ] **Step 1: Add a cross-reference note to `/scan/last`'s description** + +In `agent/openapi.yaml`, find (around line 703): + +```yaml + /scan/last: + get: + tags: + - Scan + summary: Получить последний отсканированный код + description: | + Возвращает последний код, отсканированный любым подключенным сканером. + Используется для polling при тестировании сканеров. Если сканирований + не было, code — пустая строка, time — нулевое значение + 0001-01-01T00:00:00Z. + responses: +``` + +Replace with: + +```yaml + /scan/last: + get: + tags: + - Scan + summary: Получить последний отсканированный код + description: | + Возвращает последний код, отсканированный любым подключенным сканером. + Используется для polling при тестировании сканеров. Если сканирований + не было, code — пустая строка, time — нулевое значение + 0001-01-01T00:00:00Z. + + ⚠️ Комбинация `GET /scan/last` + `POST /scan/clear` не атомарна: если + второй скан приходит в промежутке между чтением и последующей + очисткой, `POST /scan/clear` безусловно стирает и его — он теряется + без возможности восстановления. Клиентам, которым нужно гарантированно + не терять сканы, следует использовать `POST /scan/consume`. + responses: +``` + +- [ ] **Step 2: Add a similar note to `/scan/clear`'s description** + +Find (around line 721): + +```yaml + /scan/clear: + post: + tags: + - Scan + summary: Очистить последний скан + description: Сбрасывает буфер последнего отсканированного кода + responses: +``` + +Replace with: + +```yaml + /scan/clear: + post: + tags: + - Scan + summary: Очистить последний скан + description: | + Безусловно сбрасывает буфер последнего отсканированного кода — + независимо от того, что в нём находится в момент вызова. См. + предупреждение в описании `GET /scan/last` про гонку при связке с + последующим чтением; для атомарного чтения+очистки используйте + `POST /scan/consume`. + responses: +``` + +- [ ] **Step 3: Add the new `/scan/consume` path** + +Find (around line 738, the blank line between the `/scan/clear` responses block and `/openapi.yaml:`): + +```yaml + status: + type: string + example: "cleared" + + /openapi.yaml: +``` + +Replace with: + +```yaml + status: + type: string + example: "cleared" + + /scan/consume: + post: + tags: + - Scan + summary: Атомарно прочитать и очистить последний скан + description: | + Возвращает последний отсканированный код и одновременно очищает + буфер — чтение и очистка выполняются под одной блокировкой на + стороне агента, одной операцией. В отличие от связки + `GET /scan/last` + `POST /scan/clear`, здесь исключено состояние + гонки: скан, отсканированный между отдельным чтением и последующей + очисткой, не может быть потерян — он либо войдёт в текущий ответ, + либо останется в буфере нетронутым для следующего вызова + `/scan/consume`. Если сканирований не было, code — пустая строка, + time — нулевое значение 0001-01-01T00:00:00Z. + responses: + '200': + description: Считанные и одновременно очищенные данные скана + content: + application/json: + schema: + $ref: '#/components/schemas/ScanData' + + /openapi.yaml: +``` + +- [ ] **Step 4: Validate the YAML is well-formed** + +Run: `cd agent && python3 -c "import yaml, sys; yaml.safe_load(open('openapi.yaml'))" && echo OK` +Expected: `OK` (no YAML syntax errors). If `python3`/`pyyaml` is unavailable, instead start the agent (`go run . --mock`) and confirm `GET /openapi.yaml` and `GET /docs` still return `200` and render without errors. + +- [ ] **Step 5: Commit** + +```bash +git add agent/openapi.yaml +git commit -m "docs(agent): document POST /scan/consume in openapi.yaml" +``` + +--- + +### Task 5: Sync `README.md`'s endpoint table + +**Files:** +- Modify: `agent/README.md` + +- [ ] **Step 1: Add the new route to the endpoint overview table** + +In `agent/README.md`, find (around line 67): + +```markdown +| GET | `/scan/last` | Последний отсканированный код: `{code, time}` | +| POST | `/scan/clear` | Очистить буфер последнего скана | +``` + +Replace with: + +```markdown +| GET | `/scan/last` | Последний отсканированный код: `{code, time}` (не атомарно с `/scan/clear`, см. `/docs`) | +| POST | `/scan/clear` | Очистить буфер последнего скана (безусловно) | +| POST | `/scan/consume` | Атомарно получить и очистить последний скан — без риска потерять скан, пришедший между чтением и очисткой | +``` + +- [ ] **Step 2: Commit** + +```bash +git add agent/README.md +git commit -m "docs(agent): add POST /scan/consume to README endpoint table" +``` + +--- + +### Task 6: Final verification pass + +**Files:** none (verification only) + +- [ ] **Step 1: Run the full test suite with race detection and coverage, matching CI exactly** + +Run: `cd agent && go test -race -coverprofile=coverage.out -covermode=atomic ./...` +Expected: PASS, all packages. + +- [ ] **Step 2: Run `go vet`** + +Run: `cd agent && go vet ./...` +Expected: no output (clean). + +- [ ] **Step 3: Run `golangci-lint`, matching CI's version/args** + +Run: `cd agent && golangci-lint run ./...` +Expected: no issues reported. (CI pins `v2.12`; if the locally installed version differs and reports unrelated pre-existing findings, only new findings introduced by this plan's files — `scan_buffer.go`, `scan_routes.go`, `main.go` — are in scope to fix.) + +- [ ] **Step 4: Build the agent binary** + +Run: `cd agent && go build -o /tmp/idento-agent .` +Expected: builds cleanly, matching the CI "Build agent" step. + +- [ ] **Step 5: Confirm the panel is untouched** + +Run: `git status --short panel/` +Expected: no output — this plan must not have modified anything under `panel/` (the panel-side migration to `/scan/consume` is an explicit follow-up, not part of this task). + +- [ ] **Step 6: Review the full diff** + +Run: `cd agent && git diff --stat main.go scan_buffer.go scan_buffer_test.go scan_routes.go scan_routes_test.go openapi.yaml README.md` +Expected: matches the File Structure section above — no unrelated files touched. + +No commit for this task — it is a verification-only checkpoint. If anything fails, fix it as part of the task that introduced the problem (re-open that task's commit with a fixup, or a new small commit) rather than bundling unrelated fixes here. + +--- + +## Self-Review Notes + +- **Spec coverage:** Atomic consume endpoint (Task 1–3, either design 1 or 2 from the spec — this plan picks design 1, `POST /scan/consume`, since the agent's scan state was already a single mutex-guarded read/write pair local to `main()`, making a combined atomic accessor the natural, minimal extension of the existing pattern). `openapi.yaml` documentation (Task 4). Tests matching existing conventions — `package main`, `testing.T`, table-free direct assertions like `config_auth_test.go` (Tasks 1–2), plus a concurrency test since CI runs `go test -race` (Task 1). CHANGELOG/version convention checked and confirmed not applicable (Global Constraints). Panel-side client changes explicitly excluded (Global Constraints, Task 6 Step 5). +- **Placeholder scan:** No TBD/TODO markers; every step has complete, runnable code or an exact command with expected output. +- **Type consistency:** `scanBuffer`, `newScanBuffer`, `Set`, `Last`, `Clear`, `Consume` are named identically across Tasks 1, 2, and 3. `registerScanRoutes(mux *http.ServeMux, buf *scanBuffer)` is named identically across Tasks 2 and 3. diff --git a/panel/src/features/checkin/ScanInput.test.tsx b/panel/src/features/checkin/ScanInput.test.tsx index 32d69ed8..61681d9b 100644 --- a/panel/src/features/checkin/ScanInput.test.tsx +++ b/panel/src/features/checkin/ScanInput.test.tsx @@ -34,8 +34,8 @@ const ADA: Attendee = { let attendeesHitCount = 0; let lastSearchParam: string | null = null; let attendeesShouldError = false; -let scanLastResponse: { code: string; time: string } = { code: "", time: "0001-01-01T00:00:00Z" }; -let scanLastShouldError = false; +let scanConsumeResponse: { code: string; time: string } = { code: "", time: "0001-01-01T00:00:00Z" }; +let scanConsumeShouldError = false; const server = startMswServer( http.get("http://api.test/api/events/:eventId/attendees", ({ request }) => { @@ -46,11 +46,10 @@ const server = startMswServer( const matches = lastSearchParam && "Ada Lovelace ada@example.com PD-0107".includes(lastSearchParam) ? [ADA] : []; return HttpResponse.json({ attendees: matches, total: matches.length, page: 1, per_page: 8 }); }), - http.get("http://agent.test/scan/last", () => { - if (scanLastShouldError) return new HttpResponse(null, { status: 500 }); - return HttpResponse.json(scanLastResponse); + http.post("http://agent.test/scan/consume", () => { + if (scanConsumeShouldError) return new HttpResponse(null, { status: 500 }); + return HttpResponse.json(scanConsumeResponse); }), - http.post("http://agent.test/scan/clear", () => HttpResponse.json({ status: "cleared" })), ); void server; @@ -91,8 +90,8 @@ describe("ScanInput", () => { attendeesHitCount = 0; lastSearchParam = null; attendeesShouldError = false; - scanLastResponse = { code: "", time: "0001-01-01T00:00:00Z" }; - scanLastShouldError = false; + scanConsumeResponse = { code: "", time: "0001-01-01T00:00:00Z" }; + scanConsumeShouldError = false; }); it.each(["wedge", "scanner", "manual"] as const)( @@ -117,7 +116,7 @@ describe("ScanInput", () => { }); it("scanner mode: shows a waiting hint normally, and a degraded hint once the agent is unreachable", async () => { - scanLastShouldError = true; + scanConsumeShouldError = true; renderScanInput({ mode: "scanner" }); await waitFor(() => @@ -308,7 +307,7 @@ describe("ScanInput", () => { // mode, per the block above). A dedicated key with no manual-search // reference is shown instead. it("shows manual-search-free degraded copy when the scanner fails AND manualSearchEnabled is false", async () => { - scanLastShouldError = true; + scanConsumeShouldError = true; renderScanInput({ mode: "scanner", manualSearchEnabled: false }); expect( diff --git a/panel/src/features/checkin/useScanInput.test.tsx b/panel/src/features/checkin/useScanInput.test.tsx index 104d97d2..5898f9a9 100644 --- a/panel/src/features/checkin/useScanInput.test.tsx +++ b/panel/src/features/checkin/useScanInput.test.tsx @@ -15,29 +15,21 @@ import { http, HttpResponse } from "msw"; import { useScanInput, type UseScanInputOptions } from "./useScanInput"; import { startMswServer } from "../../test/msw"; -let scanLastResponse: { code: string; time: string } = { code: "", time: "0001-01-01T00:00:00Z" }; -let scanLastHitCount = 0; -let scanLastShouldError = false; -let scanClearHitCount = 0; -// PR #77 bot-review round, Finding P -- lets a test make the NEXT -// `/scan/clear` call fail without touching `/scan/last`'s own error toggle -// above (a clear failure must not also look like the agent being -// unreachable for `getLastScan`). -let scanClearShouldFailNext = false; +let scanConsumeResponse: { code: string; time: string } = { code: "", time: "0001-01-01T00:00:00Z" }; +let scanConsumeHitCount = 0; +let scanConsumeShouldError = false; const server = startMswServer( - http.get("http://agent.test/scan/last", () => { - scanLastHitCount += 1; - if (scanLastShouldError) return new HttpResponse(null, { status: 500 }); - return HttpResponse.json(scanLastResponse); - }), - http.post("http://agent.test/scan/clear", () => { - scanClearHitCount += 1; - if (scanClearShouldFailNext) { - scanClearShouldFailNext = false; - return new HttpResponse(null, { status: 500 }); - } - return HttpResponse.json({ status: "cleared" }); + http.post("http://agent.test/scan/consume", () => { + scanConsumeHitCount += 1; + if (scanConsumeShouldError) return new HttpResponse(null, { status: 500 }); + const response = scanConsumeResponse; + // Real /scan/consume atomically clears the buffer server-side -- mimic + // that here so a static scanConsumeResponse doesn't keep getting + // "re-consumed" forever, matching how the real agent behaves and + // proving the client no longer needs its own client-side dedup. + scanConsumeResponse = { code: "", time: "0001-01-01T00:00:00Z" }; + return HttpResponse.json(response); }), ); void server; @@ -80,11 +72,9 @@ function ManualHarness({ onCode }: { onCode: (code: string) => void }) { describe("useScanInput", () => { beforeEach(() => { window.__ENV__ = { API_URL: "http://api.test", AGENT_URL: "http://agent.test" }; - scanLastResponse = { code: "", time: "0001-01-01T00:00:00Z" }; - scanLastHitCount = 0; - scanLastShouldError = false; - scanClearHitCount = 0; - scanClearShouldFailNext = false; + scanConsumeResponse = { code: "", time: "0001-01-01T00:00:00Z" }; + scanConsumeHitCount = 0; + scanConsumeShouldError = false; }); describe("wedge mode", () => { @@ -206,40 +196,39 @@ describe("useScanInput", () => { }); describe("scanner mode", () => { - it("polls agentClient.getLastScan, emits onCode once for a new scan, and clears the agent buffer", async () => { - scanLastResponse = { code: "PD-0107", time: "2026-07-17T10:00:00Z" }; + it("polls agentClient.consumeLastScan, emits onCode exactly once for a scan", async () => { + scanConsumeResponse = { code: "PD-0107", time: "2026-07-17T10:00:00Z" }; const onCode = vi.fn(); render(); await waitFor(() => expect(onCode).toHaveBeenCalledTimes(1)); expect(onCode).toHaveBeenCalledWith("PD-0107"); - await waitFor(() => expect(scanClearHitCount).toBe(1)); - // A second (and third) poll cycle sees the SAME {code, time} pair - // (this mock never changes) -- must never re-emit or re-clear. + // The mock already cleared its own state on that first consume + // (mirroring the real agent's atomic consume) -- further poll ticks + // see the empty sentinel and must never re-emit. await new Promise((resolve) => setTimeout(resolve, 500)); expect(onCode).toHaveBeenCalledTimes(1); - expect(scanClearHitCount).toBe(1); - expect(scanLastHitCount).toBeGreaterThan(1); + expect(scanConsumeHitCount).toBeGreaterThan(1); }, 10000); it("does not emit while the buffer is empty (the sentinel no-scan-yet state)", async () => { const onCode = vi.fn(); render(); - await waitFor(() => expect(scanLastHitCount).toBeGreaterThan(1)); + await waitFor(() => expect(scanConsumeHitCount).toBeGreaterThan(1)); expect(onCode).not.toHaveBeenCalled(); }); it("sets degraded:true when the agent is unreachable, and clears it again once reachable", async () => { - scanLastShouldError = true; + scanConsumeShouldError = true; const onCode = vi.fn(); render(); await waitFor(() => expect(screen.getByTestId("degraded")).toHaveTextContent("true")); expect(onCode).not.toHaveBeenCalled(); - scanLastShouldError = false; + scanConsumeShouldError = false; await waitFor(() => expect(screen.getByTestId("degraded")).toHaveTextContent("false")); }, 10000); @@ -248,71 +237,72 @@ describe("useScanInput", () => { render(); await new Promise((resolve) => setTimeout(resolve, 300)); - expect(scanLastHitCount).toBe(0); + expect(scanConsumeHitCount).toBe(0); expect(onCode).not.toHaveBeenCalled(); }); - // PR #77 bot-review round, Finding P -- the CURRENT code's dedup check - // (`last.code === scan.code && last.time === scan.time`) previously - // caused the poll to see the same still-uncleared pair and exit early - // WITHOUT re-attempting the clear (since it's already in - // `lastHandledRef`), leaving the agent's buffer stuck forever after a - // single transient clear failure -- even though the scan itself was - // correctly consumed exactly once (no double-emit). - it("retries a failed clearLastScan() on the next poll of the SAME scan, without re-emitting onCode", async () => { - scanLastResponse = { code: "PD-0107", time: "2026-07-17T10:00:00Z" }; - scanClearShouldFailNext = true; + // Migration note: the OLD getLastScan/clearLastScan pair needed a + // "retry the clear" mechanism, because a transient clearLastScan() + // failure right after a successful getLastScan() left the scan already + // fired to onCode but the agent's buffer still holding it (a stuck + // buffer). The atomic consumeLastScan() has no equivalent failure mode: + // if a poll's consumeLastScan() call fails, nothing was consumed + // server-side at all -- the scan is still sitting untouched in the + // agent's buffer, and the very next successful poll consumes and emits + // it exactly once. No retry bookkeeping is needed on the client at all. + it("does not lose a scan when a poll's consumeLastScan() call fails -- the next poll consumes and emits it", async () => { + scanConsumeResponse = { code: "PD-0107", time: "2026-07-17T10:00:00Z" }; + scanConsumeShouldError = true; const onCode = vi.fn(); render(); - // First poll: onCode fires once, the clear is attempted and FAILS. - await waitFor(() => expect(onCode).toHaveBeenCalledTimes(1)); - await waitFor(() => expect(scanClearHitCount).toBe(1)); + // The failing poll(s) never reach the mock's clearing line -- the + // scan is untouched, and onCode must not fire. + await waitFor(() => expect(scanConsumeHitCount).toBeGreaterThan(0)); + expect(onCode).not.toHaveBeenCalled(); - // A later poll sees the SAME {code, time} pair (this mock never - // changes it) -- the clear is retried (a second /scan/clear hit), - // but onCode must NOT fire again. - await waitFor(() => expect(scanClearHitCount).toBe(2), { timeout: 10000 }); - expect(onCode).toHaveBeenCalledTimes(1); + scanConsumeShouldError = false; + await waitFor(() => expect(onCode).toHaveBeenCalledTimes(1)); + expect(onCode).toHaveBeenCalledWith("PD-0107"); - // And once the clear finally succeeds, no FURTHER retries happen. + // And no further re-emission on later polls. await new Promise((resolve) => setTimeout(resolve, 500)); - expect(scanClearHitCount).toBe(2); expect(onCode).toHaveBeenCalledTimes(1); - }, 15000); + }, 10000); // PR #77 bot-review round 3, Finding 4 -- the 200ms poll interval // previously started a new `poll()` on every tick regardless of whether - // the PREVIOUS `getLastScan()`/`clearLastScan()` round trip had actually - // finished. A local agent that accepts a request but stalls (a real - // possibility on a loaded/slow local network) could otherwise let - // in-flight requests accumulate indefinitely. - it("does not start a new poll while the previous getLastScan() round trip is still outstanding, and resumes once it resolves", async () => { - let releaseScanLast: (() => void) | undefined; + // the PREVIOUS round trip had actually finished. A local agent that + // accepts a request but stalls (a real possibility on a loaded/slow + // local network) could otherwise let in-flight requests accumulate + // indefinitely. This guard is untouched by the migration -- still + // verified here against the new endpoint. + it("does not start a new poll while the previous consumeLastScan() round trip is still outstanding, and resumes once it resolves", async () => { + let releaseScanConsume: (() => void) | undefined; const hang = new Promise((resolve) => { - releaseScanLast = resolve; + releaseScanConsume = resolve; }); server.use( - http.get("http://agent.test/scan/last", async () => { - scanLastHitCount += 1; + http.post("http://agent.test/scan/consume", async () => { + scanConsumeHitCount += 1; await hang; - return HttpResponse.json(scanLastResponse); + return HttpResponse.json(scanConsumeResponse); }), ); const onCode = vi.fn(); render(); // The first poll starts (and hangs on the still-unresolved response). - await waitFor(() => expect(scanLastHitCount).toBe(1)); + await waitFor(() => expect(scanConsumeHitCount).toBe(1)); // Well past several 200ms poll intervals -- a NEW poll must never // start while the first one is still outstanding. await new Promise((resolve) => setTimeout(resolve, 700)); - expect(scanLastHitCount).toBe(1); + expect(scanConsumeHitCount).toBe(1); // Releasing the hung request lets normal polling resume. - releaseScanLast?.(); - await waitFor(() => expect(scanLastHitCount).toBeGreaterThan(1)); + releaseScanConsume?.(); + await waitFor(() => expect(scanConsumeHitCount).toBeGreaterThan(1)); }, 10000); }); @@ -322,7 +312,7 @@ describe("useScanInput", () => { render(); await new Promise((resolve) => setTimeout(resolve, 300)); - expect(scanLastHitCount).toBe(0); + expect(scanConsumeHitCount).toBe(0); expect(onCode).not.toHaveBeenCalled(); expect(screen.getByTestId("degraded")).toHaveTextContent("false"); }); diff --git a/panel/src/features/checkin/useScanInput.ts b/panel/src/features/checkin/useScanInput.ts index fbed20ff..d3af4ca0 100644 --- a/panel/src/features/checkin/useScanInput.ts +++ b/panel/src/features/checkin/useScanInput.ts @@ -8,9 +8,10 @@ // hidden, always-focused text input and treat Enter as the scan boundary". // scanner: a handheld scanner the AGENT (not the browser) talks to over // serial/USB -- the panel has no direct hardware access, so it polls the -// agent's last-scan buffer instead (agentClient.getLastScan/clearLastScan, -// confirmed against agent/openapi.yaml's real /scan/last + /scan/clear -// contract -- not an invented endpoint). +// agent's last-scan buffer instead (agentClient.consumeLastScan, which +// atomically reads AND clears in one request -- confirmed against +// agent/openapi.yaml's real POST /scan/consume contract -- not an invented +// endpoint). // manual: no auto-input at all; ScanInput.tsx's always-present search box // is the only path to a pick in this mode. import * as React from "react"; @@ -40,7 +41,7 @@ export interface WedgeInputProps { } export interface UseScanInputResult { - // True only in scanner mode, only once agentClient.getLastScan() itself + // True only in scanner mode, only once agentClient.consumeLastScan() itself // has failed (agent unreachable/erroring) -- ScanInput.tsx uses this to // hint the operator toward the always-present manual search fallback // rather than silently doing nothing. @@ -53,7 +54,8 @@ export interface UseScanInputResult { } // Matches the brief verbatim ("scanner: single 200ms interval polling -// agentClient.getLastScan()"). +// agentClient.getLastScan()") -- now agentClient.consumeLastScan(), same +// 200ms interval, per the 2026-07-18 atomic-consume migration. const SCANNER_POLL_INTERVAL_MS = 200; // PR #77 bot-review round 2, Finding 5 -- the mount/`wedgeActive`-transition @@ -168,21 +170,20 @@ export function useScanInput({ mode, onCode, enabled }: UseScanInputOptions): Us if (code) onCodeRef.current(code); } - // Scanner-mode polling. A ref (not state) tracks the last {code, time} - // pair this hook has already acted on -- the dedup key the brief - // specifies ("dedup by {code, time} last-handled ... never double- - // consume"), since the agent's buffer can legitimately keep returning the - // SAME pair across several poll ticks (e.g. between this hook's own - // clearLastScan() request landing and the agent actually processing it, - // or simply because nothing new has been scanned since). - const lastHandledRef = React.useRef<{ code: string; time: string } | null>(null); - // PR #77 bot-review round, Finding P -- a transient clearLastScan() - // failure must be RETRIED, not abandoned. Tracks whether the clear for - // `lastHandledRef`'s current pair has actually succeeded yet; the dedup - // branch below only skips re-emitting `onCode` (the dedup check itself is - // untouched), it does NOT skip retrying the clear while this stays false. - const clearPendingRef = React.useRef(false); - + // Scanner-mode polling. Each tick calls agentClient.consumeLastScan(), + // which atomically reads AND clears the agent's buffer in one request + // (agent/openapi.yaml's POST /scan/consume) -- so every non-empty + // response is guaranteed to be a scan this hook has never seen before. + // This replaces the earlier GET /scan/last + POST /scan/clear pair, which + // had a real race (a second physical scan arriving between this hook's + // read and its later clear call was silently erased by that clear -- the + // CodeRabbit finding fixed agent-side by + // docs/superpowers/plans/2026-07-18-agent-atomic-scan-consume.md) and + // needed a {code, time} dedup ref plus a separate retry-the-clear-on- + // failure ref to work around it. Neither is needed anymore: there is no + // separate clear step to retry, and a repeated poll can never re-observe + // an already-handled scan (the buffer is already empty after this hook + // consumed it). React.useEffect(() => { if (mode !== "scanner" || !enabled) { setDegraded(false); @@ -196,43 +197,23 @@ export function useScanInput({ mode, onCode, enabled }: UseScanInputOptions): Us // change -- the effect's own cleanup tears the whole interval down and a // fresh run gets a fresh `false`). Without this, the 200ms `setInterval` // below started a brand-new `poll()` on EVERY tick regardless of whether - // the PREVIOUS getLastScan()/clearLastScan() round trip had actually - // finished -- a local agent that accepts a request but stalls (a real - // possibility on a loaded/slow local network) could let in-flight - // requests accumulate indefinitely with no visible indication anything - // was wrong. A tick that lands while a poll is still outstanding simply - // no-ops now; the NEXT tick after the outstanding one finally - // resolves/rejects picks polling back up normally. + // the PREVIOUS consumeLastScan() round trip had actually finished -- a + // local agent that accepts a request but stalls (a real possibility on + // a loaded/slow local network) could let in-flight requests accumulate + // indefinitely with no visible indication anything was wrong. A tick + // that lands while a poll is still outstanding simply no-ops now; the + // NEXT tick after the outstanding one finally resolves/rejects picks + // polling back up normally. let pollInFlight = false; async function poll() { if (pollInFlight) return; pollInFlight = true; try { - const scan = await agentClient.getLastScan(); + const scan = await agentClient.consumeLastScan(); if (cancelled) return; setDegraded(false); - - if (!scan.code) return; // steady state: nothing new since the last clear. - - const last = lastHandledRef.current; - const isNewScan = !last || last.code !== scan.code || last.time !== scan.time; - - if (isNewScan) { - lastHandledRef.current = { code: scan.code, time: scan.time }; - clearPendingRef.current = true; - onCodeRef.current(scan.code); - } - // else: already consumed -- onCode must NOT fire again (the dedup - // check above is unchanged), but the clear may still be owed if a - // PREVIOUS attempt for this same pair failed (clearPendingRef still - // true) -- retry it below rather than leaving the agent's buffer - // stuck forever. - - if (clearPendingRef.current) { - await agentClient.clearLastScan(); - clearPendingRef.current = false; - } + if (scan.code) onCodeRef.current(scan.code); } catch { if (!cancelled) setDegraded(true); } finally { diff --git a/panel/src/shared/agent/agentClient.test.ts b/panel/src/shared/agent/agentClient.test.ts index fc71a4f1..87041fad 100644 --- a/panel/src/shared/agent/agentClient.test.ts +++ b/panel/src/shared/agent/agentClient.test.ts @@ -19,10 +19,9 @@ const server = startMswServer( HttpResponse.json({ default: "HP_Smart_Tank_790_series" }), ), http.post("http://agent.test/print", () => HttpResponse.json({ status: "printed" })), - http.get("http://agent.test/scan/last", () => + http.post("http://agent.test/scan/consume", () => HttpResponse.json({ code: "", time: "0001-01-01T00:00:00Z" }), ), - http.post("http://agent.test/scan/clear", () => HttpResponse.json({ status: "cleared" })), ); describe("agentClient", () => { @@ -166,60 +165,52 @@ describe("agentClient", () => { }); }); - // P4.1 Task 7 -- the handheld-scanner check-in mode's polling primitives. - // Confirmed present in the agent's OWN contract (agent/openapi.yaml's - // /scan/last, /scan/clear, tag "Scan") -- not a panel-side invention. - describe("getLastScan", () => { - it("returns the empty sentinel when nothing has been scanned yet", async () => { - await expect(agentClient.getLastScan()).resolves.toEqual({ + // P4.1 Task 7 (+ 2026-07-18 atomic-consume migration) -- the handheld- + // scanner check-in mode's polling primitive. Confirmed present in the + // agent's OWN contract (agent/openapi.yaml's POST /scan/consume, tag + // "Scan") -- not a panel-side invention. Replaces the earlier + // GET /scan/last + POST /scan/clear pair, which had a real race (see + // docs/superpowers/plans/2026-07-18-agent-atomic-scan-consume.md). + describe("consumeLastScan", () => { + it("returns the empty sentinel when nothing has been scanned since the last consume", async () => { + await expect(agentClient.consumeLastScan()).resolves.toEqual({ code: "", time: "0001-01-01T00:00:00Z", }); }); - it("returns the last scanned code and time", async () => { + it("returns the consumed scan's code and time", async () => { server.use( - http.get("http://agent.test/scan/last", () => + http.post("http://agent.test/scan/consume", () => HttpResponse.json({ code: "PD-0107", time: "2026-07-17T10:00:00Z" }), ), ); - await expect(agentClient.getLastScan()).resolves.toEqual({ + await expect(agentClient.consumeLastScan()).resolves.toEqual({ code: "PD-0107", time: "2026-07-17T10:00:00Z", }); }); - it("throws on a non-2xx response (agent unreachable/error)", async () => { - server.use(http.get("http://agent.test/scan/last", () => new HttpResponse(null, { status: 500 }))); - await expect(agentClient.getLastScan()).rejects.toThrow(); - }); - - it("throws on a genuine network failure", async () => { - server.use(http.get("http://agent.test/scan/last", () => HttpResponse.error())); - await expect(agentClient.getLastScan()).rejects.toThrow(); - }); - }); - - describe("clearLastScan", () => { - it("resolves without throwing on 200", async () => { - await expect(agentClient.clearLastScan()).resolves.toBeUndefined(); - }); - it("sends Content-Type: application/json (required by the agent's Origin-allowlist auth for mutations, even with no body)", async () => { let capturedContentType: string | null = null; server.use( - http.post("http://agent.test/scan/clear", ({ request }) => { + http.post("http://agent.test/scan/consume", ({ request }) => { capturedContentType = request.headers.get("Content-Type"); - return HttpResponse.json({ status: "cleared" }); + return HttpResponse.json({ code: "", time: "0001-01-01T00:00:00Z" }); }), ); - await agentClient.clearLastScan(); + await agentClient.consumeLastScan(); expect(capturedContentType).toBe("application/json"); }); - it("throws on a non-2xx response", async () => { - server.use(http.post("http://agent.test/scan/clear", () => new HttpResponse(null, { status: 500 }))); - await expect(agentClient.clearLastScan()).rejects.toThrow(); + it("throws on a non-2xx response (agent unreachable/error)", async () => { + server.use(http.post("http://agent.test/scan/consume", () => new HttpResponse(null, { status: 500 }))); + await expect(agentClient.consumeLastScan()).rejects.toThrow(); + }); + + it("throws on a genuine network failure", async () => { + server.use(http.post("http://agent.test/scan/consume", () => HttpResponse.error())); + await expect(agentClient.consumeLastScan()).rejects.toThrow(); }); }); }); diff --git a/panel/src/shared/agent/agentClient.ts b/panel/src/shared/agent/agentClient.ts index 49b48bfb..ed0a1a1b 100644 --- a/panel/src/shared/agent/agentClient.ts +++ b/panel/src/shared/agent/agentClient.ts @@ -143,10 +143,9 @@ async function print(request: PrintRequest, timeoutMs = PRINT_TIMEOUT_MS): Promi /** * The agent's last-scanned-code buffer, per agent/openapi.yaml's `ScanData` - * schema (`GET /scan/last`) -- confirmed present in the agent's own contract - * (paths `/scan/last` + `/scan/clear`, tag "Scan"), not a panel-side - * invention. When nothing has been scanned yet (or the buffer was just - * cleared via clearLastScan()), the agent returns `code: ""`, `time: + * schema -- confirmed present in the agent's own contract (path + * `/scan/consume`, tag "Scan"), not a panel-side invention. When nothing has + * been scanned since the last consume, the agent returns `code: ""`, `time: * "0001-01-01T00:00:00Z"` -- this is a normal "no new scan" poll result, not * an error. */ @@ -157,40 +156,40 @@ export interface ScanData { /** * Polling primitive for P4.1's handheld-scanner check-in mode - * (useScanInput.ts) -- returns the last code any connected hardware scanner - * has produced. Never throws on an empty buffer (that's the normal steady - * state between scans); only a genuine transport/HTTP failure (agent - * unreachable, non-2xx) rejects, same "throw on failure" contract as - * getPrinters/getDefaultPrinter above. + * (useScanInput.ts) -- atomically reads AND clears the agent's last-scan + * buffer in one request (`POST /scan/consume`), replacing the earlier + * GET /scan/last + POST /scan/clear pair. That two-step protocol had a real + * race: a second physical scan arriving between this client's read and its + * later clear call was silently erased by the clear (the CodeRabbit finding + * on panel PR #77, fixed agent-side by + * docs/superpowers/plans/2026-07-18-agent-atomic-scan-consume.md). Because + * read+clear now happen server-side under one lock, every non-empty + * response here is guaranteed to be a scan this client has never seen + * before -- callers no longer need to dedup by {code, time} or separately + * retry a failed clear (see useScanInput.ts). Never throws on an empty + * buffer (that's the normal steady state between scans); only a genuine + * transport/HTTP failure (agent unreachable, non-2xx) rejects, same "throw + * on failure" contract as getPrinters/getDefaultPrinter above. */ -async function getLastScan(): Promise { - const response = await ensureOk(await fetch(agentUrl("/scan/last")), "agent GET /scan/last failed"); +async function consumeLastScan(): Promise { + const response = await ensureOk( + await fetch(agentUrl("/scan/consume"), { + method: "POST", + // Required for the agent's Origin-allowlist browser fallback auth on + // mutating requests (see agent/openapi.yaml's Авторизация section) -- + // without it a same-origin-allowlisted-but-tokenless request gets 415. + headers: { "Content-Type": "application/json" }, + }), + "agent POST /scan/consume failed", + ); const data = (await response.json()) as { code?: string; time?: string }; return { code: data.code ?? "", time: data.time ?? "" }; } -/** - * Resets the agent's last-scan buffer (`POST /scan/clear`) -- called after a - * scan has been consumed so the NEXT poll doesn't re-observe the same - * code/time pair. `Content-Type: application/json` is set even though there - * is no request body: every mutating agent call needs it for the agent's - * Origin-allowlist browser fallback auth (see agent/openapi.yaml's - * Авторизация section; mirrors `print`'s header above), regardless of - * whether the mutation itself carries a payload. - */ -async function clearLastScan(): Promise { - const response = await fetch(agentUrl("/scan/clear"), { - method: "POST", - headers: { "Content-Type": "application/json" }, - }); - await ensureOk(response, "agent POST /scan/clear failed"); -} - export const agentClient = { checkHealth, getPrinters, getDefaultPrinter, print, - getLastScan, - clearLastScan, + consumeLastScan, };