feat: harness improvements#44
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 ignored due to path filters (12)
📒 Files selected for processing (3)
WalkthroughAdds a harness subsystem: typed harness context resolution, descriptor-driven startup prompt sections, composite prompt-input augmenters, detached harness task submission with synthetic reentry orchestration, lifecycle observability, synthetic prompt queuing/persistence, task-run metadata persistence and DB migration, plus extensive unit/integration tests and wiring. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SessionManager
participant HarnessResolver
participant SectionSelector
participant PromptAssembler
participant Augmenter
participant Driver
participant Store
Client->>SessionManager: Create session / Start
SessionManager->>HarnessResolver: ResolveStartup(startupCtx)
HarnessResolver-->>SessionManager: ResolvedHarnessPolicy
SessionManager->>SectionSelector: Select(startupCtx, descriptors)
SectionSelector-->>SessionManager: selected descriptors
SessionManager->>PromptAssembler: AssembleStartup(startupCtx, agent, workspace)
PromptAssembler-->>SessionManager: assembled prompt
SessionManager->>Augmenter: Augment(prompt, policy)
Augmenter-->>SessionManager: augmented prompt
SessionManager->>Driver: Start(systemPrompt)
Driver-->>SessionManager: ready
SessionManager->>Store: Persist startup context
sequenceDiagram
participant TaskManager
participant DetachedBridge
participant Store
participant ReentryBridge
participant Sessions
participant HarnessResolver
TaskManager->>DetachedBridge: submitDetachedHarnessWork(req)
DetachedBridge->>Store: Lookup by idempotency / ensure task
Store-->>DetachedBridge: existing? / created
DetachedBridge->>Store: ReserveQueuedRun(..., metadata)
Store-->>DetachedBridge: reserved run
TaskManager->>Store: RecordTaskEvent(terminal)
Store->>ReentryBridge: Notify/OnTaskEvent(record)
ReentryBridge->>Store: load run metadata
ReentryBridge->>Sessions: Resolve target session state
ReentryBridge->>HarnessResolver: ResolvePrompt(... synthetic)
alt policy allows synthetic
ReentryBridge->>Sessions: PromptSynthetic(target, syntheticMeta)
Sessions-->>ReentryBridge: agent events / result
ReentryBridge->>Store: Write synthetic reentry EventSummary
else policy silent or invalid
ReentryBridge->>Store: Write dropped/silent EventSummary
end
Note: rectangles/colors omitted; components represent distinct actors. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
internal/session/stop_reason.go (1)
184-216:⚠️ Potential issue | 🟠 MajorKeep the observed-exit handle aligned with the returned state.
Line 184 snapshots
observedProcessExitfrom the original handle, but Lines 215-216 replaceprocessbefore returning. If that observed handle is cleared duringprepareStoporwaitForPromptSetup, callers take theproc == nilbranch and skipproc.Wait()/reconcileObservedTerminalStop, so the session can be finalized without the actual crash/completion result.Possible fix
- process = session.processHandle() - return session, process, false, stopWasAlreadyRequested, observedProcessExit, nil + currentProcess := session.processHandle() + if observedProcessExit && currentProcess == nil { + currentProcess = process + } + return session, currentProcess, false, stopWasAlreadyRequested, observedProcessExit, nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/session/stop_reason.go` around lines 184 - 216, The snapshot of observedProcessExit is taken too early (observedProcessExit := isProcessDone(process)) but process may change during prepareStop/waitForPromptSetup; recompute the observed exit state right before returning using the currently assigned process (e.g., call isProcessDone(session.processHandle()) or isProcessDone(process) immediately after process = session.processHandle()) so the returned observedProcessExit matches the handle callers receive and avoids skipping proc.Wait()/reconcileObservedTerminalStop.internal/daemon/boot.go (1)
258-314:⚠️ Potential issue | 🟠 Major
StartupPromptOverlayis wired but never initialized.
bootPromptProviderssets up the assembler and augmenter, but it never assignsstate.startupOverlay.sessionManagerDepstherefore passesnilforStartupPromptOverlayon every boot, so the new startup-context overlay path never runs.Also applies to: 472-495
internal/daemon/task_runtime.go (1)
226-287:⚠️ Potential issue | 🟠 MajorRelease the reentry bridge on bootstrap failures.
Once
newHarnessReentryBridge(...)succeeds, every laterreturn errinbootTasksleaks that bridge becauseshutdown()is only wired into the happy-path runtime. If the bridge starts background work, a startup failure leaves orphaned goroutines and partially initialized observers behind. Defer cleanup untilreentry.recover(ctx)has succeeded, and only then publishstate.tasks/state.deps.Tasks.🛠️ Suggested fix
reentry, err := newHarnessReentryBridge( ctx, state.harnessResolver, @@ if err != nil { return fmt.Errorf("daemon: create harness reentry bridge: %w", err) } + cleanupReentry := true + defer func() { + if cleanupReentry { + reentry.shutdown() + } + }() manager, err := taskpkg.NewManager( taskpkg.WithStore(store), @@ if err != nil { return fmt.Errorf("daemon: create detached harness bridge: %w", err) } - - state.tasks = &taskRuntime{ - manager: manager, - store: store, - detached: detached, - reentry: reentry, - } - state.deps.Tasks = manager actor, err := taskpkg.DeriveDaemonActorContext("boot-recovery", "daemon.boot") if err != nil { @@ if err := reentry.recover(ctx); err != nil { return fmt.Errorf("daemon: recover detached harness reentry bridge: %w", err) } + cleanupReentry = false + state.tasks = &taskRuntime{ + manager: manager, + store: store, + detached: detached, + reentry: reentry, + } + state.deps.Tasks = manager return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/daemon/task_runtime.go` around lines 226 - 287, After newHarnessReentryBridge(...) returns successfully, install a deferred cleanup that will call reentry.shutdown() (or the reentry bridge's shutdown/close method) to ensure the bridge is torn down on any early return; then execute reentry.recover(ctx) and only if it succeeds cancel the deferred cleanup (or mark completion) and then publish state.tasks and state.deps.Tasks. In short: add a defer immediately after newHarnessReentryBridge to shut down the reentry bridge on error, call reentry.recover(ctx), and only when recover returns nil cancel the defer and set state.tasks/state.deps.Tasks so failed boot paths don't leak the bridge.
🟡 Minor comments (9)
internal/session/stop_reason.go-194-195 (1)
194-195:⚠️ Potential issue | 🟡 MinorWrap stop-preparation failures with stage context.
These raw
return errpaths make it hard to distinguish whether stop setup failed in pre-stop hooks, metadata persistence, or prompt setup synchronization.Possible fix
if session.Info().State == StateActive && !observedProcessExit { if err := m.dispatchSessionPreStop(ctx, session); err != nil { - return nil, nil, false, false, false, err + return nil, nil, false, false, false, fmt.Errorf("session: dispatch pre-stop for %q: %w", id, err) } } writeMeta, promptSetupDone, err := session.prepareStop(m.now(), appliedCause, appliedDetail) if err != nil { - return nil, nil, false, false, false, err + return nil, nil, false, false, false, fmt.Errorf("session: prepare stop for %q: %w", id, err) } if writeMeta { if err := m.writeMeta(session); err != nil { - return nil, nil, false, false, false, err + return nil, nil, false, false, false, fmt.Errorf("session: persist stop metadata for %q: %w", id, err) } } if err := waitForPromptSetup(ctx, session, promptSetupDone); err != nil { - return nil, nil, false, false, false, err + return nil, nil, false, false, false, fmt.Errorf("session: wait for prompt setup before stopping %q: %w", id, err) }As per coding guidelines, "Use explicit error returns with wrapped context:
fmt.Errorf("context: %w", err)".Also applies to: 199-201, 204-205, 208-209
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/session/stop_reason.go` around lines 194 - 195, The pre-stop stage returns raw errors that lack stage context; wrap all errors returned from the stop-preparation calls (e.g. m.dispatchSessionPreStop(ctx, session)) with descriptive context using fmt.Errorf so callers can distinguish failures in pre-stop hooks, metadata persistence and prompt-sync steps (for example: fmt.Errorf("dispatchSessionPreStop: %w", err)). Update each raw return in this block (including the similar paths at the other noted calls) to return the wrapped error instead of the raw err.internal/api/httpapi/stream_helpers_test.go-187-229 (1)
187-229:⚠️ Potential issue | 🟡 MinorAssert that
session_idis forwarded into the observer query.This test is meant to cover
/api/observe/events/stream?session_id=sess-harness, but the fake drops thestore.EventSummaryQueryargument entirely. If the handler stopped applying that filter, this test would still pass because the stub always returns the same event. Capture the query and assert its session filter before returning the summary.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/httpapi/stream_helpers_test.go` around lines 187 - 229, The test's fake QueryEventsFn currently ignores its store.EventSummaryQuery argument so it won't catch if the handler stops forwarding session filters; update the QueryEventsFn implementation in stream_helpers_test.go to capture the incoming store.EventSummaryQuery param and assert that its SessionID (e.g. q.SessionID) equals "sess-harness" (use t.Fatalf or t.Errorf on mismatch) before returning the EventSummary; this ensures the handler actually forwards the session_id into the observer query.internal/task/manager_test.go-3068-3126 (1)
3068-3126:⚠️ Potential issue | 🟡 MinorCover conflicting metadata on duplicate enqueue.
This only locks down the happy path where the duplicate request sends the same metadata. The risky case for detached runs is reusing the same idempotency key with different metadata; without that assertion, this test will still pass if the enqueue path silently preserves the wrong wake target/payload. As per coding guidelines, focus on critical paths and ensure tests verify behavior outcomes, not just function calls.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/task/manager_test.go` around lines 3068 - 3126, The test TestManagerEnqueueRunPreservesMetadataAcrossIdempotentDuplicates currently only retries with identical metadata; add a duplicate EnqueueRun call that uses the same IdempotencyKey ("detached-metadata-1") but a different Metadata payload and assert that manager.EnqueueRun returns the original run ID and that returned run.Metadata and the stored run metadata remain equal to the first metadata (i.e., the duplicate does NOT overwrite), and that len(store.runs) is still 1; locate the EnqueueRun calls in this test and extend with the conflicting-metadata case to validate behavior.internal/observe/observer_test.go-283-339 (1)
283-339:⚠️ Potential issue | 🟡 MinorThis bypasses the recorder path the PR is adding.
Writing summaries straight into
registryonly provesQueryEventscan read stored rows. It will still pass ifharnessLifecycleRecorderemits the wrong type/summary or never writes at all, so it doesn’t really protect the new observability integration. As per coding guidelines, focus on critical paths and ensure tests verify behavior outcomes, not just function calls.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/observe/observer_test.go` around lines 283 - 339, The test currently writes summaries directly via h.observer.registry.WriteEventSummary which bypasses the new harnessLifecycleRecorder path; change the test to emit the same three events through the recorder API instead (use the harness lifecycle recorder on the observer—e.g. the harnessLifecycleRecorder or recorder methods on h.observer that the PR added) after calling h.observer.OnSessionCreated, so the recorder performs the write to the registry and QueryEvents is asserted against those recorder-emitted events (keep the same session, timestamps, Type and Summary values and the same assertions).internal/api/httpapi/httpapi_integration_test.go-201-215 (1)
201-215:⚠️ Potential issue | 🟡 MinorBound prompt collection to a test-scoped timeout.
These prompt submissions use
context.Background(), andcollectIntegrationPromptEventsblocks on a plain channel range. If one prompt stops emitting without closing, this test hangs until the package timeout instead of failing at the call site. Use a cancellable test context and abort the drain when it expires.⏱️ Suggested hardening
-func TestHTTPSessionTranscriptEndpointIncludesSyntheticTurns(t *testing.T) { +func TestHTTPSessionTranscriptEndpointIncludesSyntheticTurns(t *testing.T) { runtime := newIntegrationRuntime(t) sessionID := createIntegrationSession(t, runtime) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() - userEvents, userErr := runtime.manager.Prompt(context.Background(), sessionID, "hello") - collectIntegrationPromptEvents(t, mustIntegrationPrompt(t, userEvents, userErr)) + userEvents, userErr := runtime.manager.Prompt(ctx, sessionID, "hello") + collectIntegrationPromptEvents(t, ctx, mustIntegrationPrompt(t, userEvents, userErr)) - networkEvents, networkErr := runtime.manager.PromptNetwork(context.Background(), sessionID, "network hello") - collectIntegrationPromptEvents(t, mustIntegrationPrompt(t, networkEvents, networkErr)) + networkEvents, networkErr := runtime.manager.PromptNetwork(ctx, sessionID, "network hello") + collectIntegrationPromptEvents(t, ctx, mustIntegrationPrompt(t, networkEvents, networkErr)) - syntheticEvents, syntheticErr := runtime.manager.PromptSynthetic(context.Background(), sessionID, session.SyntheticPromptOpts{ + syntheticEvents, syntheticErr := runtime.manager.PromptSynthetic(ctx, sessionID, session.SyntheticPromptOpts{ Message: "daemon wake-up", Metadata: acp.PromptSyntheticMeta{ TaskRunID: "run-1", @@ - collectIntegrationPromptEvents(t, mustIntegrationPrompt(t, syntheticEvents, syntheticErr)) + collectIntegrationPromptEvents(t, ctx, mustIntegrationPrompt(t, syntheticEvents, syntheticErr)) } -func collectIntegrationPromptEvents(t *testing.T, events <-chan acp.AgentEvent) []acp.AgentEvent { +func collectIntegrationPromptEvents(t *testing.T, ctx context.Context, events <-chan acp.AgentEvent) []acp.AgentEvent { t.Helper() collected := make([]acp.AgentEvent, 0, 4) - for event := range events { - collected = append(collected, event) + for { + select { + case event, ok := <-events: + if !ok { + goto done + } + collected = append(collected, event) + case <-ctx.Done(): + t.Fatalf("prompt events did not complete: %v", ctx.Err()) + } } +done: if len(collected) == 0 { t.Fatal("prompt events = 0, want completed prompt stream") }Also applies to: 2424-2438
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/httpapi/httpapi_integration_test.go` around lines 201 - 215, Replace uses of context.Background() passed to runtime.manager.Prompt, PromptNetwork, and PromptSynthetic with a test-scoped cancellable context (e.g., ctx, cancel := context.WithTimeout(testCtx, ...) / defer cancel) so the prompt calls and the subsequent collectIntegrationPromptEvents drain will abort when the test deadline is reached; update calls to collectIntegrationPromptEvents/mustIntegrationPrompt to accept and respect the ctx expiration (stop draining the channel and return an error when ctx.Done() fires) to avoid hanging tests if a prompt never closes.internal/session/manager_integration_test.go-198-211 (1)
198-211:⚠️ Potential issue | 🟡 MinorFinish the blocked fake prompt with a terminal
doneevent.This stub closes the ACP stream for
turn-1without ever emittingacp.EventTypeDone. That means the test is proving that EOF unblocks the queued synthetic turn, not that the real turn-completion path does. Sending a finaldoneevent here makes the ordering assertion match the production contract more closely.🧪 Suggested fix
if req.TurnID == "turn-1" { close(firstPromptEntered) - events := make(chan acp.AgentEvent) + events := make(chan acp.AgentEvent, 1) go func() { <-releaseFirstPrompt + events <- acp.AgentEvent{ + Type: acp.EventTypeDone, + TurnID: req.TurnID, + Timestamp: time.Now().UTC(), + StopReason: "end_turn", + } close(events) }() return events, nil }Based on learnings: Check dependent package APIs before writing integration code or tests.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/session/manager_integration_test.go` around lines 198 - 211, The promptHook stub for fakeProcess closes the events channel for req.TurnID == "turn-1" without emitting a terminal acp.EventTypeDone, so change the goroutine created for turn-1 (inside promptHook) to, after receiving <-releaseFirstPrompt, send a final acp.AgentEvent with Type == acp.EventTypeDone into the events channel before closing it; locate the promptHook function, the branch checking req.TurnID == "turn-1", and modify the anonymous goroutine to emit the terminal done event (using acp.AgentEvent and acp.EventTypeDone) then close(events) so the test simulates real turn completion.internal/daemon/daemon_test.go-3634-3660 (1)
3634-3660:⚠️ Potential issue | 🟡 MinorReuse one turn ID/timestamp for the synthetic event record.
recordSyntheticEventwrites differentTurnID/Timestampvalues into the JSON payload and the outerstore.SessionEvent. Any test that decodesContentcan now observe an impossible event shape for a single row, which makes the fake less trustworthy for transcript/harness assertions.Suggested fix
func (f *fakeSessionManager) recordSyntheticEvent( sessionID string, info *session.Info, opts session.SyntheticPromptOpts, ) { if info == nil { return } + + turnID := fmt.Sprintf("turn-synthetic-%d", time.Now().UnixNano()) + timestamp := time.Now().UTC() payload, err := json.Marshal(acp.AgentEvent{ Type: acp.EventTypeSyntheticReentry, - TurnID: fmt.Sprintf("turn-synthetic-%d", time.Now().UnixNano()), - Timestamp: time.Now().UTC(), + TurnID: turnID, + Timestamp: timestamp, Text: strings.TrimSpace(opts.Message), Synthetic: &opts.Metadata, }) if err != nil { return @@ f.nextEventSequence++ f.sessionEvents[sessionID] = append(f.sessionEvents[sessionID], store.SessionEvent{ ID: fmt.Sprintf("evt-%d", f.nextEventSequence), SessionID: sessionID, Sequence: f.nextEventSequence, - TurnID: fmt.Sprintf("turn-synthetic-%d", f.nextEventSequence), + TurnID: turnID, Type: acp.EventTypeSyntheticReentry, AgentName: info.AgentName, Content: string(payload), - Timestamp: time.Now().UTC(), + Timestamp: timestamp, }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/daemon/daemon_test.go` around lines 3634 - 3660, The synthetic event currently uses different TurnID/Timestamp values in the JSON payload and the outer store.SessionEvent; fix by computing a single turnID and a single timestamp once (e.g., turnID := fmt.Sprintf("turn-synthetic-%d", ... ) and ts := time.Now().UTC() or based on f.nextEventSequence) before marshaling acp.AgentEvent, then reuse that same turnID and ts for both the acp.AgentEvent fields (TurnID, Timestamp) and the store.SessionEvent fields (TurnID, Timestamp) when appending to f.sessionEvents; update the code paths around json.Marshal(acp.AgentEvent{...}) and the store.SessionEvent construction so both use the same precomputed values (referencing acp.AgentEvent, store.SessionEvent, f.nextEventSequence, opts.Metadata, info.AgentName).internal/extension/host_api_test.go-1792-1810 (1)
1792-1810:⚠️ Potential issue | 🟡 MinorAssert the fallback text as well.
This test only checks the envelope fields. A regression that stops populating
projected.Textfrom storedContentwould still pass.Suggested assertion
if got := projected.Timestamp; !got.Equal(now) { t.Fatalf("projected.Timestamp = %s, want %s", got, now) } + if got, want := projected.Text, "daemon wake-up"; got != want { + t.Fatalf("projected.Text = %q, want %q", got, want) + }As per coding guidelines, "Ensure tests verify behavior outcomes, not just function calls".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/extension/host_api_test.go` around lines 1792 - 1810, The test for promptProjectionEventFromStoredEvent currently only asserts envelope fields; add an assertion that projected.Text equals the text pulled from the stored Content JSON (e.g., "daemon wake-up") so regressions that stop populating projected.Text fail; locate the promptProjectionEventFromStoredEvent call (using store.SessionEvent with Content `{"schema":"agh.session.event.v1","text":"daemon wake-up"}`) and add a check like assert projected.Text == "daemon wake-up" (or use t.Fatalf on mismatch) after the existing Timestamp assertion.internal/daemon/harness_context.go-396-427 (1)
396-427:⚠️ Potential issue | 🟡 MinorUse
acp.PromptTurnSourceSyntheticconstant for consistency.Line 420 should set
meta.TurnSourcetoacp.PromptTurnSourceSyntheticinstead ofstring(TurnOriginSynthetic)to match the pattern established in lines 367-369, which correctly use theacpconstants for user and network sources.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/daemon/harness_context.go` around lines 396 - 427, The meta.TurnSource assignment in normalizeSyntheticHarnessTurnContext is using string(TurnOriginSynthetic) instead of the acp constant; update the assignment so meta.TurnSource = acp.PromptTurnSourceSynthetic to match the pattern used for user/network sources (see usage of acp constants in other branches) and keep the function HarnessContextResolver.normalizeSyntheticHarnessTurnContext, the returned HarnessTurnContext, and TurnOriginSynthetic semantics unchanged.
🧹 Nitpick comments (2)
internal/daemon/prompt_input_composite_integration_test.go (1)
146-150: CloneEnableAugmentersbefore appending.Appending directly to the slice returned by
r.base.ResolvePromptcan mutate shared backing storage if that policy is reused, which makes this helper stateful across calls.Suggested fix
additional := r.extra[resolved.Policy.TurnOrigin] if len(additional) == 0 { return resolved, nil } - resolved.Policy.EnableAugmenters = append(resolved.Policy.EnableAugmenters, additional...) + resolved.Policy.EnableAugmenters = append( + append([]HarnessAugmenter(nil), resolved.Policy.EnableAugmenters...), + additional..., + ) return resolved, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/daemon/prompt_input_composite_integration_test.go` around lines 146 - 150, The test mutates the slice returned by r.base.ResolvePrompt by appending directly to resolved.Policy.EnableAugmenters; to fix, make a copy of that slice before appending (e.g., allocate a new slice and copy existing elements or use append on a nil slice) so you do not alter the original backing array from r.base.ResolvePrompt; update the code around resolved.Policy.EnableAugmenters and where additional := r.extra[resolved.Policy.TurnOrigin] is used to append to the cloned slice rather than the original.internal/daemon/task_runtime_test.go (1)
1889-1899: Replace the sleep-poll helper with synchronization.This helper makes the detached runtime tests timing-sensitive under load and under
-race. Prefer waiting on an explicit signal or a ticker/context-based helper instead of a fixedtime.Sleeploop.As per coding guidelines, "Never use time.Sleep() in orchestration — use proper synchronization primitives".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/daemon/task_runtime_test.go` around lines 1889 - 1899, The helper waitForTaskRuntimeCondition uses a tight time.Sleep polling loop which makes tests timing-sensitive; replace it with proper synchronization by accepting a context or a channel and using select with a time.After deadline or time.Ticker to wait for the check to become true or for cancellation/timeout. Modify waitForTaskRuntimeCondition (or add an overload) to take a context.Context (or a done/notify chan struct{}) and use select to either evaluate check() when the ticker fires or return/t.Fatal when the context is done/timeout, avoiding direct time.Sleep and making tests deterministic under -race.
🤖 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/acp/types_test.go`:
- Around line 139-229: Rename or wrap each top-level test into a t.Run subtest
using the "Should ..." pattern and keep t.Parallel() inside the subtest: for
TestPromptMetaValidateSyntheticRequiresWakeupReason, wrap the existing
assertions in t.Run("Should require a wakeup reason for synthetic turns", func(t
*testing.T){ t.Parallel(); ... }); for
TestPromptMetaValidateRejectsSyntheticFieldsOnUserAndNetworkTurns, convert the
table loop to call t.Run("Should reject synthetic fields on <case>", func(t
*testing.T){ t.Parallel(); ... }) for each case (keeping the existing tc.name
for clarity); and for TestPromptSyntheticMetaNormalizeAndValidate wrap the
validation steps in t.Run("Should normalize and validate synthetic meta", func(t
*testing.T){ t.Parallel(); ... }); ensure test names match the "Should..."
convention and keep all existing assertions and calls to PromptMeta.Validate,
PromptSyntheticMeta.Normalize, IsZero, and Validate unchanged.
In `@internal/api/contract/tasks.go`:
- Line 108: taskRunPayloadFromRun in internal/extension/host_api_tasks.go fails
to propagate run.Metadata into the shared TaskRunPayload.Metadata (defined as
json.RawMessage in internal/api/contract/tasks.go), causing metadata to be
present for HTTP callers but lost for extensions; update taskRunPayloadFromRun
to set TaskRunPayload.Metadata = run.Metadata (or a safe copy/clone if
necessary) when constructing the payload so the Metadata field is consistently
exposed across transports.
In `@internal/api/udsapi/stream_helpers_test.go`:
- Around line 177-179: The test currently calls close(done) unconditionally
inside the QueryEventsFn callback which races if the callback is invoked
multiple times; fix this by guarding the close with a sync.Once (e.g., declare a
doneOnce sync.Once in the test setup) and replace the direct close(done) in
QueryEventsFn with doneOnce.Do(func(){ close(done) }), ensuring only the first
invocation closes the channel and prevents "close of closed channel" panics.
In `@internal/api/udsapi/transport_parity_integration_test.go`:
- Around line 216-323: The test function
TestUDSTransportObserveHarnessLifecycleParityMatchesHTTP does not follow the
required subtest naming pattern; wrap the existing test body inside a
t.Run("Should ...") subtest (e.g., t.Run("Should match observe harness lifecycle
parity between UDS and HTTP", func(t *testing.T) { ... })) and move all current
logic into that closure, keeping the function name as the top-level test to
satisfy the test harness while ensuring the subtest uses the "Should..."
pattern; update any t references inside to use the closure parameter (func(t
*testing.T)).
In `@internal/api/udsapi/udsapi_integration_test.go`:
- Around line 116-130: Replace uses of context.Background() when calling
runtime.manager.Prompt, PromptNetwork and PromptSynthetic with a cancellable
timeout context (e.g., use context.WithTimeout) so each prompt call times out
instead of hanging; update the invocations that pass session.SyntheticPromptOpts
(and the calls referencing runtime.manager.Prompt, PromptNetwork,
PromptSynthetic) to use that timeout context and cancel when done, and also
update the channel-draining logic in collectIntegrationPromptEvents / the drain
loop (the code referenced around lines 2346-2358) to use a select with a timeout
case so draining stops after the same or a configurable deadline rather than
blocking indefinitely.
- Around line 112-173: The test function
TestUDSSessionTranscriptEndpointIncludesSyntheticTurns must be converted to use
the required subtest pattern: wrap the existing test logic inside a
t.Run("Should include synthetic turns in transcript", func(t *testing.T) { ...
}) call (keeping the same body and using the inner t *testing.T), so that
TestUDSSessionTranscriptEndpointIncludesSyntheticTurns only calls t.Run with the
described name and the current assertions and helper calls remain unchanged
inside the anonymous func.
In `@internal/daemon/composed_assembler_test.go`:
- Around line 211-391: Multiple new top-level test functions violate the repo
pattern that requires test cases to be subtests using t.Run("Should..."); locate
the functions TestComposedAssemblerAssembleStartupUsesEligibleSectionOrdering,
TestComposedAssemblerAppliesBudgetPolicies,
TestComposedAssemblerDeduplicatesEligibleSectionNames, and
TestComposedAssemblerAssembleStartupLoadsBundledNetworkSectionDescriptor and
convert them into t.Run subtests (e.g., a single TestComposedAssembler* parent
or an existing parent test) by moving each test body into t.Run("Should <short
description>", func(t *testing.T){ ... }) and keep t.Parallel() inside the
subtest; preserve the existing logic, assertions, and references to
NewComposedAssembler, WithPromptSectionDescriptors,
assembleStartupPrompt/assemblePrompt, NewHarnessContextResolver, and
defaultStartupPromptSectionDescriptors when relocating the code.
In `@internal/daemon/daemon_integration_test.go`:
- Around line 644-646: Replace the brittle time.Sleep by waiting for an
observable signal that the first run has been processed: after calling
completeDetachedHarnessRunForTest(t, daemonInstance.tasks, first.Run.ID,
"sess-owner"), block until the daemon records the wake/reentry for first.Run.ID
(e.g., poll or select on the task event channel, inspect daemonInstance.tasks
for a recorded wake/reentry event, or use a wait channel/WaitGroup you add to
the harness) and only then call completeDetachedHarnessRunForTest for
second.Run.ID; reference the existing symbols completeDetachedHarnessRunForTest,
daemonInstance.tasks, first.Run.ID and second.Run.ID to locate where to replace
the sleep with the synchronization check.
In `@internal/daemon/harness_detached_work.go`:
- Around line 494-523: The equality check in validateDetachedHarnessRunMatch is
failing because persisted run metadata can gain a mutable Reentry after
processing; change the comparison to ignore the Reentry field when comparing
decoded current (from decodeDetachedHarnessRunMetadata(run.Metadata)) to
expected (built with Reentry == nil) — e.g., clear current.Reentry (or perform a
field-by-field compare excluding Reentry) before comparing, and keep the same
error message using run.ID and req.SubmissionKey when the remaining immutable
metadata differs.
In `@internal/daemon/harness_reentry_bridge.go`:
- Around line 667-706: The awaitSyntheticWake method in harnessReentryBridge
currently loops over events without observing b.ctx.Done(), so make the loop
cancellable by replacing the range over events with a select that listens for
b.ctx.Done() and the events channel; on ctx cancellation exit the loop and treat
as a dispatch failure (set sawError or choose harnessReentryOutcomeDropped with
harnessReentryReasonDispatchFailed) before calling syntheticEventExists and
finalizeRunOutcome; retain the existing logic around detecting EventTypeError,
call syntheticEventExists(item.targetSessionID, item.runID) and then
finalizeRunOutcome(item.runID, item.targetSessionID, item.targetAgentName, ...)
as before so the run is always finalized even if the context is canceled.
- Around line 164-168: OnTaskEvent currently does a blocking send into the
bounded channel b.events (select on b.ctx.Done / case b.events <- record) which
can stall callers when the queue is full; change the send to a non-blocking send
so task state transitions never block: replace the blocking branch with a select
that attempts case b.events <- record and falls back to default (optionally
logging or metric-ing the dropped record) while still respecting b.ctx.Done.
Target the send logic around b.events in the OnTaskEvent /
harness_reentry_bridge.go bridge code and ensure you do not spin or leak
goroutines when dropping events.
- Around line 199-223: The sorting by EndedAt/ID can dispatch runs out of true
completion order because the actual completion sequence is provided by
latestDetachedTerminalSequence; to fix, obtain the terminal sequence for each
run before ordering and sort runs by (EndedAt, terminal sequence, ID) or
primarily by terminal sequence when available: call
latestDetachedTerminalSequence(ctx, run.TaskID, run.ID) for each run (skipping
those with decode errors or detachedHarnessReentryProcessed(metadata.Reentry)),
store the returned sequence and timestamp alongside the run, then sort the
enriched entries using sequence (fallback to EndedAt and then ID) and finally
iterate calling processTerminalRun with the pre-fetched sequence/timestamp to
preserve FIFO after restart.
In `@internal/daemon/prompt_input_composite_integration_test.go`:
- Around line 18-130: The test function
TestPromptInputCompositeIntegrationPreservesStoredMessagesAcrossUserAndNetworkTurns
is a top-level test and must wrap its scenario in a t.Run("Should...") subtest;
refactor the body of
TestPromptInputCompositeIntegrationPreservesStoredMessagesAcrossUserAndNetworkTurns
so that the existing logic is moved into a single t.Run call (e.g.,
t.Run("Should preserve stored messages across user and network turns", func(t
*testing.T) { ...existing body... })) while keeping the function name and all
referenced symbols unchanged (compositeResolver,
newPromptInputCompositeAugmenter, manager.Prompt, manager.PromptNetwork,
loadStoredPromptMessages, etc.).
In `@internal/daemon/prompt_input_composite.go`:
- Around line 171-174: The code is always calling c.resolver.ResolvePrompt with
an empty acp.PromptMeta{} which prevents metadata-driven selection; change the
call to pass the actual prompt metadata extracted from the current context (e.g.
use any existing metadata on info or session rather than acp.PromptMeta{}).
Locate the call to c.resolver.ResolvePrompt(info, sess.CurrentTurnSource(),
acp.PromptMeta{}) and construct/forward the real PromptMeta (from info, sess, or
the request payload) so ResolvePrompt receives the true prompt-scoped flags
instead of an empty struct.
In `@internal/daemon/section_selector.go`:
- Around line 38-40: The early return in Select when s == nil || s.resolver ==
nil skips the Provider == nil filter and name de-duplication; remove that early
return and instead short-circuit only the startup resolution logic: keep
executing the rest of Select (so the Provider==nil filter and the de-duplication
logic still run) and, when s or s.resolver is nil, treat resolved context as an
empty ResolvedHarnessContext or skip calls to s.resolver.ResolveStartup but do
not return; update the code paths in Select that reference s.resolver (e.g., any
call to s.resolver.ResolveStartup) to be guarded by a nil check so resolution is
skipped safely while still applying Provider filtering and name de-duplication.
In `@internal/memory/recall_test.go`:
- Around line 13-125: The test functions
TestNewRecallAugmenterReturnsOriginalMessageWhenSessionOrQueryIsEmpty,
TestNewRecallAugmenterPrependsRecallAndPreservesUserMessage, and
TestBuildRecallBlockSkipsZeroScoreEntriesAndCapsResults must use the project's
required subtest pattern; wrap each test body in a t.Run("Should ...", func(t
*testing.T){ ... }) with the descriptive "Should..." name (e.g., "Should return
original message when session or query is empty", "Should prepend recall and
preserve user message", "Should skip zero-score entries and cap results"), move
t.Parallel() into the subtest function, and keep all existing setup and
assertions (augmenter creation, store writes, buildRecallBlock assertions)
unchanged inside those t.Run blocks so behavior is preserved while conforming to
the t.Run("Should...") requirement.
In `@internal/session/manager.go`:
- Around line 64-66: The remove/removeActive code paths currently only clear
sessions/pending/finalizing and must also purge per-key synthetic state to avoid
leaks; under syntheticMu lock, delete entries for the session key from
syntheticQueues and syntheticDispatching (and ensure any queuedSyntheticPrompt
resources are dropped) whenever a session is removed (in the remove and
removeActive functions), and ensure syntheticMu is held while reading/modifying
syntheticQueues/syntheticDispatching to prevent races.
In `@internal/task/manager.go`:
- Around line 2574-2582: The post-commit notification code uses the caller ctx;
change it to run on a non-cancelable post-commit context so transient caller
cancellations or deadlines don't prevent best-effort follow-up work. After
CreateTaskEvent returns success, create a detached context (e.g.
context.Background() or a derived background ctx) and use that when calling
m.store.GetTaskEventRecord, m.eventObserver.OnTaskEvent,
m.emitTaskLiveRecordBestEffort and m.emitTaskLiveEventBestEffort; run this
notification path asynchronously (goroutine) or sequentially but using the
detached ctx and keep the same error handling/semantics so failures remain
best-effort.
In `@internal/transcript/transcript.go`:
- Around line 318-331: The current toolLifecycleKey function uses TurnID
whenever present, causing events sharing ToolCallID but missing TurnID on one
side to get different keys; change it so ToolCallID (parsed.ToolCallID) is the
primary key: if parsed.ToolCallID (trimmed) is non-empty return that (ignore
TurnID), otherwise fall back to parsed.ID and, only in that fallback case,
prepend TurnID if present (i.e., return turn + ":" + id when turn exists, else
id); update function toolLifecycleKey and references to
parsed.ToolCallID/parsed.ID accordingly.
---
Outside diff comments:
In `@internal/daemon/task_runtime.go`:
- Around line 226-287: After newHarnessReentryBridge(...) returns successfully,
install a deferred cleanup that will call reentry.shutdown() (or the reentry
bridge's shutdown/close method) to ensure the bridge is torn down on any early
return; then execute reentry.recover(ctx) and only if it succeeds cancel the
deferred cleanup (or mark completion) and then publish state.tasks and
state.deps.Tasks. In short: add a defer immediately after
newHarnessReentryBridge to shut down the reentry bridge on error, call
reentry.recover(ctx), and only when recover returns nil cancel the defer and set
state.tasks/state.deps.Tasks so failed boot paths don't leak the bridge.
In `@internal/session/stop_reason.go`:
- Around line 184-216: The snapshot of observedProcessExit is taken too early
(observedProcessExit := isProcessDone(process)) but process may change during
prepareStop/waitForPromptSetup; recompute the observed exit state right before
returning using the currently assigned process (e.g., call
isProcessDone(session.processHandle()) or isProcessDone(process) immediately
after process = session.processHandle()) so the returned observedProcessExit
matches the handle callers receive and avoids skipping
proc.Wait()/reconcileObservedTerminalStop.
---
Minor comments:
In `@internal/api/httpapi/httpapi_integration_test.go`:
- Around line 201-215: Replace uses of context.Background() passed to
runtime.manager.Prompt, PromptNetwork, and PromptSynthetic with a test-scoped
cancellable context (e.g., ctx, cancel := context.WithTimeout(testCtx, ...) /
defer cancel) so the prompt calls and the subsequent
collectIntegrationPromptEvents drain will abort when the test deadline is
reached; update calls to collectIntegrationPromptEvents/mustIntegrationPrompt to
accept and respect the ctx expiration (stop draining the channel and return an
error when ctx.Done() fires) to avoid hanging tests if a prompt never closes.
In `@internal/api/httpapi/stream_helpers_test.go`:
- Around line 187-229: The test's fake QueryEventsFn currently ignores its
store.EventSummaryQuery argument so it won't catch if the handler stops
forwarding session filters; update the QueryEventsFn implementation in
stream_helpers_test.go to capture the incoming store.EventSummaryQuery param and
assert that its SessionID (e.g. q.SessionID) equals "sess-harness" (use t.Fatalf
or t.Errorf on mismatch) before returning the EventSummary; this ensures the
handler actually forwards the session_id into the observer query.
In `@internal/daemon/daemon_test.go`:
- Around line 3634-3660: The synthetic event currently uses different
TurnID/Timestamp values in the JSON payload and the outer store.SessionEvent;
fix by computing a single turnID and a single timestamp once (e.g., turnID :=
fmt.Sprintf("turn-synthetic-%d", ... ) and ts := time.Now().UTC() or based on
f.nextEventSequence) before marshaling acp.AgentEvent, then reuse that same
turnID and ts for both the acp.AgentEvent fields (TurnID, Timestamp) and the
store.SessionEvent fields (TurnID, Timestamp) when appending to f.sessionEvents;
update the code paths around json.Marshal(acp.AgentEvent{...}) and the
store.SessionEvent construction so both use the same precomputed values
(referencing acp.AgentEvent, store.SessionEvent, f.nextEventSequence,
opts.Metadata, info.AgentName).
In `@internal/daemon/harness_context.go`:
- Around line 396-427: The meta.TurnSource assignment in
normalizeSyntheticHarnessTurnContext is using string(TurnOriginSynthetic)
instead of the acp constant; update the assignment so meta.TurnSource =
acp.PromptTurnSourceSynthetic to match the pattern used for user/network sources
(see usage of acp constants in other branches) and keep the function
HarnessContextResolver.normalizeSyntheticHarnessTurnContext, the returned
HarnessTurnContext, and TurnOriginSynthetic semantics unchanged.
In `@internal/extension/host_api_test.go`:
- Around line 1792-1810: The test for promptProjectionEventFromStoredEvent
currently only asserts envelope fields; add an assertion that projected.Text
equals the text pulled from the stored Content JSON (e.g., "daemon wake-up") so
regressions that stop populating projected.Text fail; locate the
promptProjectionEventFromStoredEvent call (using store.SessionEvent with Content
`{"schema":"agh.session.event.v1","text":"daemon wake-up"}`) and add a check
like assert projected.Text == "daemon wake-up" (or use t.Fatalf on mismatch)
after the existing Timestamp assertion.
In `@internal/observe/observer_test.go`:
- Around line 283-339: The test currently writes summaries directly via
h.observer.registry.WriteEventSummary which bypasses the new
harnessLifecycleRecorder path; change the test to emit the same three events
through the recorder API instead (use the harness lifecycle recorder on the
observer—e.g. the harnessLifecycleRecorder or recorder methods on h.observer
that the PR added) after calling h.observer.OnSessionCreated, so the recorder
performs the write to the registry and QueryEvents is asserted against those
recorder-emitted events (keep the same session, timestamps, Type and Summary
values and the same assertions).
In `@internal/session/manager_integration_test.go`:
- Around line 198-211: The promptHook stub for fakeProcess closes the events
channel for req.TurnID == "turn-1" without emitting a terminal
acp.EventTypeDone, so change the goroutine created for turn-1 (inside
promptHook) to, after receiving <-releaseFirstPrompt, send a final
acp.AgentEvent with Type == acp.EventTypeDone into the events channel before
closing it; locate the promptHook function, the branch checking req.TurnID ==
"turn-1", and modify the anonymous goroutine to emit the terminal done event
(using acp.AgentEvent and acp.EventTypeDone) then close(events) so the test
simulates real turn completion.
In `@internal/session/stop_reason.go`:
- Around line 194-195: The pre-stop stage returns raw errors that lack stage
context; wrap all errors returned from the stop-preparation calls (e.g.
m.dispatchSessionPreStop(ctx, session)) with descriptive context using
fmt.Errorf so callers can distinguish failures in pre-stop hooks, metadata
persistence and prompt-sync steps (for example:
fmt.Errorf("dispatchSessionPreStop: %w", err)). Update each raw return in this
block (including the similar paths at the other noted calls) to return the
wrapped error instead of the raw err.
In `@internal/task/manager_test.go`:
- Around line 3068-3126: The test
TestManagerEnqueueRunPreservesMetadataAcrossIdempotentDuplicates currently only
retries with identical metadata; add a duplicate EnqueueRun call that uses the
same IdempotencyKey ("detached-metadata-1") but a different Metadata payload and
assert that manager.EnqueueRun returns the original run ID and that returned
run.Metadata and the stored run metadata remain equal to the first metadata
(i.e., the duplicate does NOT overwrite), and that len(store.runs) is still 1;
locate the EnqueueRun calls in this test and extend with the
conflicting-metadata case to validate behavior.
---
Nitpick comments:
In `@internal/daemon/prompt_input_composite_integration_test.go`:
- Around line 146-150: The test mutates the slice returned by
r.base.ResolvePrompt by appending directly to resolved.Policy.EnableAugmenters;
to fix, make a copy of that slice before appending (e.g., allocate a new slice
and copy existing elements or use append on a nil slice) so you do not alter the
original backing array from r.base.ResolvePrompt; update the code around
resolved.Policy.EnableAugmenters and where additional :=
r.extra[resolved.Policy.TurnOrigin] is used to append to the cloned slice rather
than the original.
In `@internal/daemon/task_runtime_test.go`:
- Around line 1889-1899: The helper waitForTaskRuntimeCondition uses a tight
time.Sleep polling loop which makes tests timing-sensitive; replace it with
proper synchronization by accepting a context or a channel and using select with
a time.After deadline or time.Ticker to wait for the check to become true or for
cancellation/timeout. Modify waitForTaskRuntimeCondition (or add an overload) to
take a context.Context (or a done/notify chan struct{}) and use select to either
evaluate check() when the ticker fires or return/t.Fatal when the context is
done/timeout, avoiding direct time.Sleep and making tests deterministic under
-race.
🪄 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: 987bc8a8-d7df-4215-acfb-b888791c6def
⛔ Files ignored due to path filters (40)
.agents/skills/compozy/references/config-reference.mdis excluded by!**/*.md,!.agents/**.compozy/tasks/harness/_meta.mdis excluded by!**/*.md.compozy/tasks/harness/_tasks.mdis excluded by!**/*.md.compozy/tasks/harness/memory/MEMORY.mdis excluded by!**/*.md.compozy/tasks/harness/memory/task_01.mdis excluded by!**/*.md.compozy/tasks/harness/memory/task_02.mdis excluded by!**/*.md.compozy/tasks/harness/memory/task_03.mdis excluded by!**/*.md.compozy/tasks/harness/memory/task_04.mdis excluded by!**/*.md.compozy/tasks/harness/memory/task_05.mdis excluded by!**/*.md.compozy/tasks/harness/memory/task_06.mdis excluded by!**/*.md.compozy/tasks/harness/memory/task_07.mdis excluded by!**/*.md.compozy/tasks/harness/memory/task_08.mdis excluded by!**/*.md.compozy/tasks/harness/memory/task_09.mdis excluded by!**/*.md.compozy/tasks/harness/memory/task_10.mdis excluded by!**/*.md.compozy/tasks/harness/qa/test-cases/TC-INT-001.mdis excluded by!**/*.md.compozy/tasks/harness/qa/test-cases/TC-INT-002.mdis excluded by!**/*.md.compozy/tasks/harness/qa/test-cases/TC-INT-003.mdis excluded by!**/*.md.compozy/tasks/harness/qa/test-cases/TC-INT-004.mdis excluded by!**/*.md.compozy/tasks/harness/qa/test-cases/TC-INT-005.mdis excluded by!**/*.md.compozy/tasks/harness/qa/test-cases/TC-INT-006.mdis excluded by!**/*.md.compozy/tasks/harness/qa/test-cases/TC-INT-007.mdis excluded by!**/*.md.compozy/tasks/harness/qa/test-cases/TC-REG-001.mdis excluded by!**/*.md.compozy/tasks/harness/qa/test-plans/harness-regression.mdis excluded by!**/*.md.compozy/tasks/harness/qa/test-plans/harness-test-plan.mdis excluded by!**/*.md.compozy/tasks/harness/qa/verification-report.mdis excluded by!**/*.md.compozy/tasks/harness/task_01.mdis excluded by!**/*.md.compozy/tasks/harness/task_02.mdis excluded by!**/*.md.compozy/tasks/harness/task_03.mdis excluded by!**/*.md.compozy/tasks/harness/task_04.mdis excluded by!**/*.md.compozy/tasks/harness/task_05.mdis excluded by!**/*.md.compozy/tasks/harness/task_06.mdis excluded by!**/*.md.compozy/tasks/harness/task_07.mdis excluded by!**/*.md.compozy/tasks/harness/task_08.mdis excluded by!**/*.md.compozy/tasks/harness/task_09.mdis excluded by!**/*.md.compozy/tasks/harness/task_10.mdis excluded by!**/*.mdopenapi/agh.jsonis excluded by!**/*.jsonsdk/typescript/src/generated/contracts.tsis excluded by!**/generated/**web/AGENTS.mdis excluded by!**/*.mdweb/CLAUDE.mdis excluded by!**/*.mdweb/src/generated/agh-openapi.d.tsis excluded by!**/generated/**
📒 Files selected for processing (73)
.compozy/tasks/harness/qa/issues/.gitkeep.compozy/tasks/harness/qa/screenshots/.gitkeepextensions/bridges/teams/provider_test.gointernal/acp/types.gointernal/acp/types_test.gointernal/api/contract/tasks.gointernal/api/core/tasks.gointernal/api/httpapi/httpapi_integration_test.gointernal/api/httpapi/stream_helpers_test.gointernal/api/udsapi/stream_helpers_test.gointernal/api/udsapi/transport_parity_integration_test.gointernal/api/udsapi/udsapi_integration_test.gointernal/daemon/boot.gointernal/daemon/composed_assembler.gointernal/daemon/composed_assembler_test.gointernal/daemon/daemon.gointernal/daemon/daemon_integration_test.gointernal/daemon/daemon_test.gointernal/daemon/harness_context.gointernal/daemon/harness_context_integration_test.gointernal/daemon/harness_context_test.gointernal/daemon/harness_detached_work.gointernal/daemon/harness_observability.gointernal/daemon/harness_observability_test.gointernal/daemon/harness_reentry_bridge.gointernal/daemon/prompt_input_composite.gointernal/daemon/prompt_input_composite_integration_test.gointernal/daemon/prompt_input_composite_test.gointernal/daemon/prompt_sections.gointernal/daemon/section_selector.gointernal/daemon/task_runtime.gointernal/daemon/task_runtime_test.gointernal/extension/host_api.gointernal/extension/host_api_tasks.gointernal/extension/host_api_test.gointernal/memory/recall.gointernal/memory/recall_test.gointernal/observe/observer_test.gointernal/observe/tasks.gointernal/observe/tasks_test.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_network_skill.gointernal/session/manager_prompt.gointernal/session/manager_start.gointernal/session/manager_test.gointernal/session/prompt_overlay.gointernal/session/session.gointernal/session/stop_reason.gointernal/session/synthetic_prompt.gointernal/session/transcript_test.gointernal/store/globaldb/global_db.gointernal/store/globaldb/global_db_extra_test.gointernal/store/globaldb/global_db_task.gointernal/store/globaldb/global_db_task_aux.gointernal/store/globaldb/global_db_task_test.gointernal/store/globaldb/migrate_workspace.gointernal/task/interfaces.gointernal/task/interfaces_integration_test.gointernal/task/live.gointernal/task/live_types.gointernal/task/manager.gointernal/task/manager_test.gointernal/task/types.gointernal/task/validate.gointernal/transcript/transcript.gointernal/transcript/transcript_test.goweb/src/components/ui/empty.tsx
💤 Files with no reviewable changes (2)
- internal/observe/tasks_test.go
- internal/session/manager_network_skill.go
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (3)
internal/session/manager_prompt.go (1)
265-275: Consider extractingclonePromptSyntheticMetato a shared location.This helper function is duplicated in both
manager_prompt.go(lines 265-275) andtranscript.go(lines 824-834). Consider moving it tointernal/acp/types.goalongside thePromptSyntheticMetatype definition to avoid duplication.Also applies to: 824-834
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/session/manager_prompt.go` around lines 265 - 275, The helper clonePromptSyntheticMeta is duplicated; move it into the shared acp package next to the PromptSyntheticMeta type (e.g., add a function clonePromptSyntheticMeta(meta *acp.PromptSyntheticMeta) in internal/acp/types.go) and have it use meta.Normalize() and cloned.IsZero() exactly as before; then remove the duplicate implementations from manager_prompt.go and transcript.go and update their callers to reference the single shared function in the acp package. Ensure imports remain correct and only the centralized clonePromptSyntheticMeta that returns nil for nil or zero-normalized metas is used.internal/daemon/harness_detached_work.go (1)
600-603: Consider collision risk with truncated SHA-256 hash.The task ID uses only the first 8 bytes (16 hex chars) of a SHA-256 hash. While collisions are statistically unlikely for typical usage, this provides ~64 bits of collision resistance. For a safety-critical system with high task volume, you may want to use more bytes or document the collision probability assumptions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/daemon/harness_detached_work.go` around lines 600 - 603, The detachedHarnessTaskID function currently truncates sha256 to 8 bytes (64-bit) which increases collision risk; change the truncation to a longer slice (e.g., 16 bytes or the full sum) in detachedHarnessTaskID to raise collision resistance (replace the hex.EncodeToString(sum[:8]) usage with hex.EncodeToString(sum[:16]) or hex.EncodeToString(sum[:]) as appropriate) and add a short comment documenting the chosen truncation length and the resulting collision strength so callers know the assumption.internal/task/manager_test.go (1)
3089-3233: Use the repo’s requiredt.Run("Should...")shape for these new tests.Both additions are top-level single-case tests. Please wrap them in
t.Run("Should ...")subtests, or fold them into a small table, so they match the default Go test structure used in this repository and stay easy to extend. As per coding guidelines "Use table-driven tests with subtests (t.Run) as default in Go tests" and "**/*_test.go: MUST use t.Run("Should...") pattern for ALL test cases".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/task/manager_test.go` around lines 3089 - 3233, Both new top-level tests TestManagerEnqueueRunPreservesMetadataAcrossIdempotentDuplicates and TestManagerRecordTaskEventDetachesPostCommitNotificationsFromCallerContext must follow the repo's t.Run("Should ...") pattern; wrap each test body inside a single subtest (e.g. t.Run("Should preserve metadata across idempotent duplicates", func(t *testing.T) { ... })) or convert them to a small table-driven subtest (using t.Run for each case). Ensure you keep the existing setup and assertions intact but move them into the t.Run closures so the test names follow the "Should ..." convention and remain extendable.
🤖 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/daemon_integration_test.go`:
- Around line 221-907: The six top-level tests
TestBootWiresDetachedHarnessTaskRuntimeAcrossScopes,
TestDetachedHarnessCompletionWakeEmitsSyntheticReentryEndToEnd,
TestDetachedHarnessCompletionSilentPolicyRecordsDropEndToEnd,
TestDetachedHarnessCompletionWakePreservesFIFOAcrossRuns,
TestBootRecoveryDetachedHarnessWakeUsesPersistedSyntheticEventForDedupe, and
TestBootRecoversDetachedHarnessRunThroughTaskRuntimeRules should be consolidated
into a single parent test (e.g., TestDetachedHarnessIntegration) that uses
table-driven subtests with t.Run("Should ...", func(t *testing.T) { ... })
entries describing each scenario, moving the common setup (integrationHomePaths,
testConfig, fakeSessionManager, bootDetachedHarnessIntegrationDaemon, workspace
resolution, seeding) into shared code reused by each subtest and keeping
scenario-specific inputs/assertions in the table; additionally, update
ensureDetachedHarnessWorkspaceIndex to wrap returned errors with
fmt.Errorf("...: %w", err) (use ensureDetachedHarnessWorkspaceIndex identifier
to locate the function) for the error paths currently returned raw so they
follow Go error-wrapping conventions.
- Around line 3358-3377: The function ensureDetachedHarnessWorkspaceIndex
returns raw errors from db.GetWorkspace, os.MkdirAll, and db.InsertWorkspace;
update each return to wrap the underlying error with fmt.Errorf including
context (e.g., fmt.Errorf("GetWorkspace failed: %w", err), fmt.Errorf("mkdir
workspace root %s: %w", rootDir, err), fmt.Errorf("InsertWorkspace failed: %w",
err)) while preserving the existing workspacepkg.ErrWorkspaceNotFound check and
keeping the same calls to db.GetWorkspace(testutil.Context(t), workspaceID) and
db.InsertWorkspace(...).
In `@internal/daemon/daemon_test.go`:
- Around line 3970-3996: In PromptSynthetic of fakeSessionManager, don't swallow
JSON marshal errors when preparing the synthetic event: check the error returned
by whatever serialization step (used in recordSyntheticEvent or the marshal call
inside that flow) and return that error from PromptSynthetic instead of
returning a closed channel; update the logic around
syntheticPromptHook/recordSyntheticEvent so that if marshaling or event creation
fails you propagate the error (return nil, err) and still record the call in
syntheticPromptCalls, preserving the existing hook behavior (hook(ctx, id, opts)
remains untouched).
- Around line 3358-3649: The new tests
(TestTaskRuntimeDetachedHarnessSubmissionAllowsProcessedReentryMetadata,
TestHarnessReentryBridgeOnTaskEventSchedulesRescanWhenQueueIsFull,
TestHarnessReentryBridgeRecoverOrdersEqualTimestampsByTerminalSequence,
TestHarnessReentryBridgeShutdownFinalizesHungSyntheticWake,
TestSectionSelectorFallbackStillFiltersProvidersAndDuplicates) must be converted
to follow the repository test convention by turning each standalone case into a
t.Run("Should...") subtest (or table-driven subtests) — wrap the existing test
logic inside a t.Run with a descriptive "Should ..." name, keep calls to
t.Parallel() inside the subtest if appropriate, and if multiple scenarios exist
convert them to a table-driven slice and iterate with t.Run for each entry;
update references to functions like newDetachedHarnessTaskRuntimeForTest,
submitDetachedHarnessWorkForTest, newHarnessReentryBridge, bridge.recover,
waitForDetachedHarnessReentryState, seedDetachedHarnessRecoveryRunForTest, and
SectionSelector.Select to remain unchanged while moving their calls into the
subtest bodies so behavior is preserved.
- Around line 3813-3835: The fake Events implementation
(fakeSessionManager.Events) currently filters sessionEvents but does not
guarantee ascending sequence order; update it to sort the filtered
[]store.SessionEvent by event.Sequence in ascending order (e.g., using
sort.Slice) before applying the Limit truncation based on query.Limit and
returning; ensure you sort the filtered slice (not the original
store.sessionEvents) so behavior matches production SessionDB.Query ordering and
then apply the existing slicing logic that keeps the last N events.
In `@internal/daemon/harness_context.go`:
- Around line 247-254: The current construction of ResolvedHarnessPolicy sets
DetachedRunMode based solely on session class via
r.resolveDetachedRunMode(sessionCtx), which causes all system sessions to be
treated as detached runtimes; change this to determine DetachedRunMode from the
turn context and only when detached metadata is present: check turnCtx.Detached
!= nil (or equivalent) and call r.resolveDetachedRunMode using the turn-level
information (or a new turn-aware resolver) so DetachedRunMode is only enabled
for turns that explicitly carry detached run metadata; update both the policy
creation here and the other occurrence that mirrors lines 514-521 to follow the
same guard.
In `@internal/daemon/harness_reentry_bridge.go`:
- Around line 142-157: The bridge spawns background goroutines in
newHarnessReentryBridge/enqueueWake (run, drainWakeQueue, awaitSyntheticWake)
but shutdown only cancels the context, letting those goroutines race against
sessions/store during teardown; fix by adding explicit goroutine ownership
(e.g., a sync.WaitGroup field on harnessReentryBridge), incrementing the
WaitGroup when run/drainWakeQueue/awaitSyntheticWake start and deferring Done
when they exit, ensure all goroutines select on bridge.ctx.Done() for prompt
termination, and modify shutdown to call cancel() and then wg.Wait() (and close
any channels like events/rescan only after Wait returns) so no fire-and-forget
goroutines remain during shutdown.
In `@internal/daemon/prompt_input_composite.go`:
- Around line 194-208: The code uses a single remainingBudget for all
descriptors so per-descriptor limits (descriptor.Budget) are ignored; change the
loop in aggregatePromptInputBudget/apply flow to enforce each descriptor's cap
by computing a perDescriptorBudget = min(remainingBudget, descriptor.Budget) (or
zero if limited and descriptor.Budget==0) and pass that perDescriptorBudget into
c.applyAugmenterDescriptor instead of the full remainingBudget, then after the
call subtract the actual consumed amount from remainingBudget; update the same
pattern where applyAugmenterDescriptor is invoked (the loop using descriptors,
remainingBudget, limited) so later descriptors cannot consume more than their
own descriptor.Budget.
In `@internal/daemon/task_runtime_test.go`:
- Around line 1987-2019: newDetachedHarnessTaskRuntimeForTest creates a live
harnessReentryBridge via newHarnessReentryBridge but never stops it; register a
test cleanup to stop the bridge by calling its shutdown method (e.g.
reentry.Close() or reentry.Shutdown() as provided) — add t.Cleanup(func(){ _ =
reentry.Close() }) immediately after the reentry is created (or use the correct
method name on the returned harnessReentryBridge type) so the reentry goroutines
are stopped and won't leak across tests; ensure the taskRuntime.reentry field
remains set.
- Around line 456-1746: Several newly added tests are written as standalone
cases but must follow the project testing convention to be table-driven subtests
using t.Run("Should..."); refactor each affected test function (e.g.,
TestHarnessReentryBridgeEmitsSyntheticWakeAndObservability,
TestHarnessReentryBridgeSilentPolicyRecordsDropSummary,
TestHarnessReentryBridgeMissingAndStoppedTargetsDropWithoutWake,
TestHarnessReentryBridgeDuplicateTerminalNotificationsStayIdempotent,
TestHarnessReentryBridgePreservesSyntheticWakeFIFO,
TestHarnessReentryBridgeHelperCoverage,
TestHarnessReentryBridgeDropsWhenSyntheticDispatchFails,
TestHarnessReentryBridgeDropsWhenSyntheticPromptChannelHasNoEvent,
TestHarnessReentryBridgeDropsWhenSyntheticPromptReturnsErrorEvent,
TestHarnessReentryBridgeDispatchWakeUsesRecordedSyntheticEvent) to wrap each
logical case in t.Run("Should ...", func(t *testing.T) { t.Parallel(); ... }),
convert any inline test cases into table-driven subtests where appropriate,
preserve existing helper calls (submitDetachedHarnessWorkForTest,
completeDetachedHarnessRunForTest, waitForDetachedHarnessReentryState,
sessions.syntheticPromptHook, etc.), and ensure all new subtest names start with
"Should" and include t.Parallel() to match the required pattern.
In `@internal/extension/host_api_test.go`:
- Around line 3507-3517: The test currently rewrites task_runs.metadata_json
directly before listing, so it doesn't catch if the Host API endpoint drops
metadata; change the test to seed the metadata through the enqueueRun helper by
passing a non-nil metadata map into enqueueRun(taskID, idempotencyKey, metadata)
(which sends it via env.call to "ext-runs" "tasks/runs/enqueue"), then call the
listing endpoint and assert the returned task run's metadata equals the seeded
map; remove or stop relying on any direct writes to task_runs.metadata_json so
the assertion fails if tasks/runs/enqueue does not persist metadata.
In `@internal/session/stop_reason_test.go`:
- Around line 217-307: The two table-driven tests ("Should wrap pre-stop hook
failures" and "Should wrap metadata write failures") only check for stage text;
add explicit expected root-cause errors (set wantErr to the concrete error
values, e.g. the hook error and the underlying file/write error) and update the
verification logic after calling manager.prepareStopWithCause to always assert
the wrapped cause using errors.Is(err, tc.wantErr) (keep the existing
strings.Contains check for tc.wantStage). Locate the tests in
stop_reason_test.go where the tests slice and the call to prepareStopWithCause
are defined and update the two test case entries to include appropriate wantErr
values and ensure the final error checks use tc.wantErr for all rows.
In `@internal/task/manager.go`:
- Around line 2584-2586: Wrap the injected observer call so panics cannot
escape: protect the m.eventObserver.OnTaskEvent(postCommitCtx, record)
invocation with a recover() boundary (e.g., defer func(){ if r := recover(); r
!= nil { /* log the panic and continue */ } }()), ensuring the call still occurs
inline but any panic is caught and logged and does not prevent subsequent live
fanout; update the call site that references m.eventObserver, postCommitCtx, and
record accordingly.
---
Nitpick comments:
In `@internal/daemon/harness_detached_work.go`:
- Around line 600-603: The detachedHarnessTaskID function currently truncates
sha256 to 8 bytes (64-bit) which increases collision risk; change the truncation
to a longer slice (e.g., 16 bytes or the full sum) in detachedHarnessTaskID to
raise collision resistance (replace the hex.EncodeToString(sum[:8]) usage with
hex.EncodeToString(sum[:16]) or hex.EncodeToString(sum[:]) as appropriate) and
add a short comment documenting the chosen truncation length and the resulting
collision strength so callers know the assumption.
In `@internal/session/manager_prompt.go`:
- Around line 265-275: The helper clonePromptSyntheticMeta is duplicated; move
it into the shared acp package next to the PromptSyntheticMeta type (e.g., add a
function clonePromptSyntheticMeta(meta *acp.PromptSyntheticMeta) in
internal/acp/types.go) and have it use meta.Normalize() and cloned.IsZero()
exactly as before; then remove the duplicate implementations from
manager_prompt.go and transcript.go and update their callers to reference the
single shared function in the acp package. Ensure imports remain correct and
only the centralized clonePromptSyntheticMeta that returns nil for nil or
zero-normalized metas is used.
In `@internal/task/manager_test.go`:
- Around line 3089-3233: Both new top-level tests
TestManagerEnqueueRunPreservesMetadataAcrossIdempotentDuplicates and
TestManagerRecordTaskEventDetachesPostCommitNotificationsFromCallerContext must
follow the repo's t.Run("Should ...") pattern; wrap each test body inside a
single subtest (e.g. t.Run("Should preserve metadata across idempotent
duplicates", func(t *testing.T) { ... })) or convert them to a small
table-driven subtest (using t.Run for each case). Ensure you keep the existing
setup and assertions intact but move them into the t.Run closures so the test
names follow the "Should ..." convention and remain extendable.
🪄 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: c4bc264a-7aea-48d1-a4ec-36e896ae07f0
⛔ Files ignored due to path filters (30)
.compozy/tasks/harness/reviews-001/issue_001.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_002.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_003.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_004.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_005.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_006.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_007.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_008.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_009.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_010.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_011.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_012.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_013.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_014.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_015.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_016.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_017.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_018.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_019.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_020.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_021.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_022.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_023.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_024.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_025.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_026.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_027.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_028.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_029.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-001/issue_030.mdis excluded by!**/*.md
📒 Files selected for processing (26)
internal/api/httpapi/httpapi_integration_test.gointernal/api/httpapi/stream_helpers_test.gointernal/api/udsapi/stream_helpers_test.gointernal/api/udsapi/udsapi_integration_test.gointernal/daemon/daemon_integration_test.gointernal/daemon/daemon_test.gointernal/daemon/harness_context.gointernal/daemon/harness_detached_work.gointernal/daemon/harness_reentry_bridge.gointernal/daemon/prompt_input_composite.gointernal/daemon/prompt_input_composite_integration_test.gointernal/daemon/section_selector.gointernal/daemon/task_runtime_test.gointernal/extension/host_api_tasks.gointernal/extension/host_api_test.gointernal/memory/recall_test.gointernal/session/manager.gointernal/session/manager_integration_test.gointernal/session/manager_prompt.gointernal/session/session.gointernal/session/stop_reason.gointernal/session/stop_reason_test.gointernal/task/manager.gointernal/task/manager_test.gointernal/transcript/transcript.gointernal/transcript/transcript_test.go
✅ Files skipped from review due to trivial changes (4)
- internal/api/httpapi/httpapi_integration_test.go
- internal/daemon/prompt_input_composite_integration_test.go
- internal/api/httpapi/stream_helpers_test.go
- internal/api/udsapi/stream_helpers_test.go
🚧 Files skipped from review as they are similar to previous changes (7)
- internal/memory/recall_test.go
- internal/session/session.go
- internal/session/stop_reason.go
- internal/transcript/transcript_test.go
- internal/session/manager_integration_test.go
- internal/daemon/section_selector.go
- internal/extension/host_api_tasks.go
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
internal/daemon/task_runtime_test.go (1)
456-639:⚠️ Potential issue | 🟠 MajorWrap these new cases in
t.Run("Should ...")subtests.Several newly added tests here are still written as standalone top-level cases, so they still miss the repo’s required test structure and naming convention. Please wrap each body in
t.Run("Should ...", func(t *testing.T) { t.Parallel(); ... }), or merge related assertions into table-driven subtests. As per coding guidelines**/*_test.go: "MUST use t.Run("Should...") pattern for ALL test cases" and "Use table-driven tests with subtests (t.Run) as default".Also applies to: 720-819, 821-868, 870-998, 1000-1033, 1035-1140
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/daemon/task_runtime_test.go` around lines 456 - 639, The test function TestTaskRuntimeDetachedHarnessSubmissionPersistsMetadataAndReusesIdempotency should be converted to use the repo’s required subtest pattern: wrap the existing test body inside t.Run("Should persist metadata and reuse idempotency", func(t *testing.T) { t.Parallel(); ... }) so the assertions and setup remain identical but run as a named subtest; locate the function by its name TestTaskRuntimeDetachedHarnessSubmissionPersistsMetadataAndReusesIdempotency and move its current contents into the inner t.Run closure, ensuring t.Parallel() is called inside the subtest, and apply the same wrapping to the other new test cases referenced in the review (the other test functions/ranges noted) so all top-level cases use t.Run("Should ...", func(t *testing.T) { t.Parallel(); ... }).internal/daemon/prompt_input_composite.go (1)
314-323:⚠️ Potential issue | 🟠 Major
Budget == 0still bypasses the per-descriptor cap.When
limitedis true and a descriptor hasBudget == 0,descriptorBudgetfalls back to the fullremainingBudget. In a mixed-budget chain, that descriptor can still consume the shared pool and starve later augmenters, so descriptor-level limits are still not fully enforced.💡 Suggested change
- descriptorBudget := remainingBudget - if limited && descriptor.Budget > 0 { - descriptorBudget = min(remainingBudget, descriptor.Budget) - } + descriptorBudget := remainingBudget + if limited { + descriptorBudget = 0 + if descriptor.Budget > 0 { + descriptorBudget = min(remainingBudget, descriptor.Budget) + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/daemon/prompt_input_composite.go` around lines 314 - 323, The per-descriptor cap is bypassed when descriptor.Budget == 0 because descriptorBudget is initialized to remainingBudget and only overwritten when descriptor.Budget > 0; change the logic so that when limited is true you always apply the descriptor's explicit budget (including zero) — i.e., set descriptorBudget = min(remainingBudget, descriptor.Budget) when limited is true (instead of only when descriptor.Budget > 0) so that descriptor.Budget == 0 results in a zero descriptorBudget before calling applyPromptInputAugmenterBudget.internal/daemon/daemon_test.go (1)
4150-4223:⚠️ Potential issue | 🟡 Minor
PromptSyntheticstill hides synthetic event serialization failures.At Line 4209, marshal errors are dropped, and
PromptSyntheticstill returns success. That can mask broken test behavior and produce false positives in reentry flows.🔧 Proposed fix
func (f *fakeSessionManager) PromptSynthetic( ctx context.Context, id string, opts session.SyntheticPromptOpts, ) (<-chan acp.AgentEvent, error) { @@ - f.recordSyntheticEvent(id, info, opts) + if err := f.recordSyntheticEvent(id, info, opts); err != nil { + return nil, err + } ch := make(chan acp.AgentEvent) close(ch) return ch, nil } func (f *fakeSessionManager) recordSyntheticEvent( sessionID string, info *session.Info, opts session.SyntheticPromptOpts, -) { +) error { if info == nil { - return + return nil } @@ payload, err := json.Marshal(acp.AgentEvent{ @@ }) if err != nil { - return + return fmt.Errorf("marshal synthetic reentry event: %w", err) } @@ f.sessionEvents[sessionID] = append(f.sessionEvents[sessionID], store.SessionEvent{ @@ }) + return nil }As per coding guidelines
**/*.go: "Never ignore errors with_— every error must be handled or have a written justification".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/daemon/daemon_test.go` around lines 4150 - 4223, PromptSynthetic calls recordSyntheticEvent which swallows json.Marshal errors, causing PromptSynthetic to return success even when synthetic event serialization fails; change recordSyntheticEvent on fakeSessionManager to return an error (propagate json.Marshal errors) and update PromptSynthetic to capture and return that error instead of ignoring it so serialization failures surface to callers/tests; update any call sites for recordSyntheticEvent accordingly (keep function name recordSyntheticEvent and the fakeSessionManager receiver).
🧹 Nitpick comments (1)
internal/extension/host_api_test.go (1)
5376-5398: Make unexpectedPromptandEventscalls fail by default.The other stub methods are strict, but these two silently succeed when no callback is wired. That can let a test pass even when
HostAPIHandler.submitPrompttakes an unintended interaction path.Suggested change
func (s promptSessionManagerStub) Events( ctx context.Context, id string, query store.EventQuery, ) ([]store.SessionEvent, error) { if s.eventsFn == nil { - return nil, nil + return nil, errors.New("unexpected events call") } return s.eventsFn(ctx, id, query) } @@ func (s promptSessionManagerStub) Prompt(ctx context.Context, id string, msg string) (<-chan acp.AgentEvent, error) { if s.promptFn == nil { - ch := make(chan acp.AgentEvent) - close(ch) - return ch, nil + return nil, errors.New("unexpected prompt call") } return s.promptFn(ctx, id, msg) }As per coding guidelines, "Ensure tests verify behavior outcomes, not just function calls."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/extension/host_api_test.go` around lines 5376 - 5398, The Events and Prompt stub methods should fail by default like Stop does to catch unexpected calls: change promptSessionManagerStub.Events to return a non-nil error (e.g., errors.New("unexpected events call")) when s.eventsFn is nil, and change promptSessionManagerStub.Prompt to return an error (e.g., nil, errors.New("unexpected prompt call")) when s.promptFn is nil (instead of returning a closed channel), so unexpected interactions in HostAPIHandler.submitPrompt are surfaced; update any tests that intentionally rely on the previous permissive behavior to wire a promptFn/eventsFn or expect the new error.
🤖 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/harness_reentry_bridge.go`:
- Around line 935-940: operationContext() currently strips cancellation (uses
context.WithoutCancel) which allows downstream I/O to ignore shutdown, so change
operationContext() in harnessReentryBridge to return the bridge's real
cancellable context (i.e., return b.ctx when non-nil, otherwise
context.Background()) so operations respect cancellation from shutdown(); also
audit any goroutines started by harnessReentryBridge methods to accept that
context and ensure they are tracked and awaited by the daemon shutdown (e.g.,
use the existing shutdown wait group or add one) rather than being
fire-and-forget.
In `@internal/extension/host_api_test.go`:
- Around line 1885-1897: The stubbed eventsFn should not branch on query.Limit
(which couples the test to submitPrompt's lookup shape); instead make eventsFn
return the same event set required for the missing-boundary scenario regardless
of query.Limit (or detect the semantic intent via the query's filter/turn id if
present) so the missing-boundary test relies on the returned event content
(e.g., SessionEvent with TurnID "turn-agent" and Type acp.EventTypeAgentMessage)
rather than the numeric Limit; update the eventsFn in the test to remove the
query.Limit conditional and always produce the event(s) needed to exercise the
missing-boundary behavior.
---
Duplicate comments:
In `@internal/daemon/daemon_test.go`:
- Around line 4150-4223: PromptSynthetic calls recordSyntheticEvent which
swallows json.Marshal errors, causing PromptSynthetic to return success even
when synthetic event serialization fails; change recordSyntheticEvent on
fakeSessionManager to return an error (propagate json.Marshal errors) and update
PromptSynthetic to capture and return that error instead of ignoring it so
serialization failures surface to callers/tests; update any call sites for
recordSyntheticEvent accordingly (keep function name recordSyntheticEvent and
the fakeSessionManager receiver).
In `@internal/daemon/prompt_input_composite.go`:
- Around line 314-323: The per-descriptor cap is bypassed when descriptor.Budget
== 0 because descriptorBudget is initialized to remainingBudget and only
overwritten when descriptor.Budget > 0; change the logic so that when limited is
true you always apply the descriptor's explicit budget (including zero) — i.e.,
set descriptorBudget = min(remainingBudget, descriptor.Budget) when limited is
true (instead of only when descriptor.Budget > 0) so that descriptor.Budget == 0
results in a zero descriptorBudget before calling
applyPromptInputAugmenterBudget.
In `@internal/daemon/task_runtime_test.go`:
- Around line 456-639: The test function
TestTaskRuntimeDetachedHarnessSubmissionPersistsMetadataAndReusesIdempotency
should be converted to use the repo’s required subtest pattern: wrap the
existing test body inside t.Run("Should persist metadata and reuse idempotency",
func(t *testing.T) { t.Parallel(); ... }) so the assertions and setup remain
identical but run as a named subtest; locate the function by its name
TestTaskRuntimeDetachedHarnessSubmissionPersistsMetadataAndReusesIdempotency and
move its current contents into the inner t.Run closure, ensuring t.Parallel() is
called inside the subtest, and apply the same wrapping to the other new test
cases referenced in the review (the other test functions/ranges noted) so all
top-level cases use t.Run("Should ...", func(t *testing.T) { t.Parallel(); ...
}).
---
Nitpick comments:
In `@internal/extension/host_api_test.go`:
- Around line 5376-5398: The Events and Prompt stub methods should fail by
default like Stop does to catch unexpected calls: change
promptSessionManagerStub.Events to return a non-nil error (e.g.,
errors.New("unexpected events call")) when s.eventsFn is nil, and change
promptSessionManagerStub.Prompt to return an error (e.g., nil,
errors.New("unexpected prompt call")) when s.promptFn is nil (instead of
returning a closed channel), so unexpected interactions in
HostAPIHandler.submitPrompt are surfaced; update any tests that intentionally
rely on the previous permissive behavior to wire a promptFn/eventsFn or expect
the new error.
🪄 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: 6dbbf9eb-b73e-44f9-9d88-41e6c763b2c6
⛔ Files ignored due to path filters (21)
.compozy/tasks/harness/reviews-001/_meta.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-002/_meta.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-002/issue_001.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-002/issue_002.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-002/issue_003.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-002/issue_004.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-002/issue_005.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-002/issue_006.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-002/issue_007.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-002/issue_008.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-002/issue_009.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-002/issue_010.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-002/issue_011.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-002/issue_012.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-002/issue_013.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-002/issue_014.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-002/issue_015.mdis excluded by!**/*.md.compozy/tasks/harness/reviews-002/issue_016.mdis excluded by!**/*.mdopenapi/agh.jsonis excluded by!**/*.jsonsdk/typescript/src/generated/contracts.tsis excluded by!**/generated/**web/src/generated/agh-openapi.d.tsis excluded by!**/generated/**
📒 Files selected for processing (12)
internal/api/contract/tasks.gointernal/daemon/daemon_integration_test.gointernal/daemon/daemon_test.gointernal/daemon/harness_context.gointernal/daemon/harness_reentry_bridge.gointernal/daemon/prompt_input_composite.gointernal/daemon/task_runtime_test.gointernal/extension/host_api_tasks.gointernal/extension/host_api_test.gointernal/session/stop_reason_test.gointernal/task/manager.gointernal/task/manager_test.go
✅ Files skipped from review due to trivial changes (2)
- internal/extension/host_api_tasks.go
- internal/daemon/daemon_integration_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/api/contract/tasks.go
- internal/session/stop_reason_test.go
This was referenced Apr 19, 2026
Closed
Closed
Merged
Closed
This was referenced May 24, 2026
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 was referenced May 28, 2026
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
Improvements
Bug Fixes
Tests