refactor: hermes adjustments#69
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 (64)
📒 Files selected for processing (38)
WalkthroughIntroduces a shared tool process registry and scoped interrupts, durable scheduler state and run claiming, expanded failure classification/redaction with crash bundles, MCP OAuth2-PKCE auth and token persistence, memory operation history and endpoints, observability retention, many API/DB schema changes, and broad CLI features and tests. Changes
Sequence Diagram(s)(Skipped — changes span multiple distinct subsystems and do not present a single focused 3+ component sequential flow to visualize simply.) 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: 5
Note
Due to the large number of review comments, Critical 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 (7)
internal/api/httpapi/prompt.go (1)
137-151:⚠️ Potential issue | 🟠 MajorCancel prompt before detached drain begins.
Current ordering cancels only after draining, so the producer can continue running until drain timeout. This can keep prompt work alive after client disconnect and delay shutdown.
Suggested fix
func (h *Handlers) drainPromptEventsAsync( ctx context.Context, events <-chan acp.AgentEvent, cancelPrompt context.CancelFunc, ) { - if h == nil || cancelPrompt == nil { + if h == nil { return } + if cancelPrompt != nil { + cancelPrompt() + } if ctx == nil { - cancelPrompt() return } drainCtx, cancelDrain := context.WithTimeout(ctx, detachedPromptDrainTimeout) h.promptDrainWG.Go(func() { defer cancelDrain() - defer cancelPrompt() h.drainPromptEvents(drainCtx, events) }) }Also applies to: 163-174
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/httpapi/prompt.go` around lines 137 - 151, The code currently calls h.drainPromptEventsAsync(...) and only then cancels via cancelOnReturn, allowing the producer to keep running during the drain; change the ordering so cancelOnReturn() is invoked before starting the detached drain to stop the producer promptly, then set cancelOnReturn = nil and call h.drainPromptEventsAsync(context.WithoutCancel(c.Request.Context()), events, cancelOnReturn) immediately afterwards; apply the same reorder in both places where drainPromptEventsAsync is invoked (the branch after state.emit error and the branch handling closed events/finish involving state.finish and h.StreamDoneChannel()) and ensure you nil out cancelOnReturn after calling it.internal/api/core/more_coverage_test.go (1)
166-181:⚠️ Potential issue | 🟡 MinorAssert the mapped failure summary too.
The fixture sets
Failure.Summary, but the assertion only verifiesKind. If summary propagation breaks, this test still passes.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/api/core/more_coverage_test.go` around lines 166 - 181, Add an assertion to verify the mapped failure summary matches the fixture: after the nil/Kind checks for agentEvent.Failure, assert that agentEvent.Failure.Summary == "permission policy denied" (or use the same literal/variable from the fixture) and call t.Fatalf with a descriptive message if it does not match; this ensures the Summary field from store.SessionFailure is propagated correctly.internal/cli/session.go (1)
451-484:⚠️ Potential issue | 🟡 MinorAdd
stop_detailto Toon output for parity with human output.Line 419 exposes
Stop Detailin human format, but Toon omits it. This drops useful diagnostic context for structured CLI consumers.🔧 Proposed fix
return renderToonObject("session", []string{ "id", "name", "agent_name", "provider", "environment_backend", "workspace", "channel", "state", "stop_reason", + "stop_detail", "failure_kind", "failure_summary", "crash_bundle_path", "acp_session_id", "created_at", "updated_at", }, []string{ info.ID, info.Name, info.AgentName, info.Provider, sessionEnvironmentBackend(info), displaySessionWorkspace(info), info.Channel, string(info.State), string(info.StopReason), + info.StopDetail, sessionFailureKind(info), sessionFailureSummary(info), sessionCrashBundlePath(info), info.ACPSessionID, formatTime(info.CreatedAt), formatTime(info.UpdatedAt), }), nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/session.go` around lines 451 - 484, The Toon output omits the "stop_detail" field; update the toon closure that calls renderToonObject("session", ...) to include "stop_detail" in the header list and add the corresponding value (use the existing helper sessionStopDetail(info)) in the values slice so Toon matches the human output; modify the toon function that builds the two string slices (header names and values) to insert "stop_detail" and sessionStopDetail(info) in the same relative position as the human rendering.internal/store/globaldb/global_db_automation.go (1)
508-516:⚠️ Potential issue | 🟠 MajorList/filter runs by an effective event timestamp, not only
started_at.Runs that fail during reservation or delivery can now have
scheduled_at/delivery_error_atset whilestarted_atstays NULL. Keeping both the sort order and theSince/Untilpredicates anchored tostarted_atmeans recent delivery failures fall to the bottom and can disappear entirely from time-windowed history queries.Also applies to: 1429-1438
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/store/globaldb/global_db_automation.go` around lines 508 - 516, The query currently orders and filters by started_at only, which hides runs that never started but have scheduled_at or delivery_error_at; change ordering and time-window predicates to use an effective event timestamp (e.g., COALESCE(started_at, delivery_error_at, scheduled_at) or a GREATEST/COALESCE expression) instead of started_at alone. Update the SQL in global_db_automation.go where sqlQuery is built (the sqlQuery variable, AppendWhere/AppendLimit usage) and update buildAutomationRunClauses to apply the same effective timestamp for Since/Until predicates; make the same change in the other occurrence referenced (lines ~1429-1438) so both ORDER BY and any time range filters use the effective timestamp. Ensure the chosen expression is used consistently in SELECT/ORDER BY and in WHERE clauses so runs with scheduled_at or delivery_error_at are correctly included and sorted.internal/memory/store.go (1)
802-846: 🛠️ Refactor suggestion | 🟠 MajorAccept caller context for mutation operations to enable proper cancellation of catalog/database work.
The
Write()andDelete()methods should acceptcontext.Contextas the first parameter and propagate it throughsyncScopeAfterMutation()andlogMutationEvent()to the underlying catalog operations. Currently, these methods are called from API handlers (where request context is available) but cannot pass it, forcing the mutation helpers to usecontext.Background(). This makes catalog sync and history logging uncancellable, leaving them vulnerable to indefinite hangs on stuck database operations.Per coding guidelines, functions crossing runtime boundaries should accept context and avoid
context.Background()outside main and focused tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/memory/store.go` around lines 802 - 846, The mutation helpers (Write/Delete) must accept a context.Context and propagate it so catalog/database work can be cancelled: change the signatures of Write and Delete to take ctx context.Context as the first parameter, update calls to syncScopeAfterMutation and logMutationEvent to pass that ctx, and modify syncScopeAfterMutation(ctx context.Context, scope Scope, filename string) and logMutationEvent(ctx context.Context, action string, scope Scope, filename string) so they use ctx when calling syncScope(ctx, ...) and logCatalogEvent(ctx, ...). Ensure logCatalogEvent is called with the incoming ctx (it already accepts ctx) and remove uses of context.Background() inside these mutation-path helpers so all catalog sync/logging uses the caller-provided context.internal/automation/manager.go (1)
1971-1991:⚠️ Potential issue | 🟠 MajorDon’t use the caller context for post-write scheduler reconciliation.
By the time
applyJobToRuntimeruns, the store mutation has already succeeded at its call sites. If the API/CLI context is canceled here,Register/Updatecan fail even though the durable job change is committed, and the rollback paths above reuse that same canceled context. That leaves the persisted job definition and in-memory scheduler out of sync until restart. Use a manager-owned runtime context, orcontext.WithoutCancel(ctx)with a short timeout, for the scheduler mutation path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/automation/manager.go` around lines 1971 - 1991, In applyJobToRuntime, avoid using the caller context for scheduler reconciliation: create and use a manager-owned non-cancelable runtime context (or wrap ctx with context.WithoutCancel and a short timeout) when calling scheduler.State, scheduler.Update, and scheduler.Register so transient cancellation of the API/CLI caller doesn’t make Update/Register fail after the durable store changed; replace uses of the incoming ctx in Manager.applyJobToRuntime for the post-write scheduler mutation path (State, Update, Register) with that new runtime context and ensure rollback/retry paths also use it rather than the original caller context.internal/config/provider.go (1)
461-517:⚠️ Potential issue | 🟠 MajorName-based merges can't clear inherited remote config.
mergeMCPServerandmergeMCPAuthConfigonly copy non-empty override fields. That means an overlay can switch a base server totransport=stdio, but it cannot remove inheritedurlorauth, so the merged config stays invalid and the override cannot take effect. This needs an explicit clearing or full-replacement path when transport/auth changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/config/provider.go` around lines 461 - 517, mergeMCPServer and mergeMCPAuthConfig currently only copy non-empty fields which prevents an overlay from clearing inherited fields when changing transport or auth; update mergeMCPServer so that when override.Transport is provided and differs from the base transport you reset transport-dependent fields (e.g., set merged.URL = "" and set merged.Auth to an empty MCPAuthConfig via clone zero) before applying the override.Transport and other override fields, and update mergeMCPAuthConfig so that when override.Type is provided and differs from base.Type you perform a full-replacement (return a clone of override or set merged = cloneMCPAuthConfig(override)) instead of merging individual fields; also ensure an explicit empty/zero override.Auth (override.IsZero()) can clear inherited auth when desired.
🟠 Major comments (26)
internal/config/release_config_test.go-11-84 (1)
11-84: 🛠️ Refactor suggestion | 🟠 MajorRefactor into
t.Run("Should...")subtests (table-driven where repeated).This test is currently monolithic; split assertions into subtests so failures are isolated and guideline-compliant.
♻️ Suggested structure
func TestGoReleaserConfigPreservesTrustArtifactsAndPackageTargets(t *testing.T) { t.Parallel() root := findRepoRootForReleaseConfigTest(t) data, err := os.ReadFile(filepath.Join(root, ".goreleaser.yml")) if err != nil { t.Fatalf("os.ReadFile(.goreleaser.yml) error = %v", err) } var cfg map[string]any if err := yaml.Unmarshal(data, &cfg); err != nil { t.Fatalf("yaml.Unmarshal(.goreleaser.yml) error = %v", err) } - - checksum := mapAt(t, cfg, "checksum") - ... - nfpms := sliceAt(t, cfg, "nfpms") - ... + + t.Run("Should preserve checksum configuration", func(t *testing.T) { + t.Parallel() + checksum := mapAt(t, cfg, "checksum") + if got, want := stringAt(t, checksum, "name_template"), "checksums.txt"; got != want { + t.Fatalf("checksum.name_template = %q, want %q", got, want) + } + if got, want := stringAt(t, checksum, "algorithm"), "sha256"; got != want { + t.Fatalf("checksum.algorithm = %q, want %q", got, want) + } + }) + + for _, artifact := range []string{"archive", "package", "source"} { + artifact := artifact + t.Run("Should preserve sbom artifact "+artifact, func(t *testing.T) { + t.Parallel() + assertSBOMArtifact(t, sliceAt(t, cfg, "sboms"), artifact) + }) + } }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 pattern for Go tests".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/config/release_config_test.go` around lines 11 - 84, The test TestGoReleaserConfigPreservesTrustArtifactsAndPackageTargets is monolithic—refactor it into t.Run subtests (use "Should..." naming) and convert repeated assertions into table-driven subtests; for example, wrap top-level checks (checksum, signs, sboms via assertSBOMArtifact, homebrew_casks, nfpms) into separate t.Run blocks and for repeated checks (like verifying sbom artifact types and nfpm formats) create table-driven loops that call t.Run for each case, keeping helper calls (findRepoRootForReleaseConfigTest, mapAt, stringAt, sliceAt, asMap, stringSliceContains, assertSBOMArtifact) intact and moving their related assertions into the appropriate subtest closures so failures are isolated.internal/skills/provenance_test.go-80-106 (1)
80-106:⚠️ Potential issue | 🟠 MajorAdd
t.Run()subtest and replace string error matching with typed error assertionThis test violates two coding guidelines: it bypasses the required
t.Run("ShouldRejectSymlinkEscape", ...)subtest pattern and relies on brittlestrings.Contains(err.Error(), "reject hashed symlink")matching at line 103.Required changes:
- Wrap test logic in
t.Run("ShouldRejectSymlinkEscape", func(t *testing.T) { ... })- Define a sentinel error (e.g.,
var ErrSymlinkEscape = errors.New(...)) inprovenance.goand return it fromComputeDirectoryHash()- Replace line 103's string check with
errors.Is(err, ErrSymlinkEscape)or a typed error assertion viaerrors.As()The production code currently wraps errors with message text only (
fmt.Errorf("skills: reject hashed symlink %q: %w", ...)), but must expose a comparable error identity for proper testing per coding guidelines: "Useerrors.Is()anderrors.As()for error matching — never compare error strings."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/skills/provenance_test.go` around lines 80 - 106, The test should be converted into a t.Run subtest and assert a typed sentinel error instead of matching the error string: wrap the existing test body inside t.Run("ShouldRejectSymlinkEscape", func(t *testing.T) { ... }), add a package-level sentinel error (e.g., var ErrSymlinkEscape = errors.New("reject hashed symlink")) in provenance.go, have ComputeDirectoryHash return that sentinel (or wrap it with fmt.Errorf(...%w...)) when a hashed symlink escape is detected, and update the test to replace strings.Contains(err.Error(), ...) with errors.Is(err, ErrSymlinkEscape) (or errors.As if using a typed error).internal/bridgesdk/errors_test.go-229-261 (1)
229-261:⚠️ Potential issue | 🟠 MajorRefactor added cases into
t.Run("Should...")table-driven subtests.The new scenarios are bundled as inline assertions rather than explicit
Should...subtests, which makes case isolation/reporting weaker and conflicts with the enforced test pattern.As per coding guidelines, "
**/*_test.go: Use table-driven tests with subtests (t.Run) as default pattern for Go tests" and "MUST use t.Run(\"Should...\") pattern for ALL test cases".Also applies to: 291-331
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/bridgesdk/errors_test.go` around lines 229 - 261, Refactor TestRetryDoAppliesDefaultsAndStopsWhenDelayContextCancels to use t.Run subtests (pattern "Should...") and a table-driven structure: split the initial default-attempt case and the cancel-during-delay case into separate t.Run("Should...") subtests (each exercising RetryDo with the appropriate RetryConfig, OnRetry callback, context cancellation and assertions), and if there are multiple similar scenarios around lines 291-331 fold them into a loop over a slice of test cases that calls t.Run(tc.name, func(t *testing.T){...}) while keeping references to the same functions/types (RetryDo, RetryConfig, OnRetry, TransientError) so each case is isolated and reported independently.internal/retry/retry_test.go-10-203 (1)
10-203:⚠️ Potential issue | 🟠 MajorAlign this suite with the required
Should...subtest pattern.The scenarios are strong, but the file should be organized as table-driven
t.Run("Should...")subtests to match the repository’s mandatory test style.As per coding guidelines, "
**/*_test.go: Use table-driven tests with subtests (t.Run) as default pattern for Go tests" and "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/retry/retry_test.go` around lines 10 - 203, Refactor the tests so each scenario is a subtest using t.Run("Should...") (keeping existing names/semantics) instead of standalone Test* functions: create a single parent test (or groupings) that iterates a table of cases and calls t.Run with descriptive "Should ..." names for each original scenario (e.g., the logic inside TestDoRetriesUntilSuccessWithDeterministicJitter, TestDoStopsAtMaxAttemptsAndDoesNotSleepAfterFinalFailure, TestDoHonorsContextCancellation, TestDoValueStopsOnNonRetryableAndContextErrors, TestDoRejectsNilInputs, TestWaitHonorsContextCancellation, TestWaitHandlesNilContextAndNonPositiveDelay, TestDelayAppliesJitterBoundsAndCap); move t.Parallel() into each subtest as appropriate, preserve all assertions and helper usage (DoValue, Do, Wait, Delay, equalDurations, sequenceRand, etc.), and keep test behavior identical while updating names to follow the "Should ..." pattern.internal/bridgesdk/errors.go-344-345 (1)
344-345:⚠️ Potential issue | 🟠 MajorWrap retry wait failures with stage context.
This return path drops call-site context. Please wrap the error so downstream logs/diagnostics show that cancellation/failure happened during retry backoff waiting.
Suggested fix
if err := retrypkg.Wait(ctx, delay); err != nil { - return zero, err + return zero, fmt.Errorf("bridge retry wait: %w", err) }As per coding guidelines, "Use explicit error returns with wrapped context:
fmt.Errorf(\"context: %w\", err)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/bridgesdk/errors.go` around lines 344 - 345, The error returned from retrypkg.Wait(ctx, delay) is returned raw (return zero, err) losing stage context; change the return to wrap the error with contextual text (e.g., "retry wait" or "waiting for retry backoff") using fmt.Errorf so downstream logs show this occurred during the retry wait stage, and add fmt to imports if needed—target the retrypkg.Wait call (with variable delay) and the return that currently uses zero and err.internal/retry/retry.go-77-95 (1)
77-95:⚠️ Potential issue | 🟠 MajorWrap retry-loop propagated errors with attempt/stage context.
These returns currently pass upstream errors through raw, which makes failure provenance harder to trace during operations.
Suggested fix
import ( "context" "errors" + "fmt" "math" "math/rand" "time" ) @@ if ctxErr := ctx.Err(); ctxErr != nil { - return zero, ctxErr + return zero, fmt.Errorf("retry: context done after attempt %d: %w", attempt, ctxErr) } if isContextError(err) || attempt == policy.MaxAttempts || !retryable(shouldRetry, err) { - return zero, err + return zero, fmt.Errorf("retry: attempt %d failed: %w", attempt, err) } @@ if err := policy.Sleep(ctx, delay); err != nil { - return zero, err + return zero, fmt.Errorf("retry: wait before attempt %d failed: %w", attempt+1, err) }As per coding guidelines, "Use explicit error returns with wrapped context:
fmt.Errorf(\"context: %w\", err)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/retry/retry.go` around lines 77 - 95, The retry loop is returning raw errors which loses provenance; update the three return points to wrap errors with attempt/stage context using fmt.Errorf so callers see which attempt failed. Specifically, when returning ctx.Err(), wrap it like fmt.Errorf("context canceled during retry attempt %d/%d: %w", attempt, policy.MaxAttempts, ctxErr); when returning the propagated err (the branch checking isContextError/attempt==MaxAttempts/!retryable), wrap as fmt.Errorf("retry failed on attempt %d/%d: %w", attempt, policy.MaxAttempts, err); and when returning the error from policy.Sleep(ctx, delay), wrap it as fmt.Errorf("sleep failed on attempt %d/%d after delay %s: %w", attempt, policy.MaxAttempts, delay, err). Use the existing symbols Delay, Attempt, policy.OnRetry, retryable and policy.Sleep to locate the code to change.internal/diagnostics/redact.go-12-14 (1)
12-14:⚠️ Potential issue | 🟠 MajorQuoted JSON secrets still bypass redaction.
secretPatternonly matches unquotedkey:value/key=valueshapes, so inputs like"access_token":"abc"or"refresh_token":"abc"remain intact unless the value also happens to match the bearer-token regex. Since this helper feeds persisted diagnostics, that can leak credentials into crash bundles and failure summaries.internal/cli/lifecycle.go-201-203 (1)
201-203:⚠️ Potential issue | 🟠 MajorTreat “already exited” as success during uninstall.
There’s a race between
daemonInfo(...running)andsignalProcess. If the daemon exits in that window,SIGTERMcan returnESRCH/os.ErrProcessDoneand uninstall aborts even though the target state is already satisfied. This should continue to artifact cleanup instead of failing the command.Suggested change
if err := deps.signalProcess(info.PID, syscall.SIGTERM); err != nil { + if errors.Is(err, syscall.ESRCH) || errors.Is(err, os.ErrProcessDone) { + return false, nil + } return false, fmt.Errorf("cli: stop daemon for uninstall: %w", err) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/lifecycle.go` around lines 201 - 203, The uninstall flow should treat "process already exited" as success: in the block that calls deps.signalProcess (the call using info.PID), catch the error and if it is syscall.ESRCH or os.ErrProcessDone (use errors.Is to match) then swallow it and continue to artifact cleanup instead of returning an error; only return the formatted error ("cli: stop daemon for uninstall: %w") for other error values. Ensure you reference deps.signalProcess and the surrounding uninstall logic that follows the earlier daemonInfo(...running) check.internal/diagnostics/redact.go-29-37 (1)
29-37:⚠️ Potential issue | 🟠 MajorNon-positive budgets currently disable the bound entirely.
When
maxBytes <= 0, this returns the full redacted payload instead of a bounded result. That breaks the function contract and can still overflow storage limits if a caller passes0or a negative budget.Suggested change
func RedactAndBound(text string, maxBytes int) string { redacted := strings.TrimSpace(Redact(text)) - if maxBytes <= 0 || len(redacted) <= maxBytes { + if maxBytes <= 0 { + return "" + } + if len(redacted) <= maxBytes { return redacted } if maxBytes <= len("...[truncated]") { return redacted[:maxBytes] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/diagnostics/redact.go` around lines 29 - 37, The function RedactAndBound currently treats maxBytes <= 0 as "no bound" and returns the full redacted text; change it so non-positive budgets are enforced: in RedactAndBound, if maxBytes <= 0 return an empty string (or a minimal bounded value) immediately, otherwise proceed with the existing truncation logic using the redacted variable and the "...[truncated]" suffix; ensure you update the initial conditional that now reads "if maxBytes <= 0 || len(redacted) <= maxBytes" to first handle maxBytes <= 0, then the len(redacted) <= maxBytes check, and keep the existing truncation branch that computes redacted[:maxBytes-len("...[truncated]")] + "...[truncated]".internal/session/prompt_activity.go-21-22 (1)
21-22:⚠️ Potential issue | 🟠 MajorAvoid hardcoded stop timeout in runtime supervision path.
Line 21 introduces a fixed
5sdeadline that overrides runtime configuration when stopping timed-out sessions. This makes stop behavior non-configurable and inconsistent with the rest of supervision settings.Suggested fix
const ( runtimeActivityKindPromptStarted = "prompt_started" runtimeActivityKindAgentWaiting = "agent_waiting" runtimeActivityKindWarning = "warning" runtimeActivityKindTimeout = "timeout" - - runtimeTimeoutStopDeadline = 5 * time.Second ) @@ - stopCtx, stopCancel := context.WithTimeout(context.WithoutCancel(s.ctx), runtimeTimeoutStopDeadline) + stopCtx, stopCancel := context.WithTimeout(context.WithoutCancel(s.ctx), s.config.TimeoutCancelGrace)As per coding guidelines, "Never hardcode configuration — use TOML config or functional options."
Also applies to: 268-269
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/session/prompt_activity.go` around lines 21 - 22, The hardcoded runtimeTimeoutStopDeadline = 5 * time.Second must be replaced with a configurable value: remove the fixed declaration and read the stop timeout from the existing supervision configuration or a functional option (e.g., a field on the session/manager config or a StopTimeout option) and fall back to a sensible default if unset; update all uses of runtimeTimeoutStopDeadline (including the runtime supervision stop path referenced around the other occurrences) to use the configured value (e.g., cfg.StopTimeout or opts.StopTimeout) so the timeout is not hardcoded and can be set via TOML or options.internal/session/liveness.go-32-35 (1)
32-35:⚠️ Potential issue | 🟠 MajorPreserve an existing
meta.Failureinstead of overwriting it.These recovery branches always replace
meta.Failurewith a new{Kind, Summary}pair. If the persisted session already carried richer failure data, such as a crash-bundle path, restart recovery silently discards it. Prefer starting frommeta.Failureand only filling missingKind/Summarybefore normalizing.Also applies to: 42-45, 52-55
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/session/liveness.go` around lines 32 - 35, The current recovery branches overwrite existing failure metadata by assigning next.Failure = normalizeSessionFailure(&store.SessionFailure{...}, next.StopDetail); instead, preserve any existing failure data (meta.Failure / next.Failure) and only populate missing Kind and Summary fields before normalizing: read the existing next.Failure (or meta.Failure) as a base, set Kind = store.FailureProcess and/or Summary = next.StopDetail only if they are empty, then call normalizeSessionFailure on that enriched failure object; apply the same change in the other recovery branches referenced (the assignments at the other two locations: lines 42-45 and 52-55).internal/environment/daytona/provider.go-38-38 (1)
38-38:⚠️ Potential issue | 🟠 MajorWire
processRegistryinto the Daytona tool-host path.This option only stores the registry on
daytonaProvider; nothing in this file actually readsp.processRegistry. As a result, Daytona-owned tool processes still bypass the shared registry and won't participate in the new lifecycle/interrupt tracking.Also applies to: 100-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/environment/daytona/provider.go` at line 38, The daytonaProvider stores processRegistry but never wires it into the Daytona tool-host creation path, so Daytona-owned tool processes bypass the shared registry; update the code that constructs the Daytona tool host (the factory/constructor used in this file where daytonaProvider creates tool hosts) to accept and use p.processRegistry—either by passing p.processRegistry into the constructor (e.g., NewDaytonaToolHost/NewToolHost) or by setting the host's registry field after creation (host.Registry = p.processRegistry) so that spawned Daytona processes participate in the shared lifecycle/interrupt tracking.internal/api/core/automation_test.go-781-887 (1)
781-887: 🛠️ Refactor suggestion | 🟠 MajorRefactor into subtests using
t.Run("Should...")pattern.This test validates three distinct functions (AutomationHealthPayloadFromStatus, JobPayloadsFromJobs, RunPayloadFromRun) with sequential assertions, but lacks the required
t.Runsubtests. Split into named subtests to match the repository's mandatory test pattern:t.Run("Should expose scheduler state in health payload", ...) t.Run("Should include scheduler state in job payload", ...) t.Run("Should expose delivery error in run payload", ...)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/core/automation_test.go` around lines 781 - 887, The test currently exercises three separate behaviors in one function; refactor TestAutomationPayloadsExposeSchedulerStateAndDeliveryErrors into three t.Run subtests named "Should expose scheduler state in health payload", "Should include scheduler state in job payload", and "Should expose delivery error in run payload". For each subtest, move the related setup and assertions into its own t.Run closure and call t.Parallel() inside each subtest; keep shared fixtures (dates/schedulerState) defined in the outer test if needed or duplicate minimal setup per subtest. Ensure the assertions remain the same and reference the tested functions AutomationHealthPayloadFromStatus, JobPayloadsFromJobs, and RunPayloadFromRun respectively.internal/acp/types.go-516-525 (1)
516-525:⚠️ Potential issue | 🟠 MajorWrap checkpoint error with operation context per coding guidelines.
Line 520 returns the underlying error directly without wrapped context. Use
fmt.Errorf()to add diagnostic information consistent with other error returns in this package.🔧 Proposed fix
func (p *AgentProcess) checkpointProcessOwner(ctx context.Context) error { if p == nil || p.processRecord == nil { return nil } - return p.processRecord.Checkpoint(ctx, toolruntime.ProcessCheckpoint{ + if err := p.processRecord.Checkpoint(ctx, toolruntime.ProcessCheckpoint{ Owner: &toolruntime.ProcessOwner{ SessionID: p.SessionID, }, - }) + }); err != nil { + return fmt.Errorf("acp: checkpoint process owner: %w", err) + } + return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/acp/types.go` around lines 516 - 525, checkpointProcessOwner currently returns the raw error from p.processRecord.Checkpoint; change it to wrap that error with contextual information using fmt.Errorf (e.g., fmt.Errorf("checkpointProcessOwner: failed to checkpoint process %s: %w", p.SessionID, err)). Locate the function checkpointProcessOwner on type AgentProcess and wrap the error returned by p.processRecord.Checkpoint(ctx, toolruntime.ProcessCheckpoint{...}) so the returned error includes operation context and the original error via %w.internal/procutil/process_group_windows.go-10-23 (1)
10-23:⚠️ Potential issue | 🟠 MajorWindows implementation violates the documented blocking contract for process group shutdown.
WaitForProcessGroupIDExitreturns success immediately without waiting, andKillProcessGroupIDAndWaitonly signals without waiting for the process group to exit. The Unix implementation polls until the group is gone or the deadline is exceeded. Callers expect blocking semantics afterKillProcessGroupIDAndWaitcompletes—they proceed with cleanup, restart, and port reuse with the assumption that the process tree has been torn down. This implementation allows child processes to remain running while callers proceed, creating races. Return an explicit unsupported error or implement actual process group termination and wait behavior instead of returning success prematurely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/procutil/process_group_windows.go` around lines 10 - 23, The Windows implementation currently returns success immediately from WaitForProcessGroupIDExit and only signals in KillProcessGroupIDAndWait, violating the blocking contract; update these functions (WaitForProcessGroupIDExit, KillProcessGroupIDAndWait, and possibly SignalProcessGroupID) to either implement proper termination-and-wait semantics (e.g., enumerate and terminate child processes for pgid, then poll/wait until none remain or deadline expires) or, if that cannot be implemented safely on Windows, return a clear, documented "unsupported on Windows" error from WaitForProcessGroupIDExit and KillProcessGroupIDAndWait so callers do not assume teardown; ensure the error type/message is unique and used consistently so callers can detect and handle the unsupported case.internal/daemon/agent_probes.go-78-85 (1)
78-85:⚠️ Potential issue | 🟠 MajorDon't enqueue empty provider probes after resolution failures.
When
ResolveProviderfails,provider.Commandis the zero value, but the code still appends a target. That turns a config resolution problem into a guaranteed failed probe on every health pass. Skip the target after logging, or fall back to the raw configured command.🛠 Suggested fix
provider, err := cfg.ResolveProvider(name) - if err != nil && logger != nil { - logger.Warn("daemon: resolve provider for probe health failed", "provider", name, "error", err) - } + if err != nil { + if logger != nil { + logger.Warn("daemon: resolve provider for probe health failed", "provider", name, "error", err) + } + continue + } targets = append(targets, acp.ProbeTarget{ Provider: name, Command: strings.TrimSpace(provider.Command), })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/daemon/agent_probes.go` around lines 78 - 85, The code currently appends an acp.ProbeTarget using provider.Command even when cfg.ResolveProvider(name) returns an error, causing empty/invalid probes to be enqueued; in the block around ResolveProvider in agent_probes.go (look for ResolveProvider, logger.Warn, and the targets = append(... acp.ProbeTarget{ Provider: name, Command: strings.TrimSpace(provider.Command) })), change the logic to skip appending the probe when err != nil (after logging the warning) or, if you prefer, use the original configured command as a fallback; ensure you only append a ProbeTarget when you have a non-empty command (trimmed) to avoid creating guaranteed-failing probes.internal/session/crash_bundle.go-173-182 (1)
173-182:⚠️ Potential issue | 🟠 MajorPreserve the timestamp when truncating crash bundle names.
base[:crashBundleNameMaxBytes]trims from the front of the assembled name, so a longsessionIDcan chop off theUnixNanosuffix entirely. Repeated failures for the same session/kind then reuse the same filename and overwrite earlier crash bundles.Suggested fix
func crashBundleFileName(sessionID string, kind store.FailureKind, ts time.Time) string { - base := strings.Join([]string{ - sanitizeCrashBundleName(sessionID), - sanitizeCrashBundleName(string(kind)), - fmt.Sprintf("%d", ts.UnixNano()), - }, "-") - if len(base) > crashBundleNameMaxBytes { - base = base[:crashBundleNameMaxBytes] - } - return base + ".json" + sessionPart := sanitizeCrashBundleName(sessionID) + if sessionPart == "" { + sessionPart = "unknown" + } + kindPart := sanitizeCrashBundleName(string(kind)) + tsPart := fmt.Sprintf("%d", ts.UnixNano()) + suffix := "-" + kindPart + "-" + tsPart + maxSessionBytes := crashBundleNameMaxBytes - len(suffix) + if maxSessionBytes < len("unknown") { + maxSessionBytes = len("unknown") + } + if len(sessionPart) > maxSessionBytes { + sessionPart = sessionPart[:maxSessionBytes] + } + return sessionPart + suffix + ".json" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/session/crash_bundle.go` around lines 173 - 182, The current crashBundleFileName builds base by joining sanitized sessionID, kind, and ts.UnixNano but truncates with base[:crashBundleNameMaxBytes], which can drop the timestamp; change truncation to preserve the timestamp suffix: after assembling base (before adding ".json"), if len(base) > crashBundleNameMaxBytes then set base = base[len(base)-crashBundleNameMaxBytes:] so the UnixNano timestamp (from ts.UnixNano()) remains in the resulting filename; keep references to crashBundleFileName, base, sanitizeCrashBundleName, ts.UnixNano(), and crashBundleNameMaxBytes when making the edit.internal/mcp/auth/metadata.go-19-21 (1)
19-21:⚠️ Potential issue | 🟠 MajorAdd a timeout to the default metadata client.
Falling back to
http.DefaultClientmeans discovery has no client timeout, so a slow or wedged issuer can hang login/refresh indefinitely unless every caller always supplies a deadline onctx. Prefer a dedicated client with a sane timeout, or require callers to provide one explicitly.As per coding guidelines, external-call hazards like blocking calls without timeouts on request threads must be fixed before release.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/mcp/auth/metadata.go` around lines 19 - 21, Replace the fallback to http.DefaultClient with a dedicated HTTP client that enforces a sane timeout (e.g., set client = &http.Client{Timeout: 10 * time.Second}) to avoid indefinite blocking when the issuer is slow; update the code around the client variable assignment in internal/mcp/auth/metadata.go (the branch that currently does `if client == nil { client = http.DefaultClient }`) and ensure you import time, or alternatively change the function signature to require a non-nil client and return an error if client == nil so callers must provide one.internal/memory/store.go-466-475 (1)
466-475:⚠️ Potential issue | 🟠 MajorScope operation health metrics to the same visible workspaces.
operationStats(ctx)is aggregated across the entire shared catalog, while the entry/orphan counts above are filtered toworkspaces. In a multi-workspace daemon,HealthStats(..., []string{workspaceA})will report operation counts and last-operation timestamps produced by workspaceB.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/memory/store.go` around lines 466 - 475, operationStats(ctx) is returning metrics for the whole catalog while IndexedFiles/OrphanedFiles are already filtered to the current visible workspaces; change the code to compute operation metrics scoped to the same workspaces by calling or adding a workspace-scoped variant (e.g. s.catalog.operationStatsForWorkspaces(ctx, workspaces) or similar) and use its returned operationCount and lastOperationAt when constructing HealthStats so all fields (IndexedFiles, OrphanedFiles, OperationCount, LastOperationAt) are consistently filtered to the same workspaces.internal/mcp/auth/metadata.go-68-88 (1)
68-88:⚠️ Potential issue | 🟠 MajorRequire HTTPS for OAuth discovery and credential endpoints.
Both
resolveMetadataURL()andvalidateAbsoluteHTTPURL()accept plaintext HTTP for critical OAuth endpoints, enabling credential interception during metadata discovery, token exchange, and revocation flows. Apply HTTPS enforcement to:
- Line 68–88:
resolveMetadataURL()— enforcehttps://scheme only- Line 113–121:
validateAbsoluteHTTPURL()— rejecthttp://schemeFor local development, carve out an explicit loopback-only exception (e.g.,
localhostor127.0.0.1).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/mcp/auth/metadata.go` around lines 68 - 88, Enforce HTTPS-only for OAuth discovery and credential endpoints by updating resolveMetadataURL and validateAbsoluteHTTPURL: after parsing the URL, reject any scheme that is not "https" unless the host is a loopback address (allow "http" only for localhost, 127.0.0.1, or [::1]); specifically ensure parsed.Scheme == "https" or (scheme == "http" && hostIsLoopback(parsed.Host)), and return a clear error like "mcp auth: insecure URL; https required" when rejected; add/replace a small helper hostIsLoopback function or inline check to detect "localhost" and the loopback IPs and apply the same logic in both resolveMetadataURL and validateAbsoluteHTTPURL.internal/mcp/auth/metadata.go-81-88 (1)
81-88:⚠️ Potential issue | 🟠 MajorBuild the RFC 8414 well-known URL in the correct order.
Per RFC 8414, for issuers with a path component, the metadata endpoint must be
https://host/.well-known/oauth-authorization-server/<issuer-path>. The current code at line 85 appends the well-known path after the issuer path, resulting inhttps://host/<issuer-path>/.well-known/oauth-authorization-server, which will never resolve successfully.Suggested fix
- parsed.Path = strings.TrimRight(parsed.Path, "/") + metadataWellKnownPath + issuerPath := strings.TrimRight(parsed.Path, "/") + parsed.Path = metadataWellKnownPath + if issuerPath != "" { + parsed.Path += issuerPath + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/mcp/auth/metadata.go` around lines 81 - 88, The code builds the well-known URL in the wrong order; update the construction so the RFC 8414 well-known segment is placed immediately under the host and the original issuer path follows it (not vice versa). Concretely, after validating parsed (from url.Parse(issuer)), set parsed.Path to combine metadataWellKnownPath first and then the issuer path (trim leading/trailing slashes appropriately to avoid double slashes), clear parsed.RawQuery and parsed.Fragment, and return parsed.String(); reference parsed, issuer, and metadataWellKnownPath to locate the change.internal/observe/health.go-140-149 (1)
140-149:⚠️ Potential issue | 🟠 MajorInclude failure-health degradation in the top-level status.
collectFailureHealthcan mark the failures section as degraded, but the overallHealth.Statusignores it. That means the daemon can report top-level"ok"while simultaneously surfacing persisted lifecycle failures.Suggested fix
- Status: observeHealthStatus(persistenceHealth.Status, agentProbeStatus(agentProbes)), + Status: observeHealthStatus( + persistenceHealth.Status, + failureHealth.Status, + agentProbeStatus(agentProbes), + ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/observe/health.go` around lines 140 - 149, The top-level Health.Status ignores degradation reported by collectFailureHealth (failureHealth), so the daemon can show "ok" despite failures; update the status computation used when building Health (the call to observeHealthStatus(...)) to also consider failureHealth.Status (and treat a degraded/failed failureHealth as degrading the overall status), e.g., compute a combined status from persistenceHealth.Status, agentProbeStatus(agentProbes) and failureHealth.Status and pass that into observeHealthStatus or replace the direct call so Health.Status reflects failureHealth degradations.internal/cli/config.go-1146-1159 (1)
1146-1159:⚠️ Potential issue | 🟠 MajorReplace
strings.Fields()withshellquote.Split()to properly handle EDITOR/VISUAL values with spaces and quotes.
strings.Fields()splits on any whitespace without respecting shell quoting or escapes. This breaks real-world cases like:
EDITOR="/path with spaces/vim"→ incorrectly splits into separate argumentsEDITOR="vim --option 'value with spaces'"→ loses quoted contextThe codebase already uses
github.com/kballard/go-shellquoteextensively for this exact pattern (seeinternal/acp/client.goandinternal/session/manager_hooks.go). Replace withshellquote.Split()to handle POSIX shell word-splitting rules correctly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/config.go` around lines 1146 - 1159, In runConfigEditor replace strings.Fields(editor) with shellquote.Split(editor) so EDITOR/VISUAL values respect shell quoting/escapes; update the import to include github.com/kballard/go-shellquote, change the parts variable assignment to call shellquote.Split and handle its returned ([]string, error) by returning an error if split fails, and keep the existing len(parts)==0 check before building exec.CommandContext(parts[0], append(parts[1:], path)...).internal/store/globaldb/global_db_mcp_auth.go-32-58 (1)
32-58:⚠️ Potential issue | 🟠 MajorPersisting raw OAuth tokens in the DB is a credential exposure risk.
access_tokenandrefresh_tokenare written verbatim intomcp_auth_tokens. Anyone who can read the global DB file gets live bearer/refresh credentials for the remote MCP server. Please move these secrets to an OS keychain or encrypt them before persistence.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/store/globaldb/global_db_mcp_auth.go` around lines 32 - 58, The code currently persists raw OAuth tokens (normalized.AccessToken, normalized.RefreshToken) into the mcp_auth_tokens table; instead store only an encrypted blob or a keychain reference: replace writing normalized.AccessToken/RefreshToken in the INSERT/ON CONFLICT with either encrypted values produced by a helper (e.g., EncryptToken(ctx, plaintext) / DecryptToken(ctx,ciphertext)) or with a generated keychain ID saved via SaveTokenToKeychain(serverName, token) and persist that ID; update all readers that load from mcp_auth_tokens to call the corresponding DecryptToken/GetTokenFromKeychain before returning tokens; add/use a secure root (environment-provided envelope key or OS keychain APIs) and ensure nullableTime, store.FormatTimestamp usage remains unchanged while adjusting the parameter order to match the modified INSERT placeholders.internal/acp/client.go-239-245 (1)
239-245:⚠️ Potential issue | 🟠 MajorCancel the per-process context when registry setup fails.
If
Registerfails here, the subprocess is stopped, but theprocCtxpassed intonewLocalToolHostFromPolicystays live. Any tool-host or terminal goroutines bound to that context can outlive the failed start. CallcancelProcess()before returning from this cleanup path.Suggested fix
if err := d.registerAgentProcess(ctx, process); err != nil { + cancelProcess() stopCtx, cancelStop := context.WithTimeout(context.Background(), d.stopTimeout) defer cancelStop() if stopErr := handle.Stop(stopCtx); stopErr != nil { return nil, errors.Join(err, fmt.Errorf("acp: cleanup unregistered agent process: %w", stopErr)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/acp/client.go` around lines 239 - 245, When registerAgentProcess(ctx, process) fails, the per-process context (procCtx created for newLocalToolHostFromPolicy) is not canceled and can leak goroutines; modify the error cleanup path in the block that checks if err := d.registerAgentProcess(...) to call cancelProcess() before performing handle.Stop and returning. Specifically, ensure cancelProcess() is invoked immediately after detecting the register error (prior to stopCtx/handle.Stop and prior to returning the error) so any goroutines using procCtx are terminated when registration fails.internal/store/globaldb/global_db_automation_scheduler.go-440-452 (1)
440-452:⚠️ Potential issue | 🟠 MajorUse structured error checking instead of string matching.
Line 442 uses
strings.Contains(strings.ToLower(err.Error()), ...)to detect constraint violations, which violates the coding guideline requiringerrors.Is()or structured error inspection.The codebase already uses the correct pattern with
modernc.org/sqlite: check for*sqlite.Errorusing type assertion and inspect the error code (seeinternal/extension/registry.go:755). Replace the string matching with:var sqliteErr *sqlite.Error if errors.As(err, &sqliteErr) && sqliteErr.Code()&0xff == sqlite3.SQLITE_CONSTRAINT_UNIQUE { return fmt.Errorf(...) }Note: This same pattern should also be applied to
internal/store/globaldb/global_db_workspace.goandinternal/store/globaldb/global_db_automation.go, which use string matching for similar constraint checks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/store/globaldb/global_db_automation_scheduler.go` around lines 440 - 452, Replace the string-based constraint check in the create automation run error handling with structured SQLite error inspection: use errors.As to assert the error to *sqlite.Error (e.g., into a variable sqliteErr) and test sqliteErr.Code()&0xff == sqlite3.SQLITE_CONSTRAINT_UNIQUE to detect the unique-constraint-on-fire_id case, then return the same wrapped automation.ErrScheduledFireAlreadyClaimed; apply the identical pattern wherever similar string matching is used (notably in global_db_workspace.go and global_db_automation.go) to consistently replace strings.Contains(err.Error(), ...) with errors.As + sqlite error code checking.
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (6)
internal/session/crash_bundle_test.go (1)
27-29: Use timestamp-derived assertions instead of hardcoded"101"/"202"substrings.These checks are a bit brittle and less explicit. Derive expected timestamp tokens from
first/secondso the assertion tracks the actual test inputs.Suggested refactor
import ( + "strconv" "strings" "testing" "time" @@ - if !strings.Contains(firstName, "101") || !strings.Contains(secondName, "202") { + if !strings.Contains(firstName, strconv.FormatInt(first.UnixNano(), 10)) || + !strings.Contains(secondName, strconv.FormatInt(second.UnixNano(), 10)) { t.Fatalf("crash bundle names = %q, %q; want timestamp suffixes preserved", firstName, secondName) }As per coding guidelines, "MUST test meaningful business logic, not trivial operations".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/session/crash_bundle_test.go` around lines 27 - 29, The test currently asserts hardcoded substrings "101"/"202" in crash bundle file names (variables firstName and secondName); instead, extract the expected timestamp tokens from the test inputs (variables first and second) and assert those tokens are present in firstName and secondName respectively so the assertion reflects actual input data; locate the assertion block referencing firstName/secondName and replace the hardcoded checks with checks that derive expected suffixes from first/second (or their timestamp-formatting logic used elsewhere in the test) and assert strings.Contains(firstName, expectedFromFirst) and strings.Contains(secondName, expectedFromSecond).internal/session/prompt_activity_test.go (1)
214-229: Expand this test to lock in alltimeoutStopDeadline()branches.Line 225 currently validates only the configured positive-grace path. The method also guarantees fallback behavior for nil supervisor and non-positive grace; adding those as table-driven subtests will make regressions much harder.
📌 Suggested refactor (table-driven subtests)
func TestPromptActivitySupervisorTimeoutStopDeadline(t *testing.T) { t.Parallel() - t.Run("ShouldUseConfiguredTimeoutCancelGraceForForcedStopDeadline", func(t *testing.T) { - t.Parallel() - - supervisor := &promptActivitySupervisor{ - config: aghconfig.SessionSupervisionConfig{ - TimeoutCancelGrace: 42 * time.Millisecond, - }, - } - if got, want := supervisor.timeoutStopDeadline(), 42*time.Millisecond; got != want { - t.Fatalf("timeoutStopDeadline() = %s, want %s", got, want) - } - }) + defaultGrace := aghconfig.DefaultSessionSupervisionConfig().TimeoutCancelGrace + tests := []struct { + name string + supervisor *promptActivitySupervisor + want time.Duration + }{ + { + name: "ShouldUseConfiguredTimeoutCancelGraceForForcedStopDeadline", + supervisor: &promptActivitySupervisor{ + config: aghconfig.SessionSupervisionConfig{ + TimeoutCancelGrace: 42 * time.Millisecond, + }, + }, + want: 42 * time.Millisecond, + }, + { + name: "ShouldFallbackToDefaultWhenSupervisorIsNil", + supervisor: nil, + want: defaultGrace, + }, + { + name: "ShouldFallbackToDefaultWhenTimeoutCancelGraceIsNonPositive", + supervisor: &promptActivitySupervisor{ + config: aghconfig.SessionSupervisionConfig{TimeoutCancelGrace: 0}, + }, + want: defaultGrace, + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + if got := tc.supervisor.timeoutStopDeadline(); got != tc.want { + t.Fatalf("timeoutStopDeadline() = %s, want %s", got, tc.want) + } + }) + } }As per coding guidelines,
**/*_test.go: Use table-driven tests with subtests (t.Run) as default pattern for Go tests, and MUST test meaningful business logic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/session/prompt_activity_test.go` around lines 214 - 229, Add table-driven subtests covering all branches of promptActivitySupervisor.timeoutStopDeadline(): test a positive TimeoutCancelGrace (existing case), a zero and a negative TimeoutCancelGrace (each should fall back to the default grace value), and a nil receiver case ((*promptActivitySupervisor)(nil).timeoutStopDeadline()). Use t.Run for each row, set up promptActivitySupervisor with aghconfig.SessionSupervisionConfig{TimeoutCancelGrace: ...} where appropriate, and assert the returned duration equals the configured value for positive input and equals the known default (use the same default constant/logic used by timeoutStopDeadline()) for zero/negative/nil cases. Ensure tests are table-driven and parallelized as subtests.internal/session/resume_repair_test.go (1)
137-137: Use a fixed timestamp in this test for determinism.Using
time.Now().UTC()can make future assertions brittle if time-derived formatting/details become part of expectations. A fixed UTC value keeps this test fully reproducible.💡 Suggested tweak
- repaired, changed := ClassifyInactiveMetaForRecovery(time.Now().UTC(), meta) + fixedNow := time.Date(2026, time.January, 2, 3, 4, 5, 0, time.UTC) + repaired, changed := ClassifyInactiveMetaForRecovery(fixedNow, meta)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/session/resume_repair_test.go` at line 137, The test calls ClassifyInactiveMetaForRecovery with time.Now().UTC(), which makes the test non-deterministic; replace that call with a fixed UTC timestamp (e.g. a time.Date(...) value stored in a local variable like fixedNow) and pass that fixed value into ClassifyInactiveMetaForRecovery so the test is reproducible and assertions won't vary by current time.internal/observe/observer_test.go (1)
93-169: Prefer one table-driven retention test here.These two cases differ only by config and expected outcomes, so a single subtest table would remove duplication and make future retention branches easier to extend.
As per coding guidelines, "Use table-driven tests with subtests (
t.Run) as default pattern for Go tests".🤖 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 93 - 169, The two tests TestSweepRetentionUsesConfiguredCutoff and TestSweepRetentionDisabledKeepsHistory should be merged into a single table-driven test using subtests: create a slice of cases (name, RetentionConfig, sessionID, pre-seeded events, expected health fields like Enabled, RetentionDays, LastSweepStatus, LastCutoffAt, DeletedEventSummaries, and expected remaining events), loop over cases and call t.Run(case.name, func(t *testing.T){ ... }), reuse setup code (newHarness, h.observer.setRetentionHealth(h.observer.initialRetentionHealth()), newSession, h.observer.OnSessionCreated, h.recordEvent) and call h.observer.SweepRetention and h.observer.QueryEvents inside each subtest, asserting the case-specific expectations; reference existing symbols RetentionConfig, h.observer.SweepRetention, initialRetentionHealth, setRetentionHealth, newSession, h.recordEvent to locate and adapt the current logic into each subtest.internal/daemon/daemon.go (1)
559-563: Make the agent probe interval configurable.
2*time.Secondbakes daemon polling behavior into the composition root, so operators and tests cannot tune it without a code change. Please source this fromdeps.Config.Observabilityor an option instead of hardcoding it.As per coding guidelines,
Never hardcode configuration — use TOML config or functional options.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/daemon/daemon.go` around lines 559 - 563, The agent probe interval is hardcoded as 2*time.Second in the observe.WithAgentProbeSource call; make it configurable by reading a probe interval value from deps.Config.Observability (or add a new Observability.AgentProbeInterval field) and pass that value instead of the literal. Update the construction where observe.WithAgentProbeSource(agentProbeTargetSource(&deps.Config, deps.AgentCatalog, deps.Logger), 2*time.Second) is called to use the configured duration (with a sensible default/fallback if unset), and propagate the config field into any relevant composition or functional option so tests and operators can tune it without code changes.internal/memory/store_test.go (1)
1130-1266: Add a cross-workspace history subtest here.This scenario only ever exercises one workspace in the shared catalog, so it won't fail if
Historyloses the bound workspace default or ifscope=""events bleed across workspaces. A secondt.Run("Should isolate history by workspace")case would cover the real multi-workspace path these APIs are built around.As per coding guidelines,
**/*_test.go: MUST uset.Run("Should...")pattern for ALL test casesandFocus on critical paths: workflow execution, state management, error handling`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/memory/store_test.go` around lines 1130 - 1266, The test TestStoreOperationHistoryFiltersRedactsBoundsAndPersists only exercises one workspace and can miss cross-workspace bleed; add a t.Run("Should isolate history by workspace") subtest that creates a second workspace via NewStore(...).ForWorkspace(anotherRoot), writes at least one ScopeWorkspace and one global event (use workspaceStore.Write and workspaceStore.logCatalogEvent or the reopened store), then call History with OperationHistoryQuery scoped to the first workspace (Workspace: workspaceRoot, Scope: ScopeWorkspace) and assert results do not include records from the second workspace and vice versa; reference functions/types to touch: NewStore, ForWorkspace, Write, logCatalogEvent, History, OperationHistoryQuery, ScopeWorkspace, OperationWrite to ensure isolation.
🤖 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/client.go`:
- Around line 249-250: Replace the use of context.WithoutCancel (e.g. where
waitCtx := context.WithoutCancel(ctx) and go process.waitForExit(waitCtx)) with
the original caller context or a derived context that preserves deadlines (e.g.
pass ctx directly or use context.WithTimeout(ctx, <reasonable-duration>)) so
registry operations like Checkpoint and Complete aren't made unbounded; update
the same pattern at the other occurrences (the other context.WithoutCancel uses
noted in the comment) and ensure Stop and p.done are not blocked by operations
that lack deadlines.
In `@internal/cli/lifecycle.go`:
- Around line 277-291: The TOON renderer inside the anonymous toon func for
renderToonObject is missing the uninstall-specific fields daemon_stopped and
removed; update the field name slice (currently including
"command","status",...,"purged") to also include "daemon_stopped" and "removed",
and add the corresponding values from the lifecycleRecord into the values slice
(e.g., convert record.DaemonStopped to a string via fmt.Sprintf("%t") and
stringify record.Removed appropriately) so renderToonObject receives the same
uninstall fields that human/JSON outputs expose.
- Around line 143-163: The managed-uninstall flow currently validates
--purge/--force and calls loadRuntimeContext(deps) before checking
detectManagedState, causing managed installs to error on local validations and
runtime failures; change the logic in the uninstall handler to call
detectManagedState() first and if state.Managed is true immediately construct
the lifecycleRecord (using lifecycleRecord, lifecycleStatusDeferred,
managedRecommendation(state.Manager, "uninstall AGH")) and return
writeCommandOutput(cmd, lifecycleBundle("Uninstall", record)) without performing
the --purge validation or calling loadRuntimeContext(deps); only perform the
purge/force check and loadRuntimeContext when state.Managed is false.
- Around line 106-121: The update command currently calls deps.resolveHome()
before checking managed state, causing managed installs to fail if local home is
missing; move the detectManagedState(deps) call and the managed-install
short-circuit (creating lifecycleRecord with Command "update",
Managed/state.Manager, setting Status to lifecycleStatusDeferred, Message and
Recommendation via managedRecommendation) to run before calling
deps.resolveHome(), and only call deps.resolveHome() and set HomeDir on the
lifecycleRecord when state.Managed is false; ensure
writeCommandOutput(lifecycleBundle("Update", record)) is still returned for the
managed path.
In `@internal/cli/mcp_auth.go`:
- Around line 47-55: When mcpauth.NewService(db) fails, don't ignore the
db.Close(ctx) error or return the raw err; capture the close error and return a
wrapped error with context. Replace `_ = db.Close(ctx)` with code that does
`closeErr := db.Close(ctx)` and if closeErr != nil return nil, nil,
fmt.Errorf("mcpauth.NewService failed: %w; db.Close failed: %v", err,
closeErr)`, otherwise return nil, nil, fmt.Errorf("mcpauth.NewService failed:
%w", err). Refer to globaldb.OpenGlobalDB, mcpauth.NewService, db.Close, ctx and
homePaths.DatabaseFile to locate the relevant block.
In `@internal/daemon/daemon_test.go`:
- Around line 2236-2276: Wrap the entire test body of
TestRunShutsDownWhenObserverRetentionStartFails in a t.Run subtest with a
descriptive name starting with "Should..." (e.g., t.Run("Should shutdown when
observer retention start fails", func(t *testing.T) { ... })), ensuring you move
the existing t.Parallel() and all setup/assertions into that subtest block so
the test follows the required t.Run("Should...") pattern.
In `@internal/diagnostics/redact_test.go`:
- Around line 36-40: The loop over maxBytes in the test should be converted into
table-driven subtests that call t.Run for each case so failures are isolated;
replace the for _, maxBytes := range []int{0, -1} loop with a slice of test
cases (e.g., name and maxBytes) and run each case with t.Run("Should ...",
func(t *testing.T) { if got := RedactAndBound("token=super-secret",
tc.maxBytes); got != "" { t.Fatalf(...) } }), ensuring each subtest name follows
the "Should..." pattern and references RedactAndBound so results are checked
per-case.
In `@internal/mcp/auth/service_test.go`:
- Around line 27-84: The test's httptest handler (switch on
metadataWellKnownPath, "/token", "/revoke") and helper writeJSON must not call
t.Fatalf/t.Fatal from handler goroutines; instead create an error channel (e.g.,
errCh := make(chan error, 1) or an errors slice protected by mu) and have
handler code replace each t.Fatalf/t.Fatal and writeJSON-internal test failures
with sending errors to errCh (or return an error value from writeJSON). In
writeJSON change its signature to return error instead of calling t.Fatal*, and
have handlers check that error and send it to errCh; in the main test goroutine
select/receive from errCh after exercising the server and call t.Fatalf/t.Fatal
there if any handler-reported error exists; also stop using t.Fatal* inside
ParseForm error branches and instead send those errors to errCh. This keeps
refreshCalled/revokedToken/mu logic intact but ensures all test failures are
raised from the main test goroutine.
In `@internal/mcp/auth/service.go`:
- Around line 219-233: The current flow returns early on discoverMetadata or
s.revoke errors which prevents s.store.DeleteMCPAuthToken from running; change
the logic so metadata discovery and revoke attempts (discoverMetadata and
s.revoke) are best-effort: capture/log any metaErr or revokeErr but do NOT
return immediately, ensure s.store.DeleteMCPAuthToken(ctx, cfg.ServerName)
always runs (e.g., by restructuring the if-block or using a defer/cleanup path),
and then return statusFromToken(cfg, nil, s.now()), nil; keep unique calls
discoverMetadata, s.revoke, s.store.DeleteMCPAuthToken, and statusFromToken so
reviewers can find the changes.
- Around line 27-49: NewService currently assigns http.DefaultClient (which has
no timeout) causing network calls to potentially hang; change the constructor to
set service.client to a new http.Client with a sensible timeout (e.g., 10s) as
the default unless a ServiceOption overrides it: remove reliance on
http.DefaultClient and the dead-code fallback in discoverMetadata, initialize
service.client = &http.Client{Timeout: 10 * time.Second} in NewService (or
provide a ServiceOption to configure the timeout) so Login/Refresh/Logout
metadata discovery, code exchange, refresh and revocation requests cannot block
indefinitely.
In `@internal/memory/store.go`:
- Around line 349-356: The issue is that when opts.Scope is empty but
opts.Workspace resolves to a concrete workspace, you currently log records with
Scope="" which is treated as global; update the code that constructs the
OperationRecord for calls to s.logCatalogEvent (e.g., the OperationSearch and
OperationReindex records) to replace an empty normalized scope with a
workspace-specific scope: compute scopeNorm := scope.Normalize(); if scopeNorm
== "" && workspaceRoot != "" { scopeNorm = workspaceRoot }; then set
OperationRecord.Scope = scopeNorm so events are persisted with a concrete,
workspace-bound scope (apply the same change to the other logging site around
lines 394-404).
- Around line 487-490: The History method currently does manual normalization
(setting normalized.Scope, normalized.Workspace, normalized.Operation) but omits
the store-bound workspace behavior; replace that manual handling by calling the
existing normalizeScopeAndWorkspace helper so the store's workspace is applied
when query.Workspace is empty. Specifically, in History use
normalizeScopeAndWorkspace(&normalized, s.workspaceDir) (or the helper's actual
signature) to set Scope and Workspace, then still call Operation.Normalize() on
normalized.Operation if needed, removing the direct canonicalWorkspaceRoot and
Scope.Normalize calls in this block.
In `@internal/observe/health.go`:
- Around line 220-239: health.Recent is keeping the first 10 entries returned by
ListSessions instead of the 10 newest by UpdatedAt, so sort health.Recent by
UpdatedAt descending before you trim it: after populating health.Recent (but
before the len check/trim) use sort.Slice to order entries by UpdatedAt (newest
first), then slice to the first 10 entries; ensure you still preserve the
redaction fields and the existing Total/ByKind/Status logic (referencing
health.Recent, UpdatedAt, and the ListSessions population loop) so the "recent"
section reliably contains the newest failures.
In `@internal/observe/observer_test.go`:
- Around line 556-617: The test relies on Failures.Recent being sorted by
UpdatedAt but currently registers the newer "sess-crash" first so the test can
pass even if no sorting occurs; update the test setup in observer_test.go to
register the older session before the newer one (i.e., call
h.registry.RegisterSession for "sess-protocol" before "sess-crash") or, better,
scramble insertion order deliberately and then assert that observer.Health()
returns Failures.Recent sorted descending by UpdatedAt (check
h.observer.Health(), health.Failures.Recent, and the SessionID/FailureKind
ordering) so the test actually verifies the sorting behavior rather than
registry insertion order.
In `@internal/retry/retry_test.go`:
- Around line 159-164: The tests currently only assert that Do(...) returns a
non-nil error; update them to assert specific error content using
ErrorContains/ErrorAs (or the testing helper used in the repo) so regressions
are caught: for the call Do(nilRetryContext(), Policy{}, nil, func(...){...})
assert the error contains the expected "nil context" message or sentinel error,
and for Do(context.Background(), Policy{}, nil, nil) assert the error contains
the expected "nil operation" message (and make the same stronger assertion for
the similar checks at lines 184-186). Locate the assertions around the Do
function calls, nilRetryContext, and Policy in retry_test.go and replace the
generic non-nil checks with precise ErrorContains/ErrorAs assertions referencing
the expected error text or sentinel error values.
- Around line 248-274: The three test helper functions sequenceRand,
equalDurations, and nilRetryContext should be converted to accept a *testing.T
parameter and must call t.Helper() at the top so test failures point to the
caller; update their signatures to sequenceRand(t *testing.T, values
...float64), equalDurations(t *testing.T, left, right []time.Duration), and
nilRetryContext(t *testing.T) with t.Helper() as the first statement in each,
and then update all test callers to pass the *testing.T value.
---
Nitpick comments:
In `@internal/daemon/daemon.go`:
- Around line 559-563: The agent probe interval is hardcoded as 2*time.Second in
the observe.WithAgentProbeSource call; make it configurable by reading a probe
interval value from deps.Config.Observability (or add a new
Observability.AgentProbeInterval field) and pass that value instead of the
literal. Update the construction where
observe.WithAgentProbeSource(agentProbeTargetSource(&deps.Config,
deps.AgentCatalog, deps.Logger), 2*time.Second) is called to use the configured
duration (with a sensible default/fallback if unset), and propagate the config
field into any relevant composition or functional option so tests and operators
can tune it without code changes.
In `@internal/memory/store_test.go`:
- Around line 1130-1266: The test
TestStoreOperationHistoryFiltersRedactsBoundsAndPersists only exercises one
workspace and can miss cross-workspace bleed; add a t.Run("Should isolate
history by workspace") subtest that creates a second workspace via
NewStore(...).ForWorkspace(anotherRoot), writes at least one ScopeWorkspace and
one global event (use workspaceStore.Write and workspaceStore.logCatalogEvent or
the reopened store), then call History with OperationHistoryQuery scoped to the
first workspace (Workspace: workspaceRoot, Scope: ScopeWorkspace) and assert
results do not include records from the second workspace and vice versa;
reference functions/types to touch: NewStore, ForWorkspace, Write,
logCatalogEvent, History, OperationHistoryQuery, ScopeWorkspace, OperationWrite
to ensure isolation.
In `@internal/observe/observer_test.go`:
- Around line 93-169: The two tests TestSweepRetentionUsesConfiguredCutoff and
TestSweepRetentionDisabledKeepsHistory should be merged into a single
table-driven test using subtests: create a slice of cases (name,
RetentionConfig, sessionID, pre-seeded events, expected health fields like
Enabled, RetentionDays, LastSweepStatus, LastCutoffAt, DeletedEventSummaries,
and expected remaining events), loop over cases and call t.Run(case.name, func(t
*testing.T){ ... }), reuse setup code (newHarness,
h.observer.setRetentionHealth(h.observer.initialRetentionHealth()), newSession,
h.observer.OnSessionCreated, h.recordEvent) and call h.observer.SweepRetention
and h.observer.QueryEvents inside each subtest, asserting the case-specific
expectations; reference existing symbols RetentionConfig,
h.observer.SweepRetention, initialRetentionHealth, setRetentionHealth,
newSession, h.recordEvent to locate and adapt the current logic into each
subtest.
In `@internal/session/crash_bundle_test.go`:
- Around line 27-29: The test currently asserts hardcoded substrings "101"/"202"
in crash bundle file names (variables firstName and secondName); instead,
extract the expected timestamp tokens from the test inputs (variables first and
second) and assert those tokens are present in firstName and secondName
respectively so the assertion reflects actual input data; locate the assertion
block referencing firstName/secondName and replace the hardcoded checks with
checks that derive expected suffixes from first/second (or their
timestamp-formatting logic used elsewhere in the test) and assert
strings.Contains(firstName, expectedFromFirst) and strings.Contains(secondName,
expectedFromSecond).
In `@internal/session/prompt_activity_test.go`:
- Around line 214-229: Add table-driven subtests covering all branches of
promptActivitySupervisor.timeoutStopDeadline(): test a positive
TimeoutCancelGrace (existing case), a zero and a negative TimeoutCancelGrace
(each should fall back to the default grace value), and a nil receiver case
((*promptActivitySupervisor)(nil).timeoutStopDeadline()). Use t.Run for each
row, set up promptActivitySupervisor with
aghconfig.SessionSupervisionConfig{TimeoutCancelGrace: ...} where appropriate,
and assert the returned duration equals the configured value for positive input
and equals the known default (use the same default constant/logic used by
timeoutStopDeadline()) for zero/negative/nil cases. Ensure tests are
table-driven and parallelized as subtests.
In `@internal/session/resume_repair_test.go`:
- Line 137: The test calls ClassifyInactiveMetaForRecovery with
time.Now().UTC(), which makes the test non-deterministic; replace that call with
a fixed UTC timestamp (e.g. a time.Date(...) value stored in a local variable
like fixedNow) and pass that fixed value into ClassifyInactiveMetaForRecovery so
the test is reproducible and assertions won't vary by current time.
🪄 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: 863b5804-ea55-47b2-88fd-28508e405ed8
⛔ Files ignored due to path filters (32)
.compozy/tasks/hermes/reviews-001/_meta.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_001.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_002.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_003.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_004.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_005.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_006.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_007.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_008.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_009.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_010.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_011.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_012.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_013.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_014.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_015.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_016.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_017.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_018.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_019.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_020.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_021.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_022.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_023.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_024.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_025.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_026.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_027.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_028.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_029.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_030.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-001/issue_031.mdis excluded by!**/*.md
📒 Files selected for processing (46)
internal/acp/client.gointernal/acp/client_test.gointernal/acp/types.gointernal/api/core/automation_test.gointernal/bridgesdk/errors.gointernal/bridgesdk/errors_test.gointernal/cli/config.gointernal/cli/lifecycle.gointernal/cli/lifecycle_test.gointernal/cli/mcp_auth.gointernal/cli/mcp_auth_test.gointernal/config/release_config_test.gointernal/daemon/agent_probes.gointernal/daemon/agent_probes_test.gointernal/daemon/daemon.gointernal/daemon/daemon_test.gointernal/diagnostics/redact.gointernal/diagnostics/redact_test.gointernal/environment/daytona/provider.gointernal/environment/daytona/provider_test.gointernal/environment/daytona/tool_host.gointernal/mcp/auth/metadata.gointernal/mcp/auth/metadata_test.gointernal/mcp/auth/pkce.gointernal/mcp/auth/pkce_test.gointernal/mcp/auth/service.gointernal/mcp/auth/service_test.gointernal/memory/catalog.gointernal/memory/store.gointernal/memory/store_test.gointernal/observe/health.gointernal/observe/observer_test.gointernal/procutil/process_group_windows.gointernal/retry/retry.gointernal/retry/retry_test.gointernal/session/crash_bundle.gointernal/session/crash_bundle_test.gointernal/session/liveness.gointernal/session/prompt_activity.gointernal/session/prompt_activity_test.gointernal/session/resume_repair_test.gointernal/skills/provenance.gointernal/skills/provenance_test.gointernal/store/globaldb/global_db_automation_scheduler.gointernal/store/globaldb/global_db_mcp_auth.gointernal/store/globaldb/global_db_mcp_auth_test.go
✅ Files skipped from review due to trivial changes (2)
- internal/daemon/agent_probes.go
- internal/cli/config.go
🚧 Files skipped from review as they are similar to previous changes (10)
- internal/session/prompt_activity.go
- internal/procutil/process_group_windows.go
- internal/session/liveness.go
- internal/api/core/automation_test.go
- internal/cli/mcp_auth_test.go
- internal/diagnostics/redact.go
- internal/config/release_config_test.go
- internal/bridgesdk/errors_test.go
- internal/session/crash_bundle.go
- internal/cli/lifecycle_test.go
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
internal/cli/mcp_auth.go (1)
290-295: Ignored error on stderr write.Per the coding guidelines, errors should not be ignored with
_. While writing to stderr for terminal output rarely fails and is non-critical, adding a brief comment would satisfy the "written justification" requirement.Suggested justification comment
+ // Best-effort stderr output; failure is non-critical for CLI UX. _, _ = fmt.Fprintf( cmd.ErrOrStderr(), "Open this URL to authenticate %s:\n%s\n", cfg.ServerName, state.AuthorizationURL, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/mcp_auth.go` around lines 290 - 295, The stderr write currently ignores the returned error from fmt.Fprintf; update the call in the block that writes the auth URL (the fmt.Fprintf call using cmd.ErrOrStderr(), cfg.ServerName and state.AuthorizationURL) so the error is either captured and handled (e.g., assign to err and log or return it) or, if you intentionally want to ignore it, remove the blank assignment and add a one-line justification comment directly above the call explaining why the error is safe to ignore (e.g., non-critical terminal output). Ensure the change references the fmt.Fprintf invocation and cmd.ErrOrStderr() so reviewers can see the handling or justification.internal/acp/client.go (1)
891-898: Consider using a consistent logger reference.Line 896 uses
slog.Default()whileStopat line 670 usesd.logger. SincewaitForExitruns detached from the driver, consider storing the logger reference onAgentProcessduring construction for consistent log routing.♻️ Proposed approach
Add a logger field to
AgentProcessand set it innewAgentProcess:type AgentProcess struct { + logger *slog.Logger // ... other fields }Then in
waitForExit:- slog.Default().Warn("acp: complete process record", "pid", p.PID, "error", err) + p.logger.Warn("acp: complete process record", "pid", p.PID, "error", err)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/acp/client.go` around lines 891 - 898, The code in waitForExit uses slog.Default() for logging while other methods (like Stop) use the driver's logger; add a logger field to AgentProcess, set it in newAgentProcess (copy d.logger into the new AgentProcess instance), and replace slog.Default() calls in waitForExit (the block handling p.processRecord completion) with p.logger so all logs (e.g., the warning about Complete failing for p.PID) use the same logger reference.
🤖 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/cli/lifecycle.go`:
- Around line 119-121: The command-phase branches return raw errors from
deps.resolveHome and deps.loadRuntime; wrap these errors with context using
fmt.Errorf so callers know which phase failed (e.g., fmt.Errorf("resolving home
for update: %w", err) for the resolveHome call and fmt.Errorf("loading runtime
for uninstall: %w", err) for the loadRuntime call). Update the return statements
around calls to resolveHome and loadRuntime in lifecycle.go to wrap the
underlying err with descriptive context mentioning the operation and command
(update/uninstall) and import fmt if necessary.
- Around line 205-220: Wrap and annotate errors returned from helper calls so
failures carry context: when checking daemonInfo (the call returning info,
running, err) replace the bare "return false, err" with a wrapped error using
fmt.Errorf("daemonInfo: %w", err); likewise wrap the error returned from
waitForDaemonStop (the call waitForDaemonStop(ctx, deps, runtime, info)) as
"return false, fmt.Errorf(\"waitForDaemonStop: %w\", err)"; ensure any other
direct returns of dependency/helper errors in this block (e.g., after
signalProcess where you propagate non-ESRCH/ErrProcessDone errors) are returned
with contextual wrapping like fmt.Errorf("signalProcess: %w", err) so callers
get explicit context for uninstall failures.
- Around line 184-187: The purge branch currently calls
os.RemoveAll(runtime.HomePaths.HomeDir) without verifying the resolved path; add
a safety check before that call to reject unsafe targets (root, empty, ".", "/",
parent dirs, or paths outside the current user's home or a dedicated config
dir). Implement by resolving and cleaning the target
(filepath.Abs/filepath.Clean on runtime.HomePaths.HomeDir), obtain the current
user home (os.UserHomeDir()) or the dedicated safe parent (e.g.,
runtime.HomePaths.ConfigDir), and ensure the cleaned absolute target has the
safe parent as a prefix and is not equal to "/" or "."; if the invariant fails,
return a descriptive error instead of removing; only call os.RemoveAll when the
check passes (replace the direct RemoveAll call in the purge branch).
---
Nitpick comments:
In `@internal/acp/client.go`:
- Around line 891-898: The code in waitForExit uses slog.Default() for logging
while other methods (like Stop) use the driver's logger; add a logger field to
AgentProcess, set it in newAgentProcess (copy d.logger into the new AgentProcess
instance), and replace slog.Default() calls in waitForExit (the block handling
p.processRecord completion) with p.logger so all logs (e.g., the warning about
Complete failing for p.PID) use the same logger reference.
In `@internal/cli/mcp_auth.go`:
- Around line 290-295: The stderr write currently ignores the returned error
from fmt.Fprintf; update the call in the block that writes the auth URL (the
fmt.Fprintf call using cmd.ErrOrStderr(), cfg.ServerName and
state.AuthorizationURL) so the error is either captured and handled (e.g.,
assign to err and log or return it) or, if you intentionally want to ignore it,
remove the blank assignment and add a one-line justification comment directly
above the call explaining why the error is safe to ignore (e.g., non-critical
terminal output). Ensure the change references the fmt.Fprintf invocation and
cmd.ErrOrStderr() so reviewers can see the handling or justification.
🪄 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: 07a53835-b5d9-4222-a8c7-556dca1985cb
⛔ Files ignored due to path filters (63)
.agents/skills/favicon-gen/SKILL.mdis excluded by!**/*.md,!.agents/**.agents/skills/favicon-gen/assets/favicon-svg-circle.svgis excluded by!**/*.svg,!.agents/**.agents/skills/favicon-gen/assets/favicon-svg-shield.svgis excluded by!**/*.svg,!.agents/**.agents/skills/favicon-gen/assets/favicon-svg-square.svgis excluded by!**/*.svg,!.agents/**.agents/skills/favicon-gen/assets/manifest.webmanifestis excluded by!.agents/**.agents/skills/favicon-gen/references/extraction-methods.mdis excluded by!**/*.md,!.agents/**.agents/skills/favicon-gen/references/format-guide.mdis excluded by!**/*.md,!.agents/**.agents/skills/favicon-gen/references/monogram-patterns.mdis excluded by!**/*.md,!.agents/**.agents/skills/favicon-gen/references/shape-templates.mdis excluded by!**/*.md,!.agents/**.agents/skills/next-best-practices/SKILL.mdis excluded by!**/*.md,!.agents/**.agents/skills/next-best-practices/async-patterns.mdis excluded by!**/*.md,!.agents/**.agents/skills/next-best-practices/bundling.mdis excluded by!**/*.md,!.agents/**.agents/skills/next-best-practices/data-patterns.mdis excluded by!**/*.md,!.agents/**.agents/skills/next-best-practices/debug-tricks.mdis excluded by!**/*.md,!.agents/**.agents/skills/next-best-practices/directives.mdis excluded by!**/*.md,!.agents/**.agents/skills/next-best-practices/error-handling.mdis excluded by!**/*.md,!.agents/**.agents/skills/next-best-practices/file-conventions.mdis excluded by!**/*.md,!.agents/**.agents/skills/next-best-practices/font.mdis excluded by!**/*.md,!.agents/**.agents/skills/next-best-practices/functions.mdis excluded by!**/*.md,!.agents/**.agents/skills/next-best-practices/hydration-error.mdis excluded by!**/*.md,!.agents/**.agents/skills/next-best-practices/image.mdis excluded by!**/*.md,!.agents/**.agents/skills/next-best-practices/metadata.mdis excluded by!**/*.md,!.agents/**.agents/skills/next-best-practices/parallel-routes.mdis excluded by!**/*.md,!.agents/**.agents/skills/next-best-practices/route-handlers.mdis excluded by!**/*.md,!.agents/**.agents/skills/next-best-practices/rsc-boundaries.mdis excluded by!**/*.md,!.agents/**.agents/skills/next-best-practices/runtime-selection.mdis excluded by!**/*.md,!.agents/**.agents/skills/next-best-practices/scripts.mdis excluded by!**/*.md,!.agents/**.agents/skills/next-best-practices/self-hosting.mdis excluded by!**/*.md,!.agents/**.agents/skills/next-best-practices/suspense-boundaries.mdis excluded by!**/*.md,!.agents/**.agents/skills/seo-audit/SKILL.mdis excluded by!**/*.md,!.agents/**.agents/skills/seo-audit/evals/evals.jsonis excluded by!**/*.json,!.agents/**.agents/skills/seo-audit/references/ai-writing-detection.mdis excluded by!**/*.md,!.agents/**.agents/skills/seo-audit/references/international-seo.mdis excluded by!**/*.md,!.agents/**.claude/agents/designer.mdis excluded by!**/*.md,!.claude/**.codex/plans/2026-04-20-prompt-stream-stall.mdis excluded by!**/*.md.codex/plans/2026-04-20-session-chat-production-hardening.mdis excluded by!**/*.md.codex/plans/2026-04-25-site-hardening.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-002/_meta.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-002/issue_001.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-002/issue_002.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-002/issue_003.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-002/issue_004.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-002/issue_005.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-002/issue_006.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-002/issue_007.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-002/issue_008.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-002/issue_009.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-002/issue_010.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-002/issue_011.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-002/issue_012.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-002/issue_013.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-002/issue_014.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-002/issue_015.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-002/issue_016.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-002/issue_017.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-002/issue_018.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-002/issue_019.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-002/issue_020.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-002/issue_021.mdis excluded by!**/*.md.compozy/tasks/hermes/reviews-002/issue_022.mdis excluded by!**/*.mdAGENTS.mdis excluded by!**/*.mdCLAUDE.mdis excluded by!**/*.mdDESIGN.mdis excluded by!**/*.md
📒 Files selected for processing (37)
internal/acp/client.gointernal/acp/client_test.gointernal/cli/lifecycle.gointernal/cli/lifecycle_test.gointernal/cli/mcp_auth.gointernal/config/config.gointernal/config/config_test.gointernal/config/merge.gointernal/daemon/daemon.gointernal/daemon/daemon_test.gointernal/diagnostics/redact_test.gointernal/mcp/auth/service.gointernal/mcp/auth/service_test.gointernal/memory/store.gointernal/memory/store_test.gointernal/observe/health.gointernal/observe/observer_test.gointernal/retry/retry_test.gointernal/session/crash_bundle_test.gointernal/session/prompt_activity_test.gointernal/session/resume_repair_test.gopackages/site/app/(home)/layout.tsxpackages/site/app/error.tsxpackages/site/app/global.csspackages/site/app/layout.tsxpackages/site/app/not-found.tsxpackages/site/app/opengraph-image.tsxpackages/site/app/protocol/[[...slug]]/page.tsxpackages/site/app/protocol/layout.tsxpackages/site/app/robots.tspackages/site/app/runtime/[[...slug]]/page.tsxpackages/site/app/runtime/layout.tsxpackages/site/app/sitemap.tspackages/site/components/landing/final-cta.tsxpackages/site/components/landing/primitives/code-block.test.tsxpackages/site/components/landing/primitives/code-block.tsxpackages/site/components/landing/primitives/cta-button.tsx
✅ Files skipped from review due to trivial changes (1)
- internal/cli/lifecycle_test.go
Comment on lines
+119
to
+121
| homePaths, err := deps.resolveHome() | ||
| if err != nil { | ||
| return err |
There was a problem hiding this comment.
Wrap these command-phase errors.
These branches still return bare errors, so callers lose whether the failure came from resolving home during update or loading runtime during uninstall.
Suggested fix
homePaths, err := deps.resolveHome()
if err != nil {
- return err
+ return fmt.Errorf("cli: resolve AGH home for update: %w", err)
}
@@
runtime, err := loadRuntimeContext(deps)
if err != nil {
- return err
+ return fmt.Errorf("cli: load runtime context for uninstall: %w", err)
}As per coding guidelines **/*.go: Explicit error returns with wrapped context using fmt.Errorf("context: %w", err).
Also applies to: 160-162
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/cli/lifecycle.go` around lines 119 - 121, The command-phase branches
return raw errors from deps.resolveHome and deps.loadRuntime; wrap these errors
with context using fmt.Errorf so callers know which phase failed (e.g.,
fmt.Errorf("resolving home for update: %w", err) for the resolveHome call and
fmt.Errorf("loading runtime for uninstall: %w", err) for the loadRuntime call).
Update the return statements around calls to resolveHome and loadRuntime in
lifecycle.go to wrap the underlying err with descriptive context mentioning the
operation and command (update/uninstall) and import fmt if necessary.
Comment on lines
+184
to
+187
| if purge { | ||
| if err := os.RemoveAll(runtime.HomePaths.HomeDir); err != nil { | ||
| return fmt.Errorf("cli: purge AGH home %q: %w", runtime.HomePaths.HomeDir, err) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Definitions involved in purge path resolution:"
rg -n -C4 'func\s+(loadRuntimeContext|ResolveHome|ResolveHomePaths|ResolveHomePathsFrom)\b' internal
echo
echo "Delete sites and any nearby safety guards:"
rg -n -C3 'RemoveAll\(|filepath\.Clean|EvalSymlinks|IsAbs|unsafe|refus|root|HomeDir' internalRepository: compozy/agh
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== resolveAbsoluteDir implementation in home.go ==="
sed -n '1,150p' internal/config/home.go | tail -60
echo
echo "=== Full ResolveHomePathsFrom function ==="
sed -n '92,130p' internal/config/home.goRepository: compozy/agh
Length of output: 3507
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Full context around line 184-187 in lifecycle.go ==="
sed -n '175,200p' internal/cli/lifecycle.goRepository: compozy/agh
Length of output: 1020
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Full ResolvePath implementation ==="
grep -n "func ResolvePath" internal/config/home.go -A 30Repository: compozy/agh
Length of output: 1030
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check if there are any validations against unsafe paths like '/', '.', etc ==="
rg -n "HomeDir.*==|==.*HomeDir|forbidden|unsafe|reject.*root|IsAbs.*check" internal/config internal/cliRepository: compozy/agh
Length of output: 544
Add validation to prevent purge from deleting unsafe paths.
Line 185 recursively deletes runtime.HomePaths.HomeDir without verification. The path resolution in internal/config/home.go (via ResolvePath → filepath.Abs) accepts any path, including /, ., or other system directories. If AGH_HOME is set to an unsafe value, agh uninstall --purge --force can wipe unrelated data. Add an explicit invariant check before calling os.RemoveAll to reject paths that are not under a safe parent directory (e.g., user home or a dedicated config directory).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/cli/lifecycle.go` around lines 184 - 187, The purge branch currently
calls os.RemoveAll(runtime.HomePaths.HomeDir) without verifying the resolved
path; add a safety check before that call to reject unsafe targets (root, empty,
".", "/", parent dirs, or paths outside the current user's home or a dedicated
config dir). Implement by resolving and cleaning the target
(filepath.Abs/filepath.Clean on runtime.HomePaths.HomeDir), obtain the current
user home (os.UserHomeDir()) or the dedicated safe parent (e.g.,
runtime.HomePaths.ConfigDir), and ensure the cleaned absolute target has the
safe parent as a prefix and is not equal to "/" or "."; if the invariant fails,
return a descriptive error instead of removing; only call os.RemoveAll when the
check passes (replace the direct RemoveAll call in the purge branch).
Comment on lines
+205
to
+220
| info, running, err := daemonInfo(runtime.HomePaths, deps) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
| if !running { | ||
| return false, nil | ||
| } | ||
| if err := deps.signalProcess(info.PID, syscall.SIGTERM); err != nil { | ||
| if errors.Is(err, os.ErrProcessDone) || errors.Is(err, syscall.ESRCH) { | ||
| return true, nil | ||
| } | ||
| return false, fmt.Errorf("cli: stop daemon for uninstall: %w", err) | ||
| } | ||
| if _, err := waitForDaemonStop(ctx, deps, runtime, info); err != nil { | ||
| return false, err | ||
| } |
There was a problem hiding this comment.
Preserve helper context on uninstall failures.
Line 207, Line 219, and Line 234 still bubble dependency/helper errors up untouched, which makes daemon-stop and artifact-removal failures harder to triage.
Suggested fix
func stopDaemonForUninstall(ctx context.Context, deps commandDeps, runtime *runtimeContext) (bool, error) {
info, running, err := daemonInfo(runtime.HomePaths, deps)
if err != nil {
- return false, err
+ return false, fmt.Errorf("cli: inspect daemon state for uninstall: %w", err)
}
@@
if _, err := waitForDaemonStop(ctx, deps, runtime, info); err != nil {
- return false, err
+ return false, fmt.Errorf("cli: wait for daemon shutdown during uninstall: %w", err)
}
return true, nil
}
@@
for _, path := range candidates {
deleted, err := removeFileIfExists(path)
if err != nil {
- return nil, err
+ return nil, fmt.Errorf("cli: remove runtime artifacts: %w", err)
}As per coding guidelines **/*.go: Explicit error returns with wrapped context using fmt.Errorf("context: %w", err).
Also applies to: 232-234
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/cli/lifecycle.go` around lines 205 - 220, Wrap and annotate errors
returned from helper calls so failures carry context: when checking daemonInfo
(the call returning info, running, err) replace the bare "return false, err"
with a wrapped error using fmt.Errorf("daemonInfo: %w", err); likewise wrap the
error returned from waitForDaemonStop (the call waitForDaemonStop(ctx, deps,
runtime, info)) as "return false, fmt.Errorf(\"waitForDaemonStop: %w\", err)";
ensure any other direct returns of dependency/helper errors in this block (e.g.,
after signalProcess where you propagate non-ESRCH/ErrProcessDone errors) are
returned with contextual wrapping like fmt.Errorf("signalProcess: %w", err) so
callers get explicit context for uninstall failures.
This was referenced Apr 25, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Merged
This was referenced Apr 28, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
This was referenced May 23, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Merged
pedronauck
added a commit
that referenced
this pull request
May 26, 2026
## Release v0.0.1 This PR prepares the release of version v0.0.1. ### Changelog ## 0.0.1 - 2026-05-26 ### Other Changes - Lessons learned ### ♻️ Refactoring - Project structure (#7) - Kb improvements (#12) - Rename spaces to channels (#17) - Add extensions gaps (#21) - Improve tool calls ui (#22) - Remove web app header - Module improvements (#29) - Memory improvements (#35) - Storybook for web and ui (#38) - Enable AGH network by default for new installs (#57) - Hermes adjustments (#69) - Badges design (#84) - Storybook scenario and logos gallery - Migrate typescript tests (#114) - Internal go packages (#120) - Ui patterns (#127) - Improve e2e tests (#130) - Ui redesign - Workspace isolation across runtime surfaces (#145) - Prod ready applies (#162) - Tool card ui (#164) - Alpha on logo - Prod ready features (#167) - Thread sheet (#202) ### 🎉 Features - Implement config foundation packages - Implement sqlite store package - Add ACP client package - Add session lifecycle manager - Implement observe package - Add daemon composition root - Add uds api server - Implement cli package - Add http api server - Add system design - Add foundation types, schemas, and layout shell for web client - Add daemon health polling and agent sidebar systems for web client - Add session system CRUD, streaming core, and session store for web client - Add chat view, messages, and composer tests for web client - Add tool cards and renderers for web client - Add file-backed memory store core - Scaffold memory session seams - Add memory dream consolidation service - Wire memory assembler into daemon - Add memory api and cli - New skills system (#1) - Add workspace entity (#5) - Add new skill capabilities (#8) - Web ui v2 (#9) - Improve hooks system (#10) - Session resilience (#11) - Add extensability (#13) - Add automation (#16) - Add channels (#14) - Add network implementation (#15) - Add network, bridges and automations web pages (#18) - Ext registry (#20) - Add core tasks (#19) - Bridge adapters (#23) - Add site (#26) - Add ext refac and sandbox (#25) - Settings ui (#37) - Tasks ui (#36) - Harness improvements (#44) - Agent capabilities (#49) - Redesign ui (#48) - Unify capability (#53) - Redesign network workspace (#59) - Add task deletion and split session delete from stop (#58) - Session provider selection (#60) - Production grade adjustments (#66) - Autonomous system (#75) - Add agent session route (#80) - Tools registry (#85) - Agents soul (#88) - Add network threads (#105) - Orchestration improvements (#106) - Memory v2 (#108) - Agent categories (#113) - Providers model (#118) - Add canonical AGH bundled skill (#143) - Onboarding and improvements (#198) - Onboarding and improvements (#201) ### 🐛 Bug Fixes - Review round - Review rounds - Resolve memory extensibility review batch - Embed web into daemon - Defaults agents - Acp integration (#4) - Lint errors - Prd folder - Remove orphan web actions and dead surfaces (#55) - Qa testing and fixes (#73) - New review rounds (#82) - Security audit (#90) - Release qa round (#95) - Add missing tools (#141) - New qa round (#147) - Advanced qa round (#149) - Homebrew tap - Final review round (#151) - Daemon healthy - Reasoning models (#158) - Lint errors (#160) - Review round (#168) - Release adjustments (#171) - Stabilize release ci fixtures - Stabilize release integration gate - Stabilize release verify gates - Stabilize release integration flows - Stabilize release verify gates - Stabilize main verify shutdown - Ignore stale acpmock cancel - Marketplace search focus and filtering (#193) - Website video - Workspace command select ### 📚 Documentation - Update agents.md - Update prd - Update skills - Update compozy tasks - Update compozy - Update compozy - Add new skills - Archive prd - Update prds - Update rfc - Update prds - Update prds - Add automation prd - Channels prd - Update prd - Update prd - New prds - Archive prds - Bridges adapters prd - Sandbox prd - Update - Archive prd - Update - Add new prd - New design - Update prd - Archive prds - Update prds - Tasks-ui prd tasks - Update prd - Update design docs - Agent capabilities prd - Improve site docs - Remove old design references - Udpate - Autonomous prd - Update skills - Blog design - Agent sould prd - Final qa plan - Update - Remove codex ledgers from gitignore - Remove not needed files - Udpate ledger - Update cy-codex-loop skill - Orchestration improves prd - Update prds - Orch improvs prd - Memv2 prd - Providers model prd - Update refacs prd - New design proposal - Update rules - Update skills - New blog posts (#173) - Format docs - Remove old design files - Remove old - Skeeper update ### 📦 Build System - Initial structure - Commitlint - Frontend base structure - Update vscode settings - Add subagents - Coderabbit - Prd and tooling - Bun lock - Lint tooling - Copy.md and tooling adjusts - Add repoclone rc - Upgrade skeeper to v0.2.0 - Update go.mod - Adopt task artifacts into skeeper - Sync codex plans with skeeper - Skeeper lock - Skeeper lock - New skills - Skeeper lock - Skeeper lock - Skeeper lock - Update deps and go - Regenerate daytona sidecar assets for go 1.26.3 - Fix cliff - Ignore docs on fmt - Build web assets before goreleaser - Extend release dry-run timeout ### 🔧 CI/CD - Lint errors - Fint release pr - Fix goreleaser ### 🧪 Testing - Add e2e tests (#27) - Qa rounds (#78) - Improve test suite (#138) - Harden daemon-served restart reloads - Harden daemon-served readiness waits - Stabilize dashboard focus assertion - Stabilize release integration gates - Stabilize release e2e markers - Stabilize release e2e flows Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This was referenced May 26, 2026
Closed
Merged
pedronauck
added a commit
that referenced
this pull request
May 26, 2026
## Release v0.0.1 This PR prepares the release of version v0.0.1. ### Changelog ## 0.0.1 - 2026-05-26 ### Other Changes - Lessons learned ### ♻️ Refactoring - Project structure (#7) - Kb improvements (#12) - Rename spaces to channels (#17) - Add extensions gaps (#21) - Improve tool calls ui (#22) - Remove web app header - Module improvements (#29) - Memory improvements (#35) - Storybook for web and ui (#38) - Enable AGH network by default for new installs (#57) - Hermes adjustments (#69) - Badges design (#84) - Storybook scenario and logos gallery - Migrate typescript tests (#114) - Internal go packages (#120) - Ui patterns (#127) - Improve e2e tests (#130) - Ui redesign - Workspace isolation across runtime surfaces (#145) - Prod ready applies (#162) - Tool card ui (#164) - Alpha on logo - Prod ready features (#167) - Thread sheet (#202) ### 🎉 Features - Implement config foundation packages - Implement sqlite store package - Add ACP client package - Add session lifecycle manager - Implement observe package - Add daemon composition root - Add uds api server - Implement cli package - Add http api server - Add system design - Add foundation types, schemas, and layout shell for web client - Add daemon health polling and agent sidebar systems for web client - Add session system CRUD, streaming core, and session store for web client - Add chat view, messages, and composer tests for web client - Add tool cards and renderers for web client - Add file-backed memory store core - Scaffold memory session seams - Add memory dream consolidation service - Wire memory assembler into daemon - Add memory api and cli - New skills system (#1) - Add workspace entity (#5) - Add new skill capabilities (#8) - Web ui v2 (#9) - Improve hooks system (#10) - Session resilience (#11) - Add extensability (#13) - Add automation (#16) - Add channels (#14) - Add network implementation (#15) - Add network, bridges and automations web pages (#18) - Ext registry (#20) - Add core tasks (#19) - Bridge adapters (#23) - Add site (#26) - Add ext refac and sandbox (#25) - Settings ui (#37) - Tasks ui (#36) - Harness improvements (#44) - Agent capabilities (#49) - Redesign ui (#48) - Unify capability (#53) - Redesign network workspace (#59) - Add task deletion and split session delete from stop (#58) - Session provider selection (#60) - Production grade adjustments (#66) - Autonomous system (#75) - Add agent session route (#80) - Tools registry (#85) - Agents soul (#88) - Add network threads (#105) - Orchestration improvements (#106) - Memory v2 (#108) - Agent categories (#113) - Providers model (#118) - Add canonical AGH bundled skill (#143) - Onboarding and improvements (#198) - Onboarding and improvements (#201) ### 🐛 Bug Fixes - Review round - Review rounds - Resolve memory extensibility review batch - Embed web into daemon - Defaults agents - Acp integration (#4) - Lint errors - Prd folder - Remove orphan web actions and dead surfaces (#55) - Qa testing and fixes (#73) - New review rounds (#82) - Security audit (#90) - Release qa round (#95) - Add missing tools (#141) - New qa round (#147) - Advanced qa round (#149) - Homebrew tap - Final review round (#151) - Daemon healthy - Reasoning models (#158) - Lint errors (#160) - Review round (#168) - Release adjustments (#171) - Stabilize release ci fixtures - Stabilize release integration gate - Stabilize release verify gates - Stabilize release integration flows - Stabilize release verify gates - Stabilize main verify shutdown - Ignore stale acpmock cancel - Marketplace search focus and filtering (#193) - Website video - Workspace command select ### 📚 Documentation - Update agents.md - Update prd - Update skills - Update compozy tasks - Update compozy - Update compozy - Add new skills - Archive prd - Update prds - Update rfc - Update prds - Update prds - Add automation prd - Channels prd - Update prd - Update prd - New prds - Archive prds - Bridges adapters prd - Sandbox prd - Update - Archive prd - Update - Add new prd - New design - Update prd - Archive prds - Update prds - Tasks-ui prd tasks - Update prd - Update design docs - Agent capabilities prd - Improve site docs - Remove old design references - Udpate - Autonomous prd - Update skills - Blog design - Agent sould prd - Final qa plan - Update - Remove codex ledgers from gitignore - Remove not needed files - Udpate ledger - Update cy-codex-loop skill - Orchestration improves prd - Update prds - Orch improvs prd - Memv2 prd - Providers model prd - Update refacs prd - New design proposal - Update rules - Update skills - New blog posts (#173) - Format docs - Remove old design files - Remove old - Skeeper update ### 📦 Build System - Initial structure - Commitlint - Frontend base structure - Update vscode settings - Add subagents - Coderabbit - Prd and tooling - Bun lock - Lint tooling - Copy.md and tooling adjusts - Add repoclone rc - Upgrade skeeper to v0.2.0 - Update go.mod - Adopt task artifacts into skeeper - Sync codex plans with skeeper - Skeeper lock - Skeeper lock - New skills - Skeeper lock - Skeeper lock - Skeeper lock - Update deps and go - Regenerate daytona sidecar assets for go 1.26.3 - Fix cliff - Ignore docs on fmt - Build web assets before goreleaser - Extend release dry-run timeout ### 🔧 CI/CD - Lint errors - Fint release pr - Fix goreleaser - Fix release ### 🧪 Testing - Add e2e tests (#27) - Qa rounds (#78) - Improve test suite (#138) - Harden daemon-served restart reloads - Harden daemon-served readiness waits - Stabilize dashboard focus assertion - Stabilize release integration gates - Stabilize release e2e markers - Stabilize release e2e flows Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Merged
pedronauck
added a commit
that referenced
this pull request
May 26, 2026
## Release v0.0.2 This PR prepares the release of version v0.0.2. ### Changelog ## 0.0.2 - 2026-05-26 ### Other Changes - Lessons learned ### ♻️ Refactoring - Project structure (#7) - Kb improvements (#12) - Rename spaces to channels (#17) - Add extensions gaps (#21) - Improve tool calls ui (#22) - Remove web app header - Module improvements (#29) - Memory improvements (#35) - Storybook for web and ui (#38) - Enable AGH network by default for new installs (#57) - Hermes adjustments (#69) - Badges design (#84) - Storybook scenario and logos gallery - Migrate typescript tests (#114) - Internal go packages (#120) - Ui patterns (#127) - Improve e2e tests (#130) - Ui redesign - Workspace isolation across runtime surfaces (#145) - Prod ready applies (#162) - Tool card ui (#164) - Alpha on logo - Prod ready features (#167) - Thread sheet (#202) ### 🎉 Features - Implement config foundation packages - Implement sqlite store package - Add ACP client package - Add session lifecycle manager - Implement observe package - Add daemon composition root - Add uds api server - Implement cli package - Add http api server - Add system design - Add foundation types, schemas, and layout shell for web client - Add daemon health polling and agent sidebar systems for web client - Add session system CRUD, streaming core, and session store for web client - Add chat view, messages, and composer tests for web client - Add tool cards and renderers for web client - Add file-backed memory store core - Scaffold memory session seams - Add memory dream consolidation service - Wire memory assembler into daemon - Add memory api and cli - New skills system (#1) - Add workspace entity (#5) - Add new skill capabilities (#8) - Web ui v2 (#9) - Improve hooks system (#10) - Session resilience (#11) - Add extensability (#13) - Add automation (#16) - Add channels (#14) - Add network implementation (#15) - Add network, bridges and automations web pages (#18) - Ext registry (#20) - Add core tasks (#19) - Bridge adapters (#23) - Add site (#26) - Add ext refac and sandbox (#25) - Settings ui (#37) - Tasks ui (#36) - Harness improvements (#44) - Agent capabilities (#49) - Redesign ui (#48) - Unify capability (#53) - Redesign network workspace (#59) - Add task deletion and split session delete from stop (#58) - Session provider selection (#60) - Production grade adjustments (#66) - Autonomous system (#75) - Add agent session route (#80) - Tools registry (#85) - Agents soul (#88) - Add network threads (#105) - Orchestration improvements (#106) - Memory v2 (#108) - Agent categories (#113) - Providers model (#118) - Add canonical AGH bundled skill (#143) - Onboarding and improvements (#198) - Onboarding and improvements (#201) ### 🐛 Bug Fixes - Review round - Review rounds - Resolve memory extensibility review batch - Embed web into daemon - Defaults agents - Acp integration (#4) - Lint errors - Prd folder - Remove orphan web actions and dead surfaces (#55) - Qa testing and fixes (#73) - New review rounds (#82) - Security audit (#90) - Release qa round (#95) - Add missing tools (#141) - New qa round (#147) - Advanced qa round (#149) - Homebrew tap - Final review round (#151) - Daemon healthy - Reasoning models (#158) - Lint errors (#160) - Review round (#168) - Release adjustments (#171) - Stabilize release ci fixtures - Stabilize release integration gate - Stabilize release verify gates - Stabilize release integration flows - Stabilize release verify gates - Stabilize main verify shutdown - Ignore stale acpmock cancel - Marketplace search focus and filtering (#193) - Website video - Workspace command select ### 📚 Documentation - Update agents.md - Update prd - Update skills - Update compozy tasks - Update compozy - Update compozy - Add new skills - Archive prd - Update prds - Update rfc - Update prds - Update prds - Add automation prd - Channels prd - Update prd - Update prd - New prds - Archive prds - Bridges adapters prd - Sandbox prd - Update - Archive prd - Update - Add new prd - New design - Update prd - Archive prds - Update prds - Tasks-ui prd tasks - Update prd - Update design docs - Agent capabilities prd - Improve site docs - Remove old design references - Udpate - Autonomous prd - Update skills - Blog design - Agent sould prd - Final qa plan - Update - Remove codex ledgers from gitignore - Remove not needed files - Udpate ledger - Update cy-codex-loop skill - Orchestration improves prd - Update prds - Orch improvs prd - Memv2 prd - Providers model prd - Update refacs prd - New design proposal - Update rules - Update skills - New blog posts (#173) - Format docs - Remove old design files - Remove old - Skeeper update ### 📦 Build System - Initial structure - Commitlint - Frontend base structure - Update vscode settings - Add subagents - Coderabbit - Prd and tooling - Bun lock - Lint tooling - Copy.md and tooling adjusts - Add repoclone rc - Upgrade skeeper to v0.2.0 - Update go.mod - Adopt task artifacts into skeeper - Sync codex plans with skeeper - Skeeper lock - Skeeper lock - New skills - Skeeper lock - Skeeper lock - Skeeper lock - Update deps and go - Regenerate daytona sidecar assets for go 1.26.3 - Fix cliff - Ignore docs on fmt - Build web assets before goreleaser - Extend release dry-run timeout ### 🔧 CI/CD - Lint errors - Fint release pr - Fix goreleaser - Fix release - Fix release process ### 🧪 Testing - Add e2e tests (#27) - Qa rounds (#78) - Improve test suite (#138) - Harden daemon-served restart reloads - Harden daemon-served readiness waits - Stabilize dashboard focus assertion - Stabilize release integration gates - Stabilize release e2e markers - Stabilize release e2e flows - Improve suite speed Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This was referenced May 26, 2026
Closed
Closed
Merged
pedronauck
added a commit
that referenced
this pull request
May 27, 2026
## Release v0.0.2 This PR prepares the release of version v0.0.2. ### Changelog ## 0.0.2 - 2026-05-26 ### Other Changes - Lessons learned ### ♻️ Refactoring - Project structure (#7) - Kb improvements (#12) - Rename spaces to channels (#17) - Add extensions gaps (#21) - Improve tool calls ui (#22) - Remove web app header - Module improvements (#29) - Memory improvements (#35) - Storybook for web and ui (#38) - Enable AGH network by default for new installs (#57) - Hermes adjustments (#69) - Badges design (#84) - Storybook scenario and logos gallery - Migrate typescript tests (#114) - Internal go packages (#120) - Ui patterns (#127) - Improve e2e tests (#130) - Ui redesign - Workspace isolation across runtime surfaces (#145) - Prod ready applies (#162) - Tool card ui (#164) - Alpha on logo - Prod ready features (#167) - Thread sheet (#202) ### 🎉 Features - Implement config foundation packages - Implement sqlite store package - Add ACP client package - Add session lifecycle manager - Implement observe package - Add daemon composition root - Add uds api server - Implement cli package - Add http api server - Add system design - Add foundation types, schemas, and layout shell for web client - Add daemon health polling and agent sidebar systems for web client - Add session system CRUD, streaming core, and session store for web client - Add chat view, messages, and composer tests for web client - Add tool cards and renderers for web client - Add file-backed memory store core - Scaffold memory session seams - Add memory dream consolidation service - Wire memory assembler into daemon - Add memory api and cli - New skills system (#1) - Add workspace entity (#5) - Add new skill capabilities (#8) - Web ui v2 (#9) - Improve hooks system (#10) - Session resilience (#11) - Add extensability (#13) - Add automation (#16) - Add channels (#14) - Add network implementation (#15) - Add network, bridges and automations web pages (#18) - Ext registry (#20) - Add core tasks (#19) - Bridge adapters (#23) - Add site (#26) - Add ext refac and sandbox (#25) - Settings ui (#37) - Tasks ui (#36) - Harness improvements (#44) - Agent capabilities (#49) - Redesign ui (#48) - Unify capability (#53) - Redesign network workspace (#59) - Add task deletion and split session delete from stop (#58) - Session provider selection (#60) - Production grade adjustments (#66) - Autonomous system (#75) - Add agent session route (#80) - Tools registry (#85) - Agents soul (#88) - Add network threads (#105) - Orchestration improvements (#106) - Memory v2 (#108) - Agent categories (#113) - Providers model (#118) - Add canonical AGH bundled skill (#143) - Onboarding and improvements (#198) - Onboarding and improvements (#201) ### 🐛 Bug Fixes - Review round - Review rounds - Resolve memory extensibility review batch - Embed web into daemon - Defaults agents - Acp integration (#4) - Lint errors - Prd folder - Remove orphan web actions and dead surfaces (#55) - Qa testing and fixes (#73) - New review rounds (#82) - Security audit (#90) - Release qa round (#95) - Add missing tools (#141) - New qa round (#147) - Advanced qa round (#149) - Homebrew tap - Final review round (#151) - Daemon healthy - Reasoning models (#158) - Lint errors (#160) - Review round (#168) - Release adjustments (#171) - Stabilize release ci fixtures - Stabilize release integration gate - Stabilize release verify gates - Stabilize release integration flows - Stabilize release verify gates - Stabilize main verify shutdown - Ignore stale acpmock cancel - Marketplace search focus and filtering (#193) - Website video - Workspace command select ### 📚 Documentation - Update agents.md - Update prd - Update skills - Update compozy tasks - Update compozy - Update compozy - Add new skills - Archive prd - Update prds - Update rfc - Update prds - Update prds - Add automation prd - Channels prd - Update prd - Update prd - New prds - Archive prds - Bridges adapters prd - Sandbox prd - Update - Archive prd - Update - Add new prd - New design - Update prd - Archive prds - Update prds - Tasks-ui prd tasks - Update prd - Update design docs - Agent capabilities prd - Improve site docs - Remove old design references - Udpate - Autonomous prd - Update skills - Blog design - Agent sould prd - Final qa plan - Update - Remove codex ledgers from gitignore - Remove not needed files - Udpate ledger - Update cy-codex-loop skill - Orchestration improves prd - Update prds - Orch improvs prd - Memv2 prd - Providers model prd - Update refacs prd - New design proposal - Update rules - Update skills - New blog posts (#173) - Format docs - Remove old design files - Remove old - Skeeper update ### 📦 Build System - Initial structure - Commitlint - Frontend base structure - Update vscode settings - Add subagents - Coderabbit - Prd and tooling - Bun lock - Lint tooling - Copy.md and tooling adjusts - Add repoclone rc - Upgrade skeeper to v0.2.0 - Update go.mod - Adopt task artifacts into skeeper - Sync codex plans with skeeper - Skeeper lock - Skeeper lock - New skills - Skeeper lock - Skeeper lock - Skeeper lock - Update deps and go - Regenerate daytona sidecar assets for go 1.26.3 - Fix cliff - Ignore docs on fmt - Build web assets before goreleaser - Extend release dry-run timeout ### 🔧 CI/CD - Lint errors - Fint release pr - Fix goreleaser - Fix release - Fix release process - Fix release sync - Decouple release dry-run npm auth - Persist web assets git auth ### 🧪 Testing - Add e2e tests (#27) - Qa rounds (#78) - Improve test suite (#138) - Harden daemon-served restart reloads - Harden daemon-served readiness waits - Stabilize dashboard focus assertion - Stabilize release integration gates - Stabilize release e2e markers - Stabilize release e2e flows - Improve suite speed <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated web assets dependency to a newer version for improved stability and performance. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/compozy/agh/pull/211?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
](https://app.coderabbit.ai/change-stack/compozy/agh/pull/211?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
This was referenced May 27, 2026
Closed
Closed
Merged
pedronauck
added a commit
that referenced
this pull request
May 27, 2026
## Release v0.0.2 This PR prepares the release of version v0.0.2. ### Changelog ## 0.0.2 - 2026-05-27 ### Other Changes - Lessons learned ### ♻️ Refactoring - Project structure (#7) - Kb improvements (#12) - Rename spaces to channels (#17) - Add extensions gaps (#21) - Improve tool calls ui (#22) - Remove web app header - Module improvements (#29) - Memory improvements (#35) - Storybook for web and ui (#38) - Enable AGH network by default for new installs (#57) - Hermes adjustments (#69) - Badges design (#84) - Storybook scenario and logos gallery - Migrate typescript tests (#114) - Internal go packages (#120) - Ui patterns (#127) - Improve e2e tests (#130) - Ui redesign - Workspace isolation across runtime surfaces (#145) - Prod ready applies (#162) - Tool card ui (#164) - Alpha on logo - Prod ready features (#167) - Thread sheet (#202) ### 🎉 Features - Implement config foundation packages - Implement sqlite store package - Add ACP client package - Add session lifecycle manager - Implement observe package - Add daemon composition root - Add uds api server - Implement cli package - Add http api server - Add system design - Add foundation types, schemas, and layout shell for web client - Add daemon health polling and agent sidebar systems for web client - Add session system CRUD, streaming core, and session store for web client - Add chat view, messages, and composer tests for web client - Add tool cards and renderers for web client - Add file-backed memory store core - Scaffold memory session seams - Add memory dream consolidation service - Wire memory assembler into daemon - Add memory api and cli - New skills system (#1) - Add workspace entity (#5) - Add new skill capabilities (#8) - Web ui v2 (#9) - Improve hooks system (#10) - Session resilience (#11) - Add extensability (#13) - Add automation (#16) - Add channels (#14) - Add network implementation (#15) - Add network, bridges and automations web pages (#18) - Ext registry (#20) - Add core tasks (#19) - Bridge adapters (#23) - Add site (#26) - Add ext refac and sandbox (#25) - Settings ui (#37) - Tasks ui (#36) - Harness improvements (#44) - Agent capabilities (#49) - Redesign ui (#48) - Unify capability (#53) - Redesign network workspace (#59) - Add task deletion and split session delete from stop (#58) - Session provider selection (#60) - Production grade adjustments (#66) - Autonomous system (#75) - Add agent session route (#80) - Tools registry (#85) - Agents soul (#88) - Add network threads (#105) - Orchestration improvements (#106) - Memory v2 (#108) - Agent categories (#113) - Providers model (#118) - Add canonical AGH bundled skill (#143) - Onboarding and improvements (#198) - Onboarding and improvements (#201) ### 🐛 Bug Fixes - Review round - Review rounds - Resolve memory extensibility review batch - Embed web into daemon - Defaults agents - Acp integration (#4) - Lint errors - Prd folder - Remove orphan web actions and dead surfaces (#55) - Qa testing and fixes (#73) - New review rounds (#82) - Security audit (#90) - Release qa round (#95) - Add missing tools (#141) - New qa round (#147) - Advanced qa round (#149) - Homebrew tap - Final review round (#151) - Daemon healthy - Reasoning models (#158) - Lint errors (#160) - Review round (#168) - Release adjustments (#171) - Stabilize release ci fixtures - Stabilize release integration gate - Stabilize release verify gates - Stabilize release integration flows - Stabilize release verify gates - Stabilize main verify shutdown - Ignore stale acpmock cancel - Marketplace search focus and filtering (#193) - Website video - Workspace command select ### 📚 Documentation - Update agents.md - Update prd - Update skills - Update compozy tasks - Update compozy - Update compozy - Add new skills - Archive prd - Update prds - Update rfc - Update prds - Update prds - Add automation prd - Channels prd - Update prd - Update prd - New prds - Archive prds - Bridges adapters prd - Sandbox prd - Update - Archive prd - Update - Add new prd - New design - Update prd - Archive prds - Update prds - Tasks-ui prd tasks - Update prd - Update design docs - Agent capabilities prd - Improve site docs - Remove old design references - Udpate - Autonomous prd - Update skills - Blog design - Agent sould prd - Final qa plan - Update - Remove codex ledgers from gitignore - Remove not needed files - Udpate ledger - Update cy-codex-loop skill - Orchestration improves prd - Update prds - Orch improvs prd - Memv2 prd - Providers model prd - Update refacs prd - New design proposal - Update rules - Update skills - New blog posts (#173) - Format docs - Remove old design files - Remove old - Skeeper update ### 📦 Build System - Initial structure - Commitlint - Frontend base structure - Update vscode settings - Add subagents - Coderabbit - Prd and tooling - Bun lock - Lint tooling - Copy.md and tooling adjusts - Add repoclone rc - Upgrade skeeper to v0.2.0 - Update go.mod - Adopt task artifacts into skeeper - Sync codex plans with skeeper - Skeeper lock - Skeeper lock - New skills - Skeeper lock - Skeeper lock - Skeeper lock - Update deps and go - Regenerate daytona sidecar assets for go 1.26.3 - Fix cliff - Ignore docs on fmt - Build web assets before goreleaser - Extend release dry-run timeout - Fix release dry-run token contract ### 🔧 CI/CD - Lint errors - Fint release pr - Fix goreleaser - Fix release - Fix release process - Fix release sync - Decouple release dry-run npm auth - Persist web assets git auth - Require npm auth before release merge ### 🧪 Testing - Add e2e tests (#27) - Qa rounds (#78) - Improve test suite (#138) - Harden daemon-served restart reloads - Harden daemon-served readiness waits - Stabilize dashboard focus assertion - Stabilize release integration gates - Stabilize release e2e markers - Stabilize release e2e flows - Improve suite speed <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated dependencies to latest versions. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/compozy/agh/pull/214?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
](https://app.coderabbit.ai/change-stack/compozy/agh/pull/214?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes