docs: address audit Bundle C — README + stale-comment sync

[PaulDotterer]

Summary

Third bundle from the 2026-05-07 server tech-debt audit. Builds on
PR #141 (lint CI + gofmt) and PR #144 (error handling). Closes
audit findings F010, F011, F012, F013, F029. Filed F036 as #145
(rate-limit policy decision pending). F030 skipped — its cited
"stale line position references" don't exist in the current
comment.

• F010 — Drop the `internal/agentrelease` row from the
  package table. The package was never created — README claimed
  it under the deferred GitHub-Releases-polling design.
• F013 — Add the missing packages: `internal/actionparams`,
  `internal/asynqutil`, `internal/projectors`. Reorder
  alphabetically; move `internal/gateway/registry` next to
  `internal/gateway`.
• F011 — Rewrite the OPA / Rego section. The codebase has
  no OPA and no Rego — `internal/auth/opa.go` and
  `internal/auth/policies/authz.rego` do not exist. The actual
  authorizer is plain Go in `internal/auth/authorizer.go` with
  `:self` / `:assigned` scope variants enforced by
  `auth.EnforceSelfScope`.
• F012 — Sync README rate-limit numbers to the values
  configured in `cmd/control/main.go` (1000/min). Note added
  that these are operator-friendly defaults; tightening tracked
  in Auth: Logout + RenewCertificate are PublicProcedures with no rate-limit #142 (Logout/RenewCertificate) and Tighten Login/Register/RefreshToken rate-limits (audit F036) #145 (Login/Register).
• F029 — Refresh stale "Phase 2 dual-write" comment in
  `internal/api/action_handler.go:33`. Now describes the
  current invariant: aqClient is nil when CONTROL_VALKEY_ADDR
  is unset.
• U+201D sweep (incidental) — fixed 8 instances of the
  Unicode right-double-quotation-mark character being used as
  PostgreSQL empty-string literals in projector comments across
  `internal/projectors/role.go`, `role_test.go`,
  `identity_provider.go`. Replaced with the correct `''` SQL
  syntax. CR-CLI surfaced two during the Bundle C review;
  sibling-grep found six more.

Test plan

• Lint workflow stays green
• Tests workflow stays green (no behaviour changes here)
• Eyeball the README rendering on GitHub

Summary by CodeRabbit

• Documentation

  • Updated README: revised internal package listing, replaced OPA section with a new Authorization description (permission-map authorizer with self/assigned scopes), and updated rate-limit guidance for auth endpoints.
  • Clarified inline comments in projector tests and decoder docs for string/null semantics.

• Improvements

  • Enhanced logging for action dispatch and execution enrichment failures to improve operational visibility.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ce277422-ff69-49be-9bb1-feb0f7edb276

📥 Commits

Reviewing files that changed from the base of the PR and between 940a3da and 0f181e7.

📒 Files selected for processing (1)

• internal/api/action_handler.go

---

📝 Walkthrough

Walkthrough

This PR standardizes documentation and enhances observability across the system. README documentation adds new internal packages and replaces OPA authorization with a pure-Go permission-map implementation. Projector comments are aligned to reflect consistent PL/pgSQL quote semantics. Action handler methods now emit structured warning logs when dispatch operations fail per-device or action-name enrichment fails, replacing prior silent degradation.

Changes

Documentation, Authorization, and Logging Enhancements

Layer / File(s)	Summary
README Documentation README.md	Internal packages list updated to include internal/actionparams, internal/asynqutil, internal/projectors; OPA Authorization replaced with pure-Go permission-map authorizer documentation; Rate Limiting section updated with per-RPC limits (1000/min for Login/RefreshToken/Register).
Projector Comment Standardization internal/projectors/identity_provider.go, internal/projectors/role.go, internal/projectors/role_test.go	PL/pgSQL empty-string literals in comments updated from '' to "" across IdentityProviderUpdatedPayload, IdentityProviderUpdatedFromEvent, RoleCreatedPayload, RoleUpdatedPayload, RoleCreatedFromEvent, and RoleUpdatedFromEvent documentation.
Action Dispatch Error Logging internal/api/action_handler.go	DispatchAssignedActions, DispatchActionSet, DispatchDefinition, and DispatchToGroup methods now emit structured Warn logs on per-device dispatch failures, including RPC name, device/action/set/definition identifiers, and error details.
Action Enrichment Error Logging internal/api/action_handler.go	GetExecution and ListExecutions now log Warn when GetActionNamesByIDs enrichment fails, including action identifiers and error information instead of silently degrading.
Test Payload Formatting internal/projectors/role_test.go	Event payload formatting adjusted for whitespace and alignment in listener lifecycle tests; no changes to test logic, data, or assertions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• manchtools/power-manage-server#144: Both PRs modify internal/api/action_handler.go to add structured warning logging for dispatch failures and enrichment errors.
• manchtools/power-manage-server#91: Related through shared internal package updates and resolution/gateway infrastructure changes mentioned in README.

