refactor(store): wave B.14 — action repo (#271)#272
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (15)
🚧 Files skipped from review as they are similar to previous changes (14)
📝 WalkthroughWalkthroughThis PR adds a store-level Action domain and ActionRepo, implements a Postgres ActionRepo, and migrates action access across API handlers, workers, and search from the query-layer (store.Queries()) to the repo-layer (store.Repos().Action.*). ChangesAction Repository Foundation and Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@internal/store/postgres/action.go`:
- Around line 70-77: The UpdateSignature implementation (function
UpdateSignature) currently calls the generated UpdateActionSignature query which
was annotated with :exec so it discards affected-row counts; update the SQL
annotation to :execrows in the generated query for UpdateActionSignature, then
change UpdateSignature to capture the returned affected row count from the
generated method and if it is zero return store.ErrNotFound; keep passing
generated.UpdateActionSignatureParams (ID, Signature, ParamsCanonical) but
handle the rows-return value and map zero rows to store.ErrNotFound.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 72b2b3d0-98c0-4395-9623-72a96a935ce9
📒 Files selected for processing (15)
internal/api/action_crud.gointernal/api/action_dispatch.gointernal/api/action_handler.gointernal/api/action_set_handler.gointernal/api/assignment_handler.gointernal/api/compliance_policy_handler.gointernal/api/device_handler.gointernal/api/system_action_store.gointernal/api/user_selection_handler.gointernal/control/inbox_worker.gointernal/search/index.gointernal/store/action.gointernal/store/postgres/action.gointernal/store/postgres/wire.gointernal/store/repos.go
| func (a *Action) UpdateSignature(ctx context.Context, p store.UpdateActionSignatureParams) error { | ||
| if err := a.q.UpdateActionSignature(ctx, generated.UpdateActionSignatureParams{ | ||
| ID: p.ID, | ||
| Signature: p.Signature, | ||
| ParamsCanonical: []byte(p.ParamsCanonical), | ||
| }); err != nil { | ||
| return fmt.Errorf("action: update signature: %w", err) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Find SQL declaration for UpdateActionSignature (look for :exec vs :execrows/:one):"
rg -n -C3 --glob '*.sql' '\bUpdateActionSignature\b'
echo
echo "2) Inspect generated method signature for UpdateActionSignature:"
rg -n -C3 --glob '*.go' 'func \(.*\) UpdateActionSignature\(' internal/store/generated
echo
echo "3) Inspect repository implementation call site:"
rg -n -C3 --glob '*.go' 'func \(a \*Action\) UpdateSignature\(' internal/store/postgresRepository: manchtools/power-manage-server
Length of output: 1816
Add affected-row check to detect missing actions in signature updates.
The SQL query uses :exec, which discards the affected-row count. If the target ID does not exist, the update silently succeeds with zero rows modified. Change the SQL annotation from :exec to :execrows to detect this case and return store.ErrNotFound when no rows are updated.
🤖 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 `@internal/store/postgres/action.go` around lines 70 - 77, The UpdateSignature
implementation (function UpdateSignature) currently calls the generated
UpdateActionSignature query which was annotated with :exec so it discards
affected-row counts; update the SQL annotation to :execrows in the generated
query for UpdateActionSignature, then change UpdateSignature to capture the
returned affected row count from the generated method and if it is zero return
store.ErrNotFound; keep passing generated.UpdateActionSignatureParams (ID,
Signature, ParamsCanonical) but handle the rows-return value and map zero rows
to store.ErrNotFound.
ActionRepo is the foundation of the action-chain cluster: Get / List(filter) / Count(filter) / NamesByIDs / UpdateSignature ListActionsFilter / CountActionsFilter pair ActionTypeFilter + UnassignedOnly so paginated totals match. UpdateSignature is the single non-event write the action domain owns — used by the CA key-rotation pathway. Params / ParamsCanonical / Schedule stay as json.RawMessage at the boundary per the JSONB normalize plan; Signature stays as []byte (already opaque bytes at the wire). Call sites migrated: - action_crud.go + action_handler.go — full action CRUD path, actionToProto + persistActionSignature signatures shifted to store.Action / *store.Action - assignment_handler.go — source validation for Action + transitional inline conversion for ListAssignedActionsForDevice row (the join type migrates with the Assignment domain) - system_action_store.go — system-action rebuild lookups - user_selection_handler.go — Action source enrichment in ListAvailableActions - control/inbox_worker.go — execution dispatch, compliance trigger, agent-scheduled action lookups (3 sites) - search/index.go — action reindex sweep Non-goals: - ActionSet / Definition / Execution / Assignment repos — each gets its own follow-up sub-wave. The chain is too big for one PR. - Write-side projector queries — stay on Queries() per pattern. Closes #271.
3b45ba4 to
516411a
Compare
Summary
`ActionRepo` — foundation of the action-chain cluster. ActionSet / Definition / Execution / Assignment follow in subsequent sub-waves (the cluster is too big for one PR). Branches off main directly.
Methods
`Params` / `ParamsCanonical` / `Schedule` stay as `json.RawMessage` at the boundary per the JSONB normalize plan; `Signature` stays `[]byte` (already opaque at the wire).
Call sites migrated (~20)
Non-goals
Acceptance
Part of #242. Closes #271.
Summary by CodeRabbit