revert: remove group-variables feature entirely (#196) (#239)

Reverts the group-based variables feature shipped via 5 server PRs:
- #197 (f0934bf): server storage + handlers + RBAC + events +
  migration 043_group_variables.sql + sqlc queries + group-variable
  validator + audit-redactor schema entries for the new event types
- #198 (6dafcef): server-side {{ var.NAME }} template renderer +
  internal_handler.go SyncActions hook + main.go renderer wiring
- #236 (3bb247a): drop device-labels resolver + add DispatchAction
  template gate
- #237 (ee455b1): rewire ListAvailableVariables to group-scoped +
  SDK pin bump
- #238 (05c7bc1): distinct error code for templated-dispatch refusal

Squashed into one cohesive rollback commit alongside the SDK pin
bump to power-manage-sdk@dda53a0 (which is the post-rollback SDK
HEAD that no longer carries the group-vars proto).

Conflicts resolved during revert:
- cmd/control/main.go: dropped both the 'template' import (we are
  removing the package) AND the 'asynqutil' import the revert wanted
  to put back (it lives in valkey.go now post-#157 slice 4 extraction;
  not main.go's responsibility anymore).
- go.mod: SDK pin set to dda53a0 (post-revert SDK HEAD) rather than
  the pre-#197 SHA the revert hunk targeted, so the server compiles
  against an SDK that also doesn't have the variables proto.
- go.sum: regenerated via 'go mod tidy' against the new pin.

Why: the design has fundamental scaling problems that were not caught
at design time. Group-bound variables become monsters when many
actions are assigned to a group; assignment-bound variables suffer
the inverse N\xc3\x97M repetition problem; bundle-based reuse needs more
design work before re-implementation. Cleaner to revert wholesale
than to ship a half-shape that locks operators into the wrong API.

Companion reverts:
- SDK #64 (merged): the proto + TS-client wrappers
- Web 09a8c75 (shipped): the variables UI + hint banner + error mapping

Refs #196.

Author: PaulDotterer

cmd/control/main.go

@@ -16,7 +16,6 @@ import (
 	"github.com/manchtools/power-manage/sdk/gen/go/pm/v1/pmv1connect"
 	"github.com/manchtools/power-manage/sdk/go/logging"
 	"github.com/manchtools/power-manage/server/internal/api"
-	"github.com/manchtools/power-manage/server/internal/api/template"
 	"github.com/manchtools/power-manage/server/internal/auth"
 	"github.com/manchtools/power-manage/server/internal/ca"
 	"github.com/manchtools/power-manage/server/internal/middleware"
@@ -278,12 +277,6 @@ func main() {
 		// ProxyValidateTerminalToken.
 		internalHandler.SetTerminalTokenStore(valkey.TerminalTokenStore)
 	}
-	// Server-side `{{ var.NAME }}` substitution for action params.
-	// The renderer's StoreResolver reads device labels + group
-	// variables from the projection and decrypts SECRET-typed values
-	// via the same encryptor that wrote them. See manchtools/power-
-	// manage-server#196 (group-based variables, design #59).
-	internalHandler.SetRenderer(template.New(template.NewStoreResolver(st, encryptor, logger.With("component", "template"))))
 	internalPath, internalH := pmv1connect.NewInternalServiceHandler(internalHandler)
 
 	// Peer-class gate: InternalService handles credential-bearing

go.mod

@@ -106,4 +106,4 @@ require (
 // whatever happens to be in a local ../sdk checkout. Developers who
 // want to iterate against a local SDK override this with a per-dev
 // go.work at their workspace root — see server/README.md for setup.
-replace github.com/manchtools/power-manage/sdk => github.com/manchtools/power-manage-sdk v0.4.1-0.20260511201104-c971debf896b
+replace github.com/manchtools/power-manage/sdk => github.com/manchtools/power-manage-sdk v0.4.1-0.20260512183037-dda53a00b743

go.sum

@@ -111,8 +111,8 @@ github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 h1:6E+4a0GO5zZEnZ
 github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0/go.mod h1:zJYVVT2jmtg6P3p1VtQj7WsuWi/y4VnjVBn7F8KPB3I=
 github.com/magiconair/properties v1.8.10 h1:s31yESBquKXCV9a/ScB3ESkOjUYYv+X0rg8SYxI99mE=
 github.com/magiconair/properties v1.8.10/go.mod h1:Dhd985XPs7jluiymwWYZ0G4Z61jb3vdS329zhj2hYo0=
-github.com/manchtools/power-manage-sdk v0.4.1-0.20260511201104-c971debf896b h1:wBe6kBNNwZrqzG2jY+MD0of7s/xpLh72PWyt6V9IiEY=
-github.com/manchtools/power-manage-sdk v0.4.1-0.20260511201104-c971debf896b/go.mod h1:hG7NcdIGAG0VzhLL/W12FDAgVQJxeK+jHFtHb2eF3A8=
+github.com/manchtools/power-manage-sdk v0.4.1-0.20260512183037-dda53a00b743 h1:ANagVtDVc6PmBoFXVcT2gx23tYVrRHAE3OiQoK2sdms=
+github.com/manchtools/power-manage-sdk v0.4.1-0.20260512183037-dda53a00b743/go.mod h1:hG7NcdIGAG0VzhLL/W12FDAgVQJxeK+jHFtHb2eF3A8=
 github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY=
 github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
 github.com/mdelapenya/tlscert v0.2.0 h1:7H81W6Z/4weDvZBNOfQte5GpIMo0lGYEeWbkGp5LJHI=

internal/api/action_dispatch.go

@@ -22,7 +22,6 @@ import (
 	"github.com/oklog/ulid/v2"
 
 	pm "github.com/manchtools/power-manage/sdk/gen/go/pm/v1"
-	"github.com/manchtools/power-manage/server/internal/api/template"
 	"github.com/manchtools/power-manage/server/internal/eventtypes"
 	"github.com/manchtools/power-manage/server/internal/eventtypes/payloads"
 	"github.com/manchtools/power-manage/server/internal/store"
@@ -147,35 +146,6 @@ func (h *ActionHandler) DispatchAction(ctx context.Context, req *connect.Request
 		return nil, apiErrorCtx(ctx, ErrValidationFailed, connect.CodeInvalidArgument, "either action_id or inline_action is required")
 	}
 
-	// Templated-params gate: ad-hoc DispatchAction has no group context
-	// to resolve `{{ var.NAME }}` from (variables are exclusively a
-	// group concept; the renderer only runs on the SyncActions path
-	// where the agent's device → group memberships supply the values).
-	// Refuse here rather than enqueueing literal `{{ ... }}` markers
-	// the agent would consume verbatim.
-	//
-	// Marshals to JSON once for the scan, regardless of params shape
-	// (map[string]any from the stored branch, []byte from the inline
-	// branch, or the rare string fallback). The marshal happens again
-	// downstream for signing — a one-extra-allocation cost in exchange
-	// for one shared scan path. Marshal failure here MUST be fail-
-	// closed: a params shape we can't serialise can't be scanned for
-	// templates either, and silently bypassing the gate would let
-	// templated content through that we couldn't see. The downstream
-	// signing step would also fail on the same params, but failing
-	// here is more honest about which gate is rejecting the call.
-	paramsJSONForScan, err := json.Marshal(inputs.params)
-	if err != nil {
-		return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal,
-			fmt.Sprintf("failed to marshal action params for template scan: %v", err))
-	}
-	if template.HasReference(string(paramsJSONForScan)) {
-		return nil, apiErrorCtx(ctx, ErrTemplatedDispatchRefused, connect.CodeFailedPrecondition,
-			"this action contains templated parameters ({{ var.NAME }}) and cannot be dispatched ad-hoc to a single device. "+
-				"Variables are resolved from device-group / user-group memberships at agent sync time. "+
-				"Assign the action to a device-group or user-group instead of running it directly.")
-	}
-
 	id := ulid.Make().String()
 
 	// Compute the deferred-dispatch delay. RunAt is optional; when set

internal/api/audit_handler.go

@@ -116,18 +116,6 @@ var eventRedactionSchemas = map[string]map[string]redactionSchema{
 		// `passphrase` at the top level.
 		"LuksKeyRotated": {paths: []string{"passphrase"}},
 	},
-	"device_group_variable": {
-		// Group-scoped variables (#59). The non-secret variants
-		// (GroupVariableSet / Deleted) carry no secret-bearing fields
-		// and need no redaction; the secret variants always carry the
-		// AES-GCM ciphertext under "ciphertext", scrubbed here so the
-		// audit log never reveals it. Same trust shape as
-		// LpsPasswordRotated / LuksKeyRotated.
-		string(eventtypes.GroupSecretVariableSet): {paths: []string{"ciphertext"}},
-	},
-	"user_group_variable": {
-		string(eventtypes.GroupSecretVariableSet): {paths: []string{"ciphertext"}},
-	},
 }
 
 // actionRedactionSchemas maps an action's `params.type` value to the

internal/api/dispatch_validation_test.go

@@ -1,7 +1,6 @@
 package api_test
 
 import (
-	"errors"
 	"log/slog"
 	"testing"
 
@@ -179,59 +178,6 @@ func TestDispatchAction_PreconditionNoTaskQueue(t *testing.T) {
 	assert.Equal(t, connect.CodeFailedPrecondition, connect.CodeOf(err))
 }
 
-// TestDispatchAction_TemplatedParamsRefused pins the templated-params
-// gate added for #196 scope correction. Variables resolve only via
-// device-group / user-group memberships at agent SyncActions time; an
-// ad-hoc DispatchAction has no group context so the renderer can't
-// resolve `{{ var.NAME }}` references. The gate refuses with
-// CodeFailedPrecondition rather than enqueueing literal `{{ ... }}`
-// markers the agent would consume verbatim.
-func TestDispatchAction_TemplatedParamsRefused(t *testing.T) {
-	st := testutil.SetupPostgres(t)
-	h := api.NewActionHandler(st, slog.Default(), api.NoOpSigner{})
-
-	adminID := testutil.CreateTestUser(t, st, testutil.NewID()+"@test.com", "pass", "admin")
-	deviceID := testutil.CreateTestDevice(t, st, "templated-host")
-	ctx := testutil.AdminContext(adminID)
-
-	_, err := h.DispatchAction(ctx, connect.NewRequest(&pm.DispatchActionRequest{
-		DeviceId: deviceID,
-		ActionSource: &pm.DispatchActionRequest_InlineAction{
-			InlineAction: &pm.Action{
-				Id:           &pm.ActionId{Value: testutil.NewID()},
-				Type:         pm.ActionType_ACTION_TYPE_SHELL,
-				DesiredState: pm.DesiredState_DESIRED_STATE_PRESENT,
-				Params: &pm.Action_Shell{
-					Shell: &pm.ShellParams{
-						Script: "echo {{ var.greeting }}",
-					},
-				},
-			},
-		},
-	}))
-	require.Error(t, err)
-	assert.Equal(t, connect.CodeFailedPrecondition, connect.CodeOf(err),
-		"templated params on ad-hoc dispatch MUST surface FailedPrecondition — anything else lets literal {{ var.X }} reach the agent verbatim")
-	assert.Contains(t, err.Error(), "templated parameters",
-		"error message must explain that variables are group-only and the action should be assigned to a group instead")
-
-	// The web client reads the structured ErrorDetail code (not the
-	// human-readable message) to map the failure to a localized
-	// "assign to a group instead" UI message. Pin the code here so a
-	// future refactor that drops the structured detail breaks the test
-	// instead of silently breaking the web error mapping.
-	var connectErr *connect.Error
-	require.True(t, errors.As(err, &connectErr))
-	details := connectErr.Details()
-	require.NotEmpty(t, details, "templated-dispatch refusal MUST carry the structured ErrorDetail — the web client maps by code, not by message")
-	val, vErr := details[0].Value()
-	require.NoError(t, vErr)
-	detail, ok := val.(*pm.ErrorDetail)
-	require.True(t, ok, "first detail must be ErrorDetail; got %T", val)
-	assert.Equal(t, api.ErrTemplatedDispatchRefused, detail.Code,
-		"error code MUST be templated_dispatch_refused — the web client switches on this exact string")
-}
-
 // TestDispatchInstantAction_PreconditionNoTaskQueue pins the
 // matching fail-closed behaviour for the instant-action path.
 func TestDispatchInstantAction_PreconditionNoTaskQueue(t *testing.T) {

internal/api/errors.go

@@ -109,15 +109,6 @@ const (
 	ErrValidationFailed = "validation_failed"
 	ErrInvalidPageToken = "invalid_page_token"
 	ErrInvalidQuery     = "invalid_query"
-
-	// ErrTemplatedDispatchRefused is returned when an operator tries to
-	// dispatch an action whose params contain `{{ var.NAME }}` references
-	// directly to a single device (DispatchAction / DispatchToMultiple /
-	// DispatchActionSet / etc.). Variables resolve only via device-group
-	// or user-group memberships at agent SyncActions time; the ad-hoc
-	// dispatch path has no group context. Web maps this code to a
-	// "assign to a group instead" UI message. See #196.
-	ErrTemplatedDispatchRefused = "templated_dispatch_refused"
 )
 
 // Internal error code (generic).