Repository navigation
fix: reject automation trigger events no producer emits - #358
Merged
pedronauck merged 7 commits intoAug 12, 2026
Merged
Conversation
Trigger creation accepted any non-empty event name, but the runtime only emits activations for session.created, session.stopped, memory.consolidated, hook.<hook_name>.completed, webhook, and ext.*. A trigger created for any other name — a typo or an unsupported family such as loop.terminal — sat enabled and permanently silent, with no error on any surface and no way to notice short of waiting forever. Validate the event name in Trigger.Validate so every creation surface (CLI, HTTP, UDS, Web, native tools) fails fast with the supported families spelled out. Hook completions require a hook name and extension events require a suffix after the ext. prefix; the open ext.* namespace stays free-form past that. The builtin names move to exported model constants and the envelope producers now reference them, and the CLI --event help enumerates the families at the point of use. Co-Authored-By: Claude Fable 5 <[email protected]>
|
@franciscpd is attempting to deploy a commit to the Compozy Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
WalkthroughTrigger event validation is centralized for built-in, hook-completion, and extension formats. Creation and update paths preserve raw event values and reject invalid names. Memory consolidation now emits completion events, and migration removes invalid stored triggers. ChangesAutomation trigger event hardening
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Filename | Overview |
|---|---|
| internal/automation/model/trigger_event_validate.go | Defines the shared closed event grammar and rejects unsupported, padded, or incomplete event names. |
| internal/automation/extension.go | Completes the prior fix by rejecting a bare extension prefix at ingress, matching trigger-definition validation. |
| internal/automation/manager_triggers.go | Enforces canonical event validation at dynamic trigger creation and update boundaries. |
| internal/store/globaldb/schema/migrations/00060_trigger_event_hard_cut.sql | Applies the hard cut to persisted unsupported trigger definitions and related records. |
| internal/memory/consolidation/runtime_trigger.go | Connects successful memory consolidation to the newly supported automation event. |
| web/src/systems/automation/lib/trigger-event-id.ts | Aligns Web-side event identifiers with the backend-supported trigger families. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Producer["Built-in, hook, webhook, or extension producer"] --> Validation["Canonical event validation"]
Definition["Trigger creation or update"] --> Validation
Validation -->|valid matching grammar| Runtime["Automation trigger engine"]
Validation -->|unsupported or incomplete| Reject["Reject before persistence or dispatch"]
Runtime --> Run["Matching automation run"]
Reviews (6): Last reviewed commit: "test: wrap migration run assertion" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/automation/model/target_validate.go`:
- Around line 106-108: Update the validation call in the trigger validation flow
to pass the raw t.Event value, then modify ValidateTriggerEvent to reject values
with leading or trailing whitespace while preserving existing event validation.
Add test cases covering both leading and trailing whitespace in Trigger.Event.
In `@internal/automation/model/validate.go`:
- Around line 409-468: Move the trigger event constants and ValidateTriggerEvent
implementation out of validate.go into a focused trigger-event validation file,
such as trigger_event_validate.go. Preserve the existing exported and unexported
symbol names, validation behavior, and required imports while keeping
validate.go within the 500-line production-source limit.
🪄 Autofix
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 Plus
Run ID: a0042c5b-5022-41b9-aa2b-0ad3587190a5
⛔ Files ignored due to path filters (1)
packages/site/content/docs/automation/triggers.mdxis excluded by!**/*.mdx
📒 Files selected for processing (6)
internal/automation/model/target_validate.gointernal/automation/model/validate.gointernal/automation/trigger.gointernal/automation/trigger_envelope.gointernal/automation/validate_test.gointernal/cli/automation_trigger_commands.go
Address the PR compozy#358 review round: - Move the trigger event constants and ValidateTriggerEvent into model/trigger_event_validate.go so validate.go stays under the 500-line file cap. - Reject surrounding whitespace in ValidateTriggerEvent, passing the raw event through from Trigger.Validate. Runtime matching trims both sides, so a padded name still matched — the reject keeps stored triggers canonical and mirrors the extension ingress contract. - Close the ext-namespace divergence Greptile found: the extension ingress now rejects a bare "ext." event, matching the trigger validator, so an accepted activation always has a creatable consumer. - Regenerate the CLI reference for the --event help text (codegen-check CI failure). Co-Authored-By: Claude Fable 5 <[email protected]>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/memory/dream_test.go (1)
277-298: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftWrap each edited top-level test in a named subtest.
internal/memory/dream_test.go#L277-L298: wrap the successful spawner contract case int.Run("Should ...").internal/memory/dream_test.go#L794-L798: wrap the missing workspace resolver case int.Run("Should ...").internal/memory/dream_test.go#L832-L835: wrap the workspace resolution case int.Run("Should ...").internal/memory/dream_test.go#L864-L868: wrap the workspace resolution error case int.Run("Should ...").internal/memory/dream_test.go#L902-L906: wrap the workspace directory error case int.Run("Should ...").internal/memory/dream_test.go#L929-L931: wrap the spawner failure case int.Run("Should ...").internal/memory/dream_test.go#L960-L962: wrap the joined failure case int.Run("Should ...").internal/memory/dream_test.go#L984-L988: wrap the lock-unavailable case int.Run("Should ...").internal/memory/dream_test.go#L1005-L1012: wrap the input-validation cases int.Run("Should ...").internal/memory/dream_test.go#L1242-L1249: wrap the concurrent-run case int.Run("Should ...").internal/daemon/daemon_test.go#L2492-L2510: wrap the boot automation integration case int.Run("Should ...").As per path instructions,
**/*_test.gorequirest.Run("Should...")for all test cases.🤖 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/memory/dream_test.go` around lines 277 - 298, Wrap each specified top-level test case in a named t.Run subtest using a descriptive “Should ...” name, preserving the existing assertions and test logic. Apply this to internal/memory/dream_test.go ranges 277-298, 794-798, 832-835, 864-868, 902-906, 929-931, 960-962, 984-988, 1005-1012, and 1242-1249, plus internal/daemon/daemon_test.go range 2492-2510; ensure every *_test.go case follows the required t.Run convention.Source: Path instructions
🤖 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/automation/manager_test.go`:
- Around line 1422-1437: Add named t.Run subtests using the exact “Should...”
names around each padded-event rejection case:
internal/automation/manager_test.go lines 1422-1437 (“Should reject a padded
event on trigger creation”), internal/automation/manager_test.go lines 1554-1560
(“Should reject a padded event on trigger update”), and
internal/daemon/native_automation_tools_test.go lines 362-375 (“Should reject a
padded event on trigger creation”); keep each existing test body and assertions
inside its corresponding subtest.
In `@internal/automation/trigger_test.go`:
- Around line 749-753: Wrap each padded-event validation scenario in a named
t.Run("Should...") subtest: update internal/automation/trigger_test.go lines
749-753, internal/automation/resource_test.go lines 83-93, and
internal/store/globaldb/global_db_automation_test.go lines 2222-2230. Keep each
scenario’s existing assertions inside its subtest and use a descriptive “Should
reject...” name.
In `@internal/memory/consolidation/runtime_completion.go`:
- Around line 46-50: Update the observer delivery flow around notifyCtx and
observer to restore a bounded deadline after context.WithoutCancel(ctx), using
the established completion-delivery timeout if available. Ensure the observer
receives that deadline-bound context, and treat deadline expiry as a delivery
failure by logging the timeout through the existing failure path.
In `@internal/memory/dream_test.go`:
- Around line 1005-1012: Strengthen the two `service.Run` validation tests by
asserting the specific expected error type or message for the nil context and
nil spawner cases, rather than only checking that an error is returned. Keep the
existing test setup and ensure each assertion verifies the corresponding
validation failure.
---
Outside diff comments:
In `@internal/memory/dream_test.go`:
- Around line 277-298: Wrap each specified top-level test case in a named t.Run
subtest using a descriptive “Should ...” name, preserving the existing
assertions and test logic. Apply this to internal/memory/dream_test.go ranges
277-298, 794-798, 832-835, 864-868, 902-906, 929-931, 960-962, 984-988,
1005-1012, and 1242-1249, plus internal/daemon/daemon_test.go range 2492-2510;
ensure every *_test.go case follows the required t.Run convention.
🪄 Autofix
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 Plus
Run ID: c37090df-6c69-489b-92d9-0fffc1f8d386
⛔ Files ignored due to path filters (10)
docs/qa/charters/CH-producer-backed-trigger-events.mdis excluded by!**/*.mddocs/qa/journeys/J-create-and-activate-trigger.mdis excluded by!**/*.mddocs/qa/reports/2026-08-12-pr-358-trigger-events.mdis excluded by!**/*.mddocs/qa/scenarios/TA-057.mdis excluded by!**/*.mdinternal/store/globaldb/schema/migrations/atlas.sumis excluded by!**/*.sum,!**/*.sumpackages/site/content/docs/automation/triggers.mdxis excluded by!**/*.mdxpackages/site/content/docs/cli/automation/triggers/create.mdxis excluded by!**/*.mdxpackages/site/content/docs/cli/automation/triggers/update.mdxis excluded by!**/*.mdxpackages/site/content/docs/configuration/config-toml.mdxis excluded by!**/*.mdxskills/compozy/references/configuration.mdis excluded by!**/*.md
📒 Files selected for processing (44)
internal/api/core/automation_test.gointernal/api/core/automation_trigger_mapping.gointernal/automation/manager_definition_sync.gointernal/automation/manager_test.gointernal/automation/manager_triggers.gointernal/automation/model/trigger_event_validate.gointernal/automation/resource.gointernal/automation/resource_test.gointernal/automation/trigger_registration.gointernal/automation/trigger_test.gointernal/automation/validate.gointernal/automation/validate_test.gointernal/cli/automation_request.gointernal/cli/automation_trigger_commands.gointernal/config/automation.gointernal/config/automation_test.gointernal/daemon/boot_automation.gointernal/daemon/daemon_extension_kit_e2e_helpers_test.gointernal/daemon/daemon_test.gointernal/daemon/native_automation_tools_test.gointernal/daemon/native_automation_trigger_tools.gointernal/extension/automation_resources.gointernal/extension/automation_resources_test.gointernal/extension/host_api_automation.gointernal/extension/host_api_test.gointernal/extension/host_api_workspace_automation_mapping.gointernal/memory/consolidation/runtime.gointernal/memory/consolidation/runtime_completion.gointernal/memory/consolidation/runtime_spawner_test.gointernal/memory/consolidation/runtime_test.gointernal/memory/consolidation/runtime_trigger.gointernal/memory/dream_result.gointernal/memory/dream_run.gointernal/memory/dream_test.gointernal/store/globaldb/global_db_automation_normalize.gointernal/store/globaldb/global_db_automation_test.gointernal/store/globaldb/global_db_trigger_event_migration_test.gointernal/store/globaldb/schema/migrations/00060_trigger_event_hard_cut.sqlweb/src/systems/automation/components/__tests__/automation-trigger-form.test.tsxweb/src/systems/automation/hooks/use-automation-trigger-form.tsweb/src/systems/automation/lib/__tests__/trigger-preview.test.tsweb/src/systems/automation/lib/trigger-event-id.tsweb/src/systems/automation/lib/trigger-preview.tsweb/src/systems/automation/mocks/fixtures.ts
💤 Files with no reviewable changes (4)
- internal/automation/resource.go
- internal/store/globaldb/global_db_automation_normalize.go
- internal/automation/trigger_registration.go
- internal/extension/automation_resources.go
🚧 Files skipped from review as they are similar to previous changes (3)
- internal/cli/automation_trigger_commands.go
- internal/automation/validate_test.go
- internal/automation/model/trigger_event_validate.go
Comment on lines
+46
to
+50
| notifyCtx := context.Background() | ||
| if ctx != nil { | ||
| notifyCtx = context.WithoutCancel(ctx) | ||
| } | ||
| if err := observer(notifyCtx, result); err != nil { |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Set a deadline for completion delivery.
context.WithoutCancel removes the request deadline. The synchronous observer call can then wait forever if automation dispatch blocks. Use a bounded context for observer delivery and log a timeout as a delivery failure.
As per coding guidelines, work that outlives a request must restore a deadline when needed.
🤖 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/memory/consolidation/runtime_completion.go` around lines 46 - 50,
Update the observer delivery flow around notifyCtx and observer to restore a
bounded deadline after context.WithoutCancel(ctx), using the established
completion-delivery timeout if available. Ensure the observer receives that
deadline-bound context, and treat deadline expiry as a delivery failure by
logging the timeout through the existing failure path.
Source: Coding guidelines
Contributor
Author
There was a problem hiding this comment.
Verified but deferring to the maintainer on this one: context.WithoutCancel here is a deliberate detach so completion delivery survives the request that produced it, and the observer chain ends in automation dispatch, which has its own persistence-side effects — bounding it changes delivery semantics (a timeout would drop the trigger activation rather than the log line). If a deadline is wanted, it belongs in the observer wiring where the delivery guarantee is decided, not in this publisher. Leaving the design call to @pedronauck since this path is part of the producer-backed rework.
CodeRabbit round on 9cd0c73: wrap each new padded-event validation case in a t.Run("Should ...") subtest per the repo test convention, and assert the specific guard errors on the dream service nil-context and nil-spawner paths. Co-Authored-By: Claude Fable 5 <[email protected]>
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
This was referenced Aug 12, 2026
Merged
This was referenced Aug 13, 2026
Merged
Merged
Merged
Merged
Merged
This was referenced Aug 18, 2026
Merged
Merged
This was referenced Aug 20, 2026
Merged
Merged
Merged
Merged
This was referenced Sep 10, 2026
Merged
Merged
Merged
Merged
This was referenced Sep 27, 2026
Merged
Merged
This was referenced Oct 6, 2026
Merged
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #357.
Problem
Trigger creation accepted any non-empty event name, but the runtime only emits activations for
session.created,session.stopped,memory.consolidated,hook.<hook_name>.completed,webhook, andext.*. Any other name — a typo or an unsupported family such asloop.terminal— produced an enabled trigger that could never fire, silently, on every surface. Repro in #357.Change
ValidateTriggerEventininternal/automation/model: closed builtin set,hook.<name>.completedrequires a hook name,ext.*stays free-form past the prefix (per the extension ingress contract). Called fromTrigger.Validate, so CLI, HTTP, UDS, Web, and native-tool creation all fail fast with the supported families spelled out.internal/automationnow reference them instead of duplicating literals.--eventhelp enumerates the families at the point of use.docs/automation/triggers.mdxupdated: the "does not restrict" paragraph now documents the closed families and the rejection.Deliberately not included (discussed in #357, maintainer's call): a
loop.terminalproducer — may or may not be product intent, andwatch-eventscovers loop-reactive wakes after #356; and an event discovery endpoint for a Web picker, sincehook.*/ext.*are open grammars.Coverage
Five new
TestTriggerValidatecases: named hook completion andext.name accepted; producer-less name, hook completion without a hook name, and bareext.prefix rejected. Fullinternal/automation,internal/api,internal/daemon, andinternal/storesuites pass.🤖 Generated with Claude Code
Summary by CodeRabbit