Panel P4.3: equipment hub — device registry, agent identity, guided setup wizards - #83
Merged
Conversation
Board screens 5a-5d extracted (all four fully drawn); user-approved decisions: camera deferred entirely, server-authoritative default mirrored to agent, tenant-wide readiness rule, wedge+COM scanners, normalized registry tables keyed by agent machine id (backend #5), new agent GET /info for machine identity. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Agent /info -> migration+store -> endpoints+regen -> readiness -> panel info/cache -> registry hooks+reconcile -> hub page -> printer wizard -> scanner wizard -> default-precedence repoint -> final sweep. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… methods Adds the P4.3 per-tenant, per-machine device registry storage layer: the equipment_machines/equipment_devices tables (migration 000023, with the partial-unique-index single-default-printer guarantee and the class='printer' CHECK), the EquipmentMachine/EquipmentDevice models, and the 8-method PGStore implementation (upsert-machine, get-machine+devices, get-device-for-tenant, create/update/delete-device, set-default-printer, mark-test-passed, tenant-has-tested-default-printer) behind the Store interface's new Equipment Registry block, with ErrDeviceNotFound as its not-found sentinel. Covered by pgxmock tests pinning every statement's SQL, plus a TEST_DATABASE_URL-gated real-Postgres integration test proving the schema guarantees pgxmock can't (partial unique index, CHECK constraint, cascade delete, cross-tenant disjointness) and the id = ANY($3) UUID-array bind used by the seen-devices freshness touch.
…ule, test-passed Adds the 7 P4.3 equipment-device-registry HTTP endpoints (PUT/GET machine, POST device, PATCH/DELETE device, PUT default-printer, POST test-passed), openapi.yaml documentation, contract tests, and the regenerated panel client. ORG-level resources scoped by tenant_id alone (no requireEventOwnership); 23505 (partial-unique-index race on make_default) maps to a clean 409 instead of a raw constraint error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GetEventReadiness now sources the equipment step from TenantHasTestedDefaultPrinter(tenantID) instead of a hardcoded not_done. Equipment still never blocks ready (spec §4.3), same as zones. Extends the readiness contract tests with true/false/store-error cases and fixes two other fakeStore fixtures (badge-template P3 readiness tests) that now need the new store method wired to avoid a nil-func panic. openapi.yaml's readiness description drops the "P3/P4 wiring" caveat and states the real rule; panel schema.d.ts regenerated in the same commit.
…don't-fail cache write, machine_id-overwrite pin
Task 6 of P4.3: TanStack Query hooks over the generated equipment endpoints (machine GET/PUT, device POST/PATCH/DELETE, default-printer PUT, test-passed POST), each invalidating EQUIPMENT_MACHINE_KEY on success, plus pure reconcile.ts matching registry devices against the live agent by config.agent_name/port_name (never display_name) — wedge scanners always resolve to Liveness "none" per the honesty rule. Adds AgentScanner + agentClient.getScanners() (GET /scanners) as the scanner-side counterpart to getPrinters.
…ries name+type only
…, agent-down state Board 5a/5d: replaces the /equipment PlaceholderPage with the real hub. AgentCard is a state machine over useAgentInfo (connected/connected_legacy/ checking/disconnected), rendering board 5a's meta line (base-url · version · hostname · uptime) and board 5d's red "Start the agent" card (numbered steps, download link, Retry, auto-retry caption — the actual 8s retry timer already lives inside useAgentInfo). DeviceCard is one parameterized column component reused for Printers/Scanners, reconciling the registry (useEquipmentMachine) against the live agent (useAgentPrinters + a new local useAgentScanners) via reconcile.ts: green/amber liveness dots (never color-alone — amber pairs with visible "Saved · not seen since <date>" text), no dot at all for USB-wedge scanners (the agent has no visibility into them), unsaved live printers surfaced as a Save… affordance, and a DEFAULT chip on the registry default. The whole grid grays to opacity-.55 with actions hidden when the agent is down, but stays readable from a cached machine_id — it only fully hides for a true cold start (no cache, agent down). The reconcile-on-load PUT (seen_device_ids) fires exactly once per machine_id per visit via a ref guard, gated on info + the registry settling + both live lists settling; an empty registry (404) still upserts, since that's what registers a new machine. The overflow menu (rename/set default/delete) reuses the house ConfirmDialog pattern. Printer/scanner setup buttons and the unsaved-row Save affordance are honestly disabled (data-testid="wizard-todo") — no fake wizard ahead of Tasks 8/9. Also extends ReadinessCell's regression coverage for the equipment step's newly-reachable "done" status (no component change needed — the existing generic step renderer already handles it). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… registry-error state, v5 pending skeleton Task 7 review round (2 Important + 3 Minor, all taken): 1. Reconcile upsert now fires only when BOTH live lists (printers/ scanners) have succeeded — an errored fetch reads as an empty list and would under-report seen_device_ids (live devices later showing a false "not seen since"). A skipped attempt does not consume the once-per-machine_id ref budget, so a later successful refetch still reconciles exactly once (test drives error -> zero PUTs -> recovery -> exactly one PUT). 2. A genuine (non-404) machine-registry failure renders a distinct error state with Retry (ZonesPage's list-error shape, new equipmentRegistryLoadError key EN+RU) instead of masquerading as "No printers/scanners saved yet" — the exact silent-empty-list the board's 5d caption calls out. True 404 stays the empty state. 3. Loading skeleton now gates on v5-correct pending semantics (isPending for the enabled query + the pre-identity checking window) instead of isLoading, which is never true for a disabled query. 4. Added the missing clear-default overflow test (PUT device_id: null). 5. Pinned the legacy-agent-with-cached-identity edge: saved devices stay visible from the cached machine_id (a reachable-but-legacy agent must never show less than a fully dead one), hint renders, no reconcile. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Board 5b's 3-step wizard (Find -> Test -> Save) with physical verification via a Cyrillic test label, plus a retest entry point for a saved printer row's Test print button. Wires the hub's three printer mount points (header chooser, per-column Set up printer, unsaved-row Save) from Task 7. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… address at Save, dismissal locks Important 1: mirror failure no longer auto-closes in the same tick (the warning never painted under React 18 batching + unanimated Radix unmount) — dialog stays open with an explicit Close, BulkBar convention. Important 2: a network printer without a known address collects ip/port at the Save step instead of dead-ending on a guaranteed-400 create. Minors: in-handler busy re-checks, complete dismissal locking (+tests), Find list excludes registered printers, retest carries the real kind, label-worded timeout copy (equipmentWizardTimeout, EN+RU). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Board 5c's ScannerWizard: a segmented USB-wedge | COM toggle, each leading to its own listen step with a physical verification gate (Save/fields don't exist until a real scan lands), plus a retest entry point mirroring Task 8's controller-resolved pattern. useWedgeListen does window-scoped keydown capture with its own inter-key-gap/silence-timeout detection (a different technique from useScanInput's focused-input + Enter idiom, since this hook must first tell a scanner apart from a human typing). agentClient gains getScannerPorts/addComScanner/removeComScanner against agent/openapi.yaml's already-documented /scanners/* routes. Hub wiring: "+ Set up scanner" and the "+ Add device" chooser's Scanner option are now the real enabled affordance (the last wizard-todo placeholder is gone); scanner rows get a "Test scan" button (wedge rows always, live com rows); a saved com device's delete flow best-effort releases its agent-side port, warning (not failing) on mirror failure. Controller correction honored: retest mode calls useMarkTestPassed on any confirmed detection (wedge or com), since POST .../test-passed is device-generic and only printer tests feed the readiness gate.
… test_passed gating, abortable consume poll Task 9 review round (1 Critical + 2 Important + 1 Minor): - CRITICAL: the agent's scan buffer is process-wide (not per port/session), so the COM listen phase could pass a pre-session (or wrong-device) scan off as this session's physical verification — and retest auto-stamped test_passed_at from it with zero operator action. The poll is now an imperative effect (useScanInput's interval idiom) whose first successful consume per listen session is a pure discard; /scan/consume's atomic read-and-clear guarantees everything read after it arrived after listening began. - Important-1 (reviewer's adjudication): Save no longer requires a detection — PrinterWizard's brief-mandated test_passed:false precedent governs. Fields are editable from the start; test_passed on create is simply whether a scan landed this session. - Important-2: consumeLastScan accepts an optional AbortSignal and the poll's cleanup aborts the in-flight request on cancel/close — a post-close consume can no longer eat a scan the station needs. - Minor: COM listen tests converted to fake timers (no real 700ms intervals; equipment suite ~3.5s).
…elds can't fabricate a detection Task 9 review round 2 (1 new Important + 1 Minor): - Important: round 1's always-editable fields exposed the window-level wedge listener to fast human typing -- a 3+ char burst at wedge speed into the Device name input fabricated a detection (and a later test_passed:true) with zero physical scan. useWedgeListen now ignores keydowns targeted at editable elements (input/textarea/select/ contenteditable) and clears the accumulator on such an event (no mixed bursts). Accepted, documented limitation (controller decision): a physical scan while a text field is focused won't detect -- its chars land in the field natively (wedge physics any app has); "Scan again" re-arms with button focus. - Minor: the abort-on-cancel comment now states its honest scope -- the abort stops the client's wait and further polls; it cannot un-consume a request the agent already processed.
…ness-level agent-info/registry MSW defaults (task 10 review)
…ling meta separator RU copy: past-tense equipmentNotSeenSince (matches equipmentNotSeenYet), "штрихкод"/"сканирование" terminology consistency in the scan-confirmation strings, and a comma-splice fix in equipmentDefaultFooter. Values only, keys/EN unchanged. AgentCard: build the connected meta line by joining only non-empty segments so a blank hostname can't leave a dangling "· ·" gap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…al whole-branch review Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
Author
|
@codex review |
github-actions
Bot
added
documentation
Contributor
|
Warning Review limit reached
Next review available in: 57 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (19)
📝 WalkthroughWalkthroughAdds the P4.3 equipment hub across the agent, backend, and panel. It introduces persisted agent identity, a tenant-scoped equipment registry, readiness integration, reconciliation, connected/disconnected hub states, and printer/scanner setup and retest workflows. ChangesEquipment hub and registry
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
backend/internal/handler/equipment.go (1)
56-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRepeated inline error-response literal could be a small helper.
c.JSON(status, map[string]string{"error": "..."})is repeated roughly 15 times across this new file. A tiny helper (mirroringwriteErr's role for the tenant-context error path already used here) would reduce duplication and keep the response shape centralized for future changes (e.g., adding an error code field).♻️ Proposed helper
+func jsonErr(c echo.Context, status int, msg string) error { + return c.JSON(status, map[string]string{"error": msg}) +}Then replace call sites, e.g.:
- return c.JSON(http.StatusBadRequest, map[string]string{"error": "Invalid machine ID"}) + return jsonErr(c, http.StatusBadRequest, "Invalid machine ID")Also applies to: 265-467
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/handler/equipment.go` around lines 56 - 115, Introduce a small helper for writing JSON error responses with an HTTP status and message, colocated with the existing handler helpers such as writeErr. Replace the repeated c.JSON(status, map[string]string{"error": ...}) calls throughout equipment.go, including the handlers shown and the additional call sites, while preserving each existing status code and error message.agent/main.go (1)
57-60: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winReplace
sync.RWMutexwithsync.MutexforconfigMu. All current call sites useLock(), sosync.Mutexmatches the actual usage and removes the read-lock footgun.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agent/main.go` around lines 57 - 60, Change configMu from sync.RWMutex to sync.Mutex, keeping its existing Lock/Unlock call sites unchanged. Remove the now-unused sync.RWMutex usage while preserving config load-modify-save serialization.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agent/main.go`:
- Around line 208-219: Update generateUUIDv4 to return the generated UUID
together with any crypto/rand.Read error instead of calling log.Fatalf.
Propagate and handle that error through loadConfig’s upgrade branch and main()’s
startup path, allowing request-serving callers to fail the request without
terminating the agent while preserving startup failure handling.
In `@panel/src/features/equipment/AgentCard.tsx`:
- Around line 37-42: Update formatUptime to use the existing react-i18next
translation mechanism instead of hardcoded “uptime”, “h”, and “m” text, passing
hours and minutes as interpolation values. Add the corresponding translation key
with English and Russian strings to en.json and ru.json, preserving the current
calculated values.
In `@panel/src/features/equipment/PrinterWizard.tsx`:
- Around line 254-257: Update the manual-add validation in the PrinterWizard
flow around trimmedName, trimmedIp, and port to reject empty, non-integer, and
out-of-range ports. Match the Save-step address validation by requiring an
integer from 1 through 65535 before posting or storing config.port.
---
Nitpick comments:
In `@agent/main.go`:
- Around line 57-60: Change configMu from sync.RWMutex to sync.Mutex, keeping
its existing Lock/Unlock call sites unchanged. Remove the now-unused
sync.RWMutex usage while preserving config load-modify-save serialization.
In `@backend/internal/handler/equipment.go`:
- Around line 56-115: Introduce a small helper for writing JSON error responses
with an HTTP status and message, colocated with the existing handler helpers
such as writeErr. Replace the repeated c.JSON(status, map[string]string{"error":
...}) calls throughout equipment.go, including the handlers shown and the
additional call sites, while preserving each existing status code and error
message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: eaaa5932-eb0e-4416-89d2-68aff4feed42
📒 Files selected for processing (60)
.superpowers/sdd/p4.3-board-5a-5d-extract.md.superpowers/sdd/progress.mdagent/README.mdagent/info_test.goagent/internal/httpauth/httpauth.goagent/internal/httpauth/httpauth_test.goagent/main.goagent/openapi.yamlbackend/internal/handler/equipment.gobackend/internal/handler/handler.gobackend/internal/handler/openapi_contract_badge_template_p3_test.gobackend/internal/handler/openapi_contract_equipment_p4_test.gobackend/internal/handler/openapi_contract_events_p1_test.gobackend/internal/handler/readiness.gobackend/internal/handler/testsupport_test.gobackend/internal/models/equipment.gobackend/internal/store/interface.gobackend/internal/store/pg_store_equipment.gobackend/internal/store/pg_store_equipment_integration_test.gobackend/internal/store/pg_store_equipment_test.gobackend/migrations/000023_equipment_registry.down.sqlbackend/migrations/000023_equipment_registry.up.sqlbackend/openapi.yamldocs/superpowers/plans/2026-07-19-panel-p4.3-equipment-hub.mddocs/superpowers/specs/2026-07-19-panel-p4.3-equipment-hub-design.mdpanel/src/app/router.tsxpanel/src/features/equipment/AgentCard.test.tsxpanel/src/features/equipment/AgentCard.tsxpanel/src/features/equipment/DeviceCard.tsxpanel/src/features/equipment/EquipmentPage.test.tsxpanel/src/features/equipment/EquipmentPage.tsxpanel/src/features/equipment/PrinterWizard.test.tsxpanel/src/features/equipment/PrinterWizard.tsxpanel/src/features/equipment/ScannerWizard.test.tsxpanel/src/features/equipment/ScannerWizard.tsxpanel/src/features/equipment/deviceMeta.test.tspanel/src/features/equipment/deviceMeta.tspanel/src/features/equipment/hooks.test.tsxpanel/src/features/equipment/hooks.tspanel/src/features/equipment/reconcile.test.tspanel/src/features/equipment/reconcile.tspanel/src/features/equipment/testLabel.test.tspanel/src/features/equipment/testLabel.tspanel/src/features/equipment/useWedgeListen.test.tsxpanel/src/features/equipment/useWedgeListen.tspanel/src/features/home/ReadinessCell.test.tsxpanel/src/shared/agent/agentClient.test.tspanel/src/shared/agent/agentClient.tspanel/src/shared/agent/agentInfoCache.test.tspanel/src/shared/agent/agentInfoCache.tspanel/src/shared/agent/useAgentInfo.test.tsxpanel/src/shared/agent/useAgentInfo.tspanel/src/shared/agent/useAgentPrinters.test.tsxpanel/src/shared/agent/useAgentPrinters.tspanel/src/shared/agent/useEquipmentMachine.tspanel/src/shared/api/http.tspanel/src/shared/api/schema.d.tspanel/src/shared/i18n/en.jsonpanel/src/shared/i18n/ru.jsonpanel/src/test/msw.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d873cd6c07
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
CodeRabbit (PR #83): generateUUIDv4 called log.Fatalf on a crypto/rand failure, but it's reachable from loadConfig's MachineID-upgrade branch, which runs inside HTTP handlers (e.g. /printers) while holding configMu — a transient RNG failure would os.Exit the whole agent mid-event instead of failing one request. generateUUIDv4 now returns (string, error); loadConfig's upgrade branch propagates the error through its existing error return; main()'s startup path still Fatalf's on its own discretion (nothing to serve yet at that point). All loadConfig call sites already 500 or degrade gracefully on error, so no handler behavior changed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… shapes Two Codex findings (PR #83): 1. PUT /api/equipment/machines/{machine_id}/default-printer bound the request body's device_id into a bare *uuid.UUID, which json.Unmarshal leaves nil for both an absent key and an explicit null — so an accidentally-omitted device_id silently cleared the machine's default printer, identical to the documented clear-request. Fixed by decoding into a json.RawMessage field first (DisallowUnknownFields) so presence is observable: key absent -> 400 ("device_id is required; send null to clear"), null -> clear (unchanged), a uuid string -> set (invalid uuid -> 400). 2. validateEquipmentDeviceConfig shared one config shape across both printer kinds and both scanner kinds, so DisallowUnknownFields only caught genuinely unrecognized keys — a usb_wedge scanner could carry com's port_name, a com scanner could carry usb_wedge's terminator, and a system printer could carry network's ip/port/dpi, all stored verbatim and misleading reconciliation readers. Split into four per-kind decode shapes (network printer: agent_name/ip/port/dpi; system printer: agent_name only; com: port_name only; wedge: terminator only) so a cross-kind key is now rejected as unknown. openapi.yaml updated to match (EquipmentDefaultPrinterRequest's required- device_id behavior, EquipmentDeviceCreateRequest's per-kind key prose); panel/src/shared/api/schema.d.ts regenerated via `npm run generate:api -w panel` in this same commit (doc-comment diff only, no type-shape change). Contract tests added for both: {} on default-printer -> 400, malformed uuid -> 400; wedge+port_name, com+terminator, system printer+ip -> 400. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fixes all 10 CodeRabbit/Codex bot-review findings on the P4.3 equipment
hub (2 counted once as duplicates -> 8 distinct changes), TDD red-first
for each:
1/2. AgentCard's uptime meta line now routes through react-i18next
(equipmentUptime, EN+RU) instead of a hardcoded English string;
PrinterWizard's manual-add port guard now matches the Save-step
address form's Number.isInteger 1..65535 rule instead of
Number.isFinite (which let ""->0 and 70000+ through).
3. EquipmentPage/DeviceCard disable every Add/Set up/Save control
while machineId is null (legacy agent, no cached identity) --
their handlers already no-op there, so the affordance is now
honest about it.
4. useCreateDevice/useDeleteDevice/useSetDefaultPrinter/
useMarkTestPassed invalidate every event's readiness query (the
[method, path-template] prefix idiom) since equipment is org-level
but its readiness-gated content is per-event.
5. ScannerWizard's retest Close button (and the Escape/outside-click
paths) now gate on markTestPassed's own pending state, so a late
test-passed failure can no longer be silently dropped by closing
mid-flight.
6. PrinterWizard's shared Test/Save footer now includes `printing` in
Save's busy check, so a device row can no longer be created and the
dialog closed while the physical test send is still in flight.
7. ScannerWizard's explicit Cancel button now gates on `comAdding`
too, matching handleOpenChange/preventDialogDismiss.
8. useAgentPrinters resolves configuredDefault to null while a modern
agent's registry query is still pending, instead of racing to the
agent's own (possibly stale) configured default.
9. useEquipmentMachine sets retry:false -- a 404 there is a documented
normal state, matching useAgentInfo/useAgentPrinters convention.
10. DeviceCard's row-menu trigger is now the @idento/ui Button
(ghost/icon), replacing a hand-rolled <button>.
Gates: npm run test -w panel (121 files / 1382 tests), npm run
typecheck -w panel, npx eslint . -- all clean. Full report at
.superpowers/sdd/pr83-round1-panel-report.md.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
Author
|
@codex review |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.superpowers/sdd/pr83-round1-go-report.md:
- Around line 203-216: Specify a language on the fenced evidence block in the
report, using text or console for the command-output content, so the
markdownlint MD040 violation is resolved.
- Around line 201-214: Update the “Test evidence (final, all green)” heading to
indicate that the checks passed except for the documented pre-existing
golangci-lint issue. Keep the test and lint results unchanged.
In @.superpowers/sdd/pr83-round1-panel-report.md:
- Around line 7-8: Remove the absolute filesystem path from the report’s
checkout command and replace it with a repository-relative path or generic
<worktree> placeholder, while preserving the branch and commit information.
In `@panel/src/features/equipment/ScannerWizard.tsx`:
- Around line 314-329: Define a single comprehensive isBusy flag covering
saving, comAdding, and markTestPassed.isPending, then use it in both
handleOpenChange and preventDialogDismiss for all dismissal gating. Preserve the
current behavior while ensuring future in-flight states can be added in one
place.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c50ae389-f863-46e2-a1b6-969ee51e4fbe
📒 Files selected for processing (24)
.superpowers/sdd/pr83-round1-go-report.md.superpowers/sdd/pr83-round1-panel-report.mdagent/info_test.goagent/main.gobackend/internal/handler/equipment.gobackend/internal/handler/openapi_contract_equipment_p4_test.gobackend/openapi.yamlpanel/src/features/equipment/AgentCard.test.tsxpanel/src/features/equipment/AgentCard.tsxpanel/src/features/equipment/DeviceCard.tsxpanel/src/features/equipment/EquipmentPage.test.tsxpanel/src/features/equipment/EquipmentPage.tsxpanel/src/features/equipment/PrinterWizard.test.tsxpanel/src/features/equipment/PrinterWizard.tsxpanel/src/features/equipment/ScannerWizard.test.tsxpanel/src/features/equipment/ScannerWizard.tsxpanel/src/features/equipment/hooks.test.tsxpanel/src/features/equipment/hooks.tspanel/src/shared/agent/useAgentPrinters.test.tsxpanel/src/shared/agent/useAgentPrinters.tspanel/src/shared/agent/useEquipmentMachine.tspanel/src/shared/api/schema.d.tspanel/src/shared/i18n/en.jsonpanel/src/shared/i18n/ru.json
🚧 Files skipped from review as they are similar to previous changes (18)
- panel/src/shared/i18n/ru.json
- panel/src/shared/agent/useEquipmentMachine.ts
- panel/src/shared/i18n/en.json
- panel/src/shared/api/schema.d.ts
- panel/src/features/equipment/AgentCard.test.tsx
- panel/src/features/equipment/DeviceCard.tsx
- panel/src/shared/agent/useAgentPrinters.ts
- panel/src/features/equipment/PrinterWizard.test.tsx
- panel/src/features/equipment/ScannerWizard.test.tsx
- backend/internal/handler/openapi_contract_equipment_p4_test.go
- agent/info_test.go
- panel/src/features/equipment/AgentCard.tsx
- agent/main.go
- panel/src/features/equipment/EquipmentPage.tsx
- panel/src/features/equipment/EquipmentPage.test.tsx
- panel/src/features/equipment/PrinterWizard.tsx
- backend/openapi.yaml
- backend/internal/handler/equipment.go
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8532f41626
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…powers/sdd is scratch, gitignored by design
…ional clear on config PATCH (#83 round 2) Finding 1: UpdateEquipmentDevice now clears test_passed_at whenever the supplied config differs (jsonb-semantic) from the device's stored config, enforced in the UPDATE's own CASE over the OLD row — a PATCH swapping in different hardware can no longer leave a stale "tested" stamp that TenantHasTestedDefaultPrinter's readiness check keeps trusting. Finding 2: CreateEquipmentDevice takes testPassed and stamps test_passed_at inside its own INSERT (RETURNING), replacing the handler's separate post-create MarkEquipmentDeviceTestPassed call — closing the window where the create could commit and the second write could fail, leaving an already-visible, wrongly-unstamped device a retry would then 409/duplicate against. Pinned pgxmock SQL updated for both statements; added config-changed vs identical-config-rename cases and testPassed true/false cases. Added real-Postgres integration subtests proving the jsonb-semantic CASE behavior pgxmock can't evaluate. openapi.yaml PATCH device description updated; panel client regenerated (npm run generate:api -w panel). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…in-memory copy Addendum to the round-2 Finding 1 fix: the store's UPDATE now clears test_passed_at when config actually changes, but PatchEquipmentDevice's response was still built from the pre-fetched `existing` struct patched up in memory — so a config-changing PATCH could return a stale non-null test_passed_at for hardware that never passed a test. The handler now re-reads the row via GetEquipmentDeviceForTenant after the UPDATE and echoes that; a nil re-read (concurrent-delete race after a successful UPDATE) maps to the house 404 shape, same soft-delete-race precedent as PutCheckinSettings' ErrEventNotFound handling (PR #77 Finding C). Contract tests: config-changing PATCH response has test_passed_at null; rename-only PATCH response preserves it; vanished-row re-read is 404 (and the re-read provably happens — two get calls). The rename test's fake is now stateful so the re-read reflects the persisted update. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…9 findings) Closes the two wizard state-machine races (stale session state surviving close, letting a reopen auto-fire a print or auto-pass a retest off leftover data), gates retest-confirm/Back on an in-flight test print, swaps two hand-rolled list-item buttons for @idento/ui's Button, extracts one isBusy per wizard, routes the test-label preview through i18n, mirrors row-menu "Make default" onto the agent (warn-don't-fail, same idiom as the wizard's own mirror), and adds usePatchDevice to the readiness-invalidation set now that the backend clears test_passed_at on a config-changing PATCH. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
Author
|
@codex review |
4 tasks
This was referenced Jul 21, 2026
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
Third and final P4 ("event day") sub-cycle per the parent panel-rewrite spec — board screens 5a–5d + parent backend #5.
GET /info— persistedmachine_id(UUIDv4 inagent_config.json), hostname, version (ldflags-overridable), uptime./healthuntouched.equipment_machines+equipment_devices(single default enforced by a partial unique index + default-is-printer CHECK); 7 tenant-scoped endpoints (upsert/reconcile, device CRUD, default rule, test-passed); readinessequipmentstep is now real: done when the org has ≥1 machine with a tested default printer (readyformula deliberately unchanged — equipment does not newly block launch)./equipmenthub (3-state agent card incl. legacy-agent + agent-down-with-cache grayed registry view, per-device liveness reconciled against the live agent, honest 500-vs-404 registry states); printer wizard 5b (Find → Test → Save, «Кириллица 123» ^CI28 test label, default mirrored to agent warn-don't-fail); scanner wizard 5c (USB-wedge in-browser listen-detect with editable-target guard, COM flow with discard-first/scan/consumehygiene, abortable poll); default-printer precedence for ALL P3.2 print surfaces: server registry > agent config > first-in-list (six consumer suites pass byte-untouched).Deliberate board deviations (user-approved at spec time)
/printerscarries name+type only).Known-deferred (disclosed preemptively)
POST …/deviceson a never-registered machine_id surfaces the FK violation as a 500 (unreachable via this client — the hub always upserts the machine first); clean 4xx is a follow-up.test_passed_atat create time is a second store statement after the INSERT (narrow retry window, final-review Minor).Testing
-racewith OPENAPI_COVERAGE=1 (contract-test ledger green) + live real-PG constraint/cascade integration run during Task 2; agent 49/49-race;generate:apizero-drift; keyParity 4/4;web/diff empty.🤖 Generated with Claude Code
Summary by CodeRabbit