[stefan-ernst]

Delivers the WI-2173 audit/revision foundation ahead of Sobek script
execution. Tests live in the paired core-tests-internal PR.

What changed

Immutable, attributable revisions

• New action_definition_revisions table captures the full executable
  snapshot (trigger + config, nodes + config, edges, actor override,
  enablement, execution-role restrictions) plus editor identity/source,
  timestamp, source label, change_summary, and changed_fields.
• actions.current_revision_id points at the live revision;
  action_execution_logs.definition_revision_id records what ran.
• A migration adds the schema and backfills a source='baseline' revision
  for every existing definition with editor_user_id = NULL and
  editor_source='system', so no historical authorship is invented.

Transactional coupling

• ActionDefinitionService now owns one transaction per mutation: action
  row + graph + revision + audit rows commit or roll back together. An
  audit or revision failure aborts the mutation.
• create, update, delete, and template application all funnel through
  that service, including the v1 handler and the AI/MCP tools, so adapters
  cannot double-write or skip history.

Execution binding

• ActionService executes the frozen revision snapshot and records its id,
  rather than the mutable action row, so a concurrent edit cannot make the
  recorded revision disagree with what ran. Durable retries reuse the
  revision from the first attempt. The cached action is never mutated, so
  concurrent executions stay isolated.

Authorization hardening

• Changing execution-affecting configuration (trigger, graph, enablement,
  allowed roles) on an action with a configured actor now requires the
  global action.set_actor permission — on v2, v1, and the AI-tool paths.
  action.manage alone is no longer enough. Denied attempts are audited
  with safe metadata (attempted field names only, never the definition).
• Revision list/detail require action.manage on the workspace
  (actions:read scope).

API and UI

• GET /rest/api/v2/workspaces/{id}/actions/{action_id}/revisions
• GET /rest/api/v2/workspaces/{id}/actions/{action_id}/revisions/{revision_id}
• New editor history view listing revisions and rendering the immutable
  snapshot. No v1 surface added.

Retention

Revisions are intentionally not cascade-deleted with the action, so an
execution can always be traced back to the definition that ran. They do
cascade with the workspace, matching the existing workspace-deletion wipe.

Out of scope

Asset actions (separate domain, no actor override) and the broader
MCP/CLI authoring and restore tooling in WI-1316.

Tests

All in the paired core-tests PR; see that PR for exact commands. Coverage
includes exact snapshots/diffs and attribution, execution revision binding,
deletion/history retention, baseline migration, actor-edit denial with an
unchanged actor_user_id, and exact v2 permission contracts on SQLite and
PostgreSQL, plus frontend unit and Playwright coverage of the history view.

[github-actions]

Size Change: 0 B 🆕

Total Size: 0 B

_{compressed-size-action}

[stefan-ernst]

Review: changes requested

Three findings at d26cf7d:

1. [P1] Concurrent saves can restore a cleared execution identity.
   internal/services/action_definition_service.go:134–158
Save receives an action read before its transaction. When actorChanged is false, it preserves the database’s actor but snapshots the stale in-memory actor. If an administrator clears the actor while another user’s rename is pending, that rename can publish a revision containing the old privileged actor. Execution trusts that revision and continues impersonating the cleared identity. Read, authorize, and snapshot the effective action under the same transaction lock.

2. [P2] Retained revisions become inaccessible after action deletion.
   internal/services/action_application_service.go:421–431
   Both history methods call s.Get, which requires the live action row. Deleting an action therefore makes both endpoints return not-found despite retaining its revisions. Authorize against the workspace and scope the retained revision directly, without requiring a surviving action.

3. [P2] Several privileged-edit denials leave no audit event.
   internal/aitools/actions.go:541–542
   The AI-tool denial returns without auditing; the new v1 denial does likewise. In v2, actor-change denials return before auditDeniedPrivilegedEdit. These paths miss WI-2173’s explicit denied-attempt audit requirement. Route them through a shared safe-metadata audit helper.

Static review only; tests were not run.

[stefan-ernst]

Addressed all three findings in cef844ff (tests in core-tests 34dd9175).

1. Stale actor snapshot / authorization race (P1). ActionDefinitionService.Save now takes the actor patch instead of a pre-computed actorChanged flag. Inside the write transaction it locks the row (UpdateTx), reads actor_user_id from the locked row, resolves the effective actor, and runs an AuthorizeActor callback before snapshotting. The revision records the locked value, so a save that started before a concurrent actor clear can no longer republish the cleared identity. Authorization now runs under the same lock too. Regression test: TestActionDefinitionSaveUsesLockedActor (SQLite + PostgreSQL) seeds a stale copy, clears the actor, saves, and asserts the revision snapshot and stored actor are both nil.

2. Retained history inaccessible after deletion (P2). ListRevisions/GetRevision no longer call s.Get; they authorize with requireManage against the workspace and query revisions scoped by action_id + workspace_id. GetByIDForAction/ListSummariesByAction take the workspace predicate. Black-box test: TestActionRevisionsRemainReadableAfterDeletion deletes the action and still reads the list and detail (SQLite + PostgreSQL).

3. Missing denial audits (P2). Denied privileged-edit auditing is centralized in the definition service: the AuthorizeActor callback returns a *PrivilegedActionEditDenied, and Save writes the safe access.denied row. v2 actor-change denials, the v1 handler, and the AI/MCP tool all use the callback, so every adapter is covered. The write happens after the mutation transaction rolls back (writing it inside the transaction re-entered the single SQLite write connection and deadlocked). Tests: existing TestPrivilegedActionEditRequiresSetActor plus new TestV1PrivilegedActionEditDenialIsAudited assert the access.denied row, required_permission, unchanged actor, and no definition leak.

Targeted tests pass on SQLite and PostgreSQL (black-box action-revision set, the locked-actor service test, repository retention, and the baseline migration). No new v1 surface; OpenAPI and lint guards are green.

[github-actions]

This PR has merge conflicts that need to be resolved before it can be merged. Please rebase on the latest main branch.

[github-actions]

Merge conflicts have been resolved.

[stefan-ernst]

Merge preparation completed in 85aa3e3:

• Merged current main and resolved the adjacent migration additions, preserving both the action-revision and timer/worklog migrations.
• Corrected the PR title to use the allowed backend scope.
• Left the remaining concurrent metadata-only PATCH issue unchanged, as accepted as non-blocking.

Validation:

• make openapi-v2-check — passed.
• golangci-lint run ./internal/database/... — passed; the subsequent full pre-commit Go lint also passed.
• bun run --bun build, bun run --bun check, and bun run --bun typecheck in frontend — passed (typecheck has two warnings, zero errors).
• All pre-commit guards passed.
• ./overlay.sh ../core -- -tags=test -count=1 -run "TestMigration_ActionDefinitionRevisions|TestActionDefinitionSaveUsesLockedActor|TestActionRevision|TestActionUpdateRecordsDiff|TestActionExecutionBound|TestPrivilegedActionEdit|TestActionTemplateApplyPersistsRevision|TestV1PrivilegedActionEditDenialIsAudited" ./internal/database/ ./internal/services/ ./internal/repository/ ./tests/ — database, repository, and HTTP packages passed; services could not compile because existing timer tests still target the old timer API. Integrating the latest core-tests main into the isolated overlay did not resolve those unrelated test mismatches.
• ./overlay.sh ../core -- -tags=test -count=1 ./internal/database/ — failed only on the retained-migration inventory expectation (134 entries versus 132 expected), which does not yet include the two timer migrations now on main.

PostgreSQL and browser suites were not rerun. Refreshed GitHub checks are running before merge.