feat: add task deletion and split session delete from stop#58
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (34)
📒 Files selected for processing (40)
WalkthroughAdds explicit session and task deletion end-to-end: new HTTP handlers, manager/store delete implementations, route and OpenAPI updates, CLI/e2e adjustments, frontend adapters/hooks/UI for delete, and extensive tests; StopSession moved to POST /api/sessions/{id}/stop while DELETE /sessions/{id} deletes persisted session state. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Frontend
participant API as "API Handler"
participant SM as "SessionManager"
participant FS as "FileSystem"
User->>Frontend: Confirm "Delete session"
Frontend->>API: DELETE /api/sessions/{id}
API->>SM: Delete(ctx, id)
alt session active
Note right of SM: stopSessionBeforeDelete -> stop (CauseUserRequested)
SM->>SM: stop active session
end
SM->>FS: stat session directory
alt directory exists
SM->>FS: remove directory (RemoveAll)
SM->>SM: remove from runtime state
SM-->>API: success
else not found
SM-->>API: ErrSessionNotFound
end
API-->>Frontend: 204 No Content or 4xx/5xx
sequenceDiagram
actor User
participant Frontend
participant API as "API Handler"
participant TM as "TaskManager"
participant RS as "RecordStore"
participant DB as "Database"
User->>Frontend: Confirm "Delete task"
Frontend->>API: DELETE /api/tasks/{id}
API->>TM: DeleteTask(ctx, id, actor)
TM->>TM: validate (no children, no open runs)
alt validation fails
TM-->>API: ErrValidation
else
TM->>RS: WithDeleteTaskTransaction(fn)
RS->>DB: DELETE FROM tasks WHERE id=?
DB-->>RS: result or FK error
alt FK constraint
RS-->>TM: ErrValidation
TM-->>API: 400 Bad Request
else success
RS-->>TM: success
TM->>TM: reconcile dependents (cascade)
TM-->>API: success
end
end
API-->>Frontend: 204 No Content or 4xx/5xx
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/systems/session/adapters/session-api.ts (1)
75-99:⚠️ Potential issue | 🟠 MajorDo not throw raw
Errorfrom adapter methods.Line 82, Line 84, Line 95, and Line 97 should use a typed adapter error class for consistent handling across hooks/UI.
As per coding guidelines, "web/src/systems/*/adapters/*-api.ts: Typed error classes in adapters — never throw raw errors".Proposed direction
+export class SessionApiError extends Error { + constructor( + message: string, + public readonly status: number + ) { + super(message); + this.name = "SessionApiError"; + } +} + export async function deleteSession(id: string, signal?: AbortSignal): Promise<void> { const { error, response } = await apiClient.DELETE("/api/sessions/{id}", { params: { path: { id } }, signal, }); if (apiRequestFailed(response, error)) { if (response.status === 404) { - throw new Error(`Session not found: ${id}`); + throw new SessionApiError(`Session not found: ${id}`, 404); } - throw new Error(defaultApiErrorMessage(`Failed to delete session "${id}"`, response, error)); + throw new SessionApiError( + defaultApiErrorMessage(`Failed to delete session "${id}"`, response, error), + response.status + ); } } export async function stopSession(id: string, signal?: AbortSignal): Promise<void> { const { error, response } = await apiClient.POST("/api/sessions/{id}/stop", { params: { path: { id } }, signal, }); if (apiRequestFailed(response, error)) { if (response.status === 404) { - throw new Error(`Session not found: ${id}`); + throw new SessionApiError(`Session not found: ${id}`, 404); } - throw new Error(defaultApiErrorMessage(`Failed to stop session "${id}"`, response, error)); + throw new SessionApiError( + defaultApiErrorMessage(`Failed to stop session "${id}"`, response, error), + response.status + ); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/systems/session/adapters/session-api.ts` around lines 75 - 99, Replace the raw Error throws in deleteSession and stopSession with the project's typed adapter error class (e.g., AdapterError or ApiAdapterError): import the adapter error class and throw new AdapterError(...) instead of new Error(...); for the 404 case throw the typed error with the same "Session not found: {id}" message, and for other failures throw the typed error constructed with defaultApiErrorMessage(`Failed to delete/stop session "${id}"`, response, error) and include any response/error metadata the adapter error supports so calling code can inspect status/details. Ensure changes are made in functions deleteSession and stopSession and that the adapter error class is properly imported.
🧹 Nitpick comments (10)
web/src/systems/tasks/adapters/tasks-api.test.ts (1)
261-265: Strengthen the 404 test to assert the typed error contract.Right now it only checks the message. Also assert
TasksApiErrorso a regression to rawErroris caught.Proposed test tightening
it("throws not-found for 404", async () => { vi.mocked(globalThis.fetch).mockResolvedValue(new Response(null, { status: 404 })); - await expect(deleteTask("missing")).rejects.toThrow("Task not found: missing"); + const request = deleteTask("missing"); + await expect(request).rejects.toBeInstanceOf(TasksApiError); + await expect(request).rejects.toThrow("Task not found: missing"); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/systems/tasks/adapters/tasks-api.test.ts` around lines 261 - 265, Update the "throws not-found for 404" test to assert the typed error contract: when mocking fetch to return 404 and calling deleteTask("missing"), assert the rejection is an instance of TasksApiError (not a raw Error) and also assert the error message equals "Task not found: missing" — e.g., use expect(...).rejects.toBeInstanceOf(TasksApiError) and expect(...).rejects.toThrow("Task not found: missing") or await the rejection and assert instance and message from the caught error; reference the deleteTask function and the TasksApiError class in the test.web/src/systems/session/hooks/use-session-actions.test.tsx (1)
170-196: Add a delete-failure test case to lock cache behavior.This test is good for the success path. Please add a failure-path case (
deleteSessionrejects) to assert caches/drafts are not incorrectly cleared when deletion fails.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/systems/session/hooks/use-session-actions.test.tsx` around lines 170 - 196, Add a new test in use-session-actions.test.tsx that covers the failure path for useDeleteSession: mock vi.mocked(deleteSession).mockRejectedValue(new Error("fail")), initialize a QueryClient and setQueryData for sessionKeys.detail(createdSession.id), sessionKeys.transcript(createdSession.id), sessionKeys.history(createdSession.id) and set a draft via useSessionStore.getState().setDraft(createdSession.id, ...), render the hook with createWrapper(queryClient), call result.current.mutateAsync(createdSession.id) inside act and assert it rejects (await expect(...).rejects.toThrow()), then assert the queryClient.getQueryData for each key and useSessionStore.getState().drafts[createdSession.id] are still defined (not cleared) to ensure caches/drafts remain on delete failure.web/src/hooks/routes/use-session-page-controls.ts (1)
63-79: HardenhandleDeleteagainst concurrent control mutations.
handleDeletecurrently blocks only duplicate deletes. Consider guarding against other in-flight control mutations too (isStopping,isResuming,isClearing,isCancellingPrompt) to avoid overlapping operations through non-UI call paths.Also applies to: 92-97
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/hooks/routes/use-session-page-controls.ts` around lines 63 - 79, handleDelete only checks deleteMutation.isPending and should also bail out if any other control mutation is in-flight; update handleDelete to check deleteMutation.isPending OR the other control flags (isStopping, isResuming, isClearing, isCancellingPrompt) and return early if any are true before calling deleteMutation.mutate, keeping the existing onSuccess/onError behavior (including aui.thread().reset() and onDeleteSuccess). Apply the same multi-flag guard pattern to the other handlers around lines 92-97 (the handlers for stop/resume/clear/cancelPrompt) so each handler checks all control-mutation flags before mutating to prevent overlapping operations.internal/daemon/daemon_test.go (1)
4134-4139: Avoid aliasing delete behavior to stop in the session test double.
Delete()currently callsStop(), which can hide regressions now that delete and stop are distinct operations. Consider trackingDeletecalls independently (and optionally separate delete-specific errors/state mutation) so tests can assert the right path was used.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/daemon/daemon_test.go` around lines 4134 - 4139, The fakeSessionManager.Delete method currently forwards to Stop which masks differences between stop and delete; modify fakeSessionManager by implementing Delete to record its own invocation (e.g., increment a deleteCount or append to a deletedIDs slice) and return a configurable deleteErr/behavior instead of calling Stop; update the fake's fields to allow tests to assert that Delete was invoked separately from Stop and to simulate delete-specific errors or state mutation while leaving Stop unchanged.internal/api/core/tasks_test.go (1)
918-918: Consider adding delete to the remaining error-path matrices for parity.Nice addition in the actor-resolver error table. To fully cover this new critical endpoint, mirror
DELETE /tasks/:idin the service-unavailable and manager-error request matrices in this file as well.As per coding guidelines, "Must Check: Focus on critical paths: workflow execution, state management, error handling".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/core/tasks_test.go` at line 918, Add the DELETE /tasks/:id case to the other error-path request matrices used in these tests: in the service-unavailable request matrix and the manager-error request matrix add the same entry {method: http.MethodDelete, path: "/tasks/task-1"} so the DELETE endpoint is exercised on those error paths (look for the test slices/variables that build the "serviceUnavailableRequests" and "managerErrorRequests" or similarly named request matrices in internal/api/core/tasks_test.go and append the DELETE entry to each).internal/api/udsapi/handlers_error_test.go (1)
19-70: Refactor this endpoint-error matrix intot.Run("Should...")subtests.This block now covers multiple behaviors (create/get/resume/delete/stop) in one flow; converting it to table-driven subtests will improve failure locality and align with repo test conventions.
As per coding guidelines,
**/*_test.go: MUST use t.Run("Should...") pattern for ALL test cases.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/udsapi/handlers_error_test.go` around lines 19 - 70, Refactor TestCreateGetResumeDeleteAndStopHandlersReturnExpectedErrors into t.Run subtests: replace the single flow with a table-driven slice of cases (e.g., struct { name, method, path, body, wantStatus }) and for each case call t.Run(case.name, func(t *testing.T){ ... }); inside each subtest create the engine once using newTestRouter/newTestHandlers with the existing stubSessionManager and homePaths, call performRequest with case.method/path/body, and assert response.Code == case.wantStatus; keep the existing stubSessionManager and response expectations but split create/get/resume/delete/stop into distinct "Should..." subtest names to improve failure locality.internal/api/udsapi/handlers_test.go (1)
969-989: Consider addingt.Parallel()for test isolation.Other tests in this file use
t.Parallel()for independent execution. Adding it here would be consistent with the codebase patterns.♻️ Optional: Add t.Parallel()
func TestDeleteSessionHandlerReturnsNoContent(t *testing.T) { + t.Parallel() + homePaths := newTestHomePaths(t)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/udsapi/handlers_test.go` around lines 969 - 989, Add test isolation by calling t.Parallel() at the start of TestDeleteSessionHandlerReturnsNoContent (immediately after the function begins) so the test runs concurrently like the others; update the TestDeleteSessionHandlerReturnsNoContent function to begin with t.Parallel() while leaving the rest of the setup (newTestHomePaths, stubSessionManager, newTestHandlers, newTestRouter, performRequest assertions) unchanged.web/src/systems/tasks/hooks/use-task-actions.ts (1)
155-169: Minor redundancy in cache operations.
removeQueriescorrectly purges the deleted task's detail from cache. However,invalidateTaskQueries(queryClient, id)on line 164 will also callinvalidateQueries({ queryKey: tasksKeys.detail(id) })(see line 110), which is redundant after the query has already been removed.This is harmless but slightly wasteful. Consider either:
- Not passing
idtoinvalidateTaskQueriessince the detail query is already removed- Or keeping as-is for consistency with other mutation hooks
♻️ Optional: Remove redundant invalidation
export function useDeleteTask() { const queryClient = useQueryClient(); return useMutation({ mutationFn: ({ id }: TaskIdParams) => deleteTask(id), onSettled: (_result, _error, { id }) => { queryClient.removeQueries({ queryKey: tasksKeys.detail(id) }); return Promise.all([ - invalidateTaskQueries(queryClient, id), + invalidateTaskQueries(queryClient), invalidateAggregateQueries(queryClient), ]); }, }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/systems/tasks/hooks/use-task-actions.ts` around lines 155 - 169, In useDeleteTask, avoid redundant cache work: after queryClient.removeQueries({ queryKey: tasksKeys.detail(id) }) remove the id argument when calling invalidateTaskQueries so it doesn't re-invalidate the already-removed detail entry; update the call from invalidateTaskQueries(queryClient, id) to simply invalidateTaskQueries(queryClient) (or the equivalent overload) so only the broader task invalidations run while keeping the explicit removal of tasksKeys.detail(id).internal/api/httpapi/handlers_error_test.go (1)
23-74: Split this expanded error-mapping test into subtests for isolation.Now that this case covers five endpoints, one early failure hides the rest. Please convert to table-driven
t.Run("Should...")subtests (and mark independent ones parallel) so each route/method mapping fails independently.As per coding guidelines, "Use table-driven tests with subtests (
t.Run) as default pattern for Go tests" and "MUST use t.Run("Should...") pattern for ALL test cases".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/httpapi/handlers_error_test.go` around lines 23 - 74, The large TestCreateGetResumeDeleteAndStopHandlersReturnExpectedErrors test should be split into table-driven subtests using t.Run so each endpoint is tested in isolation: create a slice of test cases (name, method, path, body, expectedStatus) and iterate calling t.Run("Should <description>", func(t *testing.T){ t.Parallel() ... }), reusing newTestRouter/newTestHandlers and stubSessionManager setup and performRequest to execute each case and assert response codes; ensure each subtest is independent and marked parallel where safe so one failure doesn't hide others.internal/api/httpapi/handlers_test.go (1)
1074-1094: Uset.Run("Should...")for the new delete-session test case.Please wrap this new case in a
Should...subtest to match the required test pattern used by repo guidelines.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/api/httpapi/handlers_test.go` around lines 1074 - 1094, Wrap the TestDeleteSessionHandlerReturnsNoContent logic inside a t.Run subtest named "Should delete session and return no content" so it follows the repo's t.Run("Should...") pattern; specifically move the setup and assertions currently in TestDeleteSessionHandlerReturnsNoContent into a t.Run(...) closure while keeping the stubSessionManager DeleteFn, newTestHandlers, newTestRouter and performRequest calls intact (i.e., call t.Run(..., func(t *testing.T) { /* existing body using manager, handlers, engine, recorder */ })). Ensure the outer test function name stays TestDeleteSessionHandlerReturnsNoContent but delegates the actual scenario to the t.Run subtest.
🤖 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/spec/spec.go`:
- Around line 1554-1559: The OpenAPI spec for deleteTask is out of sync with
runtime error mapping from StatusForTaskError; replace the single 422 response
entry with responses that match the function's mappings: add a 400 "Invalid task
request" (Body: contract.ErrorPayload) for validation errors and a 409 "Task
conflict" (Body: contract.ErrorPayload) for conflict cases, keeping the existing
404, 503 and 500 entries; update the ResponseSpec list in the deleteTask
definition so generated clients match StatusForTaskError behavior.
In `@internal/session/manager_delete_test.go`:
- Around line 11-70: Convert the two top-level tests into table-driven subtests
using t.Run with names that begin "Should...": create a single
TestDeleteBehavior (or similar) that calls t.Parallel() once and iterates over a
cases slice where each case has a name ("Should remove stopped session from
history", "Should stop active session before removing artifacts") and a test
func. Move the existing bodies of TestDeleteRemovesStoppedSessionFromHistory and
TestDeleteStopsActiveSessionBeforeRemovingArtifacts into the case handlers,
preserving calls to h.manager.Stop, h.manager.Delete, h.manager.Status,
h.manager.ListAll, h.manager.Get, session.SessionDir(), and assertions on
h.driver.stopCalls; invoke each case with t.Run(case.name, func(t *testing.T) {
t.Parallel(); ... }) so each subtest runs in parallel and keeps original
assertions and setup (newHarness, createSession) unchanged.
In `@internal/session/manager_delete.go`:
- Around line 21-27: The returns are propagating raw errors; update the error
returns to wrap them with delete-specific context using fmt.Errorf so callers
see cause and operation (e.g., when checking m.Get(target) and when calling
m.StopWithCause(ctx, target, CauseUserRequested, "session deleted") wrap the err
values as fmt.Errorf("delete session %s: %w", target, err) or similar to add
clear delete-specific context while preserving the original error.
In `@internal/store/globaldb/global_db_task.go`:
- Around line 104-120: DeleteTask currently only maps 0-rows to
taskpkg.ErrTaskNotFound and lets DB FK constraint errors bubble up; update
DeleteTask to normalize parent-child FK failures by either (a) pre-checking for
children with a SELECT COUNT(*) FROM tasks WHERE parent_task_id = ? using the
same trimmedID and return a domain error (e.g., taskpkg.ErrTaskHasChildren) if
count>0, or (b) catch the DB constraint error from g.db.ExecContext and
translate it to taskpkg.ErrTaskHasChildren before returning; modify the logic
around ExecContext/requireRowsAffected (referencing DeleteTask,
requireTaskValue, requireRowsAffected, and taskpkg.ErrTaskNotFound) so callers
receive a stable task-domain error instead of a raw DB constraint error.
In `@internal/task/manager_test.go`:
- Line 197: The test deletes triage state using the raw taskID but
s.triageStates uses composite keys; update the cleanup to delete the exact
composite key constructed from taskID, actorKind and actorRef (the same key
format used when inserting into s.triageStates) — e.g., build the composite key
for the entry under test and call delete(s.triageStates, compositeKey) so the
triage state is fully removed.
In `@internal/task/manager.go`:
- Around line 279-300: The delete path returns raw errors from calls like
m.store.GetTask, m.ensureTaskDeleteAllowed, m.store.ListDependents,
m.store.DeleteTask and m.reconcileTaskCascade; update each error return to wrap
the underlying error with contextual text using fmt.Errorf("...: %w", err)
(e.g., include the operation and trimmedID or dependentID), so replace bare
returns like `return err` with wrapped errors that mention the failing operation
(GetTask/DeleteTask/ListDependents/reconcileTaskCascade) and the affected task
ID; apply the same wrapping for the other occurrences around lines 2199–2214.
In `@web/src/hooks/routes/use-session-page-controls.ts`:
- Around line 6-12: The imports in this hook currently reach into session
internals; replace the internal paths with the session public barrel by
importing useClearSessionConversation, useDeleteSession, useResumeSession,
useStopSession and SessionPayload from "@/systems/session" (the system's public
barrel) so all cross-system usage goes through the public API; update the import
statement to reference "@/systems/session" and remove any direct references to
adapters/hooks/types paths.
In `@web/src/systems/session/hooks/use-session-actions.ts`:
- Around line 82-86: Currently cache removal runs in the mutation's onSettled
handler which clears session data even when delete fails; move those
queryClient.removeQueries calls into the mutation's onSuccess handler instead,
and add a call to remove sessionKeys.events(id) alongside
sessionKeys.detail(id), sessionKeys.history(id), sessionKeys.transcript(id), and
still call queryClient.invalidateQueries({ queryKey: sessionKeys.lists() }) on
success so that only successful deletes purge all related caches.
---
Outside diff comments:
In `@web/src/systems/session/adapters/session-api.ts`:
- Around line 75-99: Replace the raw Error throws in deleteSession and
stopSession with the project's typed adapter error class (e.g., AdapterError or
ApiAdapterError): import the adapter error class and throw new AdapterError(...)
instead of new Error(...); for the 404 case throw the typed error with the same
"Session not found: {id}" message, and for other failures throw the typed error
constructed with defaultApiErrorMessage(`Failed to delete/stop session "${id}"`,
response, error) and include any response/error metadata the adapter error
supports so calling code can inspect status/details. Ensure changes are made in
functions deleteSession and stopSession and that the adapter error class is
properly imported.
---
Nitpick comments:
In `@internal/api/core/tasks_test.go`:
- Line 918: Add the DELETE /tasks/:id case to the other error-path request
matrices used in these tests: in the service-unavailable request matrix and the
manager-error request matrix add the same entry {method: http.MethodDelete,
path: "/tasks/task-1"} so the DELETE endpoint is exercised on those error paths
(look for the test slices/variables that build the "serviceUnavailableRequests"
and "managerErrorRequests" or similarly named request matrices in
internal/api/core/tasks_test.go and append the DELETE entry to each).
In `@internal/api/httpapi/handlers_error_test.go`:
- Around line 23-74: The large
TestCreateGetResumeDeleteAndStopHandlersReturnExpectedErrors test should be
split into table-driven subtests using t.Run so each endpoint is tested in
isolation: create a slice of test cases (name, method, path, body,
expectedStatus) and iterate calling t.Run("Should <description>", func(t
*testing.T){ t.Parallel() ... }), reusing newTestRouter/newTestHandlers and
stubSessionManager setup and performRequest to execute each case and assert
response codes; ensure each subtest is independent and marked parallel where
safe so one failure doesn't hide others.
In `@internal/api/httpapi/handlers_test.go`:
- Around line 1074-1094: Wrap the TestDeleteSessionHandlerReturnsNoContent logic
inside a t.Run subtest named "Should delete session and return no content" so it
follows the repo's t.Run("Should...") pattern; specifically move the setup and
assertions currently in TestDeleteSessionHandlerReturnsNoContent into a
t.Run(...) closure while keeping the stubSessionManager DeleteFn,
newTestHandlers, newTestRouter and performRequest calls intact (i.e., call
t.Run(..., func(t *testing.T) { /* existing body using manager, handlers,
engine, recorder */ })). Ensure the outer test function name stays
TestDeleteSessionHandlerReturnsNoContent but delegates the actual scenario to
the t.Run subtest.
In `@internal/api/udsapi/handlers_error_test.go`:
- Around line 19-70: Refactor
TestCreateGetResumeDeleteAndStopHandlersReturnExpectedErrors into t.Run
subtests: replace the single flow with a table-driven slice of cases (e.g.,
struct { name, method, path, body, wantStatus }) and for each case call
t.Run(case.name, func(t *testing.T){ ... }); inside each subtest create the
engine once using newTestRouter/newTestHandlers with the existing
stubSessionManager and homePaths, call performRequest with
case.method/path/body, and assert response.Code == case.wantStatus; keep the
existing stubSessionManager and response expectations but split
create/get/resume/delete/stop into distinct "Should..." subtest names to improve
failure locality.
In `@internal/api/udsapi/handlers_test.go`:
- Around line 969-989: Add test isolation by calling t.Parallel() at the start
of TestDeleteSessionHandlerReturnsNoContent (immediately after the function
begins) so the test runs concurrently like the others; update the
TestDeleteSessionHandlerReturnsNoContent function to begin with t.Parallel()
while leaving the rest of the setup (newTestHomePaths, stubSessionManager,
newTestHandlers, newTestRouter, performRequest assertions) unchanged.
In `@internal/daemon/daemon_test.go`:
- Around line 4134-4139: The fakeSessionManager.Delete method currently forwards
to Stop which masks differences between stop and delete; modify
fakeSessionManager by implementing Delete to record its own invocation (e.g.,
increment a deleteCount or append to a deletedIDs slice) and return a
configurable deleteErr/behavior instead of calling Stop; update the fake's
fields to allow tests to assert that Delete was invoked separately from Stop and
to simulate delete-specific errors or state mutation while leaving Stop
unchanged.
In `@web/src/hooks/routes/use-session-page-controls.ts`:
- Around line 63-79: handleDelete only checks deleteMutation.isPending and
should also bail out if any other control mutation is in-flight; update
handleDelete to check deleteMutation.isPending OR the other control flags
(isStopping, isResuming, isClearing, isCancellingPrompt) and return early if any
are true before calling deleteMutation.mutate, keeping the existing
onSuccess/onError behavior (including aui.thread().reset() and onDeleteSuccess).
Apply the same multi-flag guard pattern to the other handlers around lines 92-97
(the handlers for stop/resume/clear/cancelPrompt) so each handler checks all
control-mutation flags before mutating to prevent overlapping operations.
In `@web/src/systems/session/hooks/use-session-actions.test.tsx`:
- Around line 170-196: Add a new test in use-session-actions.test.tsx that
covers the failure path for useDeleteSession: mock
vi.mocked(deleteSession).mockRejectedValue(new Error("fail")), initialize a
QueryClient and setQueryData for sessionKeys.detail(createdSession.id),
sessionKeys.transcript(createdSession.id),
sessionKeys.history(createdSession.id) and set a draft via
useSessionStore.getState().setDraft(createdSession.id, ...), render the hook
with createWrapper(queryClient), call
result.current.mutateAsync(createdSession.id) inside act and assert it rejects
(await expect(...).rejects.toThrow()), then assert the queryClient.getQueryData
for each key and useSessionStore.getState().drafts[createdSession.id] are still
defined (not cleared) to ensure caches/drafts remain on delete failure.
In `@web/src/systems/tasks/adapters/tasks-api.test.ts`:
- Around line 261-265: Update the "throws not-found for 404" test to assert the
typed error contract: when mocking fetch to return 404 and calling
deleteTask("missing"), assert the rejection is an instance of TasksApiError (not
a raw Error) and also assert the error message equals "Task not found: missing"
— e.g., use expect(...).rejects.toBeInstanceOf(TasksApiError) and
expect(...).rejects.toThrow("Task not found: missing") or await the rejection
and assert instance and message from the caught error; reference the deleteTask
function and the TasksApiError class in the test.
In `@web/src/systems/tasks/hooks/use-task-actions.ts`:
- Around line 155-169: In useDeleteTask, avoid redundant cache work: after
queryClient.removeQueries({ queryKey: tasksKeys.detail(id) }) remove the id
argument when calling invalidateTaskQueries so it doesn't re-invalidate the
already-removed detail entry; update the call from
invalidateTaskQueries(queryClient, id) to simply
invalidateTaskQueries(queryClient) (or the equivalent overload) so only the
broader task invalidations run while keeping the explicit removal of
tasksKeys.detail(id).
🪄 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: 3e2bc545-87b9-44be-822f-bf3ca08faf9c
⛔ Files ignored due to path filters (2)
openapi/agh.jsonis excluded by!**/*.jsonweb/src/generated/agh-openapi.d.tsis excluded by!**/generated/**
📒 Files selected for processing (54)
internal/api/core/error_paths_test.gointernal/api/core/handlers.gointernal/api/core/handlers_internal_test.gointernal/api/core/handlers_test.gointernal/api/core/interfaces.gointernal/api/core/tasks.gointernal/api/core/tasks_test.gointernal/api/core/test_helpers_test.gointernal/api/httpapi/handlers_error_test.gointernal/api/httpapi/handlers_test.gointernal/api/httpapi/routes.gointernal/api/spec/spec.gointernal/api/testutil/apitest.gointernal/api/udsapi/handlers_error_test.gointernal/api/udsapi/handlers_test.gointernal/api/udsapi/routes.gointernal/cli/client.gointernal/cli/client_test.gointernal/daemon/daemon_test.gointernal/session/manager_delete.gointernal/session/manager_delete_test.gointernal/store/globaldb/global_db_task.gointernal/task/interfaces.gointernal/task/manager.gointernal/task/manager_test.gointernal/testutil/e2e/runtime_harness.gointernal/testutil/e2e/runtime_harness_helpers_test.goweb/src/components/assistant-ui/session-thread.tsxweb/src/hooks/routes/use-session-page-controls.tsweb/src/hooks/routes/use-tasks-page.tsweb/src/routes/_app/-tasks.$id.test.tsxweb/src/routes/_app/session.$id.tsxweb/src/routes/_app/tasks.$id.tsxweb/src/routes/_app/tasks.tsxweb/src/systems/session/adapters/session-api.test.tsweb/src/systems/session/adapters/session-api.tsweb/src/systems/session/components/chat-header.test.tsxweb/src/systems/session/components/chat-header.tsxweb/src/systems/session/components/session-chat-runtime-provider.test.tsxweb/src/systems/session/components/stories/chat-header.stories.tsxweb/src/systems/session/hooks/use-session-actions.test.tsxweb/src/systems/session/hooks/use-session-actions.tsweb/src/systems/session/index.tsweb/src/systems/tasks/adapters/tasks-api.test.tsweb/src/systems/tasks/adapters/tasks-api.tsweb/src/systems/tasks/components/task-delete-action.tsxweb/src/systems/tasks/components/tasks-detail-header.test.tsxweb/src/systems/tasks/components/tasks-detail-header.tsxweb/src/systems/tasks/components/tasks-detail-preview-panel.test.tsxweb/src/systems/tasks/components/tasks-detail-preview-panel.tsxweb/src/systems/tasks/hooks/use-task-actions.test.tsxweb/src/systems/tasks/hooks/use-task-actions.tsweb/src/systems/tasks/index.tsweb/vitest.config.ts
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
internal/api/spec/spec.go (1)
1554-1559:⚠️ Potential issue | 🟠 MajorAdd the missing
409response todeleteTask.The task delete path still distinguishes conflict-class failures from validation failures, but this operation only advertises
400/404/503/500. Generated clients will under-handle a real runtime response until409is declared here.Suggested spec fix
Responses: []ResponseSpec{ {Status: 204, Description: "No Content"}, {Status: 404, Description: "Task not found", Body: contract.ErrorPayload{}}, {Status: 400, Description: "Invalid task delete", Body: contract.ErrorPayload{}}, + {Status: 409, Description: "Task delete conflict", Body: contract.ErrorPayload{}}, {Status: 503, Description: "Task service is not configured", Body: contract.ErrorPayload{}}, {Status: 500, Description: "Internal server error", Body: contract.ErrorPayload{}}, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/spec/spec.go` around lines 1554 - 1559, The deleteTask operation's Responses slice (the ResponseSpec list for deleteTask) is missing a 409 Conflict entry; add a ResponseSpec with Status: 409, Description: "Conflict" (or "Task conflict") and Body: contract.ErrorPayload{} to the Responses array so generated clients can handle conflict-class failures; update the Responses slice where ResponseSpec is declared for deleteTask in internal/api/spec/spec.go accordingly.
🧹 Nitpick comments (6)
internal/store/globaldb/global_db_task_test.go (1)
245-268: Adopt the requiredt.Run("Should...")pattern for this test case.This test is valuable, but it currently skips the repository-required subtest naming/structure convention.
Suggested update
func TestGlobalDBDeleteTaskMapsChildConstraintToValidationError(t *testing.T) { t.Parallel() - - globalDB := openTestGlobalDB(t) - - parent := taskRecordForTest("task-parent-delete") - if err := globalDB.CreateTask(testutil.Context(t), parent); err != nil { - t.Fatalf("CreateTask(parent) error = %v", err) - } - - child := taskRecordForTest("task-child-delete") - child.ParentTaskID = parent.ID - if err := globalDB.CreateTask(testutil.Context(t), child); err != nil { - t.Fatalf("CreateTask(child) error = %v", err) - } - - err := globalDB.DeleteTask(testutil.Context(t), parent.ID) - if !errors.Is(err, taskpkg.ErrValidation) { - t.Fatalf("DeleteTask(parent) error = %v, want %v", err, taskpkg.ErrValidation) - } - if strings.Contains(strings.ToLower(err.Error()), "foreign key constraint failed") { - t.Fatalf("DeleteTask(parent) error = %q, want mapped task validation error", err.Error()) - } + t.Run("Should map child constraint delete failures to validation errors", func(t *testing.T) { + t.Parallel() + + globalDB := openTestGlobalDB(t) + + parent := taskRecordForTest("task-parent-delete") + if err := globalDB.CreateTask(testutil.Context(t), parent); err != nil { + t.Fatalf("CreateTask(parent) error = %v", err) + } + + child := taskRecordForTest("task-child-delete") + child.ParentTaskID = parent.ID + if err := globalDB.CreateTask(testutil.Context(t), child); err != nil { + t.Fatalf("CreateTask(child) error = %v", err) + } + + err := globalDB.DeleteTask(testutil.Context(t), parent.ID) + if !errors.Is(err, taskpkg.ErrValidation) { + t.Fatalf("DeleteTask(parent) error = %v, want %v", err, taskpkg.ErrValidation) + } + if strings.Contains(strings.ToLower(err.Error()), "foreign key constraint failed") { + t.Fatalf("DeleteTask(parent) error = %q, want mapped task validation error", err.Error()) + } + }) }As per coding guidelines, "MUST use t.Run("Should...") pattern for ALL test cases" and "Use table-driven tests with subtests (
t.Run) as default pattern for Go tests".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/store/globaldb/global_db_task_test.go` around lines 245 - 268, Wrap the existing test body of TestGlobalDBDeleteTaskMapsChildConstraintToValidationError in a t.Run subtest using a descriptive "Should..." name (e.g., t.Run("Should map child foreign key constraint to validation error", func(t *testing.T) { ... })) and move t.Parallel() into that subtest; keep the same calls to openTestGlobalDB, CreateTask, and DeleteTask and preserve the validation/assertion logic inside the subtest so the test follows the required t.Run("Should...") pattern.internal/daemon/daemon_test.go (2)
4034-4034: Add compile-time interface verification forfakeSessionManager.Since this fake now tracks the new delete surface, add an interface assertion to prevent drift as the
SessionManagercontract evolves.✅ Suggested addition
+var _ SessionManager = (*fakeSessionManager)(nil)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/daemon/daemon_test.go` at line 4034, Add a compile-time interface verification so fakeSessionManager implements the SessionManager contract (to catch future surface changes like the new delete tracking); add a declaration of the form var _ SessionManager = (*fakeSessionManager)(nil) near the fakeSessionManager type or its tests to ensure the compiler enforces the interface conformance.
4163-4178: MakeDeleteable to model not-found behavior in tests.This fake currently returns
nileven when the session ID does not exist, which can mask negative-path behavior in callers.✅ Suggested adjustment
func (f *fakeSessionManager) Delete(_ context.Context, id string) error { f.mu.Lock() defer f.mu.Unlock() f.deleteCalls = append(f.deleteCalls, id) + deleted := false filtered := f.infos[:0] for _, info := range f.infos { if info != nil && info.ID == id { + deleted = true continue } filtered = append(filtered, info) } f.infos = filtered + if !deleted { + return session.ErrSessionNotFound + } return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/daemon/daemon_test.go` around lines 4163 - 4178, The fakeSessionManager.Delete currently always returns nil, hiding “not found” cases; update Delete (in fakeSessionManager) to first check f.infos for an entry with matching id and only perform the removal when found, appending id to f.deleteCalls in all cases; if no matching entry exists return a non-nil error (e.g., a package-level ErrNotFound or errors.New("session not found")) so tests can exercise negative-path behavior instead of silently succeeding.web/src/hooks/routes/use-session-page-controls.ts (1)
50-68: Consider applyingcontrolsBusyguard to stop/resume for full control serialization.
handleDelete/handleClearare protected, buthandleStop/handleResumecan still fire while another control action is pending. Guarding them too keeps mutation concurrency policy consistent.Suggested consistency refactor
const handleStop = useCallback(() => { + if (controlsBusy) { + return; + } + if (isRunning) { handleCancelPrompt(); return; } stopMutation.mutate(sessionId); -}, [handleCancelPrompt, isRunning, sessionId, stopMutation]); +}, [controlsBusy, handleCancelPrompt, isRunning, sessionId, stopMutation]); const handleResume = useCallback(() => { + if (controlsBusy) { + return; + } + resumeMutation.mutate(sessionId); -}, [resumeMutation, sessionId]); +}, [controlsBusy, resumeMutation, sessionId]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/hooks/routes/use-session-page-controls.ts` around lines 50 - 68, handleStop and handleResume lack the same controlsBusy guard used by handleDelete/handleClear, allowing concurrent control actions; update handleStop and handleResume to early-return when controlsBusy is true (preserving the existing isRunning check/handleCancelPrompt behavior in handleStop), and ensure their dependency arrays include controlsBusy so the callbacks re-evaluate correctly; reference the functions/vars handleStop, handleResume, controlsBusy, stopMutation, resumeMutation, isRunning, and handleCancelPrompt when making the change.web/src/hooks/routes/use-session-page-controls.test.tsx (1)
137-152: Strengthen the idle delete test by executing callback side effects.Right now it only verifies callback presence. Executing captured
onSuccess/onErrorwould directly validate reset + toast + optionalonDeleteSuccessbehavior.Suggested test expansion
it("invokes delete when controls are idle", () => { - const { result } = renderHook(() => useSessionPageControls("sess-1", "active")); + const onDeleteSuccess = vi.fn(); + const { result } = renderHook(() => + useSessionPageControls("sess-1", "active", { onDeleteSuccess }) + ); act(() => { result.current.handleDelete(); }); @@ expect.objectContaining({ onError: expect.any(Function), onSuccess: expect.any(Function), }) ); + + const [, options] = routeHookMocks.deleteMutation.mutate.mock.calls[0] as [ + string, + { onSuccess?: () => void; onError?: (error: unknown) => void }, + ]; + + act(() => { + options.onSuccess?.(); + }); + expect(routeHookMocks.resetThread).toHaveBeenCalledTimes(1); + expect(routeHookMocks.toastSuccess).toHaveBeenCalledWith("Session deleted."); + expect(onDeleteSuccess).toHaveBeenCalledTimes(1); + + act(() => { + options.onError?.(new Error("delete failed")); + }); + expect(routeHookMocks.toastError).toHaveBeenCalledWith("delete failed"); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/hooks/routes/use-session-page-controls.test.tsx` around lines 137 - 152, The test "invokes delete when controls are idle" only asserts that deleteMutation.mutate was called and that it received onError/onSuccess callbacks; update the test to capture the mutate call's options and invoke those callbacks to exercise their side effects: call the captured onSuccess to assert that the controls are reset (via the reset function used in useSessionPageControls), that a success toast was shown, and that any provided onDeleteSuccess prop is invoked; also call the captured onError to assert that the error toast path runs. Locate the test around the useSessionPageControls invocation and the routeHookMocks.deleteMutation.mutate expectation and modify it to extract the second argument (options) and invoke options.onSuccess(...) and options.onError(...) with appropriate mock values, then assert reset, toast, and onDeleteSuccess behaviors.web/src/systems/session/hooks/use-session-actions.test.tsx (1)
170-198: Also assert list-query invalidation on delete success.This test validates entity cache removal and draft clearing, but it doesn’t assert the
sessionKeys.lists()invalidation contract fromuseDeleteSession, which is an important regression guard.Suggested test hardening
const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, }); + const invalidateSpy = vi.spyOn(queryClient, "invalidateQueries"); queryClient.setQueryData(sessionKeys.detail(createdSession.id), createdSession); queryClient.setQueryData(sessionKeys.transcript(createdSession.id), [ { id: "history-1", role: "assistant", content: "existing" }, ]); @@ expect(queryClient.getQueryData(sessionKeys.history(createdSession.id))).toBeUndefined(); expect(queryClient.getQueryData(sessionKeys.events(createdSession.id))).toBeUndefined(); expect(useSessionStore.getState().drafts[createdSession.id]).toBeUndefined(); + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: sessionKeys.lists() }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/systems/session/hooks/use-session-actions.test.tsx` around lines 170 - 198, Add an assertion that the session list cache is invalidated by seeding a value for sessionKeys.lists() on the QueryClient before calling useDeleteSession and then asserting it was cleared after mutation; specifically, in the test that uses useDeleteSession and QueryClient, call queryClient.setQueryData(sessionKeys.lists(), [{ id: "seed" }]) before invoking result.current.mutateAsync(createdSession.id) and then assert the list query is invalidated (e.g., queryClient.getQueryData(sessionKeys.lists()) is undefined) to enforce the useDeleteSession list invalidation contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/daemon/daemon_test.go`:
- Around line 2048-2075: Wrap the existing
TestFakeSessionManagerDeleteTracksDeleteIndependently body in a t.Run(...)
subtest using the "Should..." naming pattern (e.g. t.Run("Should delete tracks
independently", func(t *testing.T) { ... })), moving t.Parallel() inside that
subtest and keeping the same assertions that exercise fakeSessionManager, its
Delete method, manager.deleteCalls, manager.stopCalls and manager.infos so
behavior is unchanged but the test now follows the required subtest naming
convention.
In `@internal/session/manager_delete.go`:
- Around line 25-28: The current check-then-act around m.Get(target) +
m.StopWithCause can race: if the session disappears between calls StopWithCause
may return ErrSessionNotFound and abort delete; treat that outcome as benign.
Modify the code that calls m.StopWithCause(ctx, target, CauseUserRequested,
"session deleted") to continue on ErrSessionNotFound (i.e., ignore that specific
error) and only return on other errors; reference m.Get, m.StopWithCause, and
the ErrSessionNotFound sentinel when implementing the conditional handling so
the persisted session history deletion proceeds even if the runtime session
vanished concurrently.
In `@internal/task/manager.go`:
- Around line 293-305: The current flow calls m.store.DeleteTask(ctx, trimmedID)
then runs reconcileTaskCascade for dependents, which can return an error after
the resource is already deleted; fix by making delete+reconcile atomic at the
store layer or by guaranteeing post-delete repair: add a new store method (e.g.,
DeleteTaskAndReconcile/DeleteTaskWithCascade) that performs the delete and
dependent reconciliation inside the same transactional boundary (or returns
dependentIDs and reconciliation results) and call that from Manager instead of
m.store.DeleteTask, or if transactions are unavailable, modify manager.go to
treat a successful DeleteTask as definitive (do not return failure on
reconcileTaskCascade errors), log the reconcile error and enqueue an
asynchronous retry/repair job (or schedule background goroutine) so the API
returns success while dependents are eventually reconciled; update references to
m.store.DeleteTask and reconcileTaskCascade accordingly.
---
Duplicate comments:
In `@internal/api/spec/spec.go`:
- Around line 1554-1559: The deleteTask operation's Responses slice (the
ResponseSpec list for deleteTask) is missing a 409 Conflict entry; add a
ResponseSpec with Status: 409, Description: "Conflict" (or "Task conflict") and
Body: contract.ErrorPayload{} to the Responses array so generated clients can
handle conflict-class failures; update the Responses slice where ResponseSpec is
declared for deleteTask in internal/api/spec/spec.go accordingly.
---
Nitpick comments:
In `@internal/daemon/daemon_test.go`:
- Line 4034: Add a compile-time interface verification so fakeSessionManager
implements the SessionManager contract (to catch future surface changes like the
new delete tracking); add a declaration of the form var _ SessionManager =
(*fakeSessionManager)(nil) near the fakeSessionManager type or its tests to
ensure the compiler enforces the interface conformance.
- Around line 4163-4178: The fakeSessionManager.Delete currently always returns
nil, hiding “not found” cases; update Delete (in fakeSessionManager) to first
check f.infos for an entry with matching id and only perform the removal when
found, appending id to f.deleteCalls in all cases; if no matching entry exists
return a non-nil error (e.g., a package-level ErrNotFound or errors.New("session
not found")) so tests can exercise negative-path behavior instead of silently
succeeding.
In `@internal/store/globaldb/global_db_task_test.go`:
- Around line 245-268: Wrap the existing test body of
TestGlobalDBDeleteTaskMapsChildConstraintToValidationError in a t.Run subtest
using a descriptive "Should..." name (e.g., t.Run("Should map child foreign key
constraint to validation error", func(t *testing.T) { ... })) and move
t.Parallel() into that subtest; keep the same calls to openTestGlobalDB,
CreateTask, and DeleteTask and preserve the validation/assertion logic inside
the subtest so the test follows the required t.Run("Should...") pattern.
In `@web/src/hooks/routes/use-session-page-controls.test.tsx`:
- Around line 137-152: The test "invokes delete when controls are idle" only
asserts that deleteMutation.mutate was called and that it received
onError/onSuccess callbacks; update the test to capture the mutate call's
options and invoke those callbacks to exercise their side effects: call the
captured onSuccess to assert that the controls are reset (via the reset function
used in useSessionPageControls), that a success toast was shown, and that any
provided onDeleteSuccess prop is invoked; also call the captured onError to
assert that the error toast path runs. Locate the test around the
useSessionPageControls invocation and the routeHookMocks.deleteMutation.mutate
expectation and modify it to extract the second argument (options) and invoke
options.onSuccess(...) and options.onError(...) with appropriate mock values,
then assert reset, toast, and onDeleteSuccess behaviors.
In `@web/src/hooks/routes/use-session-page-controls.ts`:
- Around line 50-68: handleStop and handleResume lack the same controlsBusy
guard used by handleDelete/handleClear, allowing concurrent control actions;
update handleStop and handleResume to early-return when controlsBusy is true
(preserving the existing isRunning check/handleCancelPrompt behavior in
handleStop), and ensure their dependency arrays include controlsBusy so the
callbacks re-evaluate correctly; reference the functions/vars handleStop,
handleResume, controlsBusy, stopMutation, resumeMutation, isRunning, and
handleCancelPrompt when making the change.
In `@web/src/systems/session/hooks/use-session-actions.test.tsx`:
- Around line 170-198: Add an assertion that the session list cache is
invalidated by seeding a value for sessionKeys.lists() on the QueryClient before
calling useDeleteSession and then asserting it was cleared after mutation;
specifically, in the test that uses useDeleteSession and QueryClient, call
queryClient.setQueryData(sessionKeys.lists(), [{ id: "seed" }]) before invoking
result.current.mutateAsync(createdSession.id) and then assert the list query is
invalidated (e.g., queryClient.getQueryData(sessionKeys.lists()) is undefined)
to enforce the useDeleteSession list invalidation contract.
🪄 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: 9a5fd220-9701-4f80-95c3-1ff48ff6878d
⛔ Files ignored due to path filters (21)
.compozy/tasks/delete-session/reviews-001/_meta.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-001/issue_001.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-001/issue_002.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-001/issue_003.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-001/issue_004.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-001/issue_005.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-001/issue_006.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-001/issue_007.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-001/issue_008.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-001/issue_009.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-001/issue_010.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-001/issue_011.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-001/issue_012.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-001/issue_013.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-001/issue_014.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-001/issue_015.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-001/issue_016.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-001/issue_017.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-001/issue_018.mdis excluded by!**/*.mdopenapi/agh.jsonis excluded by!**/*.jsonweb/src/generated/agh-openapi.d.tsis excluded by!**/generated/**
📒 Files selected for processing (19)
internal/api/core/tasks_test.gointernal/api/httpapi/handlers_error_test.gointernal/api/httpapi/handlers_test.gointernal/api/spec/spec.gointernal/api/udsapi/handlers_error_test.gointernal/api/udsapi/handlers_test.gointernal/daemon/daemon_test.gointernal/session/manager_delete.gointernal/session/manager_delete_test.gointernal/store/globaldb/global_db_task.gointernal/store/globaldb/global_db_task_test.gointernal/task/manager.gointernal/task/manager_test.goweb/src/hooks/routes/use-session-page-controls.test.tsxweb/src/hooks/routes/use-session-page-controls.tsweb/src/systems/session/hooks/use-session-actions.test.tsxweb/src/systems/session/hooks/use-session-actions.tsweb/src/systems/session/index.tsweb/src/systems/tasks/adapters/tasks-api.test.ts
✅ Files skipped from review due to trivial changes (1)
- web/src/systems/tasks/adapters/tasks-api.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- web/src/systems/session/hooks/use-session-actions.ts
- web/src/systems/session/index.ts
- internal/api/udsapi/handlers_error_test.go
- internal/api/core/tasks_test.go
- internal/store/globaldb/global_db_task.go
- internal/api/httpapi/handlers_test.go
- internal/api/udsapi/handlers_test.go
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
internal/task/manager.go (1)
279-285:⚠️ Potential issue | 🟠 MajorRequire transactional delete for this flow.
If
m.storedoes not implementDeleteTaskTransactionStore, this fallback still deletes the task before reconciling dependents. A later reconcile error will return failure after the task is already gone, which recreates the half-committed behavior this PR is trying to remove.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/task/manager.go` around lines 279 - 285, The current fallback path calls m.deleteTaskWithStore without a transaction, which can delete the task before dependents are reconciled; instead, require a transactional store: if m.store does not implement DeleteTaskTransactionStore, return an error and do not perform the deletion. Update the branch around the type assertion so only the DeleteTaskTransactionStore path uses WithDeleteTaskTransaction to call m.deleteTaskWithStore(ctx, store, trimmedID), and if the assertion fails return a clear error (e.g., indicating m.store lacks DeleteTaskTransactionStore) rather than calling m.deleteTaskWithStore directly.
🧹 Nitpick comments (1)
internal/session/manager_delete_test.go (1)
13-125: LGTM! Consider addingt.Parallel()for faster test execution.The table-driven test structure follows the
t.Run("Should...")pattern and covers the key scenarios: stopped session removal, active session stop-before-delete, concurrent stop race handling, and error wrapping verification.Each subtest creates an isolated harness, so they can run in parallel:
♻️ Optional: Add parallel execution to subtests
for _, tc := range cases { + tc := tc // capture range variable t.Run(tc.name, func(t *testing.T) { + t.Parallel() tc.run(t) }) }As per coding guidelines, "Add
t.Parallel()to independent subtests in Go tests".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/session/manager_delete_test.go` around lines 13 - 125, The tests in TestManagerDelete can run in parallel; add t.Parallel() to each subtest to speed execution by calling t.Parallel() at the start of the t.Run closure (or at the top of each run func) so each case (e.g., "ShouldRemoveStoppedSessionFromHistory", "ShouldStopActiveSessionBeforeRemovingArtifacts", "ShouldIgnoreConcurrentStopRacesThatReportSessionNotFound", "ShouldWrapStopErrorsWithDeleteContext") runs concurrently; ensure any shared state in newHarness, createSession, h.manager, and h.driver is isolated per subtest before enabling parallelism.
🤖 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/task/manager.go`:
- Around line 2094-2105: The canonicalTaskStatusWithStore currently calls
reconcileTaskWithStore (which can call store.UpdateTask) and thus performs
writes during read operations; change it to be side-effect free by removing any
calls to reconcileTaskWithStore or other mutating helpers (and by not calling
store.UpdateTask), instead deriving status purely from read-only helpers like
hasUnresolvedDependenciesWithStore, the provided dependencies and runs, and any
non-mutating logic; if reconciliation logic is needed elsewhere, introduce a
separate read-only variant (e.g., reconcileTaskReadOnly or
computeCanonicalStatusNoSideEffects) or ensure reconcileTaskWithStore has a
non-mutating mode and use that here, and update canonicalTaskStatus and callers
to use the new read-only pathway.
---
Duplicate comments:
In `@internal/task/manager.go`:
- Around line 279-285: The current fallback path calls m.deleteTaskWithStore
without a transaction, which can delete the task before dependents are
reconciled; instead, require a transactional store: if m.store does not
implement DeleteTaskTransactionStore, return an error and do not perform the
deletion. Update the branch around the type assertion so only the
DeleteTaskTransactionStore path uses WithDeleteTaskTransaction to call
m.deleteTaskWithStore(ctx, store, trimmedID), and if the assertion fails return
a clear error (e.g., indicating m.store lacks DeleteTaskTransactionStore) rather
than calling m.deleteTaskWithStore directly.
---
Nitpick comments:
In `@internal/session/manager_delete_test.go`:
- Around line 13-125: The tests in TestManagerDelete can run in parallel; add
t.Parallel() to each subtest to speed execution by calling t.Parallel() at the
start of the t.Run closure (or at the top of each run func) so each case (e.g.,
"ShouldRemoveStoppedSessionFromHistory",
"ShouldStopActiveSessionBeforeRemovingArtifacts",
"ShouldIgnoreConcurrentStopRacesThatReportSessionNotFound",
"ShouldWrapStopErrorsWithDeleteContext") runs concurrently; ensure any shared
state in newHarness, createSession, h.manager, and h.driver is isolated per
subtest before enabling parallelism.
🪄 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: ea572071-b883-41f3-9bb1-7e6bc4c2efd1
⛔ Files ignored due to path filters (10)
.compozy/tasks/delete-session/reviews-002/_meta.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-002/issue_001.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-002/issue_002.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-002/issue_003.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-002/issue_004.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-002/issue_005.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-002/issue_006.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-002/issue_007.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-002/issue_008.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-002/issue_009.mdis excluded by!**/*.md
📒 Files selected for processing (12)
internal/daemon/daemon_test.gointernal/session/manager_delete.gointernal/session/manager_delete_test.gointernal/store/globaldb/global_db_task.gointernal/store/globaldb/global_db_task_aux.gointernal/store/globaldb/global_db_task_test.gointernal/task/interfaces.gointernal/task/manager.gointernal/task/manager_test.goweb/src/hooks/routes/use-session-page-controls.test.tsxweb/src/hooks/routes/use-session-page-controls.tsweb/src/systems/session/hooks/use-session-actions.test.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- internal/store/globaldb/global_db_task_test.go
- web/src/systems/session/hooks/use-session-actions.test.tsx
- internal/store/globaldb/global_db_task.go
There was a problem hiding this comment.
♻️ Duplicate comments (2)
internal/task/manager.go (2)
2028-2057:⚠️ Potential issue | 🟡 MinorWrap the new recursive/store-aware errors with operation context.
These helpers now bubble raw
GetTask/ListDependencies/ListTaskRuns/UpdateTaskfailures from nested reconcile paths, which will make stale-edge or store failures show up as opaquetask not found/store errors with no clue which dependency or step failed.As per coding guidelines, "Use explicit error returns with wrapped context:
fmt.Errorf(\"context: %w\", err)".Also applies to: 2071-2082, 2338-2359, 2378-2400
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/task/manager.go` around lines 2028 - 2057, The code is returning raw store errors from calls like store.GetTask, store.ListDependencies, store.ListTaskRuns and store.UpdateTask inside the reconcile path, which makes failures opaque; wrap each returned error with explicit context using fmt.Errorf("operation context: %w", err) (e.g., "GetTask taskID=%s: %w", "ListDependencies taskID=%s: %w", "ListTaskRuns taskID=%s: %w", "UpdateTask taskID=%s: %w") so callers of m.canonicalTaskStatusWithStore and the surrounding logic can see which operation and taskID failed; apply the same pattern to the other referenced blocks (around lines handling canonicalTaskStatusWithStore, UpdateTask, and any similar store calls at 2071-2082, 2338-2359, 2378-2400).
279-285:⚠️ Potential issue | 🟠 MajorMake the delete path transactional-or-best-effort, not both.
If
m.storedoes not implementDeleteTaskTransactionStore, this path can still delete the task and then fail during dependent reconciliation. That leaves callers seeing an error even though the resource is already gone, so retries can flip from a 5xx to a 404. Please either require transactional delete support here or stop surfacing post-delete reconcile failures as request failures.Also applies to: 311-319
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/task/manager.go` around lines 279 - 285, The delete path currently attempts a best-effort delete when m.store does not implement DeleteTaskTransactionStore, causing callers to receive errors after the resource was already removed; change it to require transactional delete support instead of doing both: in the block where you check "if txStore, ok := m.store.(DeleteTaskTransactionStore); ok { ... }" return an explicit error if the type assertion fails (i.e., when m.store is not a DeleteTaskTransactionStore), so callers never see post-delete reconciliation failures for already-deleted resources; update the same logic used around deleteTaskWithStore/WithDeleteTaskTransaction (also present in the similar code at the 311-319 area) to consistently enforce transactional deletes.
🧹 Nitpick comments (1)
internal/task/manager_test.go (1)
2982-3007: Rename these new subtests to theShould...form.They are the new coverage for this regression, so please keep them aligned with the repo’s required
t.Run("Should...")naming convention.As per coding guidelines, "MUST use t.Run("Should...") pattern for ALL test cases".
Also applies to: 3009-3043
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/task/manager_test.go` around lines 2982 - 3007, Rename the new t.Run subtests that start with "GetTask derives ..." to follow the repo convention by using the "Should..." naming pattern (e.g., change t.Run("GetTask derives dependency status without writes", ... ) to t.Run("Should derive dependency status without writes", ... )), and do the same for the other subtest referenced (lines 3009-3043) so all new cases follow t.Run("Should...") naming; update only the string names for the subtests in manager_test.go (no logic changes).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@internal/task/manager.go`:
- Around line 2028-2057: The code is returning raw store errors from calls like
store.GetTask, store.ListDependencies, store.ListTaskRuns and store.UpdateTask
inside the reconcile path, which makes failures opaque; wrap each returned error
with explicit context using fmt.Errorf("operation context: %w", err) (e.g.,
"GetTask taskID=%s: %w", "ListDependencies taskID=%s: %w", "ListTaskRuns
taskID=%s: %w", "UpdateTask taskID=%s: %w") so callers of
m.canonicalTaskStatusWithStore and the surrounding logic can see which operation
and taskID failed; apply the same pattern to the other referenced blocks (around
lines handling canonicalTaskStatusWithStore, UpdateTask, and any similar store
calls at 2071-2082, 2338-2359, 2378-2400).
- Around line 279-285: The delete path currently attempts a best-effort delete
when m.store does not implement DeleteTaskTransactionStore, causing callers to
receive errors after the resource was already removed; change it to require
transactional delete support instead of doing both: in the block where you check
"if txStore, ok := m.store.(DeleteTaskTransactionStore); ok { ... }" return an
explicit error if the type assertion fails (i.e., when m.store is not a
DeleteTaskTransactionStore), so callers never see post-delete reconciliation
failures for already-deleted resources; update the same logic used around
deleteTaskWithStore/WithDeleteTaskTransaction (also present in the similar code
at the 311-319 area) to consistently enforce transactional deletes.
---
Nitpick comments:
In `@internal/task/manager_test.go`:
- Around line 2982-3007: Rename the new t.Run subtests that start with "GetTask
derives ..." to follow the repo convention by using the "Should..." naming
pattern (e.g., change t.Run("GetTask derives dependency status without writes",
... ) to t.Run("Should derive dependency status without writes", ... )), and do
the same for the other subtest referenced (lines 3009-3043) so all new cases
follow t.Run("Should...") naming; update only the string names for the subtests
in manager_test.go (no logic changes).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2709c799-c937-4517-9f2b-aaa31317f203
⛔ Files ignored due to path filters (3)
.compozy/tasks/delete-session/reviews-003/_meta.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-003/issue_001.mdis excluded by!**/*.md.compozy/tasks/delete-session/reviews-003/issue_002.mdis excluded by!**/*.md
📒 Files selected for processing (3)
internal/session/manager_delete_test.gointernal/task/manager.gointernal/task/manager_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/session/manager_delete_test.go
pedronauck
force-pushed
the
feat/web-delete-test-and-session
branch
from
28659b4 to
f6a1bd0
Compare
This was referenced Apr 24, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
This was referenced May 23, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Merged
pedronauck
added a commit
that referenced
this pull request
May 26, 2026
## Release v0.0.1 This PR prepares the release of version v0.0.1. ### Changelog ## 0.0.1 - 2026-05-26 ### Other Changes - Lessons learned ### ♻️ Refactoring - Project structure (#7) - Kb improvements (#12) - Rename spaces to channels (#17) - Add extensions gaps (#21) - Improve tool calls ui (#22) - Remove web app header - Module improvements (#29) - Memory improvements (#35) - Storybook for web and ui (#38) - Enable AGH network by default for new installs (#57) - Hermes adjustments (#69) - Badges design (#84) - Storybook scenario and logos gallery - Migrate typescript tests (#114) - Internal go packages (#120) - Ui patterns (#127) - Improve e2e tests (#130) - Ui redesign - Workspace isolation across runtime surfaces (#145) - Prod ready applies (#162) - Tool card ui (#164) - Alpha on logo - Prod ready features (#167) - Thread sheet (#202) ### 🎉 Features - Implement config foundation packages - Implement sqlite store package - Add ACP client package - Add session lifecycle manager - Implement observe package - Add daemon composition root - Add uds api server - Implement cli package - Add http api server - Add system design - Add foundation types, schemas, and layout shell for web client - Add daemon health polling and agent sidebar systems for web client - Add session system CRUD, streaming core, and session store for web client - Add chat view, messages, and composer tests for web client - Add tool cards and renderers for web client - Add file-backed memory store core - Scaffold memory session seams - Add memory dream consolidation service - Wire memory assembler into daemon - Add memory api and cli - New skills system (#1) - Add workspace entity (#5) - Add new skill capabilities (#8) - Web ui v2 (#9) - Improve hooks system (#10) - Session resilience (#11) - Add extensability (#13) - Add automation (#16) - Add channels (#14) - Add network implementation (#15) - Add network, bridges and automations web pages (#18) - Ext registry (#20) - Add core tasks (#19) - Bridge adapters (#23) - Add site (#26) - Add ext refac and sandbox (#25) - Settings ui (#37) - Tasks ui (#36) - Harness improvements (#44) - Agent capabilities (#49) - Redesign ui (#48) - Unify capability (#53) - Redesign network workspace (#59) - Add task deletion and split session delete from stop (#58) - Session provider selection (#60) - Production grade adjustments (#66) - Autonomous system (#75) - Add agent session route (#80) - Tools registry (#85) - Agents soul (#88) - Add network threads (#105) - Orchestration improvements (#106) - Memory v2 (#108) - Agent categories (#113) - Providers model (#118) - Add canonical AGH bundled skill (#143) - Onboarding and improvements (#198) - Onboarding and improvements (#201) ### 🐛 Bug Fixes - Review round - Review rounds - Resolve memory extensibility review batch - Embed web into daemon - Defaults agents - Acp integration (#4) - Lint errors - Prd folder - Remove orphan web actions and dead surfaces (#55) - Qa testing and fixes (#73) - New review rounds (#82) - Security audit (#90) - Release qa round (#95) - Add missing tools (#141) - New qa round (#147) - Advanced qa round (#149) - Homebrew tap - Final review round (#151) - Daemon healthy - Reasoning models (#158) - Lint errors (#160) - Review round (#168) - Release adjustments (#171) - Stabilize release ci fixtures - Stabilize release integration gate - Stabilize release verify gates - Stabilize release integration flows - Stabilize release verify gates - Stabilize main verify shutdown - Ignore stale acpmock cancel - Marketplace search focus and filtering (#193) - Website video - Workspace command select ### 📚 Documentation - Update agents.md - Update prd - Update skills - Update compozy tasks - Update compozy - Update compozy - Add new skills - Archive prd - Update prds - Update rfc - Update prds - Update prds - Add automation prd - Channels prd - Update prd - Update prd - New prds - Archive prds - Bridges adapters prd - Sandbox prd - Update - Archive prd - Update - Add new prd - New design - Update prd - Archive prds - Update prds - Tasks-ui prd tasks - Update prd - Update design docs - Agent capabilities prd - Improve site docs - Remove old design references - Udpate - Autonomous prd - Update skills - Blog design - Agent sould prd - Final qa plan - Update - Remove codex ledgers from gitignore - Remove not needed files - Udpate ledger - Update cy-codex-loop skill - Orchestration improves prd - Update prds - Orch improvs prd - Memv2 prd - Providers model prd - Update refacs prd - New design proposal - Update rules - Update skills - New blog posts (#173) - Format docs - Remove old design files - Remove old - Skeeper update ### 📦 Build System - Initial structure - Commitlint - Frontend base structure - Update vscode settings - Add subagents - Coderabbit - Prd and tooling - Bun lock - Lint tooling - Copy.md and tooling adjusts - Add repoclone rc - Upgrade skeeper to v0.2.0 - Update go.mod - Adopt task artifacts into skeeper - Sync codex plans with skeeper - Skeeper lock - Skeeper lock - New skills - Skeeper lock - Skeeper lock - Skeeper lock - Update deps and go - Regenerate daytona sidecar assets for go 1.26.3 - Fix cliff - Ignore docs on fmt - Build web assets before goreleaser - Extend release dry-run timeout ### 🔧 CI/CD - Lint errors - Fint release pr - Fix goreleaser ### 🧪 Testing - Add e2e tests (#27) - Qa rounds (#78) - Improve test suite (#138) - Harden daemon-served restart reloads - Harden daemon-served readiness waits - Stabilize dashboard focus assertion - Stabilize release integration gates - Stabilize release e2e markers - Stabilize release e2e flows Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This was referenced May 26, 2026
Closed
Merged
pedronauck
added a commit
that referenced
this pull request
May 26, 2026
## Release v0.0.1 This PR prepares the release of version v0.0.1. ### Changelog ## 0.0.1 - 2026-05-26 ### Other Changes - Lessons learned ### ♻️ Refactoring - Project structure (#7) - Kb improvements (#12) - Rename spaces to channels (#17) - Add extensions gaps (#21) - Improve tool calls ui (#22) - Remove web app header - Module improvements (#29) - Memory improvements (#35) - Storybook for web and ui (#38) - Enable AGH network by default for new installs (#57) - Hermes adjustments (#69) - Badges design (#84) - Storybook scenario and logos gallery - Migrate typescript tests (#114) - Internal go packages (#120) - Ui patterns (#127) - Improve e2e tests (#130) - Ui redesign - Workspace isolation across runtime surfaces (#145) - Prod ready applies (#162) - Tool card ui (#164) - Alpha on logo - Prod ready features (#167) - Thread sheet (#202) ### 🎉 Features - Implement config foundation packages - Implement sqlite store package - Add ACP client package - Add session lifecycle manager - Implement observe package - Add daemon composition root - Add uds api server - Implement cli package - Add http api server - Add system design - Add foundation types, schemas, and layout shell for web client - Add daemon health polling and agent sidebar systems for web client - Add session system CRUD, streaming core, and session store for web client - Add chat view, messages, and composer tests for web client - Add tool cards and renderers for web client - Add file-backed memory store core - Scaffold memory session seams - Add memory dream consolidation service - Wire memory assembler into daemon - Add memory api and cli - New skills system (#1) - Add workspace entity (#5) - Add new skill capabilities (#8) - Web ui v2 (#9) - Improve hooks system (#10) - Session resilience (#11) - Add extensability (#13) - Add automation (#16) - Add channels (#14) - Add network implementation (#15) - Add network, bridges and automations web pages (#18) - Ext registry (#20) - Add core tasks (#19) - Bridge adapters (#23) - Add site (#26) - Add ext refac and sandbox (#25) - Settings ui (#37) - Tasks ui (#36) - Harness improvements (#44) - Agent capabilities (#49) - Redesign ui (#48) - Unify capability (#53) - Redesign network workspace (#59) - Add task deletion and split session delete from stop (#58) - Session provider selection (#60) - Production grade adjustments (#66) - Autonomous system (#75) - Add agent session route (#80) - Tools registry (#85) - Agents soul (#88) - Add network threads (#105) - Orchestration improvements (#106) - Memory v2 (#108) - Agent categories (#113) - Providers model (#118) - Add canonical AGH bundled skill (#143) - Onboarding and improvements (#198) - Onboarding and improvements (#201) ### 🐛 Bug Fixes - Review round - Review rounds - Resolve memory extensibility review batch - Embed web into daemon - Defaults agents - Acp integration (#4) - Lint errors - Prd folder - Remove orphan web actions and dead surfaces (#55) - Qa testing and fixes (#73) - New review rounds (#82) - Security audit (#90) - Release qa round (#95) - Add missing tools (#141) - New qa round (#147) - Advanced qa round (#149) - Homebrew tap - Final review round (#151) - Daemon healthy - Reasoning models (#158) - Lint errors (#160) - Review round (#168) - Release adjustments (#171) - Stabilize release ci fixtures - Stabilize release integration gate - Stabilize release verify gates - Stabilize release integration flows - Stabilize release verify gates - Stabilize main verify shutdown - Ignore stale acpmock cancel - Marketplace search focus and filtering (#193) - Website video - Workspace command select ### 📚 Documentation - Update agents.md - Update prd - Update skills - Update compozy tasks - Update compozy - Update compozy - Add new skills - Archive prd - Update prds - Update rfc - Update prds - Update prds - Add automation prd - Channels prd - Update prd - Update prd - New prds - Archive prds - Bridges adapters prd - Sandbox prd - Update - Archive prd - Update - Add new prd - New design - Update prd - Archive prds - Update prds - Tasks-ui prd tasks - Update prd - Update design docs - Agent capabilities prd - Improve site docs - Remove old design references - Udpate - Autonomous prd - Update skills - Blog design - Agent sould prd - Final qa plan - Update - Remove codex ledgers from gitignore - Remove not needed files - Udpate ledger - Update cy-codex-loop skill - Orchestration improves prd - Update prds - Orch improvs prd - Memv2 prd - Providers model prd - Update refacs prd - New design proposal - Update rules - Update skills - New blog posts (#173) - Format docs - Remove old design files - Remove old - Skeeper update ### 📦 Build System - Initial structure - Commitlint - Frontend base structure - Update vscode settings - Add subagents - Coderabbit - Prd and tooling - Bun lock - Lint tooling - Copy.md and tooling adjusts - Add repoclone rc - Upgrade skeeper to v0.2.0 - Update go.mod - Adopt task artifacts into skeeper - Sync codex plans with skeeper - Skeeper lock - Skeeper lock - New skills - Skeeper lock - Skeeper lock - Skeeper lock - Update deps and go - Regenerate daytona sidecar assets for go 1.26.3 - Fix cliff - Ignore docs on fmt - Build web assets before goreleaser - Extend release dry-run timeout ### 🔧 CI/CD - Lint errors - Fint release pr - Fix goreleaser - Fix release ### 🧪 Testing - Add e2e tests (#27) - Qa rounds (#78) - Improve test suite (#138) - Harden daemon-served restart reloads - Harden daemon-served readiness waits - Stabilize dashboard focus assertion - Stabilize release integration gates - Stabilize release e2e markers - Stabilize release e2e flows Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Merged
pedronauck
added a commit
that referenced
this pull request
May 26, 2026
## Release v0.0.2 This PR prepares the release of version v0.0.2. ### Changelog ## 0.0.2 - 2026-05-26 ### Other Changes - Lessons learned ### ♻️ Refactoring - Project structure (#7) - Kb improvements (#12) - Rename spaces to channels (#17) - Add extensions gaps (#21) - Improve tool calls ui (#22) - Remove web app header - Module improvements (#29) - Memory improvements (#35) - Storybook for web and ui (#38) - Enable AGH network by default for new installs (#57) - Hermes adjustments (#69) - Badges design (#84) - Storybook scenario and logos gallery - Migrate typescript tests (#114) - Internal go packages (#120) - Ui patterns (#127) - Improve e2e tests (#130) - Ui redesign - Workspace isolation across runtime surfaces (#145) - Prod ready applies (#162) - Tool card ui (#164) - Alpha on logo - Prod ready features (#167) - Thread sheet (#202) ### 🎉 Features - Implement config foundation packages - Implement sqlite store package - Add ACP client package - Add session lifecycle manager - Implement observe package - Add daemon composition root - Add uds api server - Implement cli package - Add http api server - Add system design - Add foundation types, schemas, and layout shell for web client - Add daemon health polling and agent sidebar systems for web client - Add session system CRUD, streaming core, and session store for web client - Add chat view, messages, and composer tests for web client - Add tool cards and renderers for web client - Add file-backed memory store core - Scaffold memory session seams - Add memory dream consolidation service - Wire memory assembler into daemon - Add memory api and cli - New skills system (#1) - Add workspace entity (#5) - Add new skill capabilities (#8) - Web ui v2 (#9) - Improve hooks system (#10) - Session resilience (#11) - Add extensability (#13) - Add automation (#16) - Add channels (#14) - Add network implementation (#15) - Add network, bridges and automations web pages (#18) - Ext registry (#20) - Add core tasks (#19) - Bridge adapters (#23) - Add site (#26) - Add ext refac and sandbox (#25) - Settings ui (#37) - Tasks ui (#36) - Harness improvements (#44) - Agent capabilities (#49) - Redesign ui (#48) - Unify capability (#53) - Redesign network workspace (#59) - Add task deletion and split session delete from stop (#58) - Session provider selection (#60) - Production grade adjustments (#66) - Autonomous system (#75) - Add agent session route (#80) - Tools registry (#85) - Agents soul (#88) - Add network threads (#105) - Orchestration improvements (#106) - Memory v2 (#108) - Agent categories (#113) - Providers model (#118) - Add canonical AGH bundled skill (#143) - Onboarding and improvements (#198) - Onboarding and improvements (#201) ### 🐛 Bug Fixes - Review round - Review rounds - Resolve memory extensibility review batch - Embed web into daemon - Defaults agents - Acp integration (#4) - Lint errors - Prd folder - Remove orphan web actions and dead surfaces (#55) - Qa testing and fixes (#73) - New review rounds (#82) - Security audit (#90) - Release qa round (#95) - Add missing tools (#141) - New qa round (#147) - Advanced qa round (#149) - Homebrew tap - Final review round (#151) - Daemon healthy - Reasoning models (#158) - Lint errors (#160) - Review round (#168) - Release adjustments (#171) - Stabilize release ci fixtures - Stabilize release integration gate - Stabilize release verify gates - Stabilize release integration flows - Stabilize release verify gates - Stabilize main verify shutdown - Ignore stale acpmock cancel - Marketplace search focus and filtering (#193) - Website video - Workspace command select ### 📚 Documentation - Update agents.md - Update prd - Update skills - Update compozy tasks - Update compozy - Update compozy - Add new skills - Archive prd - Update prds - Update rfc - Update prds - Update prds - Add automation prd - Channels prd - Update prd - Update prd - New prds - Archive prds - Bridges adapters prd - Sandbox prd - Update - Archive prd - Update - Add new prd - New design - Update prd - Archive prds - Update prds - Tasks-ui prd tasks - Update prd - Update design docs - Agent capabilities prd - Improve site docs - Remove old design references - Udpate - Autonomous prd - Update skills - Blog design - Agent sould prd - Final qa plan - Update - Remove codex ledgers from gitignore - Remove not needed files - Udpate ledger - Update cy-codex-loop skill - Orchestration improves prd - Update prds - Orch improvs prd - Memv2 prd - Providers model prd - Update refacs prd - New design proposal - Update rules - Update skills - New blog posts (#173) - Format docs - Remove old design files - Remove old - Skeeper update ### 📦 Build System - Initial structure - Commitlint - Frontend base structure - Update vscode settings - Add subagents - Coderabbit - Prd and tooling - Bun lock - Lint tooling - Copy.md and tooling adjusts - Add repoclone rc - Upgrade skeeper to v0.2.0 - Update go.mod - Adopt task artifacts into skeeper - Sync codex plans with skeeper - Skeeper lock - Skeeper lock - New skills - Skeeper lock - Skeeper lock - Skeeper lock - Update deps and go - Regenerate daytona sidecar assets for go 1.26.3 - Fix cliff - Ignore docs on fmt - Build web assets before goreleaser - Extend release dry-run timeout ### 🔧 CI/CD - Lint errors - Fint release pr - Fix goreleaser - Fix release - Fix release process ### 🧪 Testing - Add e2e tests (#27) - Qa rounds (#78) - Improve test suite (#138) - Harden daemon-served restart reloads - Harden daemon-served readiness waits - Stabilize dashboard focus assertion - Stabilize release integration gates - Stabilize release e2e markers - Stabilize release e2e flows - Improve suite speed Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This was referenced May 26, 2026
Closed
Closed
Merged
pedronauck
added a commit
that referenced
this pull request
May 27, 2026
## Release v0.0.2 This PR prepares the release of version v0.0.2. ### Changelog ## 0.0.2 - 2026-05-26 ### Other Changes - Lessons learned ### ♻️ Refactoring - Project structure (#7) - Kb improvements (#12) - Rename spaces to channels (#17) - Add extensions gaps (#21) - Improve tool calls ui (#22) - Remove web app header - Module improvements (#29) - Memory improvements (#35) - Storybook for web and ui (#38) - Enable AGH network by default for new installs (#57) - Hermes adjustments (#69) - Badges design (#84) - Storybook scenario and logos gallery - Migrate typescript tests (#114) - Internal go packages (#120) - Ui patterns (#127) - Improve e2e tests (#130) - Ui redesign - Workspace isolation across runtime surfaces (#145) - Prod ready applies (#162) - Tool card ui (#164) - Alpha on logo - Prod ready features (#167) - Thread sheet (#202) ### 🎉 Features - Implement config foundation packages - Implement sqlite store package - Add ACP client package - Add session lifecycle manager - Implement observe package - Add daemon composition root - Add uds api server - Implement cli package - Add http api server - Add system design - Add foundation types, schemas, and layout shell for web client - Add daemon health polling and agent sidebar systems for web client - Add session system CRUD, streaming core, and session store for web client - Add chat view, messages, and composer tests for web client - Add tool cards and renderers for web client - Add file-backed memory store core - Scaffold memory session seams - Add memory dream consolidation service - Wire memory assembler into daemon - Add memory api and cli - New skills system (#1) - Add workspace entity (#5) - Add new skill capabilities (#8) - Web ui v2 (#9) - Improve hooks system (#10) - Session resilience (#11) - Add extensability (#13) - Add automation (#16) - Add channels (#14) - Add network implementation (#15) - Add network, bridges and automations web pages (#18) - Ext registry (#20) - Add core tasks (#19) - Bridge adapters (#23) - Add site (#26) - Add ext refac and sandbox (#25) - Settings ui (#37) - Tasks ui (#36) - Harness improvements (#44) - Agent capabilities (#49) - Redesign ui (#48) - Unify capability (#53) - Redesign network workspace (#59) - Add task deletion and split session delete from stop (#58) - Session provider selection (#60) - Production grade adjustments (#66) - Autonomous system (#75) - Add agent session route (#80) - Tools registry (#85) - Agents soul (#88) - Add network threads (#105) - Orchestration improvements (#106) - Memory v2 (#108) - Agent categories (#113) - Providers model (#118) - Add canonical AGH bundled skill (#143) - Onboarding and improvements (#198) - Onboarding and improvements (#201) ### 🐛 Bug Fixes - Review round - Review rounds - Resolve memory extensibility review batch - Embed web into daemon - Defaults agents - Acp integration (#4) - Lint errors - Prd folder - Remove orphan web actions and dead surfaces (#55) - Qa testing and fixes (#73) - New review rounds (#82) - Security audit (#90) - Release qa round (#95) - Add missing tools (#141) - New qa round (#147) - Advanced qa round (#149) - Homebrew tap - Final review round (#151) - Daemon healthy - Reasoning models (#158) - Lint errors (#160) - Review round (#168) - Release adjustments (#171) - Stabilize release ci fixtures - Stabilize release integration gate - Stabilize release verify gates - Stabilize release integration flows - Stabilize release verify gates - Stabilize main verify shutdown - Ignore stale acpmock cancel - Marketplace search focus and filtering (#193) - Website video - Workspace command select ### 📚 Documentation - Update agents.md - Update prd - Update skills - Update compozy tasks - Update compozy - Update compozy - Add new skills - Archive prd - Update prds - Update rfc - Update prds - Update prds - Add automation prd - Channels prd - Update prd - Update prd - New prds - Archive prds - Bridges adapters prd - Sandbox prd - Update - Archive prd - Update - Add new prd - New design - Update prd - Archive prds - Update prds - Tasks-ui prd tasks - Update prd - Update design docs - Agent capabilities prd - Improve site docs - Remove old design references - Udpate - Autonomous prd - Update skills - Blog design - Agent sould prd - Final qa plan - Update - Remove codex ledgers from gitignore - Remove not needed files - Udpate ledger - Update cy-codex-loop skill - Orchestration improves prd - Update prds - Orch improvs prd - Memv2 prd - Providers model prd - Update refacs prd - New design proposal - Update rules - Update skills - New blog posts (#173) - Format docs - Remove old design files - Remove old - Skeeper update ### 📦 Build System - Initial structure - Commitlint - Frontend base structure - Update vscode settings - Add subagents - Coderabbit - Prd and tooling - Bun lock - Lint tooling - Copy.md and tooling adjusts - Add repoclone rc - Upgrade skeeper to v0.2.0 - Update go.mod - Adopt task artifacts into skeeper - Sync codex plans with skeeper - Skeeper lock - Skeeper lock - New skills - Skeeper lock - Skeeper lock - Skeeper lock - Update deps and go - Regenerate daytona sidecar assets for go 1.26.3 - Fix cliff - Ignore docs on fmt - Build web assets before goreleaser - Extend release dry-run timeout ### 🔧 CI/CD - Lint errors - Fint release pr - Fix goreleaser - Fix release - Fix release process - Fix release sync - Decouple release dry-run npm auth - Persist web assets git auth ### 🧪 Testing - Add e2e tests (#27) - Qa rounds (#78) - Improve test suite (#138) - Harden daemon-served restart reloads - Harden daemon-served readiness waits - Stabilize dashboard focus assertion - Stabilize release integration gates - Stabilize release e2e markers - Stabilize release e2e flows - Improve suite speed <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated web assets dependency to a newer version for improved stability and performance. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/compozy/agh/pull/211?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
](https://app.coderabbit.ai/change-stack/compozy/agh/pull/211?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
This was referenced May 27, 2026
Closed
Closed
Merged
pedronauck
added a commit
that referenced
this pull request
May 27, 2026
## Release v0.0.2 This PR prepares the release of version v0.0.2. ### Changelog ## 0.0.2 - 2026-05-27 ### Other Changes - Lessons learned ### ♻️ Refactoring - Project structure (#7) - Kb improvements (#12) - Rename spaces to channels (#17) - Add extensions gaps (#21) - Improve tool calls ui (#22) - Remove web app header - Module improvements (#29) - Memory improvements (#35) - Storybook for web and ui (#38) - Enable AGH network by default for new installs (#57) - Hermes adjustments (#69) - Badges design (#84) - Storybook scenario and logos gallery - Migrate typescript tests (#114) - Internal go packages (#120) - Ui patterns (#127) - Improve e2e tests (#130) - Ui redesign - Workspace isolation across runtime surfaces (#145) - Prod ready applies (#162) - Tool card ui (#164) - Alpha on logo - Prod ready features (#167) - Thread sheet (#202) ### 🎉 Features - Implement config foundation packages - Implement sqlite store package - Add ACP client package - Add session lifecycle manager - Implement observe package - Add daemon composition root - Add uds api server - Implement cli package - Add http api server - Add system design - Add foundation types, schemas, and layout shell for web client - Add daemon health polling and agent sidebar systems for web client - Add session system CRUD, streaming core, and session store for web client - Add chat view, messages, and composer tests for web client - Add tool cards and renderers for web client - Add file-backed memory store core - Scaffold memory session seams - Add memory dream consolidation service - Wire memory assembler into daemon - Add memory api and cli - New skills system (#1) - Add workspace entity (#5) - Add new skill capabilities (#8) - Web ui v2 (#9) - Improve hooks system (#10) - Session resilience (#11) - Add extensability (#13) - Add automation (#16) - Add channels (#14) - Add network implementation (#15) - Add network, bridges and automations web pages (#18) - Ext registry (#20) - Add core tasks (#19) - Bridge adapters (#23) - Add site (#26) - Add ext refac and sandbox (#25) - Settings ui (#37) - Tasks ui (#36) - Harness improvements (#44) - Agent capabilities (#49) - Redesign ui (#48) - Unify capability (#53) - Redesign network workspace (#59) - Add task deletion and split session delete from stop (#58) - Session provider selection (#60) - Production grade adjustments (#66) - Autonomous system (#75) - Add agent session route (#80) - Tools registry (#85) - Agents soul (#88) - Add network threads (#105) - Orchestration improvements (#106) - Memory v2 (#108) - Agent categories (#113) - Providers model (#118) - Add canonical AGH bundled skill (#143) - Onboarding and improvements (#198) - Onboarding and improvements (#201) ### 🐛 Bug Fixes - Review round - Review rounds - Resolve memory extensibility review batch - Embed web into daemon - Defaults agents - Acp integration (#4) - Lint errors - Prd folder - Remove orphan web actions and dead surfaces (#55) - Qa testing and fixes (#73) - New review rounds (#82) - Security audit (#90) - Release qa round (#95) - Add missing tools (#141) - New qa round (#147) - Advanced qa round (#149) - Homebrew tap - Final review round (#151) - Daemon healthy - Reasoning models (#158) - Lint errors (#160) - Review round (#168) - Release adjustments (#171) - Stabilize release ci fixtures - Stabilize release integration gate - Stabilize release verify gates - Stabilize release integration flows - Stabilize release verify gates - Stabilize main verify shutdown - Ignore stale acpmock cancel - Marketplace search focus and filtering (#193) - Website video - Workspace command select ### 📚 Documentation - Update agents.md - Update prd - Update skills - Update compozy tasks - Update compozy - Update compozy - Add new skills - Archive prd - Update prds - Update rfc - Update prds - Update prds - Add automation prd - Channels prd - Update prd - Update prd - New prds - Archive prds - Bridges adapters prd - Sandbox prd - Update - Archive prd - Update - Add new prd - New design - Update prd - Archive prds - Update prds - Tasks-ui prd tasks - Update prd - Update design docs - Agent capabilities prd - Improve site docs - Remove old design references - Udpate - Autonomous prd - Update skills - Blog design - Agent sould prd - Final qa plan - Update - Remove codex ledgers from gitignore - Remove not needed files - Udpate ledger - Update cy-codex-loop skill - Orchestration improves prd - Update prds - Orch improvs prd - Memv2 prd - Providers model prd - Update refacs prd - New design proposal - Update rules - Update skills - New blog posts (#173) - Format docs - Remove old design files - Remove old - Skeeper update ### 📦 Build System - Initial structure - Commitlint - Frontend base structure - Update vscode settings - Add subagents - Coderabbit - Prd and tooling - Bun lock - Lint tooling - Copy.md and tooling adjusts - Add repoclone rc - Upgrade skeeper to v0.2.0 - Update go.mod - Adopt task artifacts into skeeper - Sync codex plans with skeeper - Skeeper lock - Skeeper lock - New skills - Skeeper lock - Skeeper lock - Skeeper lock - Update deps and go - Regenerate daytona sidecar assets for go 1.26.3 - Fix cliff - Ignore docs on fmt - Build web assets before goreleaser - Extend release dry-run timeout - Fix release dry-run token contract ### 🔧 CI/CD - Lint errors - Fint release pr - Fix goreleaser - Fix release - Fix release process - Fix release sync - Decouple release dry-run npm auth - Persist web assets git auth - Require npm auth before release merge ### 🧪 Testing - Add e2e tests (#27) - Qa rounds (#78) - Improve test suite (#138) - Harden daemon-served restart reloads - Harden daemon-served readiness waits - Stabilize dashboard focus assertion - Stabilize release integration gates - Stabilize release e2e markers - Stabilize release e2e flows - Improve suite speed <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated dependencies to latest versions. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/compozy/agh/pull/214?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
](https://app.coderabbit.ai/change-stack/compozy/agh/pull/214?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
POST /api/sessions/:id/stopstops a session andDELETE /api/sessions/:idfully removes itWhy
Deleting a task and deleting a session are destructive operations with different semantics than stopping execution. This change makes those flows explicit and gives the web UI a real task-delete path plus a real session-delete path.
Key Files
internal/task/manager.go,internal/store/globaldb/global_db_task.go,internal/api/core/tasks.gointernal/session/manager_delete.go,internal/api/core/handlers.go,internal/api/httpapi/routes.go,internal/api/udsapi/routes.goweb/src/systems/tasks/**,web/src/routes/_app/tasks*.tsxweb/src/systems/session/**,web/src/components/assistant-ui/session-thread.tsx,web/src/routes/_app/session.$id.tsxopenapi/agh.json,web/src/generated/agh-openapi.d.tsTests
make verifyAnalysis
/tmp/agh-delete-analysis.mdSummary by CodeRabbit
New Features
Improvements