feat: improve hooks system#10
Merged
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
WalkthroughAdds a full hooks subsystem: taxonomy, payloads/patches, executors (native/subprocess/wasm stub), hot-reloadable registry and dispatch pipeline (sync/async, guards, depth tracking), daemon integration replacing notifier fanout, API endpoints and CLI, config parsing/merge support, and extensive unit/integration tests. Changes
Sequence DiagramsequenceDiagram
participant Client
participant HTTP as "HTTP Handler"
participant Observer
participant Hooks
participant Exec as "Executor"
Client->>HTTP: GET /api/hooks/catalog?...
HTTP->>HTTP: ParseHookCatalogFilter
HTTP->>Observer: QueryHookCatalog(filter)
Observer->>Hooks: Catalog(filter)
Hooks->>Hooks: load snapshot & filter
Hooks-->>Observer: []CatalogEntry
Observer-->>HTTP: mapped contract payloads
HTTP-->>Client: 200 {hooks:[...]}
Note over Client,Hooks: Dispatch flow
Client->>Hooks: DispatchSessionPostCreate(payload)
Hooks->>Hooks: select matching hooks (sync/async)
Hooks->>Exec: Execute(hook, serialized_payload)
Exec-->>Hooks: patch/result
Hooks->>Hooks: apply patch, check deny/guards
Hooks->>Hooks: submit async hooks to worker pool
Hooks-->>Client: final payload / error or denied
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/hooks/executor_subprocess_unix.go (1)
33-37:⚠️ Potential issue | 🟡 MinorWrap syscall errors with operation context.
Line 37 returns
errunwrapped, violating the coding guideline to usefmt.Errorf("context: %w", err)for explicit error context.💡 Proposed fix
import ( "errors" + "fmt" "os/exec" "syscall" ) @@ if err := syscall.Kill(-cmd.Process.Pid, sig); err != nil { if errors.Is(err, syscall.ESRCH) { return nil } - return err + return fmt.Errorf("hooks: signal subprocess process-group with %s: %w", sig, err) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/hooks/executor_subprocess_unix.go` around lines 33 - 37, The syscall.Kill error is returned directly; wrap it with contextual information using fmt.Errorf before returning to follow the error-wrapping guideline. Update the branch that checks the result of syscall.Kill(-cmd.Process.Pid, sig) so that when err is not syscall.ESRCH you return fmt.Errorf("kill process group (pid %d, sig %v): %w", cmd.Process.Pid, sig, err); also add the fmt import if missing. Ensure the ESRCH case still returns nil unchanged.
🧹 Nitpick comments (17)
internal/hooks/ordering_test.go (1)
85-112: Add an invalid-source case forDefaultHookPrioritywith specific error assertion.Current coverage only exercises success paths; please add the unknown-source branch and assert the error contract explicitly.
💡 Example test addition
func TestDefaultHookPriority(t *testing.T) { t.Parallel() @@ for _, tt := range tests { @@ } + + t.Run("Should return error for unknown source", func(t *testing.T) { + t.Parallel() + _, err := DefaultHookPriority(HookSource(999)) + if err == nil { + t.Fatal("DefaultHookPriority() error = nil, want non-nil") + } + }) }As per coding guidelines:
MUST have specific error assertions (ErrorContains, ErrorAs).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/hooks/ordering_test.go` around lines 85 - 112, Add a failing-case to TestDefaultHookPriority that passes an invalid HookSource (e.g., HookSource(-1) or a clearly out-of-range value) to DefaultHookPriority and assert that it returns an error using a specific error assertion (e.g., require.ErrorContains/require.ErrorAs or t.Fatalf with strings.Contains). Reference the test function TestDefaultHookPriority and the function DefaultHookPriority so you add a new subtest (t.Run("invalid"...) ) that checks got is ignored and the returned error contains the expected message/marker for unknown sources.internal/api/udsapi/handlers_test.go (1)
200-236: Addt.Parallel()for test independence.Per coding guidelines, independent tests should use
t.Parallel(). This test doesn't share state with other tests and can run concurrently.♻️ Proposed fix
func TestHookEventsHandlerAvailableOnUDSRouter(t *testing.T) { + t.Parallel() + homePaths := newTestHomePaths(t)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/udsapi/handlers_test.go` around lines 200 - 236, Add t.Parallel() at the start of the TestHookEventsHandlerAvailableOnUDSRouter test to allow it to run concurrently; open the TestHookEventsHandlerAvailableOnUDSRouter function and insert a call to t.Parallel() immediately after the function begins (before creating homePaths/observer/engine) so this independent test can execute in parallel with others.internal/hooks/matcher_test.go (1)
108-211: Consider splittingTestHookMatcherMatchesAdditionalFamiliesinto focused tests.This test covers 10+ different matcher methods in a single function. While comprehensive, splitting into smaller focused tests (e.g.,
TestHookMatcherMatchesInput,TestHookMatcherMatchesTool) would improve readability and make failures easier to diagnose.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/hooks/matcher_test.go` around lines 108 - 211, The current TestHookMatcherMatchesAdditionalFamilies bundles many assertions; split it into focused test functions (e.g., TestHookMatcherMatchesInput/MatchesPrompt/MatchesAgentPreStart/MatchesAgentLifecycle/MatchesTurn, TestHookMatcherMatchesEvent, TestHookMatcherMatchesTool, TestHookMatcherMatchesPermissionResolution) that each create only the HookMatcher and corresponding payload (reuse the SessionContext/ToolCall/PermissionToolCall fixtures), call the single matcher method under test (MatchesInput, MatchesPrompt, MatchesAgentPreStart, MatchesAgentLifecycle, MatchesTurn, MatchesEvent, MatchesToolPostCall, MatchesToolPostError, MatchesPermissionResolution) and assert the boolean with a clear t.Fatal message; ensure each new test calls t.Parallel() and preserves the same payload values and expectations from the original test.internal/hooks/events_test.go (1)
5-26: Consider using a constant or comment for the expected event count.The hardcoded
27at line 9 will break if new events are added. If this is intentional to catch unintentional event additions, consider adding a comment explaining this. Otherwise, consider making the test more resilient.💡 Alternative: Document the intent
func TestAllHookEvents(t *testing.T) { t.Parallel() events := AllHookEvents() + // Explicitly assert count to catch unintentional event additions/removals. + // Update this when the event taxonomy changes. if len(events) != 27 { t.Fatalf("len(AllHookEvents()) = %d, want 27", len(events)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/hooks/events_test.go` around lines 5 - 26, Replace the magic number 27 in TestAllHookEvents with a named constant (e.g., ExpectedHookEventCount) and add a one-line comment explaining whether the test is intended to assert an exact count (to catch accidental additions/removals) or to be resilient; update the length assertion to use ExpectedHookEventCount (or change the assertion to a >= check if resilience is desired) so the intent is clear when inspecting TestAllHookEvents and AllHookEvents.internal/daemon/daemon_integration_test.go (1)
669-691: Consider usingt.Runsubtests or table-driven assertions for payload fields.The helper provides clear assertions but could benefit from subtests for better failure isolation. However, this is a minor suggestion given the helper's focused scope.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/daemon/daemon_integration_test.go` around lines 669 - 691, The helper assertLifecycleHookPayload should use t.Run subtests to isolate failures for each field; update assertLifecycleHookPayload to wrap each assertion (Event, WorkspaceID, Workspace/rootDir and the json unmarshal/read error checks) in separate t.Run blocks named like "read file", "unmarshal", "event", "workspace id", "workspace path" and use the payload variable and wantEvent/wantWorkspace to drive the expectations so failing checks report which assertion failed clearly.internal/api/core/parsers.go (1)
115-119: Redundant string trimming.
query.Eventis already assigned fromstrings.TrimSpace(c.Query("event"))on line 105. The trim on line 115 is unnecessary.🔧 Suggested simplification
- if event := strings.TrimSpace(query.Event); event != "" { - if err := hookspkg.HookEvent(event).Validate(); err != nil { + if query.Event != "" { + if err := hookspkg.HookEvent(query.Event).Validate(); err != nil { return store.HookRunQuery{}, err } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/core/parsers.go` around lines 115 - 119, The extra strings.TrimSpace call is redundant because query.Event was already trimmed earlier; replace the trimming in the conditional with a simple check using the already-trimmed value (e.g., use event := query.Event or if query.Event != ""), then call hookspkg.HookEvent(event).Validate() as before (refer to the query.Event variable in internal/api/core/parsers.go to locate the code).internal/config/hooks.go (2)
58-68: Add compile-time interface verification.The
hookValidationExecutortype implements theExecutorinterface (frominternal/hooks) but lacks a compile-time verification to catch interface drift.As per coding guidelines: "Use compile-time interface verification:
var _ Interface = (*Type)(nil)".Suggested fix
type hookValidationExecutor struct { kind hookspkg.HookExecutorKind } + +var _ hookspkg.Executor = hookValidationExecutor{}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/config/hooks.go` around lines 58 - 68, Add a compile-time interface verification to ensure hookValidationExecutor continues to satisfy the Executor interface: add a package-level var assertion referencing hookValidationExecutor and the Executor interface (e.g. var _ hooks.Executor = (*hookValidationExecutor)(nil)) so the compiler will catch any interface drift; place this next to the hookValidationExecutor type definition or other type assertions in the file.
71-76: Capacity pre-allocation underestimates when agents have multiple hooks.The slice capacity is calculated as
len(cfg.Hooks.Declarations) + len(agents), but eachAgentDefcan have multiple hooks. This won't cause bugs but results in unnecessary allocations when agents define multiple hooks.More accurate capacity calculation
-func HookDeclarations(cfg Config, agents []AgentDef) ([]hookspkg.HookDecl, error) { - raw := make([]hookspkg.HookDecl, 0, len(cfg.Hooks.Declarations)+len(agents)) +func HookDeclarations(cfg Config, agents []AgentDef) ([]hookspkg.HookDecl, error) { + capacity := len(cfg.Hooks.Declarations) + for _, agent := range agents { + capacity += len(agent.Hooks) + } + raw := make([]hookspkg.HookDecl, 0, capacity)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/config/hooks.go` around lines 71 - 76, HookDeclarations currently preallocates capacity with len(cfg.Hooks.Declarations)+len(agents) which underestimates when each AgentDef may have multiple hooks; change the allocation to compute the exact needed capacity by summing len(cfg.Hooks.Declarations) plus the sum of len(agent.Hooks) for each agent in agents, then use that total to make the raw slice before appending the cloned declarations (references: HookDeclarations, cfg.Hooks.Declarations, agents, AgentDef, cloneHookDecls).internal/hooks/hooks_test.go (1)
191-278: Consider addingt.Parallel()to the concurrency test.
TestHooksConcurrentRebuildAndDispatchtests concurrent behavior but doesn't callt.Parallel(). While this test is a stress test, Go's race detector works correctly with parallel tests, and addingt.Parallel()would allow it to run alongside other tests, improving overall test suite performance.Add t.Parallel()
func TestHooksConcurrentRebuildAndDispatch(t *testing.T) { + t.Parallel() + declsA := []HookDecl{🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/hooks/hooks_test.go` around lines 191 - 278, The test TestHooksConcurrentRebuildAndDispatch lacks t.Parallel(); add a call to t.Parallel() near the start of the test body (immediately after the func(t *testing.T) { opens) so the test runs in parallel with other tests; verify there are no globals or shared test-wide resources being mutated (the test uses local variables like seq, hooks, wg, errCh) to avoid interference when running in parallel.internal/config/hooks_test.go (1)
12-12: Addt.Parallel()to independent tests.Several test functions are missing
t.Parallel()calls. Based on the test implementations, these tests appear to be independent (they each create isolated temp directories and don't share mutable state):
TestLoadParsesConfigHookDeclarationWithAllFields(Line 12)TestLoadParsesMinimalConfigHookAndAppliesDefaults(Line 89)TestLoadRejectsInvalidConfigHookEvent(Line 126)TestLoadRejectsRequiredAsyncConfigHook(Line 144)TestLoadMergesConfigHooksAcrossPrecedenceLevels(Line 164)TestHookDeclarationsReturnsCombinedConfigAndAgentHooks(Line 310)TestHookDeclarationsReturnsEmptySliceForEmptyHooksSection(Line 361)As per coding guidelines: "Use
t.Parallel()for independent subtests".Also applies to: 89-89, 126-126, 144-144, 164-164, 310-310, 361-361
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/config/hooks_test.go` at line 12, Several independent tests are missing t.Parallel(): add a call to t.Parallel() at the start of each of the listed test functions (TestLoadParsesConfigHookDeclarationWithAllFields, TestLoadParsesMinimalConfigHookAndAppliesDefaults, TestLoadRejectsInvalidConfigHookEvent, TestLoadRejectsRequiredAsyncConfigHook, TestLoadMergesConfigHooksAcrossPrecedenceLevels, TestHookDeclarationsReturnsCombinedConfigAndAgentHooks, TestHookDeclarationsReturnsEmptySliceForEmptyHooksSection) so they run concurrently; open each function and insert t.Parallel() as the first statement inside the test body (before any shared setup) to enable parallel execution.internal/hooks/executor_test.go (1)
124-143: Missingt.Parallel()call.This test is missing the
t.Parallel()call that all other tests in this file have. Since it usest.Setenv(), verify that parallel execution won't cause environment variable conflicts with other tests.func TestSubprocessExecutorExecuteFiltersEnvironment(t *testing.T) { + t.Parallel() + if runtime.GOOS == "windows" { t.Skip("subprocess shell test requires POSIX shell") }Note:
t.Setenvautomatically restores the original value after the test completes and is safe witht.Parallel()in Go 1.17+.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/hooks/executor_test.go` around lines 124 - 143, The test TestSubprocessExecutorExecuteFiltersEnvironment is missing t.Parallel(); add a call to t.Parallel() near the top of the test (after the runtime.GOOS skip) so it runs concurrently like the other tests; ensure you place it before t.Setenv(...) to allow the test's environment changes (via t.Setenv) to be isolated and safe when run in parallel.internal/hooks/events.go (1)
129-164: Consider adding a compile-time or init-time consistency check.The
allHookEventsslice andhookEventSpecsmap must stay synchronized. If an event is added to one but not the other, it could cause subtle bugs:
- Event in
allHookEventsbut nothookEventSpecs→Family()returns"",Validate()returns error- Event in
hookEventSpecsbut notallHookEvents→ won't appear inAllHookEvents()iterationConsider adding an
init()check or a test that verifies both collections have the same events.🛡️ Example init-time validation
func init() { for _, event := range allHookEvents { if _, ok := hookEventSpecs[event]; !ok { panic(fmt.Sprintf("hooks: event %q in allHookEvents but not hookEventSpecs", event)) } } for event := range hookEventSpecs { found := false for _, e := range allHookEvents { if e == event { found = true break } } if !found { panic(fmt.Sprintf("hooks: event %q in hookEventSpecs but not allHookEvents", event)) } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/hooks/events.go` around lines 129 - 164, Add a compile/init-time consistency check that ensures allHookEvents and hookEventSpecs contain the same HookEvent entries: iterate allHookEvents and verify each key exists in hookEventSpecs, then iterate hookEventSpecs keys and verify each appears in allHookEvents; if any mismatch is found, fail fast (e.g., panic or log.Fatalf) during init() so the discrepancy is caught at startup (reference allHookEvents, hookEventSpecs, and the AllHookEvents function to locate the definitions).internal/config/agent.go (1)
204-222: Remove redundantTrimSpacecalls.Lines 214-219 duplicate the trimming already performed in lines 205-212 when constructing the
AgentDef. The values are trimmed when assigned, then immediately trimmed again.🧹 Proposed cleanup
agent := AgentDef{ Name: strings.TrimSpace(parsed.Name), Provider: strings.TrimSpace(parsed.Provider), Command: strings.TrimSpace(parsed.Command), Model: strings.TrimSpace(parsed.Model), Tools: cloneStrings(parsed.Tools), Permissions: strings.TrimSpace(parsed.Permissions), MCPServers: cloneMCPServers(parsed.MCPServers), Prompt: strings.TrimSpace(body), } - agent.Name = strings.TrimSpace(agent.Name) - agent.Provider = strings.TrimSpace(agent.Provider) - agent.Command = strings.TrimSpace(agent.Command) - agent.Model = strings.TrimSpace(agent.Model) - agent.Permissions = strings.TrimSpace(agent.Permissions) - agent.Prompt = strings.TrimSpace(body) if len(agent.Tools) == 0 { agent.Tools = []string{"*"} }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/config/agent.go` around lines 204 - 222, The trimmed assignments after constructing the AgentDef are redundant—remove the repeated strings.TrimSpace calls for agent.Name, agent.Provider, agent.Command, agent.Model, agent.Permissions, and agent.Prompt that occur immediately after the AgentDef literal; keep the initial trimming done inside the AgentDef constructor (i.e., the trimmed values assigned when creating agent := AgentDef{...}) and leave the subsequent logic that defaults agent.Tools to []string{"*"} intact.internal/hooks/executor_subprocess.go (1)
138-138: Consider adding brief justification comments for ignored errors.Per coding guidelines, every error must be handled or have a written justification. The
terminateSubprocessCommandandkillSubprocessCommanderrors are intentionally ignored here (best-effort cleanup during context cancellation), which is reasonable. A brief inline comment would satisfy the guideline and clarify intent for future readers.case <-ctx.Done(): - _ = terminateSubprocessCommand(cmd) + _ = terminateSubprocessCommand(cmd) // best-effort; process may already be gone timer := time.NewTimer(subprocessShutdownGrace) defer timer.Stop() select { case err := <-waitCh: return err case <-timer.C: - _ = killSubprocessCommand(cmd) + _ = killSubprocessCommand(cmd) // best-effort; process may already be gone return <-waitCh }Also applies to: 146-146
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/hooks/executor_subprocess.go` at line 138, Add a one-line inline justification comment next to the ignored error returns from terminateSubprocessCommand(cmd) and killSubprocessCommand(cmd) in executor_subprocess.go explaining that these errors are intentionally not handled because cleanup is best-effort during context cancellation and failure to terminate the subprocess does not change the main control flow (optionally note you could log at debug level if desired); place the comments immediately above or beside the calls to terminateSubprocessCommand and killSubprocessCommand so future readers know the ignores are deliberate.internal/api/contract/contract.go (1)
7-9: Consider whether embeddinghookspkg.HookMatcherin the contract layer is the right abstraction.The
contractpackage importsinternal/hooksto embedHookMatcherdirectly inHookCatalogPayload. While hooks is a domain package (not api/cli/daemon), this creates coupling between the API contract and internal hook types.If
HookMatcherfields change, the API contract changes implicitly. Consider whether:
- This coupling is intentional and acceptable (hook matchers are part of the public API)
- A separate
MatcherPayloadDTO with explicit fields would provide better API stabilityThis is likely intentional given the design, but worth confirming the API surface is meant to expose internal matcher semantics directly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/contract/contract.go` around lines 7 - 9, The contract package currently embeds hookspkg.HookMatcher into HookCatalogPayload, which couples the API contract to internal hook types; either confirm that HookMatcher is intended as part of the public API or introduce a dedicated DTO to decouple them: create a new MatcherPayload struct in this package with explicit fields matching the public parts of hookspkg.HookMatcher, replace the embedded HookMatcher in HookCatalogPayload with MatcherPayload, and add conversion helpers (e.g., NewMatcherPayloadFromHookMatcher and ToHookMatcher if needed) to map between hookspkg.HookMatcher and the new DTO so contract changes are explicit and stable.internal/daemon/hooks_bridge.go (1)
91-95: Empty interface methods lack documentation.These empty implementations of
OnSessionCreatedandOnSessionStoppedare intentional no-ops since lifecycle observation is handled via hook dispatch. A brief comment would clarify this design decision for future maintainers.Suggested documentation
+// OnSessionCreated is a no-op; session creation observation is handled via hook dispatch. func (n *hooksNotifier) OnSessionCreated(ctx context.Context, sess *session.Session) { } +// OnSessionStopped is a no-op; session stop observation is handled via hook dispatch. func (n *hooksNotifier) OnSessionStopped(ctx context.Context, sess *session.Session) { }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/daemon/hooks_bridge.go` around lines 91 - 95, Add concise comments to the empty methods OnSessionCreated and OnSessionStopped on hooksNotifier explaining they are intentional no-ops because lifecycle observation is handled elsewhere via hook dispatch; update the functions (OnSessionCreated and OnSessionStopped) to include a one-line comment each stating "no-op: lifecycle observation handled via hook dispatch" (or similar) so future maintainers understand the purpose and that leaving them empty is deliberate.internal/hooks/hooks.go (1)
379-418: JSON-based fingerprinting may have performance implications.Using
json.Marshalfor fingerprinting is correct for semantic comparison but could be expensive for large hook registries (hundreds of hooks with complex matchers/metadata). Consider if this becomes a bottleneck in hot-reload scenarios.For now this is acceptable, but if profiling shows
Rebuildlatency issues, consider:
- Incremental fingerprinting per source
- Hash-based fingerprinting (e.g., using
hash/fnvon sorted keys)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/hooks/hooks.go` around lines 379 - 418, The current fingerprintHookSnapshot function uses json.Marshal on the full fingerprints slice which can be costly; replace it with a deterministic, compact hash-based fingerprint: iterate AllHookEvents() and for each snapshotFingerprint and its resolvedHookFingerprint entries produce a stable byte stream (e.g., write Event, Name, Source, Mode, Required, Priority, Timeout, Matcher, sorted Metadata keys/values, ExecutorKind, Command, Args, sorted Env keys/values, SkillSource) and feed that into a non-cryptographic hasher (e.g., hash/fnv) to produce a single string fingerprint; ensure ordering of map keys (Metadata, Env) and slice elements is deterministic and keep the function name fingerprintHookSnapshot and input/output semantics the same (returning the hash string and error) so callers (e.g., Rebuild logic) remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/config/config.go`:
- Around line 327-329: The call to c.Hooks.Validate() returns raw errors that
lose section context; update the return to wrap the error with context using
fmt.Errorf so callers know the failure came from the Hooks section (e.g.,
replace returning err from the c.Hooks.Validate() check with return
fmt.Errorf("hooks validation: %w", err)); ensure fmt is imported if not already
and reference the c.Hooks.Validate() call in internal/config/config.go when
making this change.
In `@internal/daemon/hooks_bridge.go`:
- Around line 254-256: The nil-context fallback using context.Background() must
be removed from the production dispatch path: in the function that checks "if
ctx == nil { ctx = context.Background() }" (look for the ctx parameter usage in
hooks_bridge.go), replace that fallback with explicit validation that returns an
error (or panics in init code) when ctx is nil so cancellation/deadline
propagation is preserved; update the function signature/contract and adjust
callers/tests to pass a non-nil context instead of relying on a background
context.
In `@internal/hooks/dispatch.go`:
- Around line 443-448: The early-return branches that currently return
context.Canceled when h == nil or ctx == nil are misleading; replace those
returns with descriptive errors (e.g., errors.New("nil dispatcher: h is nil")
and errors.New("nil context: ctx is nil")) so callers get accurate failure
reasons, updating the function that contains the h == nil / ctx == nil checks
and adding the necessary import for the errors package (or fmt.Errorf if you
prefer formatted errors).
In `@internal/hooks/executor_wasm_stub.go`:
- Around line 5-16: Add a compile-time assertion that WasmExecutor implements
the HookExecutor interface by adding a package-level var like `var _
HookExecutor = (*WasmExecutor)(nil)` near the `WasmExecutor` type declaration;
this uses the `WasmExecutor` and `HookExecutor` symbols to lock the interface
contract and will fail compilation if `Execute`, `Kind`, or other required
methods drift.
In `@internal/hooks/executor.go`:
- Around line 29-36: Define a package-level sentinel error (e.g.,
ErrInvalidHookExecutorKind) and update HookExecutorKind.Validate to return a
wrapped error using errors wrapping so callers can use errors.Is; specifically,
replace the current fmt.Errorf return in the default case with a wrapped error
that includes the sentinel (for example return fmt.Errorf("hooks: invalid hook
executor kind %q: %w", k, ErrInvalidHookExecutorKind)). Ensure the sentinel is
exported if it should be matched from other packages and keep the existing
Validate signature.
In `@internal/hooks/introspection.go`:
- Around line 85-111: The loop building CatalogEntry may panic because it calls
hook.Executor.Kind() without ensuring hook.Executor is non-nil; add a defensive
nil-check inside the inner loop that skips or substitutes a safe default when
hook.Executor == nil (e.g., set ExecutorKind to an empty string or "unknown")
before constructing the CatalogEntry, updating the code around the CatalogEntry
creation (where hook.Executor.Kind() is used) so cloneHookMatcher,
cloneStringMap and other fields are still populated but ExecutorKind is guarded.
In `@internal/hooks/matcher.go`:
- Around line 257-270: matchStringField currently swallows errors from
path.Match which can hide malformed patterns like “[z-a]”; validate patterns
earlier (e.g., in normalizeHookMatcher or ValidateMatcherForEvent) by calling
path.Match or a dedicated validation helper for each pattern string and
return/log validation errors during hook normalization, and update
matchStringField to treat runtime errors as a validation-failure fallback
(optionally logging the invalid pattern via the package logger when err != nil)
so invalid patterns are reported instead of silently ignored.
In `@internal/hooks/ordering_test.go`:
- Around line 5-83: Convert the standalone tests for SortResolvedHooks and
OrderedResolvedHooks into table-driven subtests using t.Run("Should ...") per
the repo pattern: group the cases from TestSortResolvedHooksOrdersBySource,
TestSortResolvedHooksOrdersByPriorityThenName,
TestSortResolvedHooksOrdersSkillSourcesBeforeName,
TestSortResolvedHooksIsStableAcrossMultipleSorts and
TestOrderedResolvedHooksReturnsSortedCopy into a single TestSortResolvedHooks
(or similarly named) that iterates a slice of cases, calling t.Run("Should
<behavior>", func(t *testing.T) { t.Parallel(); ... }) for each case; keep using
testResolvedHook, SortResolvedHooks, OrderedResolvedHooks and assertHookNames
inside each subtest, preserve parallelism semantics, and ensure stability case
asserts compare the original pointers as before.
In `@internal/hooks/ordering.go`:
- Around line 9-21: The function DefaultHookPriority returns a formatted error
for an invalid HookSource that cannot be matched with errors.Is; add a
package-level sentinel error (e.g., ErrInvalidHookSource) and change the default
return to wrap or return that sentinel so callers can use errors.Is.
Specifically, declare ErrInvalidHookSource = errors.New("hooks: invalid hook
source") in the same package and modify DefaultHookPriority's default case to
return the sentinel (or fmt.Errorf("%w: %d", ErrInvalidHookSource, source))
instead of only fmt.Errorf(...); update any tests or callers that assert the
error to use errors.Is(..., ErrInvalidHookSource).
---
Outside diff comments:
In `@internal/hooks/executor_subprocess_unix.go`:
- Around line 33-37: The syscall.Kill error is returned directly; wrap it with
contextual information using fmt.Errorf before returning to follow the
error-wrapping guideline. Update the branch that checks the result of
syscall.Kill(-cmd.Process.Pid, sig) so that when err is not syscall.ESRCH you
return fmt.Errorf("kill process group (pid %d, sig %v): %w", cmd.Process.Pid,
sig, err); also add the fmt import if missing. Ensure the ESRCH case still
returns nil unchanged.
---
Nitpick comments:
In `@internal/api/contract/contract.go`:
- Around line 7-9: The contract package currently embeds hookspkg.HookMatcher
into HookCatalogPayload, which couples the API contract to internal hook types;
either confirm that HookMatcher is intended as part of the public API or
introduce a dedicated DTO to decouple them: create a new MatcherPayload struct
in this package with explicit fields matching the public parts of
hookspkg.HookMatcher, replace the embedded HookMatcher in HookCatalogPayload
with MatcherPayload, and add conversion helpers (e.g.,
NewMatcherPayloadFromHookMatcher and ToHookMatcher if needed) to map between
hookspkg.HookMatcher and the new DTO so contract changes are explicit and
stable.
In `@internal/api/core/parsers.go`:
- Around line 115-119: The extra strings.TrimSpace call is redundant because
query.Event was already trimmed earlier; replace the trimming in the conditional
with a simple check using the already-trimmed value (e.g., use event :=
query.Event or if query.Event != ""), then call
hookspkg.HookEvent(event).Validate() as before (refer to the query.Event
variable in internal/api/core/parsers.go to locate the code).
In `@internal/api/udsapi/handlers_test.go`:
- Around line 200-236: Add t.Parallel() at the start of the
TestHookEventsHandlerAvailableOnUDSRouter test to allow it to run concurrently;
open the TestHookEventsHandlerAvailableOnUDSRouter function and insert a call to
t.Parallel() immediately after the function begins (before creating
homePaths/observer/engine) so this independent test can execute in parallel with
others.
In `@internal/config/agent.go`:
- Around line 204-222: The trimmed assignments after constructing the AgentDef
are redundant—remove the repeated strings.TrimSpace calls for agent.Name,
agent.Provider, agent.Command, agent.Model, agent.Permissions, and agent.Prompt
that occur immediately after the AgentDef literal; keep the initial trimming
done inside the AgentDef constructor (i.e., the trimmed values assigned when
creating agent := AgentDef{...}) and leave the subsequent logic that defaults
agent.Tools to []string{"*"} intact.
In `@internal/config/hooks_test.go`:
- Line 12: Several independent tests are missing t.Parallel(): add a call to
t.Parallel() at the start of each of the listed test functions
(TestLoadParsesConfigHookDeclarationWithAllFields,
TestLoadParsesMinimalConfigHookAndAppliesDefaults,
TestLoadRejectsInvalidConfigHookEvent, TestLoadRejectsRequiredAsyncConfigHook,
TestLoadMergesConfigHooksAcrossPrecedenceLevels,
TestHookDeclarationsReturnsCombinedConfigAndAgentHooks,
TestHookDeclarationsReturnsEmptySliceForEmptyHooksSection) so they run
concurrently; open each function and insert t.Parallel() as the first statement
inside the test body (before any shared setup) to enable parallel execution.
In `@internal/config/hooks.go`:
- Around line 58-68: Add a compile-time interface verification to ensure
hookValidationExecutor continues to satisfy the Executor interface: add a
package-level var assertion referencing hookValidationExecutor and the Executor
interface (e.g. var _ hooks.Executor = (*hookValidationExecutor)(nil)) so the
compiler will catch any interface drift; place this next to the
hookValidationExecutor type definition or other type assertions in the file.
- Around line 71-76: HookDeclarations currently preallocates capacity with
len(cfg.Hooks.Declarations)+len(agents) which underestimates when each AgentDef
may have multiple hooks; change the allocation to compute the exact needed
capacity by summing len(cfg.Hooks.Declarations) plus the sum of len(agent.Hooks)
for each agent in agents, then use that total to make the raw slice before
appending the cloned declarations (references: HookDeclarations,
cfg.Hooks.Declarations, agents, AgentDef, cloneHookDecls).
In `@internal/daemon/daemon_integration_test.go`:
- Around line 669-691: The helper assertLifecycleHookPayload should use t.Run
subtests to isolate failures for each field; update assertLifecycleHookPayload
to wrap each assertion (Event, WorkspaceID, Workspace/rootDir and the json
unmarshal/read error checks) in separate t.Run blocks named like "read file",
"unmarshal", "event", "workspace id", "workspace path" and use the payload
variable and wantEvent/wantWorkspace to drive the expectations so failing checks
report which assertion failed clearly.
In `@internal/daemon/hooks_bridge.go`:
- Around line 91-95: Add concise comments to the empty methods OnSessionCreated
and OnSessionStopped on hooksNotifier explaining they are intentional no-ops
because lifecycle observation is handled elsewhere via hook dispatch; update the
functions (OnSessionCreated and OnSessionStopped) to include a one-line comment
each stating "no-op: lifecycle observation handled via hook dispatch" (or
similar) so future maintainers understand the purpose and that leaving them
empty is deliberate.
In `@internal/hooks/events_test.go`:
- Around line 5-26: Replace the magic number 27 in TestAllHookEvents with a
named constant (e.g., ExpectedHookEventCount) and add a one-line comment
explaining whether the test is intended to assert an exact count (to catch
accidental additions/removals) or to be resilient; update the length assertion
to use ExpectedHookEventCount (or change the assertion to a >= check if
resilience is desired) so the intent is clear when inspecting TestAllHookEvents
and AllHookEvents.
In `@internal/hooks/events.go`:
- Around line 129-164: Add a compile/init-time consistency check that ensures
allHookEvents and hookEventSpecs contain the same HookEvent entries: iterate
allHookEvents and verify each key exists in hookEventSpecs, then iterate
hookEventSpecs keys and verify each appears in allHookEvents; if any mismatch is
found, fail fast (e.g., panic or log.Fatalf) during init() so the discrepancy is
caught at startup (reference allHookEvents, hookEventSpecs, and the
AllHookEvents function to locate the definitions).
In `@internal/hooks/executor_subprocess.go`:
- Line 138: Add a one-line inline justification comment next to the ignored
error returns from terminateSubprocessCommand(cmd) and
killSubprocessCommand(cmd) in executor_subprocess.go explaining that these
errors are intentionally not handled because cleanup is best-effort during
context cancellation and failure to terminate the subprocess does not change the
main control flow (optionally note you could log at debug level if desired);
place the comments immediately above or beside the calls to
terminateSubprocessCommand and killSubprocessCommand so future readers know the
ignores are deliberate.
In `@internal/hooks/executor_test.go`:
- Around line 124-143: The test TestSubprocessExecutorExecuteFiltersEnvironment
is missing t.Parallel(); add a call to t.Parallel() near the top of the test
(after the runtime.GOOS skip) so it runs concurrently like the other tests;
ensure you place it before t.Setenv(...) to allow the test's environment changes
(via t.Setenv) to be isolated and safe when run in parallel.
In `@internal/hooks/hooks_test.go`:
- Around line 191-278: The test TestHooksConcurrentRebuildAndDispatch lacks
t.Parallel(); add a call to t.Parallel() near the start of the test body
(immediately after the func(t *testing.T) { opens) so the test runs in parallel
with other tests; verify there are no globals or shared test-wide resources
being mutated (the test uses local variables like seq, hooks, wg, errCh) to
avoid interference when running in parallel.
In `@internal/hooks/hooks.go`:
- Around line 379-418: The current fingerprintHookSnapshot function uses
json.Marshal on the full fingerprints slice which can be costly; replace it with
a deterministic, compact hash-based fingerprint: iterate AllHookEvents() and for
each snapshotFingerprint and its resolvedHookFingerprint entries produce a
stable byte stream (e.g., write Event, Name, Source, Mode, Required, Priority,
Timeout, Matcher, sorted Metadata keys/values, ExecutorKind, Command, Args,
sorted Env keys/values, SkillSource) and feed that into a non-cryptographic
hasher (e.g., hash/fnv) to produce a single string fingerprint; ensure ordering
of map keys (Metadata, Env) and slice elements is deterministic and keep the
function name fingerprintHookSnapshot and input/output semantics the same
(returning the hash string and error) so callers (e.g., Rebuild logic) remain
unchanged.
In `@internal/hooks/matcher_test.go`:
- Around line 108-211: The current TestHookMatcherMatchesAdditionalFamilies
bundles many assertions; split it into focused test functions (e.g.,
TestHookMatcherMatchesInput/MatchesPrompt/MatchesAgentPreStart/MatchesAgentLifecycle/MatchesTurn,
TestHookMatcherMatchesEvent, TestHookMatcherMatchesTool,
TestHookMatcherMatchesPermissionResolution) that each create only the
HookMatcher and corresponding payload (reuse the
SessionContext/ToolCall/PermissionToolCall fixtures), call the single matcher
method under test (MatchesInput, MatchesPrompt, MatchesAgentPreStart,
MatchesAgentLifecycle, MatchesTurn, MatchesEvent, MatchesToolPostCall,
MatchesToolPostError, MatchesPermissionResolution) and assert the boolean with a
clear t.Fatal message; ensure each new test calls t.Parallel() and preserves the
same payload values and expectations from the original test.
In `@internal/hooks/ordering_test.go`:
- Around line 85-112: Add a failing-case to TestDefaultHookPriority that passes
an invalid HookSource (e.g., HookSource(-1) or a clearly out-of-range value) to
DefaultHookPriority and assert that it returns an error using a specific error
assertion (e.g., require.ErrorContains/require.ErrorAs or t.Fatalf with
strings.Contains). Reference the test function TestDefaultHookPriority and the
function DefaultHookPriority so you add a new subtest (t.Run("invalid"...) )
that checks got is ignored and the returned error contains the expected
message/marker for unknown sources.
🪄 Autofix (Beta)
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
Run ID: 158a8bf5-f723-4c36-b3ed-adcb2749e8f6
⛔ Files ignored due to path filters (89)
.codex/plans/2026-04-09-hooks-cli-endpoints.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/_meta.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/_tasks.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/_techspec.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/adrs/adr-001.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/adrs/adr-002.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/adrs/adr-003.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/adrs/adr-004.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/memory/MEMORY.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/memory/task_01.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/memory/task_02.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/memory/task_03.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/memory/task_04.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/memory/task_05.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/memory/task_06.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/memory/task_07.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/memory/task_08.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/task_01.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/task_02.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/task_03.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/task_04.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/task_05.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/task_06.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/task_07.mdis excluded by!**/*.md.compozy/tasks/_archived/20260409-183155-web-ui-redesign/task_08.mdis excluded by!**/*.md.compozy/tasks/extensability/_meta.mdis excluded by!**/*.md.compozy/tasks/hooks/_meta.mdis excluded by!**/*.md.compozy/tasks/hooks/_tasks.mdis excluded by!**/*.md.compozy/tasks/hooks/_techspec.mdis excluded by!**/*.md.compozy/tasks/hooks/adrs/adr-001.mdis excluded by!**/*.md.compozy/tasks/hooks/adrs/adr-002.mdis excluded by!**/*.md.compozy/tasks/hooks/adrs/adr-003.mdis excluded by!**/*.md.compozy/tasks/hooks/adrs/adr-004.mdis excluded by!**/*.md.compozy/tasks/hooks/adrs/adr-005.mdis excluded by!**/*.md.compozy/tasks/hooks/adrs/adr-006.mdis excluded by!**/*.md.compozy/tasks/hooks/adrs/adr-007.mdis excluded by!**/*.md.compozy/tasks/hooks/adrs/adr-008.mdis excluded by!**/*.md.compozy/tasks/hooks/adrs/adr-009.mdis excluded by!**/*.md.compozy/tasks/hooks/adrs/adr-010.mdis excluded by!**/*.md.compozy/tasks/hooks/adrs/adr-011.mdis excluded by!**/*.md.compozy/tasks/hooks/adrs/adr-012.mdis excluded by!**/*.md.compozy/tasks/hooks/adrs/adr-013.mdis excluded by!**/*.md.compozy/tasks/hooks/memory/MEMORY.mdis excluded by!**/*.md.compozy/tasks/hooks/memory/task_01.mdis excluded by!**/*.md.compozy/tasks/hooks/memory/task_02.mdis excluded by!**/*.md.compozy/tasks/hooks/memory/task_03.mdis excluded by!**/*.md.compozy/tasks/hooks/memory/task_04.mdis excluded by!**/*.md.compozy/tasks/hooks/memory/task_05.mdis excluded by!**/*.md.compozy/tasks/hooks/memory/task_06.mdis excluded by!**/*.md.compozy/tasks/hooks/memory/task_07.mdis excluded by!**/*.md.compozy/tasks/hooks/memory/task_08.mdis excluded by!**/*.md.compozy/tasks/hooks/memory/task_09.mdis excluded by!**/*.md.compozy/tasks/hooks/memory/task_10.mdis excluded by!**/*.md.compozy/tasks/hooks/memory/task_11.mdis excluded by!**/*.md.compozy/tasks/hooks/memory/task_12.mdis excluded by!**/*.md.compozy/tasks/hooks/task_01.mdis excluded by!**/*.md.compozy/tasks/hooks/task_02.mdis excluded by!**/*.md.compozy/tasks/hooks/task_03.mdis excluded by!**/*.md.compozy/tasks/hooks/task_04.mdis excluded by!**/*.md.compozy/tasks/hooks/task_05.mdis excluded by!**/*.md.compozy/tasks/hooks/task_06.mdis excluded by!**/*.md.compozy/tasks/hooks/task_07.mdis excluded by!**/*.md.compozy/tasks/hooks/task_08.mdis excluded by!**/*.md.compozy/tasks/hooks/task_09.mdis excluded by!**/*.md.compozy/tasks/hooks/task_10.mdis excluded by!**/*.md.compozy/tasks/hooks/task_11.mdis excluded by!**/*.md.compozy/tasks/hooks/task_12.mdis excluded by!**/*.md.compozy/tasks/session-resilience/_tasks.mdis excluded by!**/*.md.compozy/tasks/session-resilience/_techspec.mdis excluded by!**/*.md.compozy/tasks/session-resilience/adrs/adr-001.mdis excluded by!**/*.md.compozy/tasks/session-resilience/adrs/adr-002.mdis excluded by!**/*.md.compozy/tasks/session-resilience/adrs/adr-003.mdis excluded by!**/*.md.compozy/tasks/session-resilience/adrs/adr-004.mdis excluded by!**/*.md.compozy/tasks/session-resilience/adrs/adr-005.mdis excluded by!**/*.md.compozy/tasks/session-resilience/analysis_claude_code.mdis excluded by!**/*.md.compozy/tasks/session-resilience/analysis_goclaw.mdis excluded by!**/*.md.compozy/tasks/session-resilience/analysis_hermes.mdis excluded by!**/*.md.compozy/tasks/session-resilience/analysis_openclaw.mdis excluded by!**/*.md.compozy/tasks/session-resilience/analysis_openfang.mdis excluded by!**/*.md.compozy/tasks/session-resilience/analysis_pi_mono.mdis excluded by!**/*.md.compozy/tasks/session-resilience/task_01.mdis excluded by!**/*.md.compozy/tasks/session-resilience/task_02.mdis excluded by!**/*.md.compozy/tasks/session-resilience/task_03.mdis excluded by!**/*.md.compozy/tasks/session-resilience/task_04.mdis excluded by!**/*.mdAGENTS.mdis excluded by!**/*.mdinternal/skills/testdata/loader/combined/SKILL.mdis excluded by!**/*.mdinternal/skills/testdata/loader/hooks-only/SKILL.mdis excluded by!**/*.mdinternal/skills/testdata/loader/invalid-hook-command/SKILL.mdis excluded by!**/*.mdinternal/skills/testdata/loader/invalid-hook/SKILL.mdis excluded by!**/*.md
📒 Files selected for processing (98)
internal/api/contract/contract.gointernal/api/core/handlers.gointernal/api/core/interfaces.gointernal/api/core/parsers.gointernal/api/core/payloads.gointernal/api/httpapi/handlers_test.gointernal/api/httpapi/hooks_integration_test.gointernal/api/httpapi/hooks_test.gointernal/api/httpapi/httpapi_integration_test.gointernal/api/httpapi/server.gointernal/api/testutil/apitest.gointernal/api/udsapi/handlers_test.gointernal/api/udsapi/routes.gointernal/api/udsapi/udsapi_integration_test.gointernal/cli/cli_integration_test.gointernal/cli/client.gointernal/cli/client_test.gointernal/cli/helpers_test.gointernal/cli/hooks.gointernal/cli/hooks_test.gointernal/cli/root.gointernal/config/agent.gointernal/config/bootstrap.gointernal/config/config.gointernal/config/hooks.gointernal/config/hooks_test.gointernal/config/merge.gointernal/daemon/boot.gointernal/daemon/daemon.gointernal/daemon/daemon_integration_test.gointernal/daemon/daemon_test.gointernal/daemon/hooks_bridge.gointernal/daemon/notifier.gointernal/daemon/notifier_integration_test.gointernal/daemon/notifier_test.gointernal/hooks/agent_event.gointernal/hooks/depth.gointernal/hooks/dispatch.gointernal/hooks/dispatch_integration_test.gointernal/hooks/doc.gointernal/hooks/events.gointernal/hooks/events_test.gointernal/hooks/executor.gointernal/hooks/executor_native.gointernal/hooks/executor_subprocess.gointernal/hooks/executor_subprocess_unix.gointernal/hooks/executor_subprocess_unix_test.gointernal/hooks/executor_subprocess_windows.gointernal/hooks/executor_test.gointernal/hooks/executor_wasm_stub.gointernal/hooks/hooks.gointernal/hooks/hooks_test.gointernal/hooks/introspection.gointernal/hooks/introspection_test.gointernal/hooks/matcher.gointernal/hooks/matcher_test.gointernal/hooks/normalize.gointernal/hooks/normalize_test.gointernal/hooks/ordering.gointernal/hooks/ordering_test.gointernal/hooks/payloads.gointernal/hooks/payloads_test.gointernal/hooks/permission.gointernal/hooks/permission_test.gointernal/hooks/pipeline.gointernal/hooks/pipeline_test.gointernal/hooks/pool.gointernal/hooks/pool_test.gointernal/hooks/telemetry.gointernal/hooks/telemetry_test.gointernal/hooks/types.gointernal/hooks/types_test.gointernal/observe/hooks_test.gointernal/observe/observer.gointernal/observe/query.gointernal/session/interfaces.gointernal/session/manager.gointernal/session/manager_helpers.gointernal/session/manager_hooks.gointernal/session/manager_hooks_test.gointernal/session/manager_integration_test.gointernal/session/manager_lifecycle.gointernal/session/manager_prompt.gointernal/session/manager_test.gointernal/skills/hook_decl.gointernal/skills/hook_process_windows.gointernal/skills/hooks.gointernal/skills/hooks_test.gointernal/skills/loader.gointernal/skills/loader_test.gointernal/skills/registry.gointernal/skills/registry_test.gointernal/skills/types.gointernal/skills/watcher.gointernal/store/sessiondb/hook_runs_test.gointernal/store/sessiondb/session_db.gointernal/store/types.gointernal/workspace/clone.go
💤 Files with no reviewable changes (2)
- internal/daemon/notifier_integration_test.go
- internal/daemon/notifier.go
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
internal/config/config_test.go (1)
605-625: Addt.Parallel()for test isolation.The test doesn't modify shared state and can run in parallel with other tests.
♻️ Proposed fix
func TestValidateWrapsHooksConfigErrors(t *testing.T) { + t.Parallel() + homePaths, err := ResolveHomePathsFrom(filepath.Join(t.TempDir(), "home"))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/config/config_test.go` around lines 605 - 625, Test TestValidateWrapsHooksConfigErrors should call t.Parallel() to allow safe concurrent execution; add a single t.Parallel() invocation near the top of the TestValidateWrapsHooksConfigErrors function (immediately inside the function before resolving home paths) so the test runs in parallel with others without changing any other logic in cfg.Validate or the hook declaration setup.internal/hooks/events.go (1)
200-203: Redundant package-level function duplicates method behavior.
SyncEligible(event HookEvent) boolsimply delegates toevent.SyncEligible(). Consider removing the package-level function to reduce API surface, or document why both forms are needed.♻️ Proposed removal
-// SyncEligible reports whether the event accepts sync hooks. -func SyncEligible(event HookEvent) bool { - return event.SyncEligible() -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/hooks/events.go` around lines 200 - 203, Remove the redundant package-level function SyncEligible(event HookEvent) which only forwards to the HookEvent method; delete the function declaration and update all call sites to invoke the HookEvent method directly (i.e., replace SyncEligible(x) with x.SyncEligible()). Ensure imports/tests compile and run after removing SyncEligible and adjust any public API docs/comments that referenced the function.internal/hooks/executor_subprocess.go (1)
58-83: Add an explicitExecutorsatisfaction check.
SubprocessExecutoris a core executor implementation, so it should get the same compile-time interface guard already used elsewhere in this PR.Suggested fix
type SubprocessExecutor struct { command string args []string dir string env map[string]string } + +var _ Executor = (*SubprocessExecutor)(nil)As per coding guidelines,
**/*.goshould use compile-time interface verification:var _ Interface = (*Type)(nil).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/hooks/executor_subprocess.go` around lines 58 - 83, Add a compile-time interface satisfaction assertion for SubprocessExecutor by declaring the interface guard: add a top-level line like `var _ HookExecutor = (*SubprocessExecutor)(nil)` (using the actual interface name used in the codebase, e.g., HookExecutor or Executor) adjacent to the SubprocessExecutor type or immediately after its methods so the compiler verifies SubprocessExecutor implements the HookExecutor interface.internal/hooks/hooks_test.go (1)
328-415: Refactor this smoke test into table-driven subtests.This block repeats the same assertion pattern for every dispatch method, which makes failures harder to isolate and expands linearly every time a new event is added.
Refactor pattern
func TestDispatchMethodsSmokeNoHooks(t *testing.T) { t.Parallel() - - hooks := newTestHooks(t) - ctx := t.Context() - - if _, err := hooks.DispatchSessionPreCreate(ctx, SessionPreCreatePayload{PayloadBase: PayloadBase{Event: HookSessionPreCreate}}); err != nil { - t.Fatalf("DispatchSessionPreCreate() error = %v, want nil", err) - } - // ... + cases := []struct { + name string + run func(context.Context, *Hooks) error + }{ + { + name: "Should dispatch session.pre_create without hooks", + run: func(ctx context.Context, hooks *Hooks) error { + _, err := hooks.DispatchSessionPreCreate(ctx, SessionPreCreatePayload{ + PayloadBase: PayloadBase{Event: HookSessionPreCreate}, + }) + return err + }, + }, + // ... + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + hooks := newTestHooks(t) + if err := tc.run(t.Context(), hooks); err != nil { + t.Fatalf("%s: %v", tc.name, err) + } + }) + } }As per coding guidelines,
**/*_test.goshould use table-driven tests with subtests by default, and must use thet.Run("Should...")pattern for test cases.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/hooks/hooks_test.go` around lines 328 - 415, The TestDispatchMethodsSmokeNoHooks test repeats the same assertion for many Dispatch* methods; refactor it into a table-driven test that runs each case as a subtest with t.Run("Should ...") and t.Parallel where appropriate. Create a slice of cases each containing a name (e.g. "Should handle SessionPreCreate"), a function value that calls the corresponding dispatch (e.g. hooks.DispatchSessionPreCreate, hooks.DispatchSessionPostCreate, DispatchAgentPreStart, DispatchToolPostError, etc.), and the appropriate payload value (SessionPreCreatePayload, SessionPostCreatePayload, AgentPreStartPayload, ToolPostErrorPayload, PermissionRequestPayload, ContextPreCompactPayload, etc.); iterate cases and in each t.Run invoke the function with ctx and the payload, assert err == nil using t.Fatalf on failure, and keep TestDispatchMethodsSmokeNoHooks as the top-level test entry that delegates to these subtests. Ensure method references match existing symbols (DispatchSessionPreCreate, DispatchMessageDelta, DispatchContextPostCompact, etc.) so the compiler resolves the function values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/daemon/hooks_bridge.go`:
- Around line 396-403: The subprocess executor currently drops the workspace
scope so hooks run from the daemon cwd; update defaultDaemonExecutorResolver to
pass the workspace root attached to the HookDecl (the value set by
scopeWorkspaceHookDecls, e.g. decl.Matcher.WorkspaceRoot) into
NewSubprocessExecutor as the working-directory option (use the appropriate
option like hookspkg.WithSubprocessWorkdir or equivalent) so the subprocess runs
with the workspace as its cwd while preserving existing env options such as
hookspkg.WithSubprocessEnv(decl.Env).
In `@internal/hooks/dispatch.go`:
- Around line 485-494: The code currently always calls submitAsyncHooks even
when sync hooks failed or denied the event; update the call site so async hooks
are only submitted when there is no dispatch error and the sync report did not
deny the event. Specifically, after pipe.executeWithDisposition (and after
applying cfg.denyErr to set dispatchErr), wrap the submitAsyncHooks(h, ctx,
result, asyncHooks, pipe) call with a guard that checks dispatchErr == nil and
(report == nil || !report.Denied) and only invoke submitAsyncHooks when that
condition is true.
---
Nitpick comments:
In `@internal/config/config_test.go`:
- Around line 605-625: Test TestValidateWrapsHooksConfigErrors should call
t.Parallel() to allow safe concurrent execution; add a single t.Parallel()
invocation near the top of the TestValidateWrapsHooksConfigErrors function
(immediately inside the function before resolving home paths) so the test runs
in parallel with others without changing any other logic in cfg.Validate or the
hook declaration setup.
In `@internal/hooks/events.go`:
- Around line 200-203: Remove the redundant package-level function
SyncEligible(event HookEvent) which only forwards to the HookEvent method;
delete the function declaration and update all call sites to invoke the
HookEvent method directly (i.e., replace SyncEligible(x) with x.SyncEligible()).
Ensure imports/tests compile and run after removing SyncEligible and adjust any
public API docs/comments that referenced the function.
In `@internal/hooks/executor_subprocess.go`:
- Around line 58-83: Add a compile-time interface satisfaction assertion for
SubprocessExecutor by declaring the interface guard: add a top-level line like
`var _ HookExecutor = (*SubprocessExecutor)(nil)` (using the actual interface
name used in the codebase, e.g., HookExecutor or Executor) adjacent to the
SubprocessExecutor type or immediately after its methods so the compiler
verifies SubprocessExecutor implements the HookExecutor interface.
In `@internal/hooks/hooks_test.go`:
- Around line 328-415: The TestDispatchMethodsSmokeNoHooks test repeats the same
assertion for many Dispatch* methods; refactor it into a table-driven test that
runs each case as a subtest with t.Run("Should ...") and t.Parallel where
appropriate. Create a slice of cases each containing a name (e.g. "Should handle
SessionPreCreate"), a function value that calls the corresponding dispatch (e.g.
hooks.DispatchSessionPreCreate, hooks.DispatchSessionPostCreate,
DispatchAgentPreStart, DispatchToolPostError, etc.), and the appropriate payload
value (SessionPreCreatePayload, SessionPostCreatePayload, AgentPreStartPayload,
ToolPostErrorPayload, PermissionRequestPayload, ContextPreCompactPayload, etc.);
iterate cases and in each t.Run invoke the function with ctx and the payload,
assert err == nil using t.Fatalf on failure, and keep
TestDispatchMethodsSmokeNoHooks as the top-level test entry that delegates to
these subtests. Ensure method references match existing symbols
(DispatchSessionPreCreate, DispatchMessageDelta, DispatchContextPostCompact,
etc.) so the compiler resolves the function values.
🪄 Autofix (Beta)
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
Run ID: 387bd81c-3eef-40a8-a16d-9355ff65d27e
📒 Files selected for processing (25)
internal/api/core/parsers.gointernal/api/udsapi/handlers_test.gointernal/config/agent.gointernal/config/config.gointernal/config/config_test.gointernal/config/hooks.gointernal/daemon/daemon_integration_test.gointernal/daemon/hooks_bridge.gointernal/daemon/notifier_test.gointernal/hooks/dispatch.gointernal/hooks/events.gointernal/hooks/events_test.gointernal/hooks/executor.gointernal/hooks/executor_subprocess.gointernal/hooks/executor_subprocess_unix.gointernal/hooks/executor_wasm_stub.gointernal/hooks/hooks_test.gointernal/hooks/introspection.gointernal/hooks/introspection_test.gointernal/hooks/matcher.gointernal/hooks/matcher_test.gointernal/hooks/normalize_test.gointernal/hooks/ordering.gointernal/hooks/ordering_test.gointernal/hooks/types_test.go
✅ Files skipped from review due to trivial changes (2)
- internal/hooks/executor_wasm_stub.go
- internal/hooks/ordering_test.go
🚧 Files skipped from review as they are similar to previous changes (7)
- internal/hooks/events_test.go
- internal/hooks/ordering.go
- internal/hooks/executor_subprocess_unix.go
- internal/hooks/matcher_test.go
- internal/daemon/daemon_integration_test.go
- internal/hooks/introspection_test.go
- internal/hooks/matcher.go
This was referenced Apr 10, 2026
Merged
Merged
This was referenced Apr 17, 2026
Closed
Closed
This was referenced May 23, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Merged
pedronauck
added a commit
that referenced
this pull request
May 26, 2026
## Release v0.0.1 This PR prepares the release of version v0.0.1. ### Changelog ## 0.0.1 - 2026-05-26 ### Other Changes - Lessons learned ### ♻️ Refactoring - Project structure (#7) - Kb improvements (#12) - Rename spaces to channels (#17) - Add extensions gaps (#21) - Improve tool calls ui (#22) - Remove web app header - Module improvements (#29) - Memory improvements (#35) - Storybook for web and ui (#38) - Enable AGH network by default for new installs (#57) - Hermes adjustments (#69) - Badges design (#84) - Storybook scenario and logos gallery - Migrate typescript tests (#114) - Internal go packages (#120) - Ui patterns (#127) - Improve e2e tests (#130) - Ui redesign - Workspace isolation across runtime surfaces (#145) - Prod ready applies (#162) - Tool card ui (#164) - Alpha on logo - Prod ready features (#167) - Thread sheet (#202) ### 🎉 Features - Implement config foundation packages - Implement sqlite store package - Add ACP client package - Add session lifecycle manager - Implement observe package - Add daemon composition root - Add uds api server - Implement cli package - Add http api server - Add system design - Add foundation types, schemas, and layout shell for web client - Add daemon health polling and agent sidebar systems for web client - Add session system CRUD, streaming core, and session store for web client - Add chat view, messages, and composer tests for web client - Add tool cards and renderers for web client - Add file-backed memory store core - Scaffold memory session seams - Add memory dream consolidation service - Wire memory assembler into daemon - Add memory api and cli - New skills system (#1) - Add workspace entity (#5) - Add new skill capabilities (#8) - Web ui v2 (#9) - Improve hooks system (#10) - Session resilience (#11) - Add extensability (#13) - Add automation (#16) - Add channels (#14) - Add network implementation (#15) - Add network, bridges and automations web pages (#18) - Ext registry (#20) - Add core tasks (#19) - Bridge adapters (#23) - Add site (#26) - Add ext refac and sandbox (#25) - Settings ui (#37) - Tasks ui (#36) - Harness improvements (#44) - Agent capabilities (#49) - Redesign ui (#48) - Unify capability (#53) - Redesign network workspace (#59) - Add task deletion and split session delete from stop (#58) - Session provider selection (#60) - Production grade adjustments (#66) - Autonomous system (#75) - Add agent session route (#80) - Tools registry (#85) - Agents soul (#88) - Add network threads (#105) - Orchestration improvements (#106) - Memory v2 (#108) - Agent categories (#113) - Providers model (#118) - Add canonical AGH bundled skill (#143) - Onboarding and improvements (#198) - Onboarding and improvements (#201) ### 🐛 Bug Fixes - Review round - Review rounds - Resolve memory extensibility review batch - Embed web into daemon - Defaults agents - Acp integration (#4) - Lint errors - Prd folder - Remove orphan web actions and dead surfaces (#55) - Qa testing and fixes (#73) - New review rounds (#82) - Security audit (#90) - Release qa round (#95) - Add missing tools (#141) - New qa round (#147) - Advanced qa round (#149) - Homebrew tap - Final review round (#151) - Daemon healthy - Reasoning models (#158) - Lint errors (#160) - Review round (#168) - Release adjustments (#171) - Stabilize release ci fixtures - Stabilize release integration gate - Stabilize release verify gates - Stabilize release integration flows - Stabilize release verify gates - Stabilize main verify shutdown - Ignore stale acpmock cancel - Marketplace search focus and filtering (#193) - Website video - Workspace command select ### 📚 Documentation - Update agents.md - Update prd - Update skills - Update compozy tasks - Update compozy - Update compozy - Add new skills - Archive prd - Update prds - Update rfc - Update prds - Update prds - Add automation prd - Channels prd - Update prd - Update prd - New prds - Archive prds - Bridges adapters prd - Sandbox prd - Update - Archive prd - Update - Add new prd - New design - Update prd - Archive prds - Update prds - Tasks-ui prd tasks - Update prd - Update design docs - Agent capabilities prd - Improve site docs - Remove old design references - Udpate - Autonomous prd - Update skills - Blog design - Agent sould prd - Final qa plan - Update - Remove codex ledgers from gitignore - Remove not needed files - Udpate ledger - Update cy-codex-loop skill - Orchestration improves prd - Update prds - Orch improvs prd - Memv2 prd - Providers model prd - Update refacs prd - New design proposal - Update rules - Update skills - New blog posts (#173) - Format docs - Remove old design files - Remove old - Skeeper update ### 📦 Build System - Initial structure - Commitlint - Frontend base structure - Update vscode settings - Add subagents - Coderabbit - Prd and tooling - Bun lock - Lint tooling - Copy.md and tooling adjusts - Add repoclone rc - Upgrade skeeper to v0.2.0 - Update go.mod - Adopt task artifacts into skeeper - Sync codex plans with skeeper - Skeeper lock - Skeeper lock - New skills - Skeeper lock - Skeeper lock - Skeeper lock - Update deps and go - Regenerate daytona sidecar assets for go 1.26.3 - Fix cliff - Ignore docs on fmt - Build web assets before goreleaser - Extend release dry-run timeout ### 🔧 CI/CD - Lint errors - Fint release pr - Fix goreleaser ### 🧪 Testing - Add e2e tests (#27) - Qa rounds (#78) - Improve test suite (#138) - Harden daemon-served restart reloads - Harden daemon-served readiness waits - Stabilize dashboard focus assertion - Stabilize release integration gates - Stabilize release e2e markers - Stabilize release e2e flows Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This was referenced May 26, 2026
Closed
Merged
pedronauck
added a commit
that referenced
this pull request
May 26, 2026
## Release v0.0.1 This PR prepares the release of version v0.0.1. ### Changelog ## 0.0.1 - 2026-05-26 ### Other Changes - Lessons learned ### ♻️ Refactoring - Project structure (#7) - Kb improvements (#12) - Rename spaces to channels (#17) - Add extensions gaps (#21) - Improve tool calls ui (#22) - Remove web app header - Module improvements (#29) - Memory improvements (#35) - Storybook for web and ui (#38) - Enable AGH network by default for new installs (#57) - Hermes adjustments (#69) - Badges design (#84) - Storybook scenario and logos gallery - Migrate typescript tests (#114) - Internal go packages (#120) - Ui patterns (#127) - Improve e2e tests (#130) - Ui redesign - Workspace isolation across runtime surfaces (#145) - Prod ready applies (#162) - Tool card ui (#164) - Alpha on logo - Prod ready features (#167) - Thread sheet (#202) ### 🎉 Features - Implement config foundation packages - Implement sqlite store package - Add ACP client package - Add session lifecycle manager - Implement observe package - Add daemon composition root - Add uds api server - Implement cli package - Add http api server - Add system design - Add foundation types, schemas, and layout shell for web client - Add daemon health polling and agent sidebar systems for web client - Add session system CRUD, streaming core, and session store for web client - Add chat view, messages, and composer tests for web client - Add tool cards and renderers for web client - Add file-backed memory store core - Scaffold memory session seams - Add memory dream consolidation service - Wire memory assembler into daemon - Add memory api and cli - New skills system (#1) - Add workspace entity (#5) - Add new skill capabilities (#8) - Web ui v2 (#9) - Improve hooks system (#10) - Session resilience (#11) - Add extensability (#13) - Add automation (#16) - Add channels (#14) - Add network implementation (#15) - Add network, bridges and automations web pages (#18) - Ext registry (#20) - Add core tasks (#19) - Bridge adapters (#23) - Add site (#26) - Add ext refac and sandbox (#25) - Settings ui (#37) - Tasks ui (#36) - Harness improvements (#44) - Agent capabilities (#49) - Redesign ui (#48) - Unify capability (#53) - Redesign network workspace (#59) - Add task deletion and split session delete from stop (#58) - Session provider selection (#60) - Production grade adjustments (#66) - Autonomous system (#75) - Add agent session route (#80) - Tools registry (#85) - Agents soul (#88) - Add network threads (#105) - Orchestration improvements (#106) - Memory v2 (#108) - Agent categories (#113) - Providers model (#118) - Add canonical AGH bundled skill (#143) - Onboarding and improvements (#198) - Onboarding and improvements (#201) ### 🐛 Bug Fixes - Review round - Review rounds - Resolve memory extensibility review batch - Embed web into daemon - Defaults agents - Acp integration (#4) - Lint errors - Prd folder - Remove orphan web actions and dead surfaces (#55) - Qa testing and fixes (#73) - New review rounds (#82) - Security audit (#90) - Release qa round (#95) - Add missing tools (#141) - New qa round (#147) - Advanced qa round (#149) - Homebrew tap - Final review round (#151) - Daemon healthy - Reasoning models (#158) - Lint errors (#160) - Review round (#168) - Release adjustments (#171) - Stabilize release ci fixtures - Stabilize release integration gate - Stabilize release verify gates - Stabilize release integration flows - Stabilize release verify gates - Stabilize main verify shutdown - Ignore stale acpmock cancel - Marketplace search focus and filtering (#193) - Website video - Workspace command select ### 📚 Documentation - Update agents.md - Update prd - Update skills - Update compozy tasks - Update compozy - Update compozy - Add new skills - Archive prd - Update prds - Update rfc - Update prds - Update prds - Add automation prd - Channels prd - Update prd - Update prd - New prds - Archive prds - Bridges adapters prd - Sandbox prd - Update - Archive prd - Update - Add new prd - New design - Update prd - Archive prds - Update prds - Tasks-ui prd tasks - Update prd - Update design docs - Agent capabilities prd - Improve site docs - Remove old design references - Udpate - Autonomous prd - Update skills - Blog design - Agent sould prd - Final qa plan - Update - Remove codex ledgers from gitignore - Remove not needed files - Udpate ledger - Update cy-codex-loop skill - Orchestration improves prd - Update prds - Orch improvs prd - Memv2 prd - Providers model prd - Update refacs prd - New design proposal - Update rules - Update skills - New blog posts (#173) - Format docs - Remove old design files - Remove old - Skeeper update ### 📦 Build System - Initial structure - Commitlint - Frontend base structure - Update vscode settings - Add subagents - Coderabbit - Prd and tooling - Bun lock - Lint tooling - Copy.md and tooling adjusts - Add repoclone rc - Upgrade skeeper to v0.2.0 - Update go.mod - Adopt task artifacts into skeeper - Sync codex plans with skeeper - Skeeper lock - Skeeper lock - New skills - Skeeper lock - Skeeper lock - Skeeper lock - Update deps and go - Regenerate daytona sidecar assets for go 1.26.3 - Fix cliff - Ignore docs on fmt - Build web assets before goreleaser - Extend release dry-run timeout ### 🔧 CI/CD - Lint errors - Fint release pr - Fix goreleaser - Fix release ### 🧪 Testing - Add e2e tests (#27) - Qa rounds (#78) - Improve test suite (#138) - Harden daemon-served restart reloads - Harden daemon-served readiness waits - Stabilize dashboard focus assertion - Stabilize release integration gates - Stabilize release e2e markers - Stabilize release e2e flows Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Merged
pedronauck
added a commit
that referenced
this pull request
May 26, 2026
## Release v0.0.2 This PR prepares the release of version v0.0.2. ### Changelog ## 0.0.2 - 2026-05-26 ### Other Changes - Lessons learned ### ♻️ Refactoring - Project structure (#7) - Kb improvements (#12) - Rename spaces to channels (#17) - Add extensions gaps (#21) - Improve tool calls ui (#22) - Remove web app header - Module improvements (#29) - Memory improvements (#35) - Storybook for web and ui (#38) - Enable AGH network by default for new installs (#57) - Hermes adjustments (#69) - Badges design (#84) - Storybook scenario and logos gallery - Migrate typescript tests (#114) - Internal go packages (#120) - Ui patterns (#127) - Improve e2e tests (#130) - Ui redesign - Workspace isolation across runtime surfaces (#145) - Prod ready applies (#162) - Tool card ui (#164) - Alpha on logo - Prod ready features (#167) - Thread sheet (#202) ### 🎉 Features - Implement config foundation packages - Implement sqlite store package - Add ACP client package - Add session lifecycle manager - Implement observe package - Add daemon composition root - Add uds api server - Implement cli package - Add http api server - Add system design - Add foundation types, schemas, and layout shell for web client - Add daemon health polling and agent sidebar systems for web client - Add session system CRUD, streaming core, and session store for web client - Add chat view, messages, and composer tests for web client - Add tool cards and renderers for web client - Add file-backed memory store core - Scaffold memory session seams - Add memory dream consolidation service - Wire memory assembler into daemon - Add memory api and cli - New skills system (#1) - Add workspace entity (#5) - Add new skill capabilities (#8) - Web ui v2 (#9) - Improve hooks system (#10) - Session resilience (#11) - Add extensability (#13) - Add automation (#16) - Add channels (#14) - Add network implementation (#15) - Add network, bridges and automations web pages (#18) - Ext registry (#20) - Add core tasks (#19) - Bridge adapters (#23) - Add site (#26) - Add ext refac and sandbox (#25) - Settings ui (#37) - Tasks ui (#36) - Harness improvements (#44) - Agent capabilities (#49) - Redesign ui (#48) - Unify capability (#53) - Redesign network workspace (#59) - Add task deletion and split session delete from stop (#58) - Session provider selection (#60) - Production grade adjustments (#66) - Autonomous system (#75) - Add agent session route (#80) - Tools registry (#85) - Agents soul (#88) - Add network threads (#105) - Orchestration improvements (#106) - Memory v2 (#108) - Agent categories (#113) - Providers model (#118) - Add canonical AGH bundled skill (#143) - Onboarding and improvements (#198) - Onboarding and improvements (#201) ### 🐛 Bug Fixes - Review round - Review rounds - Resolve memory extensibility review batch - Embed web into daemon - Defaults agents - Acp integration (#4) - Lint errors - Prd folder - Remove orphan web actions and dead surfaces (#55) - Qa testing and fixes (#73) - New review rounds (#82) - Security audit (#90) - Release qa round (#95) - Add missing tools (#141) - New qa round (#147) - Advanced qa round (#149) - Homebrew tap - Final review round (#151) - Daemon healthy - Reasoning models (#158) - Lint errors (#160) - Review round (#168) - Release adjustments (#171) - Stabilize release ci fixtures - Stabilize release integration gate - Stabilize release verify gates - Stabilize release integration flows - Stabilize release verify gates - Stabilize main verify shutdown - Ignore stale acpmock cancel - Marketplace search focus and filtering (#193) - Website video - Workspace command select ### 📚 Documentation - Update agents.md - Update prd - Update skills - Update compozy tasks - Update compozy - Update compozy - Add new skills - Archive prd - Update prds - Update rfc - Update prds - Update prds - Add automation prd - Channels prd - Update prd - Update prd - New prds - Archive prds - Bridges adapters prd - Sandbox prd - Update - Archive prd - Update - Add new prd - New design - Update prd - Archive prds - Update prds - Tasks-ui prd tasks - Update prd - Update design docs - Agent capabilities prd - Improve site docs - Remove old design references - Udpate - Autonomous prd - Update skills - Blog design - Agent sould prd - Final qa plan - Update - Remove codex ledgers from gitignore - Remove not needed files - Udpate ledger - Update cy-codex-loop skill - Orchestration improves prd - Update prds - Orch improvs prd - Memv2 prd - Providers model prd - Update refacs prd - New design proposal - Update rules - Update skills - New blog posts (#173) - Format docs - Remove old design files - Remove old - Skeeper update ### 📦 Build System - Initial structure - Commitlint - Frontend base structure - Update vscode settings - Add subagents - Coderabbit - Prd and tooling - Bun lock - Lint tooling - Copy.md and tooling adjusts - Add repoclone rc - Upgrade skeeper to v0.2.0 - Update go.mod - Adopt task artifacts into skeeper - Sync codex plans with skeeper - Skeeper lock - Skeeper lock - New skills - Skeeper lock - Skeeper lock - Skeeper lock - Update deps and go - Regenerate daytona sidecar assets for go 1.26.3 - Fix cliff - Ignore docs on fmt - Build web assets before goreleaser - Extend release dry-run timeout ### 🔧 CI/CD - Lint errors - Fint release pr - Fix goreleaser - Fix release - Fix release process ### 🧪 Testing - Add e2e tests (#27) - Qa rounds (#78) - Improve test suite (#138) - Harden daemon-served restart reloads - Harden daemon-served readiness waits - Stabilize dashboard focus assertion - Stabilize release integration gates - Stabilize release e2e markers - Stabilize release e2e flows - Improve suite speed Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This was referenced May 26, 2026
Closed
Closed
Merged
pedronauck
added a commit
that referenced
this pull request
May 27, 2026
## Release v0.0.2 This PR prepares the release of version v0.0.2. ### Changelog ## 0.0.2 - 2026-05-26 ### Other Changes - Lessons learned ### ♻️ Refactoring - Project structure (#7) - Kb improvements (#12) - Rename spaces to channels (#17) - Add extensions gaps (#21) - Improve tool calls ui (#22) - Remove web app header - Module improvements (#29) - Memory improvements (#35) - Storybook for web and ui (#38) - Enable AGH network by default for new installs (#57) - Hermes adjustments (#69) - Badges design (#84) - Storybook scenario and logos gallery - Migrate typescript tests (#114) - Internal go packages (#120) - Ui patterns (#127) - Improve e2e tests (#130) - Ui redesign - Workspace isolation across runtime surfaces (#145) - Prod ready applies (#162) - Tool card ui (#164) - Alpha on logo - Prod ready features (#167) - Thread sheet (#202) ### 🎉 Features - Implement config foundation packages - Implement sqlite store package - Add ACP client package - Add session lifecycle manager - Implement observe package - Add daemon composition root - Add uds api server - Implement cli package - Add http api server - Add system design - Add foundation types, schemas, and layout shell for web client - Add daemon health polling and agent sidebar systems for web client - Add session system CRUD, streaming core, and session store for web client - Add chat view, messages, and composer tests for web client - Add tool cards and renderers for web client - Add file-backed memory store core - Scaffold memory session seams - Add memory dream consolidation service - Wire memory assembler into daemon - Add memory api and cli - New skills system (#1) - Add workspace entity (#5) - Add new skill capabilities (#8) - Web ui v2 (#9) - Improve hooks system (#10) - Session resilience (#11) - Add extensability (#13) - Add automation (#16) - Add channels (#14) - Add network implementation (#15) - Add network, bridges and automations web pages (#18) - Ext registry (#20) - Add core tasks (#19) - Bridge adapters (#23) - Add site (#26) - Add ext refac and sandbox (#25) - Settings ui (#37) - Tasks ui (#36) - Harness improvements (#44) - Agent capabilities (#49) - Redesign ui (#48) - Unify capability (#53) - Redesign network workspace (#59) - Add task deletion and split session delete from stop (#58) - Session provider selection (#60) - Production grade adjustments (#66) - Autonomous system (#75) - Add agent session route (#80) - Tools registry (#85) - Agents soul (#88) - Add network threads (#105) - Orchestration improvements (#106) - Memory v2 (#108) - Agent categories (#113) - Providers model (#118) - Add canonical AGH bundled skill (#143) - Onboarding and improvements (#198) - Onboarding and improvements (#201) ### 🐛 Bug Fixes - Review round - Review rounds - Resolve memory extensibility review batch - Embed web into daemon - Defaults agents - Acp integration (#4) - Lint errors - Prd folder - Remove orphan web actions and dead surfaces (#55) - Qa testing and fixes (#73) - New review rounds (#82) - Security audit (#90) - Release qa round (#95) - Add missing tools (#141) - New qa round (#147) - Advanced qa round (#149) - Homebrew tap - Final review round (#151) - Daemon healthy - Reasoning models (#158) - Lint errors (#160) - Review round (#168) - Release adjustments (#171) - Stabilize release ci fixtures - Stabilize release integration gate - Stabilize release verify gates - Stabilize release integration flows - Stabilize release verify gates - Stabilize main verify shutdown - Ignore stale acpmock cancel - Marketplace search focus and filtering (#193) - Website video - Workspace command select ### 📚 Documentation - Update agents.md - Update prd - Update skills - Update compozy tasks - Update compozy - Update compozy - Add new skills - Archive prd - Update prds - Update rfc - Update prds - Update prds - Add automation prd - Channels prd - Update prd - Update prd - New prds - Archive prds - Bridges adapters prd - Sandbox prd - Update - Archive prd - Update - Add new prd - New design - Update prd - Archive prds - Update prds - Tasks-ui prd tasks - Update prd - Update design docs - Agent capabilities prd - Improve site docs - Remove old design references - Udpate - Autonomous prd - Update skills - Blog design - Agent sould prd - Final qa plan - Update - Remove codex ledgers from gitignore - Remove not needed files - Udpate ledger - Update cy-codex-loop skill - Orchestration improves prd - Update prds - Orch improvs prd - Memv2 prd - Providers model prd - Update refacs prd - New design proposal - Update rules - Update skills - New blog posts (#173) - Format docs - Remove old design files - Remove old - Skeeper update ### 📦 Build System - Initial structure - Commitlint - Frontend base structure - Update vscode settings - Add subagents - Coderabbit - Prd and tooling - Bun lock - Lint tooling - Copy.md and tooling adjusts - Add repoclone rc - Upgrade skeeper to v0.2.0 - Update go.mod - Adopt task artifacts into skeeper - Sync codex plans with skeeper - Skeeper lock - Skeeper lock - New skills - Skeeper lock - Skeeper lock - Skeeper lock - Update deps and go - Regenerate daytona sidecar assets for go 1.26.3 - Fix cliff - Ignore docs on fmt - Build web assets before goreleaser - Extend release dry-run timeout ### 🔧 CI/CD - Lint errors - Fint release pr - Fix goreleaser - Fix release - Fix release process - Fix release sync - Decouple release dry-run npm auth - Persist web assets git auth ### 🧪 Testing - Add e2e tests (#27) - Qa rounds (#78) - Improve test suite (#138) - Harden daemon-served restart reloads - Harden daemon-served readiness waits - Stabilize dashboard focus assertion - Stabilize release integration gates - Stabilize release e2e markers - Stabilize release e2e flows - Improve suite speed <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated web assets dependency to a newer version for improved stability and performance. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/compozy/agh/pull/211?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
](https://app.coderabbit.ai/change-stack/compozy/agh/pull/211?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
This was referenced May 27, 2026
Closed
Closed
Merged
pedronauck
added a commit
that referenced
this pull request
May 27, 2026
## Release v0.0.2 This PR prepares the release of version v0.0.2. ### Changelog ## 0.0.2 - 2026-05-27 ### Other Changes - Lessons learned ### ♻️ Refactoring - Project structure (#7) - Kb improvements (#12) - Rename spaces to channels (#17) - Add extensions gaps (#21) - Improve tool calls ui (#22) - Remove web app header - Module improvements (#29) - Memory improvements (#35) - Storybook for web and ui (#38) - Enable AGH network by default for new installs (#57) - Hermes adjustments (#69) - Badges design (#84) - Storybook scenario and logos gallery - Migrate typescript tests (#114) - Internal go packages (#120) - Ui patterns (#127) - Improve e2e tests (#130) - Ui redesign - Workspace isolation across runtime surfaces (#145) - Prod ready applies (#162) - Tool card ui (#164) - Alpha on logo - Prod ready features (#167) - Thread sheet (#202) ### 🎉 Features - Implement config foundation packages - Implement sqlite store package - Add ACP client package - Add session lifecycle manager - Implement observe package - Add daemon composition root - Add uds api server - Implement cli package - Add http api server - Add system design - Add foundation types, schemas, and layout shell for web client - Add daemon health polling and agent sidebar systems for web client - Add session system CRUD, streaming core, and session store for web client - Add chat view, messages, and composer tests for web client - Add tool cards and renderers for web client - Add file-backed memory store core - Scaffold memory session seams - Add memory dream consolidation service - Wire memory assembler into daemon - Add memory api and cli - New skills system (#1) - Add workspace entity (#5) - Add new skill capabilities (#8) - Web ui v2 (#9) - Improve hooks system (#10) - Session resilience (#11) - Add extensability (#13) - Add automation (#16) - Add channels (#14) - Add network implementation (#15) - Add network, bridges and automations web pages (#18) - Ext registry (#20) - Add core tasks (#19) - Bridge adapters (#23) - Add site (#26) - Add ext refac and sandbox (#25) - Settings ui (#37) - Tasks ui (#36) - Harness improvements (#44) - Agent capabilities (#49) - Redesign ui (#48) - Unify capability (#53) - Redesign network workspace (#59) - Add task deletion and split session delete from stop (#58) - Session provider selection (#60) - Production grade adjustments (#66) - Autonomous system (#75) - Add agent session route (#80) - Tools registry (#85) - Agents soul (#88) - Add network threads (#105) - Orchestration improvements (#106) - Memory v2 (#108) - Agent categories (#113) - Providers model (#118) - Add canonical AGH bundled skill (#143) - Onboarding and improvements (#198) - Onboarding and improvements (#201) ### 🐛 Bug Fixes - Review round - Review rounds - Resolve memory extensibility review batch - Embed web into daemon - Defaults agents - Acp integration (#4) - Lint errors - Prd folder - Remove orphan web actions and dead surfaces (#55) - Qa testing and fixes (#73) - New review rounds (#82) - Security audit (#90) - Release qa round (#95) - Add missing tools (#141) - New qa round (#147) - Advanced qa round (#149) - Homebrew tap - Final review round (#151) - Daemon healthy - Reasoning models (#158) - Lint errors (#160) - Review round (#168) - Release adjustments (#171) - Stabilize release ci fixtures - Stabilize release integration gate - Stabilize release verify gates - Stabilize release integration flows - Stabilize release verify gates - Stabilize main verify shutdown - Ignore stale acpmock cancel - Marketplace search focus and filtering (#193) - Website video - Workspace command select ### 📚 Documentation - Update agents.md - Update prd - Update skills - Update compozy tasks - Update compozy - Update compozy - Add new skills - Archive prd - Update prds - Update rfc - Update prds - Update prds - Add automation prd - Channels prd - Update prd - Update prd - New prds - Archive prds - Bridges adapters prd - Sandbox prd - Update - Archive prd - Update - Add new prd - New design - Update prd - Archive prds - Update prds - Tasks-ui prd tasks - Update prd - Update design docs - Agent capabilities prd - Improve site docs - Remove old design references - Udpate - Autonomous prd - Update skills - Blog design - Agent sould prd - Final qa plan - Update - Remove codex ledgers from gitignore - Remove not needed files - Udpate ledger - Update cy-codex-loop skill - Orchestration improves prd - Update prds - Orch improvs prd - Memv2 prd - Providers model prd - Update refacs prd - New design proposal - Update rules - Update skills - New blog posts (#173) - Format docs - Remove old design files - Remove old - Skeeper update ### 📦 Build System - Initial structure - Commitlint - Frontend base structure - Update vscode settings - Add subagents - Coderabbit - Prd and tooling - Bun lock - Lint tooling - Copy.md and tooling adjusts - Add repoclone rc - Upgrade skeeper to v0.2.0 - Update go.mod - Adopt task artifacts into skeeper - Sync codex plans with skeeper - Skeeper lock - Skeeper lock - New skills - Skeeper lock - Skeeper lock - Skeeper lock - Update deps and go - Regenerate daytona sidecar assets for go 1.26.3 - Fix cliff - Ignore docs on fmt - Build web assets before goreleaser - Extend release dry-run timeout - Fix release dry-run token contract ### 🔧 CI/CD - Lint errors - Fint release pr - Fix goreleaser - Fix release - Fix release process - Fix release sync - Decouple release dry-run npm auth - Persist web assets git auth - Require npm auth before release merge ### 🧪 Testing - Add e2e tests (#27) - Qa rounds (#78) - Improve test suite (#138) - Harden daemon-served restart reloads - Harden daemon-served readiness waits - Stabilize dashboard focus assertion - Stabilize release integration gates - Stabilize release e2e markers - Stabilize release e2e flows - Improve suite speed <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated dependencies to latest versions. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/compozy/agh/pull/214?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
](https://app.coderabbit.ai/change-stack/compozy/agh/pull/214?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
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
1 participant
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.
Summary by CodeRabbit
New Features
hookscommand withlist,info,events,runs(human/json/toon output).Bug Fixes / Behavior
Tests