Poem

🐰 Documentation blooms with proper quote,
PL/pgSQL semantics now take note,
Dispatch logs whisper of each device,
Enrichment failures now arrive,
Clearer paths for bugs to find,
Observability refined!

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/readme-and-stale-comment-sync

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (3)

internal/api/helpers.go (1)

104-109: ⚡ Quick win

logEnrichmentErr drops handler-scoped logger context

Using the package-level slog.Warn discards per-handler attributes added via .With("component", ...). Handlers' loggers carry structured context (e.g. component=api/assignment) that won't appear in these enrichment warnings, making them harder to correlate in logs. The existing appendEvent helper in the same file already takes a *slog.Logger — the same pattern applies here.

♻️ Proposed refactor

-func logEnrichmentErr(operation, idKey, idValue string, err error) {
-	slog.Warn("enrichment lookup failed",
+func logEnrichmentErr(logger *slog.Logger, operation, idKey, idValue string, err error) {
+	logger.Warn("enrichment lookup failed",
 		"operation", operation,
 		idKey, idValue,
 		"error", err)
 }

All call sites should then pass their handler's h.logger.

🤖 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/api/helpers.go` around lines 104 - 109, The package-level slog.Warn
in logEnrichmentErr drops handler-scoped context; change the signature of
logEnrichmentErr to accept a *slog.Logger (e.g., logger *slog.Logger) and call
logger.Warn instead of slog.Warn, preserving the same keys ("operation", idKey,
"error"); update all call sites to pass the handler's logger (e.g., h.logger)
similar to appendEvent so per-handler attributes like component=... are retained
in enrichment warnings.

internal/api/assignment_handler.go (1)

327-398: ⚡ Quick win

Silent continue on error in action-set and definition loops — inconsistent with the compliance-policy fix above

The compliance-policy fetch now calls logEnrichmentErr on failure, but the action-set and definition enrichment loops still silently discard errors. Per the handler coding guidelines ("Never silently ignore errors"), these should use the same pattern. As the PR is fixing exactly this anti-pattern in this function, applying it uniformly avoids leaving orphaned silent failures.

♻️ Proposed fix — apply logEnrichmentErr consistently

 	for id := range actionSetIDs {
 		set, err := h.store.Queries().GetActionSetByID(ctx, id)
 		if err != nil {
+			logEnrichmentErr("GetActionSetByID", "action_set_id", id, err)
 			continue
 		}
 		// ... build protoSet ...

 		members, err := h.store.Queries().ListActionSetMembers(ctx, id)
 		if err != nil {
+			logEnrichmentErr("ListActionSetMembers", "action_set_id", id, err)
 			continue
 		}

 	for id := range definitionIDs {
 		def, err := h.store.Queries().GetDefinitionByID(ctx, id)
 		if err != nil {
+			logEnrichmentErr("GetDefinitionByID", "definition_id", id, err)
 			continue
 		}
 		// ... build protoDef ...

 		members, err := h.store.Queries().ListDefinitionMembers(ctx, id)
 		if err != nil {
+			logEnrichmentErr("ListDefinitionMembers", "definition_id", id, err)
 			continue
 		}

(If the logEnrichmentErr signature is updated to accept a *slog.Logger as suggested in helpers.go, pass h.logger here as well.)

As per coding guidelines, internal/api/*_handler.go must never silently ignore errors.

🤖 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/api/assignment_handler.go` around lines 327 - 398, The loops that
call h.store.Queries().GetActionSetByID / ListActionSetMembers and
GetDefinitionByID / ListDefinitionMembers currently silently continue on error;
replace each bare "continue" with a call to logEnrichmentErr so failures are
logged consistently (e.g., invoke logEnrichmentErr with context, h.logger if the
helper now requires it, a short operation identifier like "GetActionSetByID" or
"ListDefinitionMembers", the error, and the id) and then continue; update all
four error branches (both Get* and List* calls) to use this pattern to match the
compliance-policy changes elsewhere.

.github/workflows/lint.yml (1)

41-55: ⚡ Quick win

Pin the lint tool versions instead of installing @latest.

Using @latest makes CI non-deterministic: a new upstream staticcheck or govulncheck release can fail unchanged commits. Pin explicit versions and update them intentionally. Based on your declared Go 1.25.0, use staticcheck@v0.7.0 and govulncheck@v1.3.0.

🤖 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 @.github/workflows/lint.yml around lines 41 - 55, Update the GitHub Actions
steps that install lint tools to pin explicit versions instead of using `@latest`:
change the staticcheck install in the "staticcheck" job to use
staticcheck@v0.7.0 and change the govulncheck install in the "govulncheck
(production packages)" job to use govulncheck@v1.3.0 so CI is deterministic;
locate the install lines for the staticcheck and govulncheck steps in the
lint.yml workflow and replace the `@latest` suffixes with the specified versions.

🤖 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.

Nitpick comments:
In @.github/workflows/lint.yml:
- Around line 41-55: Update the GitHub Actions steps that install lint tools to
pin explicit versions instead of using `@latest`: change the staticcheck install
in the "staticcheck" job to use staticcheck@v0.7.0 and change the govulncheck
install in the "govulncheck (production packages)" job to use govulncheck@v1.3.0
so CI is deterministic; locate the install lines for the staticcheck and
govulncheck steps in the lint.yml workflow and replace the `@latest` suffixes with
the specified versions.

In `@internal/api/assignment_handler.go`:
- Around line 327-398: The loops that call h.store.Queries().GetActionSetByID /
ListActionSetMembers and GetDefinitionByID / ListDefinitionMembers currently
silently continue on error; replace each bare "continue" with a call to
logEnrichmentErr so failures are logged consistently (e.g., invoke
logEnrichmentErr with context, h.logger if the helper now requires it, a short
operation identifier like "GetActionSetByID" or "ListDefinitionMembers", the
error, and the id) and then continue; update all four error branches (both Get*
and List* calls) to use this pattern to match the compliance-policy changes
elsewhere.

In `@internal/api/helpers.go`:
- Around line 104-109: The package-level slog.Warn in logEnrichmentErr drops
handler-scoped context; change the signature of logEnrichmentErr to accept a
*slog.Logger (e.g., logger *slog.Logger) and call logger.Warn instead of
slog.Warn, preserving the same keys ("operation", idKey, "error"); update all
call sites to pass the handler's logger (e.g., h.logger) similar to appendEvent
so per-handler attributes like component=... are retained in enrichment
warnings.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 20c4b5ae-932a-4224-95e4-810632f09bee

📥 Commits

Reviewing files that changed from the base of the PR and between 44ef974 and 3f994c8.

⛔ Files ignored due to path filters (1)

• go.sum is excluded by !**/*.sum

📒 Files selected for processing (68)

• .github/workflows/lint.yml
• README.md
• cmd/control/main.go
• cmd/indexer/main.go
• go.mod
• internal/api/action_handler.go
• internal/api/action_set_handler.go
• internal/api/action_set_handler_test.go
• internal/api/assignment_handler.go
• internal/api/audit_handler.go
• internal/api/compliance_policy_handler.go
• internal/api/definition_handler.go
• internal/api/device_group_handler.go
• internal/api/device_handler.go
• internal/api/errors.go
• internal/api/errors_parity_test.go
• internal/api/helpers.go
• internal/api/internal_handler.go
• internal/api/role_handler.go
• internal/api/search_handler.go
• internal/api/search_listener_test.go
• internal/api/service.go
• internal/api/sso_handler.go
• internal/api/system_actions_listener_test.go
• internal/api/terminal_handler.go
• internal/api/terminal_handler_test.go
• internal/api/user_group_handler.go
• internal/api/user_handler.go
• internal/api/user_selection_handler.go
• internal/api/validator_test.go
• internal/auth/interceptor.go
• internal/auth/interceptor_test.go
• internal/auth/jwt.go
• internal/auth/reconcile_test.go
• internal/config/config.go
• internal/connection/manager.go
• internal/gateway/registry/registry.go
• internal/gateway/task_handlers.go
• internal/gateway/task_handlers_test.go
• internal/handler/terminal_bridge.go
• internal/idp/linker.go
• internal/idp/linker_test.go
• internal/idp/oidc.go
• internal/middleware/middleware_test.go
• internal/mtls/mtls.go
• internal/mtls/peer_class_test.go
• internal/projectors/identity_provider_test.go
• internal/projectors/luks_key_listener.go
• internal/projectors/luks_key_test.go
• internal/projectors/role_test.go
• internal/projectors/scim_group_mapping.go
• internal/projectors/scim_group_mapping_test.go
• internal/projectors/security_alert_test.go
• internal/projectors/token.go
• internal/projectors/token_test.go
• internal/projectors/user_role_test.go
• internal/projectors/user_selection_listener.go
• internal/resolution/resolve_tree_test.go
• internal/scim/discovery.go
• internal/scim/groups.go
• internal/scim/handler.go
• internal/scim/scim.go
• internal/search/index.go
• internal/store/store.go
• internal/taskqueue/client.go
• internal/taskqueue/payloads.go
• internal/taskqueue/types_test.go
• internal/terminal/store_test.go

💤 Files with no reviewable changes (10)

• internal/auth/reconcile_test.go
• internal/resolution/resolve_tree_test.go
• internal/projectors/user_selection_listener.go
• internal/api/compliance_policy_handler.go
• internal/gateway/task_handlers_test.go
• internal/gateway/task_handlers.go
• internal/api/definition_handler.go
• internal/api/action_set_handler.go
• internal/api/device_group_handler.go
• internal/mtls/mtls.go

[coderabbitai]

🧹 Nitpick comments (1)

.github/workflows/lint.yml (1)

60-61: ⚡ Quick win

Consider using an anchored grep pattern for more defensive filtering.

Line 60's substring filter (grep -v 'internal/testutil') works correctly in this repository since no similarly named directories exist. However, an anchored regex pattern would be more robust and prevent future accidental exclusions if package structures change:

Suggested refinement

-          PKGS=$(go list ./... | grep -v 'internal/testutil')
-          govulncheck $PKGS
+          go list ./... \
+            | grep -vE '(^|/)internal/testutil($|/)' \
+            | xargs -r govulncheck

🤖 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 @.github/workflows/lint.yml around lines 60 - 61, The current PKGS filter
uses an unanchored substring grep (-v 'internal/testutil') which may
accidentally exclude packages with similar names; update the filtering in the
PKGS assignment so grep uses an anchored/regex pattern that precisely matches
the internal/testutil package path (e.g., anchor the segment boundaries or use a
path-segment-aware regex) instead of a plain substring, ensuring govulncheck
still receives the correct package list; change the grep used where PKGS=$(go
list ./... | grep -v 'internal/testutil') is defined to an anchored/regex form
that only excludes the exact internal/testutil package.

🤖 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.

Nitpick comments:
In @.github/workflows/lint.yml:
- Around line 60-61: The current PKGS filter uses an unanchored substring grep
(-v 'internal/testutil') which may accidentally exclude packages with similar
names; update the filtering in the PKGS assignment so grep uses an
anchored/regex pattern that precisely matches the internal/testutil package path
(e.g., anchor the segment boundaries or use a path-segment-aware regex) instead
of a plain substring, ensuring govulncheck still receives the correct package
list; change the grep used where PKGS=$(go list ./... | grep -v
'internal/testutil') is defined to an anchored/regex form that only excludes the
exact internal/testutil package.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6a317dbd-474f-48ab-9801-a7864eae652b

📥 Commits

Reviewing files that changed from the base of the PR and between 3f994c8 and 647bb3b.

📒 Files selected for processing (1)

• .github/workflows/lint.yml

[PaulDotterer]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.