diff --git a/CHANGELOG.md b/CHANGELOG.md index a8cbceb..eddc58c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## 0.2.0 - 2026-07-21 + +- Clarified GTM skill readiness checks, site-snippet prerequisite, and when to use raw upstream commands versus extending `gtm-agent`. +- Added validated `createTrigger.config` compilation and `createTag` trigger bindings through singular `firingTriggerId` or plural `firingTriggerIds`, while preserving dry-run and publish gates. + ## 0.1.0 - 2026-05-24 - Added `gtm-agent` CLI with doctor, install, inventory, snapshot, diff, plan, apply, backup, raw passthrough, and guide commands. diff --git a/README.md b/README.md index af9a2a9..dae1b68 100644 --- a/README.md +++ b/README.md @@ -69,29 +69,55 @@ gtm-agent guide ## Declarative Plan +Phase 1 creates the trigger only: + ```yaml accountId: "123" containerId: "456" workspaceId: "7" actions: - - kind: enableBuiltInVariables - types: ["pageUrl", "clickText"] - kind: createTrigger - name: "All Pages" - type: "pageview" + name: "CE - article_product_click" + type: "CUSTOM_EVENT" + config: + customEventFilter: + - type: EQUALS + parameter: + - {type: TEMPLATE, key: arg0, value: "{{_event}}"} + - {type: TEMPLATE, key: arg1, value: article_product_click} +``` + +`createTrigger.config` and `createTag.config` are JSON objects expressed as YAML. Resource `type` values must be strings. Config cannot redefine declarative `name`, `type`, or tag firing-trigger fields. Tags accept either one quoted decimal `firingTriggerId` or a non-empty list of unique quoted decimal `firingTriggerIds`; both compile to the upstream `--firing-trigger-id` flag. The validator rejects numeric YAML values, blanks, zero, comma-packed singular values, duplicate IDs, and use of trigger IDs on non-tag actions. + +GTM assigns a trigger ID only after creation, so do not guess it or pretend a later action in the same plan can reference the earlier result. Use a two-phase, exact-name workflow: + +1. Run `inventory` and confirm there is no exact-name trigger already present. +2. Dry-run and execute a trigger-only plan. +3. Run `inventory` again and copy the returned numeric `triggerId`. +4. Confirm there is no exact-name tag already present, then dry-run and execute the tag plan with that ID. + +Phase 2 is a separate file created only after inventory returns the assigned ID (`"20"` is an example inventory result): + +```yaml +accountId: "123" +containerId: "456" +workspaceId: "7" +actions: - kind: createTag - name: "GA4 purchase" + name: "GA4 - article_product_click" type: "gaawe" + firingTriggerIds: ["20"] config: parameter: - type: template key: eventName - value: purchase - - kind: createVersion - name: "agent release" - notes: "Created by gtm-agent" + value: article_product_click ``` +The control plane deliberately does not perform live duplicate checks during an offline dry-run. Inventory and snapshots remain the explicit source of live-state evidence. + +Independent plans can also use `enableBuiltInVariables`, `createVariable`, `createVersion`, and the separately gated `publishVersion` action. + Publish action: ```yaml diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 2f9864c..2c2731d 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -38,33 +38,46 @@ Build a strong agent-friendly Google Tag Manager control-plane CLI without reinv Plans can be YAML or JSON: +Trigger phase: + ```yaml accountId: "123" containerId: "456" workspaceId: "7" actions: - - kind: enableBuiltInVariables - types: ["pageUrl", "clickText"] - kind: createTrigger - name: "All Pages" - type: "pageview" + name: "CE - article_product_click" + type: "CUSTOM_EVENT" + config: + customEventFilter: + - type: EQUALS + parameter: + - {type: TEMPLATE, key: arg0, value: "{{_event}}"} + - {type: TEMPLATE, key: arg1, value: article_product_click} +``` + +After execution and a fresh inventory read, the returned ID is used in a separate tag plan: + +```yaml +accountId: "123" +containerId: "456" +workspaceId: "7" +actions: - kind: createTag - name: "GA4 purchase" + name: "GA4 - article_product_click" type: "gaawe" + firingTriggerIds: ["20"] config: parameter: - type: template key: eventName - value: purchase - - kind: createVersion - name: "agent release" - notes: "Created by gtm-agent" - - kind: publishVersion - versionId: "42" + value: article_product_click ``` The first release intentionally supports the highest-value safe workflow primitives and leaves deep endpoint-specific authoring to raw upstream passthrough. +Trigger configuration is encoded into the upstream `--config` JSON argument. Reserved declarative fields (`name`, `type`, and tag firing-trigger fields) cannot be overridden inside config. A tag can bind to one `firingTriggerId` or several `firingTriggerIds`; the plan compiler validates quoted positive-decimal IDs and emits the upstream comma-separated `--firing-trigger-id` form. Because GTM generates IDs at mutation time, references to a newly created trigger use two reviewed plans with an inventory read between them. Offline plan compilation intentionally does not claim live exact-name uniqueness. + ## Verification Required checks before claiming completion: @@ -75,4 +88,3 @@ Required checks before claiming completion: - `go build -o ./gtm-agent ./cmd/gtm-agent` - Fake upstream E2E: install a temporary `gtm` script on `PATH`, run doctor, inventory, snapshot, diff, dry-run apply, guarded publish failure, allowed publish success, and raw passthrough. - If real GTM credentials are already available safely, run read-only `auth status` / inventory smoke. Do not create or publish real GTM resources without an explicit safe target. - diff --git a/internal/cli/root.go b/internal/cli/root.go index 3bc91e1..6872863 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -28,7 +28,7 @@ type runtime struct { asJSON bool } -const Version = "0.1.0" +const Version = "0.2.0" func NewRoot(options Options) *cobra.Command { if options.Out == nil { @@ -610,6 +610,12 @@ Recommended loop: 6. Snapshot after edits and diff snapshots. 7. Publish only with both gates: --allow-publish --confirm +Trigger-to-tag workflow: + Create a new trigger in one reviewed plan, then inventory the workspace and + copy its exact numeric triggerId into a second tag plan. Check the inventory + for an exact-name match before creating either resource. This avoids duplicate + resources and avoids guessing an ID that GTM assigns only after creation. + Raw escape hatch: gtm-agent raw -- @@ -620,20 +626,20 @@ const planTemplate = `accountId: "123" containerId: "456" workspaceId: "7" actions: - - kind: enableBuiltInVariables - types: ["pageUrl", "clickText"] - kind: createTrigger - name: "All Pages" - type: "pageview" - - kind: createTag - name: "GA4 purchase" - type: "gaawe" + name: "CE - article_product_click" + type: "CUSTOM_EVENT" config: - parameter: - - type: template - key: eventName - value: purchase - - kind: createVersion - name: "agent release" - notes: "Created by gtm-agent" + customEventFilter: + - type: EQUALS + parameter: + - {type: TEMPLATE, key: arg0, value: "{{_event}}"} + - {type: TEMPLATE, key: arg1, value: article_product_click} + +# After executing this trigger-only plan, run inventory and copy the assigned ID. +# Then create a separate tag plan whose action contains, for example: +# - kind: createTag +# name: "GA4 - article_product_click" +# type: "gaawe" +# firingTriggerIds: ["20"] # Replace with the verified inventory value. ` diff --git a/internal/cli/root_test.go b/internal/cli/root_test.go index 49f3bf7..5916bdd 100644 --- a/internal/cli/root_test.go +++ b/internal/cli/root_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/vecyang1/gtm-agent-cli/internal/cli" + planpkg "github.com/vecyang1/gtm-agent-cli/internal/plan" "github.com/vecyang1/gtm-agent-cli/internal/runner" ) @@ -106,7 +107,7 @@ actions: func TestCLIVersion(t *testing.T) { out := runCLI(t, runner.NewFake(nil), "--version") - if !strings.Contains(out, "gtm-agent version 0.1.0") { + if !strings.Contains(out, "gtm-agent version 0.2.0") { t.Fatalf("unexpected version output: %s", out) } } @@ -193,6 +194,53 @@ actions: } } +func TestCLIApplyKeepsConfiguredTriggerAndTagDryRunFirst(t *testing.T) { + triggerCommand := `gtm triggers create --name CE - article_product_click --type CUSTOM_EVENT --config {"customEventFilter":[{"parameter":[{"key":"arg0","type":"TEMPLATE","value":"{{_event}}"},{"key":"arg1","type":"TEMPLATE","value":"article_product_click"}],"type":"EQUALS"}]} --account-id 123 --container-id 456 --workspace-id 7 --output json` + tagCommand := "gtm tags create --name GA4 - article_product_click --type gaawe --firing-trigger-id 20 --account-id 123 --container-id 456 --workspace-id 7 --output json" + fake := runner.NewFake(map[string]runner.Result{ + triggerCommand: {Stdout: `{"triggerId":"20","name":"CE - article_product_click"}` + "\n"}, + tagCommand: {Stdout: `{"tagId":"30","name":"GA4 - article_product_click"}` + "\n"}, + }) + tmp := t.TempDir() + planPath := filepath.Join(tmp, "article-click.yaml") + if err := os.WriteFile(planPath, []byte(`accountId: "123" +containerId: "456" +workspaceId: "7" +actions: + - kind: createTrigger + name: "CE - article_product_click" + type: "CUSTOM_EVENT" + config: + customEventFilter: + - type: EQUALS + parameter: + - {type: TEMPLATE, key: arg0, value: "{{_event}}"} + - {type: TEMPLATE, key: arg1, value: article_product_click} + - kind: createTag + name: "GA4 - article_product_click" + type: "gaawe" + firingTriggerIds: ["20"] +`), 0o600); err != nil { + t.Fatalf("write plan: %v", err) + } + + dryRun := runCLI(t, fake, "apply", planPath, "--json") + if !strings.Contains(dryRun, `"dryRun": true`) || !strings.Contains(dryRun, `--firing-trigger-id 20`) { + t.Fatalf("dry-run did not expose the trigger binding: %s", dryRun) + } + if len(fake.Calls) != 0 { + t.Fatalf("dry-run unexpectedly called upstream GTM: %v", fake.Calls) + } + + executed := runCLI(t, fake, "apply", planPath, "--execute", "--json") + if !strings.Contains(executed, `"dryRun": false`) || !strings.Contains(executed, `"tagId": "30"`) { + t.Fatalf("execute output missing upstream result: %s", executed) + } + if len(fake.Calls) != 2 || fake.Calls[0] != triggerCommand || fake.Calls[1] != tagCommand { + t.Fatalf("unexpected execute calls: %v", fake.Calls) + } +} + func TestCLIPlanValidateAndTemplate(t *testing.T) { tmp := t.TempDir() planPath := filepath.Join(tmp, "plan.yaml") @@ -220,8 +268,15 @@ actions: if err != nil { t.Fatalf("read template: %v", err) } - if !strings.Contains(string(templateData), "enableBuiltInVariables") || !strings.Contains(string(templateData), "createTag") { - t.Fatalf("template missing expected action examples: %s", string(templateData)) + parsedTemplate, err := planpkg.Parse(templateData) + if err != nil { + t.Fatalf("generated template should validate: %v", err) + } + if len(parsedTemplate.Actions) != 1 || parsedTemplate.Actions[0].Kind != "createTrigger" { + t.Fatalf("starter template must contain only the trigger phase, got %#v", parsedTemplate.Actions) + } + if !strings.Contains(string(templateData), "create a separate tag plan") || !strings.Contains(string(templateData), "firingTriggerIds") { + t.Fatalf("template missing second-phase guidance: %s", string(templateData)) } } diff --git a/internal/plan/plan.go b/internal/plan/plan.go index bbe614c..2269665 100644 --- a/internal/plan/plan.go +++ b/internal/plan/plan.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/json" "fmt" + "strconv" "strings" "gopkg.in/yaml.v3" @@ -17,13 +18,15 @@ type Plan struct { } type Action struct { - Kind string `json:"kind" yaml:"kind"` - Name string `json:"name,omitempty" yaml:"name,omitempty"` - Type string `json:"type,omitempty" yaml:"type,omitempty"` - Types []string `json:"types,omitempty" yaml:"types,omitempty"` - Config map[string]any `json:"config,omitempty" yaml:"config,omitempty"` - VersionID string `json:"versionId,omitempty" yaml:"versionId,omitempty"` - Notes string `json:"notes,omitempty" yaml:"notes,omitempty"` + Kind string `json:"kind" yaml:"kind"` + Name string `json:"name,omitempty" yaml:"name,omitempty"` + Type string `json:"type,omitempty" yaml:"type,omitempty"` + Types []string `json:"types,omitempty" yaml:"types,omitempty"` + Config map[string]any `json:"config,omitempty" yaml:"config,omitempty"` + FiringTriggerID *string `json:"firingTriggerId,omitempty" yaml:"firingTriggerId,omitempty"` + FiringTriggerIDs *[]string `json:"firingTriggerIds,omitempty" yaml:"firingTriggerIds,omitempty"` + VersionID string `json:"versionId,omitempty" yaml:"versionId,omitempty"` + Notes string `json:"notes,omitempty" yaml:"notes,omitempty"` } type Options struct { @@ -44,6 +47,9 @@ func Parse(raw []byte) (Plan, error) { if err := decoder.Decode(&p); err != nil { return Plan{}, err } + if err := validateActionFieldNodes(raw); err != nil { + return Plan{}, err + } if strings.TrimSpace(p.AccountID) == "" { return Plan{}, fmt.Errorf("accountId is required") } @@ -94,11 +100,18 @@ func (p Plan) argsFor(action Action, options Options) ([]string, error) { return withOutput(args), nil case "createTrigger": args := []string{"triggers", "create", "--name", action.Name, "--type", action.Type} + if action.Config != nil { + encoded, err := encodeConfig(action.Config) + if err != nil { + return nil, err + } + args = append(args, "--config", encoded) + } args = append(args, workspaceBase...) return withOutput(args), nil case "createVariable": args := []string{"variables", "create", "--name", action.Name, "--type", action.Type} - if len(action.Config) > 0 { + if action.Config != nil { encoded, err := encodeConfig(action.Config) if err != nil { return nil, err @@ -109,13 +122,20 @@ func (p Plan) argsFor(action Action, options Options) ([]string, error) { return withOutput(args), nil case "createTag": args := []string{"tags", "create", "--name", action.Name, "--type", action.Type} - if len(action.Config) > 0 { + if action.Config != nil { encoded, err := encodeConfig(action.Config) if err != nil { return nil, err } args = append(args, "--config", encoded) } + triggerIDs, err := firingTriggerIDs(action) + if err != nil { + return nil, err + } + if len(triggerIDs) > 0 { + args = append(args, "--firing-trigger-id", strings.Join(triggerIDs, ",")) + } args = append(args, workspaceBase...) return withOutput(args), nil case "createVersion": @@ -141,6 +161,9 @@ func (p Plan) argsFor(action Action, options Options) ([]string, error) { } func validateAction(action Action) error { + if action.Kind != "createTag" && (action.FiringTriggerID != nil || action.FiringTriggerIDs != nil) { + return fmt.Errorf("firingTriggerId and firingTriggerIds are only valid for createTag") + } switch action.Kind { case "enableBuiltInVariables": if len(action.Types) == 0 { @@ -153,6 +176,19 @@ func validateAction(action Action) error { if strings.TrimSpace(action.Type) == "" { return fmt.Errorf("type is required") } + if action.Config != nil { + if err := validateConfigKeys(action); err != nil { + return err + } + if _, err := encodeConfig(action.Config); err != nil { + return fmt.Errorf("config must be valid JSON: %w", err) + } + } + if action.Kind == "createTag" { + if _, err := firingTriggerIDs(action); err != nil { + return err + } + } case "createVersion": if strings.TrimSpace(action.Name) == "" { return fmt.Errorf("name is required") @@ -167,6 +203,107 @@ func validateAction(action Action) error { return nil } +func validateConfigKeys(action Action) error { + reserved := map[string]struct{}{ + "name": {}, + "type": {}, + } + if action.Kind == "createTag" { + reserved["firingTriggerId"] = struct{}{} + reserved["firingTriggerIds"] = struct{}{} + } + for key := range action.Config { + if _, found := reserved[key]; found { + return fmt.Errorf("config key %q must use the declarative action field instead", key) + } + } + return nil +} + +func firingTriggerIDs(action Action) ([]string, error) { + if action.FiringTriggerID != nil && action.FiringTriggerIDs != nil { + return nil, fmt.Errorf("use either firingTriggerId or firingTriggerIds, not both") + } + var ids []string + switch { + case action.FiringTriggerID != nil: + ids = []string{*action.FiringTriggerID} + case action.FiringTriggerIDs != nil: + ids = append([]string{}, (*action.FiringTriggerIDs)...) + if len(ids) == 0 { + return nil, fmt.Errorf("firingTriggerIds must contain at least one ID") + } + default: + return nil, nil + } + seen := make(map[string]struct{}, len(ids)) + for i, id := range ids { + if strings.TrimSpace(id) != id || id == "" { + return nil, fmt.Errorf("firing trigger ID at index %d must be a non-empty decimal ID without surrounding whitespace", i) + } + if id[0] == '0' { + return nil, fmt.Errorf("firing trigger ID at index %d must be a positive decimal ID", i) + } + if _, err := strconv.ParseUint(id, 10, 64); err != nil { + return nil, fmt.Errorf("firing trigger ID at index %d must be a positive decimal ID", i) + } + if _, duplicate := seen[id]; duplicate { + return nil, fmt.Errorf("firing trigger ID %q is duplicated", id) + } + seen[id] = struct{}{} + } + return ids, nil +} + +func validateActionFieldNodes(raw []byte) error { + var document yaml.Node + if err := yaml.Unmarshal(raw, &document); err != nil { + return err + } + if len(document.Content) != 1 || document.Content[0].Kind != yaml.MappingNode { + return nil + } + root := document.Content[0] + for i := 0; i+1 < len(root.Content); i += 2 { + if root.Content[i].Value != "actions" { + continue + } + actions := root.Content[i+1] + if actions.Kind != yaml.SequenceNode { + return nil + } + for actionIndex, action := range actions.Content { + if action.Kind != yaml.MappingNode { + continue + } + for fieldIndex := 0; fieldIndex+1 < len(action.Content); fieldIndex += 2 { + name := action.Content[fieldIndex].Value + value := action.Content[fieldIndex+1] + switch name { + case "type": + if value.Kind != yaml.ScalarNode || value.Tag != "!!str" { + return fmt.Errorf("actions[%d]: type must be a string", actionIndex) + } + case "firingTriggerId": + if value.Kind != yaml.ScalarNode || value.Tag != "!!str" { + return fmt.Errorf("actions[%d]: firingTriggerId must be a quoted string", actionIndex) + } + case "firingTriggerIds": + if value.Kind != yaml.SequenceNode { + return fmt.Errorf("actions[%d]: firingTriggerIds must be a list of quoted strings", actionIndex) + } + for idIndex, id := range value.Content { + if id.Kind != yaml.ScalarNode || id.Tag != "!!str" { + return fmt.Errorf("actions[%d]: firingTriggerIds[%d] must be a quoted string", actionIndex, idIndex) + } + } + } + } + } + } + return nil +} + func encodeConfig(config map[string]any) (string, error) { encoded, err := json.Marshal(config) if err != nil { diff --git a/internal/plan/plan_test.go b/internal/plan/plan_test.go index 0de9338..ae7f793 100644 --- a/internal/plan/plan_test.go +++ b/internal/plan/plan_test.go @@ -54,6 +54,201 @@ actions: } } +func TestCreateTriggerCompilesConfigAsJSON(t *testing.T) { + raw := []byte(` +accountId: "123" +containerId: "456" +workspaceId: "7" +actions: + - kind: createTrigger + name: "CE - article_product_click" + type: "CUSTOM_EVENT" + config: + customEventFilter: + - type: EQUALS + parameter: + - type: TEMPLATE + key: arg0 + value: "{{_event}}" + - type: TEMPLATE + key: arg1 + value: article_product_click +`) + + p, err := plan.Parse(raw) + if err != nil { + t.Fatalf("Parse returned error: %v", err) + } + commands, err := p.Commands(plan.Options{}) + if err != nil { + t.Fatalf("Commands returned error: %v", err) + } + if len(commands) != 1 { + t.Fatalf("expected one command, got %d", len(commands)) + } + config := flagValue(commands[0].Args, "--config") + if !strings.Contains(config, `"customEventFilter"`) || !strings.Contains(config, `"article_product_click"`) { + t.Fatalf("trigger config was not compiled as JSON: %s", config) + } +} + +func TestCreateTagCompilesPluralFiringTriggerIDs(t *testing.T) { + raw := []byte(` +accountId: "123" +containerId: "456" +workspaceId: "7" +actions: + - kind: createTag + name: "GA4 - article_product_click" + type: "gaawe" + firingTriggerIds: ["20", "21"] +`) + + p, err := plan.Parse(raw) + if err != nil { + t.Fatalf("Parse returned error: %v", err) + } + commands, err := p.Commands(plan.Options{}) + if err != nil { + t.Fatalf("Commands returned error: %v", err) + } + if got := flagValue(commands[0].Args, "--firing-trigger-id"); got != "20,21" { + t.Fatalf("unexpected firing trigger flag: %q", got) + } +} + +func TestCreateTagCompilesSingularFiringTriggerID(t *testing.T) { + raw := []byte(` +accountId: "123" +containerId: "456" +workspaceId: "7" +actions: + - kind: createTag + name: "GA4 - article_product_click" + type: "gaawe" + firingTriggerId: "20" +`) + + p, err := plan.Parse(raw) + if err != nil { + t.Fatalf("Parse returned error: %v", err) + } + commands, err := p.Commands(plan.Options{}) + if err != nil { + t.Fatalf("Commands returned error: %v", err) + } + if got := flagValue(commands[0].Args, "--firing-trigger-id"); got != "20" { + t.Fatalf("unexpected firing trigger flag: %q", got) + } +} + +func TestParseRejectsMalformedFiringTriggerIDs(t *testing.T) { + tests := []struct { + name string + fields string + }{ + {name: "empty singular", fields: `firingTriggerId: ""`}, + {name: "non numeric singular", fields: `firingTriggerId: "all-pages"`}, + {name: "comma separated singular", fields: `firingTriggerId: "20,21"`}, + {name: "zero singular", fields: `firingTriggerId: "0"`}, + {name: "empty plural", fields: `firingTriggerIds: []`}, + {name: "blank plural member", fields: `firingTriggerIds: ["20", " "]`}, + {name: "duplicate plural member", fields: `firingTriggerIds: ["20", "20"]`}, + {name: "both forms", fields: "firingTriggerId: \"20\"\n firingTriggerIds: [\"21\"]"}, + {name: "wrong plural type", fields: `firingTriggerIds: "20"`}, + {name: "numeric singular type", fields: `firingTriggerId: 20`}, + {name: "numeric plural member type", fields: `firingTriggerIds: [20]`}, + {name: "null singular type", fields: `firingTriggerId: null`}, + {name: "null plural type", fields: `firingTriggerIds: null`}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + raw := []byte("accountId: \"123\"\n" + + "containerId: \"456\"\n" + + "workspaceId: \"7\"\n" + + "actions:\n" + + " - kind: createTag\n" + + " name: \"GA4 event\"\n" + + " type: \"gaawe\"\n" + + " " + test.fields + "\n") + if _, err := plan.Parse(raw); err == nil { + t.Fatalf("expected malformed firing trigger IDs to be rejected") + } + }) + } +} + +func TestParseRejectsFiringTriggerIDsOnNonTagActions(t *testing.T) { + _, err := plan.Parse([]byte(` +accountId: "123" +containerId: "456" +workspaceId: "7" +actions: + - kind: createTrigger + name: "All Pages" + type: "PAGEVIEW" + firingTriggerId: "20" +`)) + if err == nil || !strings.Contains(err.Error(), "only valid for createTag") { + t.Fatalf("expected scoped firing trigger validation error, got %v", err) + } +} + +func TestParseRejectsNonStringResourceTypes(t *testing.T) { + tests := []struct { + name string + kind string + typeValue string + }{ + {name: "numeric trigger type", kind: "createTrigger", typeValue: "123"}, + {name: "boolean tag type", kind: "createTag", typeValue: "true"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + raw := []byte("accountId: \"123\"\n" + + "containerId: \"456\"\n" + + "workspaceId: \"7\"\n" + + "actions:\n" + + " - kind: " + test.kind + "\n" + + " name: \"resource\"\n" + + " type: " + test.typeValue + "\n") + if _, err := plan.Parse(raw); err == nil || !strings.Contains(err.Error(), "type must be a string") { + t.Fatalf("expected non-string type to be rejected, got %v", err) + } + }) + } +} + +func TestParseRejectsConfigThatOverridesDeclarativeFields(t *testing.T) { + tests := []struct { + name string + kind string + config string + }{ + {name: "trigger name", kind: "createTrigger", config: `name: "hidden override"`}, + {name: "trigger type", kind: "createTrigger", config: `type: "hidden override"`}, + {name: "tag firing trigger", kind: "createTag", config: `firingTriggerId: ["20"]`}, + {name: "tag plural firing trigger", kind: "createTag", config: `firingTriggerIds: ["20"]`}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + raw := []byte("accountId: \"123\"\n" + + "containerId: \"456\"\n" + + "workspaceId: \"7\"\n" + + "actions:\n" + + " - kind: " + test.kind + "\n" + + " name: \"resource\"\n" + + " type: \"safe-type\"\n" + + " config:\n" + + " " + test.config + "\n") + if _, err := plan.Parse(raw); err == nil || !strings.Contains(err.Error(), "config key") { + t.Fatalf("expected config override to be rejected, got %v", err) + } + }) + } +} + func TestPublishRequiresExplicitGateAndContainerConfirmation(t *testing.T) { raw := []byte(` accountId: "123" @@ -157,3 +352,12 @@ func render(commands []plan.Command) string { } return b.String() } + +func flagValue(args []string, name string) string { + for i, arg := range args { + if arg == name && i+1 < len(args) { + return args[i+1] + } + } + return "" +} diff --git a/scripts/e2e-fake-gtm.sh b/scripts/e2e-fake-gtm.sh index f34281f..53ea8c5 100755 --- a/scripts/e2e-fake-gtm.sh +++ b/scripts/e2e-fake-gtm.sh @@ -4,10 +4,13 @@ set -euo pipefail ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" TMP="$(mktemp -d)" trap 'rm -rf "$TMP"' EXIT +export FAKE_GTM_STATE="$TMP/state" +mkdir -p "$FAKE_GTM_STATE" cat > "$TMP/gtm" <<'SH' #!/usr/bin/env bash set -euo pipefail +STATE="${FAKE_GTM_STATE:?}" case "$*" in "--version") echo "gtm version 1.5.8" @@ -28,10 +31,18 @@ case "$*" in echo '[{"workspaceId":"7","name":"Default Workspace"}]' ;; "tags list --account-id 123 --container-id 456 --workspace-id 7 --output json") - echo '[{"tagId":"1","name":"GA4 purchase","type":"gaawe"}]' + if [[ -f "$STATE/tag-created" ]]; then + echo '[{"tagId":"1","name":"GA4 purchase","type":"gaawe"},{"tagId":"30","name":"GA4 - article_product_click","type":"gaawe"}]' + else + echo '[{"tagId":"1","name":"GA4 purchase","type":"gaawe"}]' + fi ;; "triggers list --account-id 123 --container-id 456 --workspace-id 7 --output json") - echo '[{"triggerId":"2","name":"All Pages","type":"pageview"}]' + if [[ -f "$STATE/trigger-created" ]]; then + echo '[{"triggerId":"2","name":"All Pages","type":"pageview"},{"triggerId":"20","name":"CE - article_product_click","type":"CUSTOM_EVENT"}]' + else + echo '[{"triggerId":"2","name":"All Pages","type":"pageview"}]' + fi ;; "variables list --account-id 123 --container-id 456 --workspace-id 7 --output json") echo '[]' @@ -45,6 +56,15 @@ case "$*" in "triggers create --name All Pages --type pageview --account-id 123 --container-id 456 --workspace-id 7 --output json") echo '{"triggerId":"2","name":"All Pages"}' ;; + triggers\ create\ --name\ CE\ -\ article_product_click\ --type\ CUSTOM_EVENT\ --config\ *\ --account-id\ 123\ --container-id\ 456\ --workspace-id\ 7\ --output\ json) + touch "$STATE/trigger-created" + echo '{"triggerId":"20","name":"CE - article_product_click"}' + ;; + "tags create --name GA4 - article_product_click --type gaawe --firing-trigger-id 20 --account-id 123 --container-id 456 --workspace-id 7 --output json") + [[ -f "$STATE/trigger-created" ]] + touch "$STATE/tag-created" + echo '{"tagId":"30","name":"GA4 - article_product_click"}' + ;; "versions publish --version-id 42 --account-id 123 --container-id 456 --output json") echo '{"containerVersionId":"42","published":true}' ;; @@ -71,18 +91,44 @@ perl -0pi -e 's/GA4 purchase/GA4 purchase updated/g' "$TMP/after.json" "$TMP/gtm-agent" diff "$TMP/before.json" "$TMP/after.json" --json | grep -q 'GA4 purchase updated' "$TMP/gtm-agent" plan template --out "$TMP/template.yaml" --json | grep -q "$TMP/template.yaml" -cat > "$TMP/plan.yaml" <<'YAML' +if "$TMP/gtm-agent" inventory --account-id 123 --container-id 456 --workspace-id 7 --json | grep -q 'CE - article_product_click'; then + echo "trigger unexpectedly existed before the trigger plan" >&2 + exit 1 +fi + +cat > "$TMP/trigger-plan.yaml" <<'YAML' accountId: "123" containerId: "456" workspaceId: "7" actions: - kind: createTrigger - name: "All Pages" - type: "pageview" + name: "CE - article_product_click" + type: "CUSTOM_EVENT" + config: + customEventFilter: + - type: EQUALS + parameter: + - {type: TEMPLATE, key: arg0, value: "{{_event}}"} + - {type: TEMPLATE, key: arg1, value: article_product_click} +YAML +"$TMP/gtm-agent" plan validate "$TMP/trigger-plan.yaml" --json | grep -q '"valid": true' +"$TMP/gtm-agent" apply "$TMP/trigger-plan.yaml" --json | grep -q '"dryRun": true' +"$TMP/gtm-agent" apply "$TMP/trigger-plan.yaml" --execute --json | grep -q '"triggerId": "20"' +"$TMP/gtm-agent" inventory --account-id 123 --container-id 456 --workspace-id 7 --json | grep -q 'CE - article_product_click' + +cat > "$TMP/tag-plan.yaml" <<'YAML' +accountId: "123" +containerId: "456" +workspaceId: "7" +actions: + - kind: createTag + name: "GA4 - article_product_click" + type: "gaawe" + firingTriggerIds: ["20"] YAML -"$TMP/gtm-agent" plan validate "$TMP/plan.yaml" --json | grep -q '"valid": true' -"$TMP/gtm-agent" apply "$TMP/plan.yaml" --json | grep -q '"dryRun": true' -"$TMP/gtm-agent" apply "$TMP/plan.yaml" --execute --json | grep -q '"triggerId": "2"' +"$TMP/gtm-agent" plan validate "$TMP/tag-plan.yaml" --json | grep -q '"valid": true' +"$TMP/gtm-agent" apply "$TMP/tag-plan.yaml" --json | grep -q -- '--firing-trigger-id 20' +"$TMP/gtm-agent" apply "$TMP/tag-plan.yaml" --execute --json | grep -q '"tagId": "30"' cat > "$TMP/publish.yaml" <<'YAML' accountId: "123" diff --git a/skills/gtm-agent/SKILL.md b/skills/gtm-agent/SKILL.md index aea37fc..e88878c 100644 --- a/skills/gtm-agent/SKILL.md +++ b/skills/gtm-agent/SKILL.md @@ -10,14 +10,24 @@ Use this skill for Google Tag Manager operations. ## Rules - Wheel first: use `gtm-agent`, which wraps `@owntag/gtm-cli`. +- If `gtm-agent doctor --json` says `gtm` is missing, install the pinned upstream wheel with `gtm-agent install --execute` before diagnosing auth or config. - Read-only discovery first: `gtm-agent doctor --json`, then `inventory` or `snapshot`. - Mutations must start with dry-run `gtm-agent apply --json`. - Real mutation requires `--execute`. - Publishing requires `--allow-publish --confirm `. - Never paste or commit service-account JSON, OAuth tokens, live snapshots, or backups. +- A site must already load the GTM container snippet, or the CMS/app must support adding it, before CLI-created tags can fire on that site. - Prefer a dedicated GTM workspace for changes. - Use `gtm-agent raw -- ...` only when the declarative safety layer lacks a needed upstream command. - Mutating raw commands require `--allow-mutation`; raw publish requires `--allow-publish --confirm `. +- Declarative trigger plans support `config`; tag plans support either `firingTriggerId` or `firingTriggerIds`, using quoted positive-decimal IDs only. +- When a tag needs a newly created trigger, use two plans: create the trigger, inventory the workspace to obtain its assigned ID, then create the tag. Before each create, inspect inventory for an exact-name match. + +## Enough Or Extend + +Use the existing toolchain first. `gtm-agent` is enough for normal agent-safe GTM work: doctor, inventory, snapshots, diffs, backups, basic declarative tag/trigger/variable/version plans, and guarded publish. + +Drop to `gtm-agent raw -- ...` when the upstream `@owntag/gtm-cli` already has a command that the declarative plan layer does not expose. Add code to `gtm-agent` only when a repeated workflow needs safer declarative plans, stronger validation, idempotent upsert behavior, or a reusable site-specific guardrail. ## Standard Workflow @@ -46,3 +56,7 @@ go test ./... go vet ./... ./scripts/e2e-fake-gtm.sh ``` + +## References + +- [Google Ads API Integration & Tooling Reference](file:///Users/vecsatfoxmailcom/.gemini/config/skills/gtm-agent/references/google_ads_api_integration.md) (Developer Token requirements and comparison of Composio vs. google-ads-open-cli)