refactor: project structure#7
Merged
Merged
Conversation
|
Caution Review failedPull request was closed or merged during review WalkthroughIntroduces a shared API contract and core HTTP handler layer, migrates transport handlers to delegate to core, refactors session lifecycle and prompt/permission flows, adds consolidation/dream runtime and daemon boot logic, and introduces multiple utility packages and test helpers. Changes are primarily API, handler, session, memory, daemon, CLI, and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant HTTP as HTTP Server
participant Core as core.BaseHandlers
participant Sessions as SessionManager
participant DB as Session DB
participant Driver as ACP Driver
Client->>HTTP: POST /api/sessions
HTTP->>Core: CreateSession(c)
Core->>Sessions: Create(ctx, opts)
Sessions->>DB: Open session DB / write metadata
Sessions->>Driver: Start process
Driver-->>Sessions: Process handle
Sessions-->>Core: created session
Core-->>Client: 201 Created (SessionPayload)
Client->>HTTP: POST /api/sessions/{id}/prompt
HTTP->>Core: StreamSession(c)
Core->>Sessions: Prompt(ctx, id, message)
Sessions->>Driver: Prompt(ctx, req)
Driver-->>Sessions: <-chan AgentEvent
loop stream
Sessions->>DB: Record Event
Sessions-->>Core: event
Core->>Core: PrepareSSE/WriteSSE
Core-->>Client: SSE frame
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/api/httpapi/httpapi_integration_test.go (1)
1007-1012:⚠️ Potential issue | 🟠 MajorFix stale stop-session status expectation in helper.
Line 1008 still expects
http.StatusOK, but stop-session handlers now returnhttp.StatusNoContent. This can make integration flows fail even when behavior is correct.🔧 Proposed fix
func stopIntegrationSession(t *testing.T, runtime integrationRuntime, sessionID string) { t.Helper() resp := mustHTTPRequest(t, runtime.client, http.MethodDelete, mustURL(runtime.host, runtime.port, "/api/sessions/"+sessionID), nil, nil) - if resp.StatusCode != http.StatusOK { + if resp.StatusCode != http.StatusNoContent { body, _ := io.ReadAll(resp.Body) _ = resp.Body.Close() - t.Fatalf("stop status = %d, want %d; body=%s", resp.StatusCode, http.StatusOK, string(body)) + t.Fatalf("stop status = %d, want %d; body=%s", resp.StatusCode, http.StatusNoContent, string(body)) } _ = resp.Body.Close() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/httpapi/httpapi_integration_test.go` around lines 1007 - 1012, The helper currently asserts stop-session DELETE responses against http.StatusOK; update the assertion to expect http.StatusNoContent instead (change the comparison from http.StatusOK to http.StatusNoContent and adjust the failure message accordingly) in the test code that calls mustHTTPRequest for DELETE "/api/sessions/"+sessionID (the block using resp := mustHTTPRequest(...), resp.StatusCode check, and t.Fatalf).internal/cli/client.go (1)
463-465:⚠️ Potential issue | 🟡 MinorAvoid
context.Background()fallback in production code.Creating a fallback context with
context.Background()whenctxis nil violates the coding guideline requiring callers to provide a proper context. This masks bugs where callers forget to pass context and prevents proper cancellation/timeout propagation.Proposed fix: require non-nil context
func (c *unixSocketClient) doRequest(ctx context.Context, method string, path string, query url.Values, requestBody any, lastEventID string) (*http.Response, error) { - if ctx == nil { - ctx = context.Background() - } + if ctx == nil { + return nil, errors.New("cli: context is required") + } target := baseURL + pathAlternatively, if maintaining backward compatibility is essential, document this behavior and consider logging a warning.
As per coding guidelines: "Use
context.Contextas first argument to functions crossing runtime boundaries — avoidcontext.Background()outsidemainand focused tests".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/client.go` around lines 463 - 465, The current nil-check that assigns ctx = context.Background() masks caller errors; instead remove the fallback and enforce a non-nil context by returning an error (or panic) when ctx == nil (replace the context.Background() assignment with something like "return fmt.Errorf(\"nil context passed to <functionName>\")" or appropriate error handling), and update callers to pass a valid context; if backward compatibility is required, log a clear warning when ctx == nil before returning the error. Ensure you modify the block that currently checks "if ctx == nil { ctx = context.Background() }" and reference the variable "ctx" and the function where this check lives.
🟡 Minor comments (11)
internal/api/httpapi/memory.go-1-1 (1)
1-1:⚠️ Potential issue | 🟡 MinorRemove empty package declaration files from httpapi and udsapi packages.
These 8 files contain only package declarations with no implementation:
internal/api/httpapi/{memory.go, observe.go, agents.go, stream.go, daemon.go}internal/api/udsapi/{daemon.go, observe.go, stream.go, payloads.go}All handler functionality (ListAgents, GetAgent, ObserveEvents, StreamObserveEvents, ListMemory, etc.) is already centralized in
internal/api/core/handlers.goand wired into both transports viaserver.go(httpapi) androutes.go(udsapi). These empty files serve no purpose and should be deleted to reduce maintenance overhead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/httpapi/memory.go` at line 1, Delete the eight no-op files that contain only package declarations to reduce dead code: in package httpapi remove memory.go, observe.go, agents.go, stream.go, daemon.go; in package udsapi remove daemon.go, observe.go, stream.go, payloads.go. Ensure you only remove files that are truly empty (no exported symbols or init logic), run the build/tests, and confirm handler functionality remains provided by internal/api/core/handlers.go and wired via server.go (httpapi) and routes.go (udsapi).internal/api/udsapi/agents.go-1-1 (1)
1-1:⚠️ Potential issue | 🟡 MinorRemove empty
agents.gofile or implement its intended functionality.This file contains only a package declaration with no code, functions, or types. The file is not referenced anywhere in the codebase and serves no purpose. Either remove this file or add the implementation it was scaffolded for.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/udsapi/agents.go` at line 1, This file only contains "package udsapi" and should be removed if unused; otherwise implement the intended agents API inside package udsapi by adding concrete symbols that other code expects (for example define an exported type Agent and functions such as NewAgent, RegisterAgent, and ListAgents or GetAgents) so the package is non-empty and provides the required functionality; choose removal if nothing depends on package udsapi or implement the minimal Agent struct and API methods (Agent, NewAgent, RegisterAgent, ListAgents) to satisfy compilation and future usage.internal/api/httpapi/daemon.go-1-1 (1)
1-1:⚠️ Potential issue | 🟡 MinorRemove unused package scaffolding file.
daemon.gocontains only a package declaration and is not referenced anywhere in the codebase. Delete this file along withagents.go, which has the same issue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/httpapi/daemon.go` at line 1, Delete the unused scaffolding files that only declare the httpapi package: remove daemon.go and agents.go from the repository; ensure there are no imports or references to package httpapi remaining and run a build/test to confirm nothing else depends on those files (the unique identifiers are the files named daemon.go and agents.go and the package declaration "package httpapi").internal/procutil/procutil_test.go-40-53 (1)
40-53:⚠️ Potential issue | 🟡 MinorUse
errors.Is()to verify the specific error in the missing-process case.The first test (non-positive PID) correctly rejects the call, but the second test only checks that
Signal(999999, 0)returns some error—not that it's the missing-process error. SinceSignalwraps the underlyingsyscall.Killerror with%w(line 26 of procutil.go), the missing-process case should assert:if err := Signal(999999, syscall.Signal(0)); !errors.Is(err, syscall.ESRCH) { t.Fatalf("Signal(missing pid, 0) error = %v, want ESRCH", err) }This locks down that the specific error is "no such process," not another failure mode. Per guidelines: use
errors.Is()anderrors.As()for error matching.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/procutil/procutil_test.go` around lines 40 - 53, Update the TestSignalReturnsErrorForMissingProcess test to assert the specific "no such process" error using errors.Is rather than just checking non-nil; call Signal(999999, syscall.Signal(0)) and verify errors.Is(err, syscall.ESRCH) (since Signal wraps syscall.Kill with %w), failing the test with a clear message if the check is false; this ensures the test verifies the exact missing-process error from Signal.internal/acp/handlers_test.go-317-323 (1)
317-323:⚠️ Potential issue | 🟡 MinorUse
t.Run("Should ...")naming for subtests.Please rename these subtest names to the required
Should ...style for test policy consistency.As per coding guidelines,
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/acp/handlers_test.go` around lines 317 - 323, The subtests in the tests slice use short names like "interactive pending" and "auto allow once"; rename each test case name to the required "Should ..." style (e.g., "Should handle interactive pending", "Should allow once automatically", "Should reject once on timeout") so that t.Run(tt.name, ...) follows the mandated pattern; update the entries in the tests array used by the loop that calls t.Run and ensure decision constants (decisionAllowOnce, decisionRejectOnce) remain unchanged.internal/acp/handlers_test.go-339-339 (1)
339-339:⚠️ Potential issue | 🟡 MinorMutate
rawwith a different byte so clone validation is real.Line 339 writes
'{', which usually keeps the payload unchanged for object JSON, so this can miss aliasing bugs inemitPermissionEvent.Suggested fix
- raw[0] = '{' + raw[0] = '['🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/acp/handlers_test.go` at line 339, The test mutates the raw byte slice using raw[0] = '{', which often leaves JSON object payloads unchanged and can hide aliasing bugs in emitPermissionEvent; change the mutation to a different byte that will alter the serialized payload (e.g., flip a letter or use a byte that is not identical to the original JSON first byte) so the clone validation is meaningful, update the test that constructs raw and calls emitPermissionEvent to assert that the original raw remains unchanged after the call, and reference the raw variable and emitPermissionEvent function when making the assertion.internal/daemon/daemon_integration_test.go-23-30 (1)
23-30:⚠️ Potential issue | 🟡 MinorMissing bounds check can cause test panics.
The
promptCallmethod accessesf.promptCalls[index]without validating thatindexis within bounds. This could cause panics in tests if an incorrect index is passed.🛡️ Proposed fix to add bounds validation
func (f *fakeSessionManager) promptCall(index int) struct { id string msg string } { f.mu.Lock() defer f.mu.Unlock() + if index < 0 || index >= len(f.promptCalls) { + return struct{ id, msg string }{} + } return f.promptCalls[index] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/daemon/daemon_integration_test.go` around lines 23 - 30, The promptCall method reads f.promptCalls[index] without bounds checking which can panic; update fakeSessionManager.promptCall to validate index (index >= 0 && index < len(f.promptCalls)) while holding f.mu and if out of range return the zero-value struct (or a clear error/indicator) instead of indexing out of bounds; reference the method name promptCall and the slice f.promptCalls when making this change.internal/api/httpapi/prompt.go-79-80 (1)
79-80:⚠️ Potential issue | 🟡 MinorKeep the 400 bind error message client-facing.
RespondErrorreturns the raw error text for non-5xx statuses, so this wrapped error will exposehttpapi: decode prompt request: ...in the public response body. Pass a request-level bind error here and keep the transport-specific context for logs only.🤖 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 79 - 80, The current bind error passed to core.RespondError wraps the transport-specific context and will be exposed to clients; replace the wrapped error with a simple request-level error message (e.g., "invalid JSON in request body" or "invalid request payload") when calling core.RespondError after c.ShouldBindJSON(&req) fails, and move the fmt.Errorf("httpapi: decode prompt request: %w", err) into a separate server-side log call so the detailed context is retained in logs but not returned to the client; update the call site around c.ShouldBindJSON, core.RespondError, and the local err variable accordingly.internal/session/manager_lifecycle.go-386-394 (1)
386-394:⚠️ Potential issue | 🟡 MinorSame pattern: cancel not deferred.
Same issue as above —
cancel()should be deferred for safety.🛡️ Proposed fix
if proc != nil { stopCtx, cancel := context.WithTimeout(context.Background(), defaultLifecycleTimeout) + defer cancel() if err := m.driver.Stop(stopCtx, proc); err != nil { errs = append(errs, err) } - cancel() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/session/manager_lifecycle.go` around lines 386 - 394, The cleanupFailedStart function creates a cancellable context (stopCtx, cancel) but calls cancel() directly instead of deferring it; change the code in cleanupFailedStart to call defer cancel() immediately after creating the context so the cancel function is always executed (even on early returns or panics) and keep the call sites that use m.driver.Stop(stopCtx, proc) unchanged.internal/session/manager_lifecycle.go-362-369 (1)
362-369:⚠️ Potential issue | 🟡 MinorContext created but cancel not deferred.
The
cancel()is called afterrecorder.Close, but ifClosepanics, the context won't be cancelled. Consider usingdefer cancel()immediately after context creation.🛡️ Proposed fix
if recorder := session.recorderHandle(); recorder != nil { closeCtx, cancel := context.WithTimeout(context.Background(), defaultLifecycleTimeout) + defer cancel() if err := recorder.Close(closeCtx); err != nil { errs = append(errs, err) } - cancel() session.setRecorder(nil) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/session/manager_lifecycle.go` around lines 362 - 369, The context created for closing the recorder uses context.WithTimeout but calls cancel() after recorder.Close, which can leak if Close panics; change the block around session.recorderHandle() so that after creating closeCtx, cancel := context.WithTimeout(...) you immediately defer cancel() to guarantee cancellation, then call recorder.Close(closeCtx) and handle errors (append to errs) and finally call session.setRecorder(nil) as before; ensure the defer is placed right after the WithTimeout call in the same scope that calls recorder.Close.internal/api/testutil/apitest.go-340-340 (1)
340-340:⚠️ Potential issue | 🟡 MinorSSE multi-line data handling may lose newlines.
Per the SSE specification, multiple
data:lines should be concatenated with newline characters between them. The current implementation appends bytes directly without inserting newlines between consecutive data lines.Proposed fix to preserve newlines between data lines
case strings.HasPrefix(line, "data: "): - current.Data = append(current.Data, []byte(strings.TrimPrefix(line, "data: "))...) + if len(current.Data) > 0 { + current.Data = append(current.Data, '\n') + } + current.Data = append(current.Data, []byte(strings.TrimPrefix(line, "data: "))...) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/testutil/apitest.go` at line 340, The SSE data assembly currently appends each "data: " line bytes directly (the append to current.Data), which drops required newlines between multi-line SSE data fields; update the logic that handles lines with the "data: " prefix (the code touching current.Data) to insert a newline into current.Data before appending when current.Data is non-empty so consecutive data: lines are joined with '\n' as per the SSE spec.
🧹 Nitpick comments (26)
internal/cli/helpers_test.go (1)
3-15: Import grouping inconsistency.The
testutilimport at line 9 is placed within the standard library imports block. Per Go conventions, imports should be grouped: standard library first, then a blank line, then third-party/internal packages.♻️ Suggested import grouping
import ( "bytes" "context" "encoding/json" "errors" "fmt" - "github.com/pedronauck/agh/internal/testutil" "testing" "time" aghconfig "github.com/pedronauck/agh/internal/config" "github.com/pedronauck/agh/internal/memory" + "github.com/pedronauck/agh/internal/testutil" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/helpers_test.go` around lines 3 - 15, The import block in helpers_test.go incorrectly places github.com/pedronauck/agh/internal/testutil inside the standard library group; reorder imports to follow Go conventions: first group standard libraries (bytes, context, encoding/json, errors, fmt, testing, time), then a blank line, then internal/third-party packages (github.com/pedronauck/agh/internal/testutil, aghconfig from github.com/pedronauck/agh/internal/config, and github.com/pedronauck/agh/internal/memory) so the file compiles with consistent import grouping.internal/procutil/procutil_test.go (1)
20-29: Use namedShould...subtests for the negativeAlivecases.Repeated
"pid"subtests end up aspid/pid#01, which makes failures harder to scan and misses the repo’s test naming convention.♻️ Suggested refactor
- testCases := []int{0, -1} - for _, pid := range testCases { - pid := pid - t.Run("pid", func(t *testing.T) { + testCases := []struct { + name string + pid int + }{ + {name: "ShouldRejectPID0", pid: 0}, + {name: "ShouldRejectNegativePID", pid: -1}, + } + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { t.Parallel() - if Alive(pid) { - t.Fatalf("Alive(%d) = true, want false", pid) + if Alive(tc.pid) { + t.Fatalf("Alive(%d) = true, want false", tc.pid) } }) }As per coding guidelines,
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/procutil/procutil_test.go` around lines 20 - 29, The subtests use a repeated literal name "pid" which produces non-descriptive names and violates the repo convention; update the loop over testCases in procutil_test.go to call t.Run with a descriptive name like fmt.Sprintf("ShouldReturnFalseForPID_%d", pid) (or "ShouldNotBeAlive_<pid>") and pass the pid into the closure (keeping the existing pid := pid capture pattern) so each invocation of Alive(pid) runs as a uniquely named subtest; ensure you reference the same testCases slice and Alive function and preserve t.Parallel() inside each subtest.internal/memory/lock_test.go (1)
348-356: Move this check back tointernal/procutilor rewrite it around lock behavior.
TestProcessAlivenow exercisesprocutil.Alivedirectly, sointernal/memoryis no longer testing its own behavior here. That duplicates the newinternal/procutilcoverage and adds cross-package maintenance for no extra signal.As per coding guidelines,
MUST test meaningful business logic, not trivial operationsandMUST avoid redundant validation tests across packages.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/memory/lock_test.go` around lines 348 - 356, The TestProcessAlive in internal/memory is directly calling procutil.Alive and should be moved or repurposed: either relocate this test to internal/procutil (keeping the name TestProcessAlive and asserting procutil.Alive(os.Getpid()) true and procutil.Alive(-1) false), or change the test in internal/memory to exercise lock-related behavior instead (e.g., exercise the Lock/Unlock or TryLock semantics in the memory locker implementation and assert expected liveliness/error conditions) so it no longer duplicates procutil coverage; update or remove TestProcessAlive in internal/memory accordingly and ensure any referenced symbols (TestProcessAlive, procutil.Alive, the memory lock methods like Lock/Unlock/TryLock) are adjusted to match the chosen approach.internal/daemon/notifier.go (1)
10-13: Add the compile-timesession.Notifierassertion.This type is clearly intended to satisfy
session.Notifier; pinning that with avar _ ...assertion will catch interface drift at build time instead of during wiring or tests.♻️ Suggested fix
type notifierFanout struct { notifiers []session.Notifier onSessionStopped func(context.Context, *session.Session) } + +var _ session.Notifier = (*notifierFanout)(nil)As per coding guidelines,
Compile-time interface verification with var _ Interface = (*Type)(nil).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/daemon/notifier.go` around lines 10 - 13, Add a compile-time interface verification asserting that notifierFanout implements session.Notifier by adding a var declaration that assigns a nil pointer of notifierFanout to a variable of type session.Notifier (place this statement near the notifierFanout type or at the bottom of the file so build-time interface drift is caught early).internal/config/agent.go (1)
227-235: Keep the mapped frontmatter error in the chain.Returning fresh
errors.New(...)values here discards the original sentinel, so downstream code can’t useerrors.Isand tests are forced into exact string matching. Prefer config-level sentinel errors wrapped with%wover replacing the error outright.As per coding guidelines,
Use errors.Is() and errors.As() for error matching — never compare error stringsandUse 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/config/agent.go` around lines 227 - 235, Wrap and preserve the original frontmatter sentinel errors in wrapFrontmatterError so callers can use errors.Is/As: instead of returning fresh errors.New("config: ..."), return config-level sentinel errors that wrap the original error using %w (e.g., map frontmatter.ErrMissing and frontmatter.ErrUnterminated to distinct config package sentinel errors and wrap the original err). Update wrapFrontmatterError to return those wrapped sentinel errors (referencing wrapFrontmatterError, frontmatter.ErrMissing, frontmatter.ErrUnterminated and the new config sentinel names) so downstream code and tests can match using errors.Is.internal/memory/consolidation/runtime.go (1)
391-394: Detach cleanup without dropping context values.The fresh stop context makes sense, but building it from
context.Background()loses request-scoped values and hardcodes the10spolicy in code. Prefer deriving a non-canceling context fromctxand inject the timeout from config/options.As per coding guidelines,
Use context.Context as first argument to functions crossing runtime boundaries — avoid context.Background() outside main and focused testsandNever 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/memory/consolidation/runtime.go` around lines 391 - 394, Replace the ad-hoc background context and hardcoded 10s with a timeout derived from the incoming request context and a configurable timeout option: call context.WithTimeout(ctx, opts.SessionStopTimeout) (or similar config value) instead of context.WithTimeout(context.Background(), 10*time.Second), keep the defer cancel(), and pass the resulting stopCtx to sessions.Stop(dreamSession.ID) so request-scoped values are preserved and the timeout is configurable; update any surrounding code to accept or access the options value used for SessionStopTimeout.internal/session/query_test.go (1)
4-14: Import ordering: group standard library imports first.The
testutilimport is placed before standard library imports (os,path/filepath,strings,testing,time). Consider grouping imports conventionally.♻️ Suggested import ordering
import ( "context" "errors" - "github.com/pedronauck/agh/internal/testutil" "os" "path/filepath" "strings" "testing" "time" "github.com/pedronauck/agh/internal/store" + "github.com/pedronauck/agh/internal/testutil" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/session/query_test.go` around lines 4 - 14, Reorder the import block so standard library packages (context, errors, os, path/filepath, strings, testing, time) are grouped first, followed by internal and third-party imports like github.com/pedronauck/agh/internal/testutil and github.com/pedronauck/agh/internal/store; update the import declaration at the top of the query_test.go file (the import block that currently places testutil before os/path/filepath) to follow Go's conventional grouping and ordering.internal/observe/observer_integration_test.go (1)
5-13: Import ordering: group standard library imports before local packages.The
testutilimport is placed beforetestingandtime. Go convention groups imports as: standard library, then external packages, then local packages.♻️ Suggested import ordering
import ( - "github.com/pedronauck/agh/internal/testutil" "testing" "time" "github.com/pedronauck/agh/internal/acp" "github.com/pedronauck/agh/internal/session" "github.com/pedronauck/agh/internal/store" + "github.com/pedronauck/agh/internal/testutil" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/observe/observer_integration_test.go` around lines 5 - 13, Reorder the import block so standard library packages come first: place "testing" and "time" before all other imports, then external packages, and finally local packages like "github.com/pedronauck/agh/internal/testutil", "github.com/pedronauck/agh/internal/acp", "github.com/pedronauck/agh/internal/session", and "github.com/pedronauck/agh/internal/store" to follow Go import grouping conventions.internal/api/testutil/apitest_test.go (1)
10-12: Consider removing this redundant test file.Per coding guidelines, compile-time interface verification should use
var _ Interface = (*Type)(nil)in production code, not in tests. The AI summary indicates thatapitest.goalready contains compile-time interface compliance assertions forStubSessionManager,StubObserver, andStubWorkspaceService. This test file duplicates that verification without adding value.If the assertion in
apitest.gois present, this test can be deleted. If not, move the assertion there:// In apitest.go var _ core.SessionManager = StubSessionManager{}As per coding guidelines: "Use compile-time interface verification:
var _ Interface = (*Type)(nil)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/testutil/apitest_test.go` around lines 10 - 12, The test TestStubSessionManagerSatisfiesInterface is redundant; remove this test file or the test function and instead ensure a compile-time interface assertion exists in apitest.go using the StubSessionManager type against core.SessionManager (e.g., add a package-level assertion there referencing StubSessionManager and core.SessionManager); if apitest.go already contains the assertion, simply delete TestStubSessionManagerSatisfiesInterface to avoid duplication.internal/cli/render_test.go (1)
199-206: Addt.Helper()to the test helper function.Test helper functions should call
t.Helper()so that test failure line numbers point to the calling test rather than within the helper.♻️ Proposed fix
-func newOutputTestCommand(mode OutputFormat) (*cobra.Command, *bytes.Buffer) { +func newOutputTestCommand(t *testing.T, mode OutputFormat) (*cobra.Command, *bytes.Buffer) { + t.Helper() cmd := &cobra.Command{Use: "test"} output := &bytes.Buffer{} cmd.SetOut(output) cmd.Flags().String(outputFlagName, string(OutputHuman), "output format") _ = cmd.Flags().Set(outputFlagName, string(mode)) return cmd, output }Note: Update the call site at line 149 to pass
tas well.As per coding guidelines: "Use
t.Helper()on test helper functions in Go tests"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/render_test.go` around lines 199 - 206, The test helper newOutputTestCommand lacks a t.Helper() call, so add t.Helper() at the start of newOutputTestCommand and change its signature to accept testing.TB (or *testing.T) to allow calling t.Helper(); update the caller at the previous call site to pass t into newOutputTestCommand and adjust references to outputFlagName and OutputFormat parameters accordingly so tests report failures at the caller frame.internal/api/core/conversions_parsers_test.go (2)
78-79: Variable shadowscontextpackage import.The variable
contexton line 79 shadows thecontextpackage imported on line 4. While this doesn't cause a bug here since the test doesn't use thecontextpackage directly after this point, it's a code smell that can cause confusion.♻️ Rename variable to avoid shadowing
recorder := httptest.NewRecorder() - context, _ := gin.CreateTestContext(recorder) - context.Request = httptest.NewRequest(http.MethodGet, "/events?type=agent_message&agent_name=coder&turn_id=turn-1&after_sequence=5&limit=10&since=2026-04-03T12:00:00Z", nil) + ginCtx, _ := gin.CreateTestContext(recorder) + ginCtx.Request = httptest.NewRequest(http.MethodGet, "/events?type=agent_message&agent_name=coder&turn_id=turn-1&after_sequence=5&limit=10&since=2026-04-03T12:00:00Z", nil) - query, err := core.ParseSessionEventQuery(context) + query, err := core.ParseSessionEventQuery(ginCtx)Apply the same rename pattern throughout the file for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/core/conversions_parsers_test.go` around lines 78 - 79, The test code declares a local variable named `context` which shadows the imported `context` package; rename the local variable returned by `gin.CreateTestContext` (for example to `ctx` or `ginCtx`) in the `conversions_parsers_test.go` test and update all uses in that file (e.g., references that currently use `context`) to the new name so the package import is no longer shadowed.
135-149: Consider usingt.Parallel()for subtests.The table-driven test subtests are independent and could benefit from parallel execution per coding guidelines.
♻️ Add t.Parallel() to subtests
t.Run(tc.name, func(t *testing.T) { + t.Parallel() recorder := httptest.NewRecorder() context, _ := gin.CreateTestContext(recorder)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/core/conversions_parsers_test.go` around lines 135 - 149, The subtests inside the table-driven test use t.Run and are independent, so add t.Parallel() as the first statement inside the subtest closure (the anonymous func passed to t.Run) to enable parallel execution; ensure any shared state is not accessed concurrently (the current recorder and context creation inside the closure is fine). Locate the t.Run(...) anonymous function around core.RespondError usage and insert t.Parallel() at the top of that closure.internal/session/manager_test.go (1)
483-497: Consider usingt.Parallel()in table-driven subtests.Per coding guidelines, independent subtests should use
t.Parallel(). The subtests here are independent and could benefit from parallel execution.♻️ Optional: Add t.Parallel() to subtests
for _, tc := range testCases { - tc := tc t.Run(tc.name, func(t *testing.T) { + t.Parallel() h.driver.approveHook = func(*fakeProcess, acp.ApproveRequest) error { return tc.hookErr }Note: This may require adjusting the test setup since
h.driver.approveHookis shared state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/session/manager_test.go` around lines 483 - 497, The subtests in the table-driven loop should run in parallel: inside the t.Run closure call t.Parallel() as the first statement to enable parallel execution of each tc; because h.driver.approveHook is shared state, ensure each subtest gets its own isolated state before calling h.manager.ApprovePermission — e.g., clone or recreate the handler/driver per test case or set a per-subtest local driver (avoid mutating the shared h.driver), and assign the approveHook on that local instance so concurrent subtests do not race on h.driver.approveHook while still testing the same behavior.internal/api/core/memory_workspace_test.go (1)
4-6: Avoid ignoringjson.Marshalin the helper.Even in tests, the repo's Go rules require every error to be handled. Here you're only using JSON encoding to quote a string, so
strconv.Quotekeeps the helper simple without dropping an error.♻️ Suggested change
-import ( - "encoding/json" +import ( "errors" "net/http" "path/filepath" + "strconv" "testing" "time" @@ func escapeJSON(value string) string { - payload, _ := json.Marshal(value) - return string(payload[1 : len(payload)-1]) + quoted := strconv.Quote(value) + return quoted[1 : len(quoted)-1] }As per coding guidelines,
Never ignore errors with _ — every error must be handled or have a written justification.Also applies to: 254-256
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/core/memory_workspace_test.go` around lines 4 - 6, The test helper currently uses json.Marshal to quote a string and ignores its error; replace that usage with strconv.Quote (or, if you must keep json.Marshal, capture and handle its error) so no error is discarded. Locate the helper in memory_workspace_test.go that calls json.Marshal and swap it to strconv.Quote for simplicity (or add proper error handling around json.Marshal) and update imports (remove "encoding/json", add "strconv" if needed).internal/api/core/parsers.go (1)
13-13: Minor: Unnecessary constant redefinition.
timeRFC3339Nanois justtime.RFC3339Nano. Consider using the standard library constant directly.-const timeRFC3339Nano = time.RFC3339NanoThen update usages to
time.RFC3339Nano.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/core/parsers.go` at line 13, Remove the redundant constant timeRFC3339Nano and replace all its usages with the standard library constant time.RFC3339Nano; specifically delete the const declaration in internal/api/core/parsers.go (timeRFC3339Nano) and update any references in functions or methods that currently use timeRFC3339Nano to use time.RFC3339Nano instead.internal/api/httpapi/server.go (1)
19-20: Minor: Redundant import alias.The alias
coreon line 20 matches the package name, making it redundant. Consider removing it for consistency."github.com/pedronauck/agh/internal/api/contract" - core "github.com/pedronauck/agh/internal/api/core" + "github.com/pedronauck/agh/internal/api/core"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/httpapi/server.go` around lines 19 - 20, Remove the redundant import alias `core` in the import block (the import currently written as `core "github.com/pedronauck/agh/internal/api/core"`); change it to a plain import of the package and ensure any references to core.* remain valid (no code changes should be necessary because the alias matches the package name).internal/api/core/error_paths_test.go (2)
69-95: Consider usingt.Runfor table-driven subtests.The loop over requests would benefit from
t.Runfor better test isolation and clearer failure messages. This aligns with the coding guidelines for table-driven tests.for _, request := range requests { + t.Run(request.method+" "+request.path, func(t *testing.T) { + t.Parallel() resp := performRequest(t, fixture.Engine, request.method, request.path, request.body) if resp.Code != request.want { t.Fatalf("%s %s status = %d, want %d; body=%s", request.method, request.path, resp.Code, request.want, resp.Body.String()) } + }) }As per coding guidelines: "Use table-driven tests with subtests (
t.Run) as default in Go tests".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/core/error_paths_test.go` around lines 69 - 95, The test iterates over the requests table without subtests; wrap each table entry in t.Run (use a descriptive name like request.method+" "+request.path) and run the body of the loop inside the subtest closure (capture request into a local variable to avoid loop variable capture) where you call performRequest(t, fixture.Engine, request.method, request.path, request.body) and assert resp.Code == request.want; this will provide isolated failures and clearer messages for the requests slice checks in the test (reference performRequest and the requests variable).
226-241: Consider usingt.Runfor table-driven subtests.Similar to the earlier suggestion, this loop would benefit from
t.Runfor better test isolation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/core/error_paths_test.go` around lines 226 - 241, The table-driven loop over requests should use t.Run for isolation: inside the loop over the requests slice (the variable named requests) wrap each iteration in t.Run(fmt.Sprintf("%s %s", req.method, req.path), func(t *testing.T) { req := request // capture loop variable; resp := performRequest(t, noStoreFixture.Engine, req.method, req.path, req.body); if resp.Code != http.StatusInternalServerError { t.Fatalf(...) } }), and optionally call t.Parallel() inside the subtest if parallel execution is desired; this ensures each performRequest invocation is isolated and avoids loop-variable capture issues.internal/api/core/more_coverage_test.go (1)
154-158: Questionable assertion logic for TransportName default.The condition
handlers.TransportName != "" && handlers.TransportName != "apicore"will only fail ifTransportNameis a non-empty string other than"apicore". This means empty string passes the test, which seems to be the expected default. Consider clarifying the assertion:handlers := core.NewBaseHandlers(core.BaseHandlerConfig{}) - if handlers.TransportName != "" && handlers.TransportName != "apicore" { - t.Fatalf("TransportName default = %q", handlers.TransportName) + if handlers.TransportName != "" { + t.Fatalf("TransportName default = %q, want empty", handlers.TransportName) }🤖 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 154 - 158, The current assertion uses a compound condition that lets an empty TransportName slip through; change the test in more_coverage_test.go to explicitly assert the expected default for TransportName returned by core.NewBaseHandlers(core.BaseHandlerConfig{}). Locate the creation of handlers (handlers := core.NewBaseHandlers(core.BaseHandlerConfig{})) and replace the ambiguous check with a clear assertion such as verifying handlers.TransportName == "" (or the exact expected string like "apicore" if that is intended) and call t.Fatalf with the actual value when it doesn't match.internal/session/manager_prompt.go (1)
14-62: Consider documenting or tracking the goroutine lifecycle.The goroutine at line 60 (
go m.pumpPrompt(...)) is not tracked with async.WaitGroupor equivalent. While the channel-based design ensures the goroutine terminates when the source closes, callers have no way to wait for pump completion.This may be intentional for the streaming use case, but per coding guidelines: "No fire-and-forget goroutines — track with
sync.WaitGroupor equivalent."If tracking is not needed, consider adding a brief comment explaining why (e.g., "// goroutine terminates when source channel closes").
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/session/manager_prompt.go` around lines 14 - 62, The goroutine started in Manager.Prompt (go m.pumpPrompt(...)) is currently fire-and-forget; either track it with a sync.WaitGroup or document why it's safe. To fix, add a sync.WaitGroup field (e.g., wg sync.WaitGroup) to Manager, call m.wg.Add(1) immediately before spawning the goroutine in Prompt, and update pumpPrompt to call defer m.wg.Done() at the start so callers can wait on m.wg.Wait() when needed; alternatively, if you intentionally do not track it, add a concise comment above the go m.pumpPrompt(...) line explaining that the goroutine always terminates when the source channel closes and therefore is safe to spawn without a WaitGroup.internal/api/core/workspaces.go (1)
184-198: Consider removing trivial wrapper functions.These one-liner functions add an unnecessary layer of indirection without providing additional functionality. They simply delegate to
*Internalcounterparts with identical signatures.If the intent is to allow future extension or testing seams, consider documenting that purpose. Otherwise, calling the internal functions directly would reduce code surface area.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/core/workspaces.go` around lines 184 - 198, Remove the trivial one-line wrapper functions filterSessionInfosByWorkspaceID, validateAbsolutePath, validateAbsolutePaths, and trimStringSlice and update all call sites to call their corresponding internal implementations (filterSessionInfosByWorkspaceIDInternal, validateAbsolutePathInternal, validateAbsolutePathsInternal, trimStringSliceInternal) directly; if these wrappers were intended as a documented extension or test seam, instead add a short comment above each wrapper explaining that intent—otherwise delete the wrappers, run tests, and fix any import/visibility issues that arise from the change.internal/api/core/handlers_test.go (2)
262-277: Test verifies stub behavior rather than handler logic.This test calls
manager.ApprovePermissiondirectly on the stub without invoking any HTTP handler. While it validates the stub wiring, it doesn't exercise the actual HTTP endpoint or handler path.Consider either removing this test (if stub behavior is obvious) or converting it to test the actual
/sessions/{id}/approveendpoint if one exists.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/core/handlers_test.go` around lines 262 - 277, The test TestBaseHandlersApprovePermissionGapResolvedInStub is exercising the stub directly (calling manager.ApprovePermission) instead of the HTTP handler; update it to exercise the actual handler for the /sessions/{id}/approve endpoint or remove it. Specifically, replace the direct stub call in TestBaseHandlersApprovePermissionGapResolvedInStub with an HTTP request against the handler that routes to the session manager (or construct the handler and call ServeHTTP) so the test invokes the handler path that uses the session manager's Approve method, asserting the same id and acp.ApproveRequest.TurnID behavior; alternatively, if the stub wiring is trivial, delete this test to avoid redundant coverage.
20-147: Consider usingt.Runsubtests for better test isolation and reporting.This test function validates multiple endpoints but uses sequential assertions without
t.Run. Per coding guidelines, tests should uset.Run("Should...")pattern for each test case. This would provide:
- Better failure isolation (one failing case doesn't skip others)
- Clearer test output showing which specific endpoint failed
- Ability to run individual subtests
Example structure:
t.Run("Should list sessions", func(t *testing.T) { listResp := performRequest(t, fixture.Engine, http.MethodGet, "/sessions", nil) if listResp.Code != http.StatusOK { t.Fatalf("list status = %d, want %d", listResp.Code, http.StatusOK) } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/core/handlers_test.go` around lines 20 - 147, The test TestBaseHandlersSessionEndpoints runs many endpoint checks sequentially; refactor it to use t.Run subtests so each endpoint assertion is isolated and reported separately: wrap each performRequest + assertion block in t.Run with descriptive names (e.g., "Should list sessions", "Should create session", "Should get session", "Should return 404 for missing session", "Should stop session", "Should resume session", "Should get events", "Should get history", "Should get transcript"), keeping the same fixture, manager (including createCalled atomic), and assertions, and ensure any per-subtest state (if needed) is local to that t.Run closure; keep function name TestBaseHandlersSessionEndpoints and the existing performRequest calls and response checks unchanged except for moving them into the t.Run closures.internal/api/core/handlers.go (1)
561-593: DaemonStatus usesos.Getpid()directly instead of injected dependency.The
DaemonStatushandler callsos.Getpid()directly (Line 583), while the daemon struct has an injectedpidfunction for testability. For consistency and testability, consider using an injected PID source.This is a minor inconsistency since status endpoints are typically not unit-tested in isolation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/core/handlers.go` around lines 561 - 593, DaemonStatus calls os.Getpid() directly; replace that call with the injected PID provider on the handler (e.g., use h.Pid() or the handler's pid function) so the PID is sourced from the injected dependency for testability and consistency with BaseHandlers; update the struct reference in DaemonStatus (where PID is set) to call the injected function instead of os.Getpid().internal/api/core/interfaces.go (1)
21-33: Consider documenting the distinction betweenList()andListAll().
List()takes no context whileListAll()does. This asymmetry suggestsList()returns cached in-memory sessions whileListAll()may perform I/O. A brief doc comment clarifying when to use each would help implementers and consumers.Suggested documentation
// SessionManager is the runtime session surface exposed by API transports. type SessionManager interface { Create(ctx context.Context, opts session.CreateOpts) (*session.Session, error) + // List returns currently active in-memory sessions without I/O. List() []*session.SessionInfo + // ListAll retrieves all sessions including persisted ones; may perform I/O. ListAll(ctx context.Context) ([]*session.SessionInfo, error) Status(ctx context.Context, id string) (*session.SessionInfo, error)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/core/interfaces.go` around lines 21 - 33, Add a brief doc comment on the SessionManager interface clarifying the behavioral difference between List() and ListAll(): state that List() is a fast/cached, in-memory snapshot that takes no context and should be used for inexpensive, non-blocking reads of currently tracked sessions, whereas ListAll(ctx context.Context) may perform I/O or long-running operations (and thus accepts a context) to return a full authoritative list; attach this comment immediately above the SessionManager type and mention the two methods by name (List and ListAll) so implementers and callers understand which to use.internal/api/testutil/apitest.go (1)
52-61: Silently ignoring error fromListAllFnfallback.The
List()method falls back to callingListAllFnbut discards any error it returns. While this is test utility code, silently dropping errors can mask issues in tests that rely on this fallback behavior.Consider logging or handling the error
If the fallback is intentional, consider whether tests using
List()would benefit from knowing ifListAllFnfailed. Alternatively, document this behavior explicitly:func (s StubSessionManager) List() []*session.SessionInfo { if s.ListFn != nil { return s.ListFn() } if s.ListAllFn != nil { - infos, _ := s.ListAllFn(context.Background()) + // Fallback: errors are intentionally ignored since List() has no error return. + infos, _ := s.ListAllFn(context.Background()) return infos } return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/testutil/apitest.go` around lines 52 - 61, The List method on StubSessionManager currently ignores the error returned by ListAllFn; update StubSessionManager.List to check the (infos, err) result from ListAllFn and handle the error instead of discarding it — e.g., if err != nil produce a visible failure (log or panic) with a descriptive message so tests won't silently hide failures, otherwise return infos; reference the StubSessionManager.List method and the ListFn/ListAllFn fields when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5996c634-6a84-4c31-b7cb-a9d1a00f3019
📒 Files selected for processing (182)
internal/acp/client_integration_test.gointernal/acp/client_test.gointernal/acp/handlers.gointernal/acp/handlers_test.gointernal/acp/rawjson.gointernal/api/contract/contract.gointernal/api/contract/contract_test.gointernal/api/core/conversions.gointernal/api/core/conversions_parsers_test.gointernal/api/core/error_paths_test.gointernal/api/core/errors.gointernal/api/core/handlers.gointernal/api/core/handlers_test.gointernal/api/core/interfaces.gointernal/api/core/memory.gointernal/api/core/memory_workspace_test.gointernal/api/core/more_coverage_test.gointernal/api/core/parsers.gointernal/api/core/payloads.gointernal/api/core/session_workspace.gointernal/api/core/session_workspace_internal_test.gointernal/api/core/sse.gointernal/api/core/test_helpers_test.gointernal/api/core/workspaces.gointernal/api/httpapi/agents.gointernal/api/httpapi/daemon.gointernal/api/httpapi/handlers_error_test.gointernal/api/httpapi/handlers_test.gointernal/api/httpapi/helpers_integration_test.gointernal/api/httpapi/helpers_test.gointernal/api/httpapi/httpapi_integration_test.gointernal/api/httpapi/memory.gointernal/api/httpapi/memory_test.gointernal/api/httpapi/observe.gointernal/api/httpapi/prompt.gointernal/api/httpapi/prompt_contract_test.gointernal/api/httpapi/server.gointernal/api/httpapi/server_test.gointernal/api/httpapi/sessions.gointernal/api/httpapi/shared_test.gointernal/api/httpapi/static.gointernal/api/httpapi/static_test.gointernal/api/httpapi/stream.gointernal/api/httpapi/stream_helpers_test.gointernal/api/httpapi/workspaces.gointernal/api/testutil/apitest.gointernal/api/testutil/apitest_test.gointernal/api/udsapi/agents.gointernal/api/udsapi/daemon.gointernal/api/udsapi/handlers_error_test.gointernal/api/udsapi/handlers_test.gointernal/api/udsapi/helpers_test.gointernal/api/udsapi/memory.gointernal/api/udsapi/memory_test.gointernal/api/udsapi/observe.gointernal/api/udsapi/payloads.gointernal/api/udsapi/prompt.gointernal/api/udsapi/routes.gointernal/api/udsapi/server.gointernal/api/udsapi/server_test.gointernal/api/udsapi/sessions.gointernal/api/udsapi/shared_test.gointernal/api/udsapi/stream.gointernal/api/udsapi/stream_helpers_test.gointernal/api/udsapi/udsapi_integration_test.gointernal/api/udsapi/workspaces.gointernal/cli/agent.gointernal/cli/agent_commands_test.gointernal/cli/cli_integration_test.gointernal/cli/client.gointernal/cli/client_test.gointernal/cli/command_paths_test.gointernal/cli/daemon.gointernal/cli/daemon_wait_test.gointernal/cli/format.gointernal/cli/helpers_test.gointernal/cli/memory.gointernal/cli/memory_test.gointernal/cli/observe.gointernal/cli/render_test.gointernal/cli/root.gointernal/cli/session.gointernal/cli/session_test.gointernal/cli/skill.gointernal/cli/skill_test.gointernal/cli/workspace.gointernal/config/agent.gointernal/config/agent_test.gointernal/config/home.gointernal/config/home_test.gointernal/daemon/boot.gointernal/daemon/boundary.gointernal/daemon/daemon.gointernal/daemon/daemon_integration_test.gointernal/daemon/daemon_test.gointernal/daemon/lock.gointernal/daemon/notifier.gointernal/daemon/orphan.gointernal/filesnap/filesnap.gointernal/filesnap/filesnap_test.gointernal/fileutil/atomic.gointernal/fileutil/atomic_test.gointernal/frontmatter/frontmatter.gointernal/frontmatter/frontmatter_test.gointernal/httpapi/agents.gointernal/httpapi/daemon.gointernal/httpapi/helpers_test.gointernal/httpapi/observe.gointernal/httpapi/sessions.gointernal/httpapi/stream.gointernal/httpapi/workspaces.gointernal/memory/consolidation/runtime.gointernal/memory/consolidation/runtime_test.gointernal/memory/dream_test.gointernal/memory/lock.gointernal/memory/lock_test.gointernal/memory/store.gointernal/observe/health.gointernal/observe/helpers_test.gointernal/observe/observer.gointernal/observe/observer_integration_test.gointernal/observe/observer_test.gointernal/observe/reconcile_test.gointernal/procutil/procutil.gointernal/procutil/procutil_test.gointernal/session/additional_test.gointernal/session/interfaces.gointernal/session/manager.gointernal/session/manager_helpers.gointernal/session/manager_integration_test.gointernal/session/manager_lifecycle.gointernal/session/manager_prompt.gointernal/session/manager_stop_integration_test.gointernal/session/manager_test.gointernal/session/manager_workspace.gointernal/session/query_test.gointernal/session/transcript.gointernal/session/transcript_test.gointernal/skills/loader.gointernal/skills/loader_test.gointernal/skills/registry.gointernal/skills/registry_test.gointernal/skills/types.gointernal/skills/watcher.gointernal/store/global_db.gointernal/store/globaldb/global_db.gointernal/store/globaldb/global_db_extra_test.gointernal/store/globaldb/global_db_observe.gointernal/store/globaldb/global_db_permission.gointernal/store/globaldb/global_db_session.gointernal/store/globaldb/global_db_test.gointernal/store/globaldb/global_db_workspace.gointernal/store/globaldb/migrate_workspace.gointernal/store/meta.gointernal/store/schema.gointernal/store/sessiondb/session_db.gointernal/store/sessiondb/session_db_extra_test.gointernal/store/sessiondb/session_db_integration_test.gointernal/store/sessiondb/session_db_test.gointernal/store/sql_helpers.gointernal/store/sqlite.gointernal/store/store.gointernal/store/store_extra_test.gointernal/store/store_helpers_test.gointernal/store/types.gointernal/store/validation.gointernal/testutil/testutil.gointernal/testutil/testutil_test.gointernal/transcript/transcript.gointernal/transcript/transcript_test.gointernal/udsapi/handlers.gointernal/udsapi/helpers_test.gointernal/udsapi/memory.gointernal/udsapi/routes.gointernal/udsapi/workspaces.gointernal/workspace/clone.gointernal/workspace/helpers.gointernal/workspace/resolver.gointernal/workspace/resolver_crud.gointernal/workspace/resolver_integration_test.gointernal/workspace/resolver_test.gointernal/workspace/scanner.go
💤 Files with no reviewable changes (7)
- internal/httpapi/daemon.go
- internal/httpapi/sessions.go
- internal/httpapi/stream.go
- internal/httpapi/workspaces.go
- internal/httpapi/observe.go
- internal/httpapi/helpers_test.go
- internal/httpapi/agents.go
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (2)
internal/api/core/sse.go (1)
97-115:⚠️ Potential issue | 🔴 Critical
event.IDis still not a stable same-timestamp cursor key.Line 109 and Line 115 both assume lexicographic
event.IDorder matches actual emission order. When two summaries share a timestamp, reconnects can replay or drop the wrong item unless the cursor uses a monotonic DB sequence or insertion-order key instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/core/sse.go` around lines 97 - 115, The comparison and Last-Event-ID construction in ObserveEventAfterCursor and ObserveEventID assume event.ID orders lexicographically, which is unstable for same-timestamp events; change the cursor and ID generation to use a monotonic insertion-order/sequence value (e.g., event.Sequence or event.Serial) instead of relying on event.ID string ordering: update ObserveEventAfterCursor to, after timestamp equality, compare the monotonic sequence (fallback to event.ID only if sequence is absent), and update ObserveEventID to include the timestamp + a fixed-width zero-padded sequence (or the monotonic key) after the separator so the Last-Event-ID is stable and preserves emission order for identical timestamps. Ensure the cursor type ObserveCursor is updated to hold the sequence and that any consumers use that sequence in comparisons and parsing.internal/session/manager_lifecycle.go (1)
274-286:⚠️ Potential issue | 🟠 MajorPass context.Context to watchProcess and use select with ctx.Done() for cancellation.
This is the same issue flagged in a previous review. The goroutine spawned here violates coding guidelines:
- Missing context propagation:
watchProcess(session)is called fromactivateAndWatch(ctx, ...)butctxis not passed.- No goroutine tracking: The goroutine cannot be cancelled and is fire-and-forget.
Per context snippet from
internal/session/interfaces.go,AgentProcesshas aDone() <-chan struct{}method that can be used withselect.As per coding guidelines: "Every goroutine must have explicit ownership and shutdown via context.Context cancellation" and "Use
selectwithctx.Done()in all long-running goroutine loops".,
Proposed fix
-func (m *Manager) watchProcess(session *Session) { +func (m *Manager) watchProcess(ctx context.Context, session *Session) { proc := session.processHandle() if proc == nil { return } go func() { - waitErr := proc.Wait() + select { + case <-ctx.Done(): + return + case <-proc.Done(): + } + waitErr := proc.Wait() if err := m.handleProcessExit(session, waitErr); err != nil { m.sessionLogger(session).Warn("session: process exit handling failed", "error", err) } }() }And update the call in
activateAndWatch(inmanager_helpers.go):- m.watchProcess(session) + m.watchProcess(ctx, session)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/session/manager_lifecycle.go` around lines 274 - 286, The goroutine in Manager.watchProcess is created without context propagation or cancellation; change the signature to accept ctx (e.g., func (m *Manager) watchProcess(ctx context.Context, session *Session)) and update the caller in activateAndWatch to pass its ctx. Inside watchProcess, run the wait logic in a cancellable select that listens on ctx.Done() and the process completion (use session.processHandle().Wait() result or the process Done() channel from AgentProcess.Done()), then call m.handleProcessExit(session, waitErr) only when the process actually exits; ensure the goroutine returns promptly on ctx cancellation so it is properly owned and shutdownable.
🧹 Nitpick comments (5)
internal/fileutil/atomic.go (1)
28-32: Add a comment justifying the ignored error on temp file cleanup.The ignored error
_ = os.Remove(tempPath)violates the coding guideline requiring every ignored error to have a written justification. While ignoring this error is reasonable (it's best-effort cleanup and failure doesn't affect correctness), adding a brief comment will satisfy the guideline and document the intent.💡 Suggested fix
defer func() { if cleanup { - _ = os.Remove(tempPath) + _ = os.Remove(tempPath) // Best-effort cleanup; temp file may already be gone or dir unwritable } }()As per coding guidelines: "Never ignore errors with
_— every error must be handled or have a written justification".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/fileutil/atomic.go` around lines 28 - 32, The deferred cleanup currently calls `_ = os.Remove(tempPath)` in the anonymous func (guarded by `cleanup`) and ignores the error; add a brief comment above that line explaining why the error is intentionally ignored (e.g., best-effort cleanup, non-fatal, does not affect correctness or resource safety) so it satisfies the guideline; reference the deferred anonymous function, the `cleanup` variable, and the `os.Remove(tempPath)` call when adding the justification comment.internal/api/testutil/apitest.go (1)
53-65: Consider returning empty slice instead of panicking in test fallback.The
List()fallback toListAllFnpanics on error (line 60), which could cause unhelpful test failures if a test accidentally triggers this path. Since this is test utility code, a more defensive approach would help debugging.♻️ Proposed alternative: return empty slice on fallback error
func (s StubSessionManager) List() []*session.SessionInfo { if s.ListFn != nil { return s.ListFn() } if s.ListAllFn != nil { infos, err := s.ListAllFn(context.Background()) - if err != nil { - panic(fmt.Errorf("testutil: StubSessionManager.List fallback failed: %w", err)) - } + if err != nil { + return nil // Test should set ListFn if error behavior matters + } return infos } return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/testutil/apitest.go` around lines 53 - 65, The StubSessionManager.List fallback currently panics when ListAllFn returns an error; change this to be defensive: in the List method (StubSessionManager.List) call s.ListAllFn(context.Background()), and if it returns an error, do not panic—return an empty slice (e.g., []*session.SessionInfo{}) instead so tests hitting the fallback get an empty result rather than crashing; keep existing behavior of returning s.ListFn() when present and nil only if neither handler is provided.internal/cli/client_test.go (1)
424-461: Consider using "Should..." naming pattern for subtests.The table-driven test structure is good, but the subtest names like
"createSessionRequest"don't follow thet.Run("Should...")pattern. As per coding guidelines: "MUST use t.Run("Should...") pattern for ALL test cases".♻️ Suggested naming pattern
tests := []struct { - name string + name string // e.g., "Should_alias_CreateSessionRequest_to_contract" cliType any want any }{ - {name: "createSessionRequest", cliType: CreateSessionRequest{}, want: contract.CreateSessionRequest{}}, - {name: "sessionRecord", cliType: SessionRecord{}, want: contract.SessionPayload{}}, + {name: "Should alias CreateSessionRequest to contract type", cliType: CreateSessionRequest{}, want: contract.CreateSessionRequest{}}, + {name: "Should alias SessionRecord to contract type", cliType: SessionRecord{}, want: contract.SessionPayload{}}, // ... similar for others🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/client_test.go` around lines 424 - 461, Update the subtest names in TestCLIUsesSharedContractAliases to follow the required t.Run("Should ...") pattern: modify the test table entries (the name fields) or the t.Run invocation so each subtest name is prefixed with "Should", e.g. "Should create session request" for the CreateSessionRequest case; ensure you update all entries (createSessionRequest, sessionRecord, sessionEventRecord, etc.) or prepend "Should " at t.Run(tt.name, ...) (e.g. t.Run(fmt.Sprintf("Should %s", tt.name), ...)) to preserve intent and keep reflect checks unchanged.internal/api/core/handlers.go (1)
101-103: Default StreamDone channel never signals shutdown.When
cfg.StreamDoneis nil, a new channel is created at line 102 that is never closed. This means streaming handlers will only terminate via request context cancellation, never via server shutdown signal. This could leave streams running longer than expected during graceful shutdown if the caller forgets to set StreamDone.Consider documenting this behavior or logging a warning when StreamDone is not provided.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/core/handlers.go` around lines 101 - 103, The code creates a default cfg.StreamDone channel but never closes it, so update the initialization to either log a warning when StreamDone is nil or (preferably) wire the new channel to the server shutdown lifecycle: when you allocate cfg.StreamDone, spawn a short goroutine that waits on the server shutdown context or shutdown signal (e.g., a provided context, cfg.ShutdownCtx, or equivalent) and closes cfg.StreamDone when shutdown begins; reference cfg.StreamDone and add a log entry so callers know a default channel was created if no explicit shutdown hook exists.internal/session/manager_lifecycle.go (1)
288-299: Consider accepting context parameter instead of using context.Background().Line 298 uses
context.Background()which the coding guidelines discourage outsidemainand tests. However, sincehandleProcessExitis called from thewatchProcessgoroutine after a process exits (potentially long after the original context was cancelled), usingcontext.Background()is a reasonable choice here to ensure cleanup completes.If
watchProcessis updated to accept context (per previous comment), that context could be passed tohandleProcessExit, though it might already be cancelled at this point.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/session/manager_lifecycle.go` around lines 288 - 299, Change Manager.handleProcessExit to accept a context.Context parameter and use that context when calling finalizeStopped instead of using context.Background(); update its callers (e.g., watchProcess) to pass through the available context (even if it may be cancelled) so the function signature is Manager.handleProcessExit(ctx context.Context, session *Session, waitErr error) and the call site uses that ctx when invoking m.finalizeStopped(ctx, session, waitErr); keep references to Manager.handleProcessExit, watchProcess, and finalizeStopped when making the edits.
🤖 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/api/core/handlers.go`:
- Around line 182-187: The handler currently omits the session Type when calling
Sessions.Create (session.CreateOpts in internal/api/core/handlers.go), which
causes normalizeSessionType to default empty string to SessionTypeUser; either
(A) add a Type field to the API contract (CreateSessionRequest) and map req.Type
into session.CreateOpts.Type so callers can create Dream/System sessions,
ensuring you validate/convert the incoming string to the internal enum
(normalizeSessionType or equivalent), or (B) if the API should only create user
sessions, explicitly set opts.Type = session.SessionTypeUser before calling
h.Sessions.Create and document that CreateSessionRequest does not support other
types — update the call site in handlers.go (the sess creation block) and the
CreateSessionRequest definition accordingly.
In `@internal/api/core/memory_workspace_test.go`:
- Line 68: Test requests interpolate workspace, rootDir, and addDir directly
into URLs and JSON, which breaks on Windows due to backslashes/spaces; update
the tests (where performRequest is called) to build query strings with
url.Values or url.QueryEscape for the workspace parameter and build JSON bodies
via json.Marshal (instead of string concatenation) for payloads that include
rootDir/addDir, ensuring all request URLs and JSON bodies are properly
escaped/encoded.
- Around line 69-71: The tests currently only assert HTTP status (e.g., the
listResp.Code check) which misses payload regressions; update each handler
subtest (those using variables like listResp, getResp, createResp, deleteResp)
to decode the JSON response body into the concrete response structs (memory
list/item struct, health/workspace DTOs used by the handlers) and add assertions
on key business fields (IDs, WorkspaceID, Name/Content, timestamps or deleted
flags, and any health fields) to verify the actual payload shape and persisted
state rather than just the status code. Ensure you call
json.NewDecoder(resp.Body).Decode(&<expectedStruct>), check for decode errors,
and replace the status-only t.Fatalf checks with both status assertions and
explicit field asserts so tests fail on contract regressions.
In `@internal/api/core/sse.go`:
- Around line 81-90: The observe cursor (ObserveCursor) is advanced before
attempting to write the SSE frame, causing skipped events on write failure;
modify EmitObserveEvents so that next = ObserveCursor{Timestamp:
event.Timestamp.UTC(), ID: event.ID} is assigned only after WriteSSE(writer,
SSEMessage{ID: ObserveEventID(event), Name: event.Type, Data:
ObserveEventPayloadFromEvent(event)}) returns nil, i.e. move the next assignment
below the successful WriteSSE call and keep returning the prior cursor on error
so reconnects can retry the unsent event.
In `@internal/daemon/boundary.go`:
- Around line 47-54: The shouldVerifyBoundaries method can panic if d.getenv is
nil; update shouldVerifyBoundaries to guard: if d.verifyBoundaries return true;
otherwise resolve an envGetter function pointer (use d.getenv if non-nil else
use os.Getenv) and call it for "AGH_DEV_VERIFY_BOUNDARIES", trim/lower it and
compare to "1"/"true"/"yes"; reference the shouldVerifyBoundaries method and the
d.getenv field when making this change.
In `@internal/session/manager_helpers.go`:
- Around line 67-84: activateAndWatch currently attaches the live AgentProcess
via session.updateFromProcess early and can return before subsequent fallible
calls (session.activate, m.writeMeta, m.activate) complete, leaking the process
or leaving session state inconsistent; fix this by reordering and/or adding
rollback: perform the fallible operations (call session.activate(now),
m.writeMeta(session), and m.activate(session)) before calling
session.updateFromProcess(ctx, proc) and only invoke m.notifier.OnSessionCreated
and m.watchProcess after those succeed, and if you prefer rollback instead,
ensure on any error after updateFromProcess you stop/terminate the AgentProcess
and clear the attached state on Session (undo whatever session.updateFromProcess
sets) and revert any written metadata (or call the corresponding metadata/
deactivate helper) so no active process or active session state remains on
error.
In `@internal/session/manager_prompt.go`:
- Around line 103-118: The pumpPrompt goroutine can hang if ctx is canceled
while still ranging over source; change the loop in Manager.pumpPrompt to select
on ctx.Done() when receiving and when sending so the goroutine exits promptly on
cancellation: replace the "for event := range source" with a loop using "select
{ case <-ctx.Done(): return; case event, ok := <-source: if !ok { return } ...
}" and keep the existing send guarded by "select { case out <- normalized: case
<-ctx.Done(): return }"; use the same symbols (pumpPrompt, normalizeEvent,
recordEvent, notifier.OnAgentEvent, sessionLogger) so the function returns when
ctx is done and out is closed via the existing defer.
---
Duplicate comments:
In `@internal/api/core/sse.go`:
- Around line 97-115: The comparison and Last-Event-ID construction in
ObserveEventAfterCursor and ObserveEventID assume event.ID orders
lexicographically, which is unstable for same-timestamp events; change the
cursor and ID generation to use a monotonic insertion-order/sequence value
(e.g., event.Sequence or event.Serial) instead of relying on event.ID string
ordering: update ObserveEventAfterCursor to, after timestamp equality, compare
the monotonic sequence (fallback to event.ID only if sequence is absent), and
update ObserveEventID to include the timestamp + a fixed-width zero-padded
sequence (or the monotonic key) after the separator so the Last-Event-ID is
stable and preserves emission order for identical timestamps. Ensure the cursor
type ObserveCursor is updated to hold the sequence and that any consumers use
that sequence in comparisons and parsing.
In `@internal/session/manager_lifecycle.go`:
- Around line 274-286: The goroutine in Manager.watchProcess is created without
context propagation or cancellation; change the signature to accept ctx (e.g.,
func (m *Manager) watchProcess(ctx context.Context, session *Session)) and
update the caller in activateAndWatch to pass its ctx. Inside watchProcess, run
the wait logic in a cancellable select that listens on ctx.Done() and the
process completion (use session.processHandle().Wait() result or the process
Done() channel from AgentProcess.Done()), then call m.handleProcessExit(session,
waitErr) only when the process actually exits; ensure the goroutine returns
promptly on ctx cancellation so it is properly owned and shutdownable.
---
Nitpick comments:
In `@internal/api/core/handlers.go`:
- Around line 101-103: The code creates a default cfg.StreamDone channel but
never closes it, so update the initialization to either log a warning when
StreamDone is nil or (preferably) wire the new channel to the server shutdown
lifecycle: when you allocate cfg.StreamDone, spawn a short goroutine that waits
on the server shutdown context or shutdown signal (e.g., a provided context,
cfg.ShutdownCtx, or equivalent) and closes cfg.StreamDone when shutdown begins;
reference cfg.StreamDone and add a log entry so callers know a default channel
was created if no explicit shutdown hook exists.
In `@internal/api/testutil/apitest.go`:
- Around line 53-65: The StubSessionManager.List fallback currently panics when
ListAllFn returns an error; change this to be defensive: in the List method
(StubSessionManager.List) call s.ListAllFn(context.Background()), and if it
returns an error, do not panic—return an empty slice (e.g.,
[]*session.SessionInfo{}) instead so tests hitting the fallback get an empty
result rather than crashing; keep existing behavior of returning s.ListFn() when
present and nil only if neither handler is provided.
In `@internal/cli/client_test.go`:
- Around line 424-461: Update the subtest names in
TestCLIUsesSharedContractAliases to follow the required t.Run("Should ...")
pattern: modify the test table entries (the name fields) or the t.Run invocation
so each subtest name is prefixed with "Should", e.g. "Should create session
request" for the CreateSessionRequest case; ensure you update all entries
(createSessionRequest, sessionRecord, sessionEventRecord, etc.) or prepend
"Should " at t.Run(tt.name, ...) (e.g. t.Run(fmt.Sprintf("Should %s", tt.name),
...)) to preserve intent and keep reflect checks unchanged.
In `@internal/fileutil/atomic.go`:
- Around line 28-32: The deferred cleanup currently calls `_ =
os.Remove(tempPath)` in the anonymous func (guarded by `cleanup`) and ignores
the error; add a brief comment above that line explaining why the error is
intentionally ignored (e.g., best-effort cleanup, non-fatal, does not affect
correctness or resource safety) so it satisfies the guideline; reference the
deferred anonymous function, the `cleanup` variable, and the
`os.Remove(tempPath)` call when adding the justification comment.
In `@internal/session/manager_lifecycle.go`:
- Around line 288-299: Change Manager.handleProcessExit to accept a
context.Context parameter and use that context when calling finalizeStopped
instead of using context.Background(); update its callers (e.g., watchProcess)
to pass through the available context (even if it may be cancelled) so the
function signature is Manager.handleProcessExit(ctx context.Context, session
*Session, waitErr error) and the call site uses that ctx when invoking
m.finalizeStopped(ctx, session, waitErr); keep references to
Manager.handleProcessExit, watchProcess, and finalizeStopped when making the
edits.
🪄 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: 50797426-42aa-402b-a345-a753833424d2
📒 Files selected for processing (49)
internal/acp/handlers_test.gointernal/api/core/conversions_parsers_test.gointernal/api/core/error_paths_test.gointernal/api/core/handlers.gointernal/api/core/handlers_test.gointernal/api/core/interfaces.gointernal/api/core/memory_workspace_test.gointernal/api/core/more_coverage_test.gointernal/api/core/parsers.gointernal/api/core/sse.gointernal/api/core/workspaces.gointernal/api/httpapi/httpapi_integration_test.gointernal/api/httpapi/prompt.gointernal/api/httpapi/prompt_contract_test.gointernal/api/httpapi/server.gointernal/api/testutil/apitest.gointernal/api/udsapi/udsapi_integration_test.gointernal/cli/client.gointernal/cli/client_test.gointernal/cli/helpers_test.gointernal/cli/render_test.gointernal/config/agent.gointernal/config/agent_test.gointernal/daemon/boundary.gointernal/daemon/daemon_integration_test.gointernal/daemon/daemon_test.gointernal/daemon/notifier.gointernal/filesnap/filesnap.gointernal/filesnap/filesnap_test.gointernal/fileutil/atomic.gointernal/fileutil/atomic_dirsync_unix.gointernal/fileutil/atomic_dirsync_windows.gointernal/fileutil/atomic_test.gointernal/frontmatter/frontmatter.gointernal/frontmatter/frontmatter_test.gointernal/memory/consolidation/runtime.gointernal/memory/consolidation/runtime_test.gointernal/memory/lock_test.gointernal/observe/observer_integration_test.gointernal/procutil/procutil.gointernal/procutil/procutil_test.gointernal/procutil/procutil_windows.gointernal/session/manager_helpers.gointernal/session/manager_integration_test.gointernal/session/manager_lifecycle.gointernal/session/manager_prompt.gointernal/session/manager_stop_integration_test.gointernal/session/manager_test.gointernal/session/query_test.go
💤 Files with no reviewable changes (1)
- internal/memory/lock_test.go
✅ Files skipped from review due to trivial changes (10)
- internal/cli/helpers_test.go
- internal/observe/observer_integration_test.go
- internal/filesnap/filesnap_test.go
- internal/cli/render_test.go
- internal/api/core/conversions_parsers_test.go
- internal/filesnap/filesnap.go
- internal/api/core/more_coverage_test.go
- internal/api/core/parsers.go
- internal/api/core/error_paths_test.go
- internal/api/core/interfaces.go
🚧 Files skipped from review as they are similar to previous changes (13)
- internal/acp/handlers_test.go
- internal/config/agent.go
- internal/procutil/procutil_test.go
- internal/procutil/procutil.go
- internal/config/agent_test.go
- internal/daemon/daemon_integration_test.go
- internal/frontmatter/frontmatter_test.go
- internal/frontmatter/frontmatter.go
- internal/daemon/notifier.go
- internal/api/httpapi/prompt.go
- internal/api/core/handlers_test.go
- internal/memory/consolidation/runtime_test.go
- internal/memory/consolidation/runtime.go
This was referenced Apr 8, 2026
Merged
Merged
Merged
Merged
Merged
Merged
This was referenced Apr 17, 2026
Closed
Closed
Closed
Closed
Closed
This was referenced May 24, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
This was referenced May 26, 2026
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
Bug Fixes
Improvements