refactor: internal go packages#120
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughLarge PR: implements ACP terminal lifecycle and handler refactors, introduces agentidentity snapshot/errors, compiles/validates trigger filters, improves bundle/bridge projection & provider-config JSON handling, hardens bridgesdk runtime/batching/peer, tightens API/HTTP/UDS server and routes, updates CLI/codegen, and adds many test utilities, stubs, benchmarks and test reorganizations. ChangesACP Terminal & Handlers
Agent identity
Automation triggers & filters
Bundles & Resource Projection
Bridges & Provider Config
Bridges SDK & Runtime
API contract, spec, prompt SSE
HTTP/UDS server & handlers
CLI, codegen, docpost, format
Test utilities, stubs & benchmarks
Sequence Diagram(s)No sequence diagrams generated for visible section. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
internal/bridgesdk/batching.go (1)
192-204:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClose can still dispatch a pending batch after shutdown starts.
On Line 192,
b.cancel()runs beforeb.closed = true. If a timer callback fires in that window,flushKey()still sees!b.closed, removes the batch, and callsdispatch; a dispatcher that does not checkctx.Done()will emit work thatClose()says it cancels.Suggested fix
func (b *InboundBatcher) Close() { if b == nil { return } - - b.cancel() b.mu.Lock() if b.closed { b.mu.Unlock() return } b.closed = true for _, entry := range b.pending { b.stopTimerLocked(entry) } b.pending = make(map[string]*pendingInboundBatch) b.mu.Unlock() + + b.cancel() b.wg.Wait() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/bridgesdk/batching.go` around lines 192 - 204, The race is that b.cancel() is called before b.closed is set so a timer's callback can run flushKey() and dispatch a batch after shutdown starts; fix it by setting b.closed = true (under b.mu lock) before calling b.cancel() so timers see closed and won't dispatch—specifically, in the Close method adjust the order: acquire b.mu, check/return if already closed, set b.closed = true, clear/stop pending entries with stopTimerLocked, then call b.cancel() (or call b.cancel() while still holding the lock) to ensure callbacks to flushKey()/dispatch cannot run after shutdown begins.internal/bridgesdk/errors.go (1)
341-355:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe new exhaustion error is never returned.
Line 341 already returns the last attempt's
err, so Line 355 is dead code. If you want a distinct exhaustion failure, split out the final-attempt case and wrap the last error with%w.💡 Suggested fix
- if !recovery.Retry || attempt == config.Attempts { + if !recovery.Retry { return zero, err } + if attempt == config.Attempts { + return zero, fmt.Errorf("bridgesdk: retry attempts exhausted: %w", err) + }As per coding guidelines, "Use error wrapping with
%win format strings when wrapping errors in Go code".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/bridgesdk/errors.go` around lines 341 - 355, The final "retry attempts exhausted" return is dead because the loop already returns the last attempt's err; modify the retry loop so that when !recovery.Retry || attempt == config.Attempts you return a wrapped error (e.g., fmt.Errorf("bridgesdk: retry attempts exhausted: %w", err)) instead of returning raw err, and then remove the unreachable errors.New(...) at the end; locate the logic around retryDelay(config, attempt, recovery), config.OnRetry, and retrypkg.Wait to apply this change.internal/bridges/resource_projection.go (1)
169-175:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a nil check before the type assertion to unify nil error handling.
When
planis an untypednilinterface, the type assertion fails and returnsfmt.Errorf("bridges: bridge resource plan has type %T", plan)with%Tprinting<nil>. Whenplanis(*ResourceProjectionPlan)(nil), the assertion succeeds and the second check catches it with the clearer"plan is required"error. Addif plan == nil { return errors.New("bridges: bridge resource plan is required") }before the type assertion to handle both cases consistently.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/bridges/resource_projection.go` around lines 169 - 175, Before performing the type assertion on plan in the block that currently does "typed, ok := plan.(*ResourceProjectionPlan)", add an explicit nil check "if plan == nil { return errors.New(\"bridges: bridge resource plan is required\") }" so untyped nil interfaces are handled the same as a typed nil (*ResourceProjectionPlan)(nil); keep the existing assertion and subsequent nil check for typed to preserve the clearer error path for typed nil pointers.internal/cli/cli_integration_test.go (1)
2931-2961:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCache the daemon error to avoid silent loss during cleanup.
integrationDaemonProcess.Wait()andintegrationDaemon.waitForExit()both consumewaitCh. During a daemon startup failure,waitForDaemonStart()drains the buffered error viachild.Wait(). When test cleanup later callswaitForExit(), it reads from an already-closed channel and silently loses the real daemon error.Store the run result once and have every waiter read the cached error after
donecloses, rather than sharing the same one-shot result channel across multiple consumers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/cli/cli_integration_test.go` around lines 2931 - 2961, The wait channel result is being consumed multiple times causing the real daemon error to be lost; modify spawnDetached and integrationDaemonProcess so the goroutine stores the run error into a cached field (e.g., add an err error field on integrationDaemonProcess) before closing done, and change integrationDaemonProcess.Wait() to wait for done to close and then return the cached error instead of reading directly from waitCh; update spawnDetached to write the error into that cached field (from the goroutine that currently does waitCh <- err / close(waitCh)) and ensure other callers like waitForExit and waitForDaemonStart read the cached error after done closes rather than draining the original one-shot waitCh.
🟡 Minor comments (15)
internal/cli/json_flags_test.go-32-37 (1)
32-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStrengthen invalid JSON assertion to validate the actual error path.
This subtest currently accepts any non-nil error. Add a specific assertion so it fails if the code returns the wrong error class.
As per coding guidelines, "MUST have specific error assertions (ErrorContains, ErrorAs)".Proposed test tightening
import ( "errors" + "strings" "testing" ) @@ t.Run("Should reject invalid JSON", func(t *testing.T) { t.Parallel() - if _, err := parseRequiredJSONRawMessage("{"); err == nil { - t.Fatal("parseRequiredJSONRawMessage() error = nil, want non-nil") - } + _, err := parseRequiredJSONRawMessage("{") + if err == nil { + t.Fatal("parseRequiredJSONRawMessage() error = nil, want non-nil") + } + if errors.Is(err, errEmptyJSONFlag) { + t.Fatalf("parseRequiredJSONRawMessage() error = %v, want JSON parse error", err) + } + if !strings.Contains(err.Error(), "unexpected end of JSON input") { + t.Fatalf("parseRequiredJSONRawMessage() error = %v, want parse failure detail", err) + } })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/cli/json_flags_test.go` around lines 32 - 37, The test currently only checks for a non-nil error from parseRequiredJSONRawMessage("{") — change it to assert the specific error class/path: capture err, then use errors.As to verify it is (or wraps) a *json.SyntaxError (or use ErrorContains to match the expected syntax error message) so the subtest fails if a different error type is returned; reference the parseRequiredJSONRawMessage call and replace the loose nil-check with a concrete errors.As/ErrorContains assertion.internal/cli/format.go-195-195 (1)
195-195:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep underline and separator sizing on the same width metric.
The new renderer uses
humanTableCellWidthfor column sizing, but these spots still uselen(...). For non-ASCII titles or headers that mixes byte length with rune width, so the underline/separator can render too wide and even expand the computed table width.💡 Suggested change
- if _, err := fmt.Fprintf(&buffer, "%s\n", strings.Repeat("=", len(title))); err != nil { + if _, err := fmt.Fprintf(&buffer, "%s\n", strings.Repeat("=", humanTableCellWidth(title))); err != nil { return "", fmt.Errorf("cli: write section underline: %w", err) } @@ - builder.WriteString(strings.Repeat("=", len(title))) + builder.WriteString(strings.Repeat("=", humanTableCellWidth(title))) @@ - separators = append(separators, strings.Repeat("-", max(3, len(header)))) + separators = append(separators, strings.Repeat("-", max(3, humanTableCellWidth(header)))) }Also applies to: 218-218, 225-227
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/cli/format.go` at line 195, The underline/separator is using byte length (len(title)/len(header)) which mismatches the renderer’s column sizing; change those formatting calls to use the humanTableCellWidth(...) metric instead (e.g. replace len(title) and any len(header)/len(column) uses in the fmt.Fprintf repeat calls with humanTableCellWidth(title) / humanTableCellWidth(header) so the underline/separator width matches the humanTableCellWidth used by the renderer); update the three similar spots (the current fmt.Fprintf for title underline and the other separator/underline writes) to use humanTableCellWidth to compute repeat counts.internal/bridgesdk/webhook_refac_test.go-31-61 (1)
31-61:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing status code and response body assertions in the retry-after subtest.
The test only validates the
Retry-Afterheader but doesn't assertrecorder.Codeor the response body, which leaves the 429 status path untested.✅ Proposed additions
handler.ServeHTTP(recorder, request) + if got, want := recorder.Code, http.StatusTooManyRequests; got != want { + t.Fatalf("Status = %d, want %d", got, want) + } + if !strings.Contains(recorder.Body.String(), "slow down") { + t.Fatalf("Body = %q, want it to contain %q", recorder.Body.String(), "slow down") + } if got, want := recorder.Header().Get("Retry-After"), "1"; got != want { t.Fatalf("Retry-After = %q, want %q", got, want) }As per coding guidelines: "Always assert both HTTP status code AND response body in tests; status-code-only assertions are insufficient."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/bridgesdk/webhook_refac_test.go` around lines 31 - 61, The subtest currently only checks the Retry-After header; update the test that builds the handler via NewWebhookHandler and returns an *HTTPError (RetryAfter 100ms) to also assert the HTTP status code and response body: verify recorder.Code equals http.StatusTooManyRequests (429) and that the response body contains the expected error message ("slow down") or the serialized error payload produced by the webhook handler so the 429 path is fully validated.internal/bridgesdk/errors_refac_test.go-11-34 (1)
11-34:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAssert the concrete precondition error in both subtests.
Right now both cases only check
err != nil, so they'll still pass on any unrelated failure. These are contract tests for new guard clauses; assert the expected message or a sentinel error.As per coding guidelines, "MUST have specific error assertions (ErrorContains, ErrorAs)".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/bridgesdk/errors_refac_test.go` around lines 11 - 34, Update both subtests to assert the concrete precondition error returned by RetryDo instead of only checking err != nil: when calling RetryDo(nilRetryContext(), RetryConfig{}, ...) and RetryDo[string](context.Background(), RetryConfig{}, nil) verify the error matches the expected sentinel or message (use ErrorContains or ErrorAs per guidelines) so the tests assert the specific guard-clause error from RetryDo rather than any non-nil error.internal/bridgesdk/runtime_refac_test.go-115-121 (1)
115-121:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAssert that the decoded object is empty, not just non-nil.
This test is meant to lock down "
nullbecomes{}". A non-nil map with unexpected keys would still pass, so add alen(target) == 0assertion.As per coding guidelines, "Ensure tests verify behavior outcomes, not just function calls".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/bridgesdk/runtime_refac_test.go` around lines 115 - 121, The test currently only checks that decodeParams(json.RawMessage(" \n null \t "), &target) yields a non-nil map; update the assertion to validate the map is empty as well by asserting len(target) == 0 (after verifying target != nil) so the behavior "null becomes {}" is enforced; reference the decodeParams call and the target variable in runtime_refac_test.go and replace or extend the existing t.Fatal/t.Fatalf check to fail when target has any keys.internal/bridgesdk/hostapi_refac_test.go-12-26 (1)
12-26:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake the nil-context subtest assert the expected error.
err == nildoes not prove the nil-context guard fired; any unrelated failure would satisfy this test. Check the returned message or a sentinel error so the contract is actually locked down.As per coding guidelines, "MUST have specific error assertions (ErrorContains, ErrorAs)".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/bridgesdk/hostapi_refac_test.go` around lines 12 - 26, The test currently only checks err != nil which can be satisfied by unrelated failures; change the nil-context subtest to assert the specific error contract by verifying the returned error is the sentinel error or contains an expected message: call client.Call(nilHostAPIContext(), "bridges/instances/list", nil, nil) and then assert either errors.Is(err, ErrNilHostAPIContext) (if ErrNilHostAPIContext exists) or use strings.Contains(err.Error(), "nil context") / t.Fatalf-style ErrorContains to ensure the guard in NewHostAPIClientFromCall / Call actually triggered; keep the existing check that the transport (the closure created by NewHostAPIClientFromCall) was not invoked (called == false).internal/bridges/resource_test.go-379-411 (1)
379-411:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winConvert this flat case into a
Should ...subtest and assert the exact rejection.Right now this only checks
err != nil, so it will still pass ifApplyResourceStatefails for the wrong reason. Please wrap it in a named subtest and check the expected error text/type, ideally with a sibling case for the untypednilinterface as well.As per coding guidelines, "Use
t.Run('Should ...')pattern for Go test subtests instead of flat test structures" and "MUST have specific error assertions (ErrorContains, ErrorAs)".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/bridges/resource_test.go` around lines 379 - 411, Convert the flat test TestBridgeResourceApplyRejectsTypedNilPlanWithoutReplacingInstances into a t.Run subtest using the "Should ..." naming convention, e.g. t.Run("Should reject typed nil plan without replacing instances", func(t *testing.T) {...}); inside the subtest call bridgepkg.ApplyResourceState with the typed nil plan variable and assert the exact rejection using a specific assertion (e.g. errors.As to check the error type or require.ErrorContains/strings.Contains to check the error text) rather than only checking err != nil; keep the existing projectionStore instance checks (store.replacements and store.instances) and add a sibling subtest that passes an untyped nil interface (nil as interface{}) to ApplyResourceState to verify the behavior for untyped nil as well, referencing the ApplyResourceState and projectionStore symbols to locate the code to modify.internal/api/httpapi/middleware_refac_test.go-70-78 (1)
70-78:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAssert the response body in the successful branches.
Both success paths stop after status/header checks, so a regression that starts returning an unexpected payload would still pass.
As per coding guidelines, "Assert both HTTP status code AND response body in tests; status-code-only assertions are insufficient".💡 Minimal assertion to add
if recorder.Code != http.StatusNoContent { t.Fatalf("status = %d, want %d; body=%s", recorder.Code, http.StatusNoContent, recorder.Body.String()) } + if body := recorder.Body.String(); body != "" { + t.Fatalf("body = %q, want empty", body) + } if got := recorder.Header().Get("Access-Control-Allow-Methods"); !strings.Contains(got, http.MethodPatch) { t.Fatalf("Access-Control-Allow-Methods = %q, want it to include %q", got, http.MethodPatch) }if recorder.Code != tt.wantStatus { t.Fatalf("status = %d, want %d; body=%s", recorder.Code, tt.wantStatus, recorder.Body.String()) } if tt.wantError == "" { + if body := recorder.Body.String(); body != "" { + t.Fatalf("body = %q, want empty", body) + } return }Also applies to: 115-120
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/httpapi/middleware_refac_test.go` around lines 70 - 78, The test currently only checks status and headers for the preflight branch (the block using recorder and http.StatusNoContent and Access-Control-Allow-* headers) and a second success branch around lines 115-120; add assertions that also validate the response body in both places: after the StatusNoContent/header checks assert recorder.Body.String() is the expected empty payload (or specific expected string) and in the other success branch assert recorder.Body.String() equals the expected response payload; use the same t.Fatalf style including recorder.Body.String() in the failure message so regressions returning unexpected bodies fail the test.internal/api/testutil/task_stub.go-390-399 (1)
390-399:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReturn the task sentinel from the default
EnqueueRunpath.
EnqueueRuncreates a run from a task, so falling back toErrTaskRunNotFoundsends tests down the wrong not-found branch before a run even exists.🩹 Suggested fix
if s.EnqueueRunFn != nil { return s.EnqueueRunFn(ctx, spec, actor) } - return nil, taskpkg.ErrTaskRunNotFound + return nil, taskpkg.ErrTaskNotFound }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/testutil/task_stub.go` around lines 390 - 399, The default StubTaskManager.EnqueueRun path should return a sentinel run instead of ErrTaskRunNotFound so tests get a created run; update the EnqueueRun method to construct and return a minimal *taskpkg.Run representing the sentinel (e.g., with ID set to the package sentinel constant such as taskpkg.SentinelRunID or using any taskpkg helper like NewSentinelRun if available) when s.EnqueueRunFn is nil, rather than returning taskpkg.ErrTaskRunNotFound.internal/api/testutil/network_stub.go-89-98 (1)
89-98:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHoist the default
WaitInboxerror into a sentinel.Allocating a fresh
errors.New(...)here prevents callers from usingerrors.Is()for error comparison. This violates the guideline to useerrors.Isanderrors.Asonly for error type checking. The codebase establishes sentinel error pattern (e.g.,var errXxx = errors.New(...)) across all packages — apply the same pattern here.🩹 Suggested fix
+var errStubWaitInboxNotImplemented = errors.New("stub network service WaitInbox not implemented") + func (s StubNetworkService) WaitInbox( ctx context.Context, sessionID string, channel string, ) ([]network.Envelope, error) { if s.WaitInboxFn != nil { return s.WaitInboxFn(ctx, sessionID, channel) } - return nil, errors.New("stub network service WaitInbox not implemented") + return nil, errStubWaitInboxNotImplemented }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/testutil/network_stub.go` around lines 89 - 98, The default error returned by StubNetworkService.WaitInbox is created inline which prevents callers from using errors.Is; hoist that error into a package-level sentinel (e.g., var ErrWaitInboxNotImplemented = errors.New("stub network service WaitInbox not implemented")) and change WaitInbox to return that sentinel when WaitInboxFn is nil; reference StubNetworkService.WaitInbox and the WaitInboxFn field when making the change so callers can compare with errors.Is(…, ErrWaitInboxNotImplemented).internal/api/core/bridges_test.go-546-577 (1)
546-577:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAssert the rejection body, not just the 400.
These cases currently pass for any bad-request path, including unrelated decode failures. Check the error payload for the rejected field so the test fails only when the operational-state validation regresses.
As per coding guidelines, "Assert both HTTP status code AND response body in tests — status-code-only assertions are insufficient".💡 Suggested fix
tests := []struct { - name string - body []byte + name string + body []byte + wantErr string }{ { name: "Should reject status", body: []byte( `{"scope":"global","platform":"telegram","extension_name":"ext-telegram","display_name":"Support","enabled":true,"status":"ready","routing_policy":{"include_peer":true}}`, ), + wantErr: "status", }, { name: "Should reject degradation", body: []byte( `{"scope":"global","platform":"telegram","extension_name":"ext-telegram","display_name":"Support","enabled":true,"degradation":{"reason":"rate_limited"},"routing_policy":{"include_peer":true}}`, ), + wantErr: "degradation", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() resp := performRequest(t, engine, http.MethodPost, "/bridges", tt.body) if resp.Code != http.StatusBadRequest { t.Fatalf( "create status = %d, want %d body=%s", resp.Code, http.StatusBadRequest, resp.Body.String(), ) } + var payload contract.ErrorPayload + testutil.DecodeJSONResponse(t, resp, &payload) + if !strings.Contains(payload.Error, tt.wantErr) { + t.Fatalf("error payload = %#v, want mention of %q", payload, tt.wantErr) + } }) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/core/bridges_test.go` around lines 546 - 577, The test only checks for HTTP 400 but must also assert the response body contains the specific rejected field to avoid masking unrelated decode errors; after calling performRequest in the t.Run loop for the "/bridges" POST cases, decode or inspect resp.Body and assert the error payload includes the rejected field name ("status" for the first case, "degradation" for the second) or an explicit "rejected" key indicating which field was rejected, failing the test if the expected field is not present.internal/api/testutil/sse.go-33-49 (1)
33-49:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid emitting empty SSE records and accept valid
field:valuelines.Line 33 appends a record for every blank separator, even when
currentis empty, which can produce spurious entries. Also, Lines 40-48 only parse"id: ","event: ","data: "(with space), so valid SSEfield:valuelines are silently ignored.Suggested patch
func ParseSSE(t *testing.T, body string) []SSERecord { t.Helper() scanner := bufio.NewScanner(strings.NewReader(body)) records := make([]SSERecord, 0) current := SSERecord{} for scanner.Scan() { line := scanner.Text() if line == "" { - records = append(records, current) + if current.Event != "" || current.ID != "" || len(current.Data) > 0 { + records = append(records, current) + } current = SSERecord{} continue } + if strings.HasPrefix(line, ":") { + continue + } switch { - case strings.HasPrefix(line, "id: "): - current.ID = strings.TrimPrefix(line, "id: ") - case strings.HasPrefix(line, "event: "): - current.Event = strings.TrimPrefix(line, "event: ") - case strings.HasPrefix(line, "data: "): + case strings.HasPrefix(line, "id:"): + current.ID = strings.TrimPrefix(strings.TrimPrefix(line, "id:"), " ") + case strings.HasPrefix(line, "event:"): + current.Event = strings.TrimPrefix(strings.TrimPrefix(line, "event:"), " ") + case strings.HasPrefix(line, "data:"): if len(current.Data) > 0 { current.Data = append(current.Data, '\n') } - current.Data = append(current.Data, []byte(strings.TrimPrefix(line, "data: "))...) + current.Data = append(current.Data, []byte(strings.TrimPrefix(strings.TrimPrefix(line, "data:"), " "))...) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/testutil/sse.go` around lines 33 - 49, The parser currently appends an SSERecord on every blank separator and only recognizes keys with a trailing space ("id: ", "event: ", "data: "), causing empty/spurious records and ignoring valid "field:value" lines; update the logic that handles blank lines to only append `current` to `records` when `current` has any non-empty field (e.g., `current.ID`, `current.Event`, or `len(current.Data) > 0`), and replace the strict HasPrefix checks in the parsing switch (the branches around `strings.HasPrefix(line, "id: ")`, `... "event: "`, `... "data: "`) with a robust split-on-first-colon approach (or accept both "field: value" and "field:value") to extract the key and value, trimming a single optional leading space from the value before assigning to `current.ID`, `current.Event`, or appending to `current.Data` (keep using SSERecord and current identifiers to locate the code).internal/api/core/coverage_helpers_test.go-675-677 (1)
675-677:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing
t.Parallel()in subtest.The subtest at line 676 is missing the
t.Parallel()call, unlike other subtests in this file that consistently include it.Proposed fix
func TestObserveHealthPayloadIncludesRuntimeActivity(t *testing.T) { + t.Parallel() + t.Run("Should include runtime activity", func(t *testing.T) { t.Parallel()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/core/coverage_helpers_test.go` around lines 675 - 677, The subtest inside TestObserveHealthPayloadIncludesRuntimeActivity is missing t.Parallel(); open the subtest defined by t.Run("Should include runtime activity", func(t *testing.T) { ... }) and add t.Parallel() as the first statement in that closure so it mirrors other subtests and runs in parallel.internal/api/testutil/bridge_stub.go-139-157 (1)
139-157:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
ResolveOrCreateRouteandUpsertRoutedefaults returnErrBridgeRouteNotFound— semantically wrong for write/create ops.Both operations guarantee the caller a route object (by creating it if necessary); returning "not found" as their default zero-stub behaviour contradicts that contract and is inconsistent with every other write-op default in the same file (
CreateInstance→nil, nil,PutSecretBinding→nil,PutBridgeTaskSubscription→nil).Tests that rely on unconfigured stubs will silently get a "not found" failure from code paths that should succeed, producing confusing test output.
🐛 Align defaults with write-op semantics
func (s StubBridgeService) ResolveOrCreateRoute( ctx context.Context, route bridgepkg.BridgeRoute, ) (*bridgepkg.BridgeRoute, bool, error) { if s.ResolveOrCreateRouteFn != nil { return s.ResolveOrCreateRouteFn(ctx, route) } - return nil, false, bridgepkg.ErrBridgeRouteNotFound + return &route, true, nil } func (s StubBridgeService) UpsertRoute( ctx context.Context, route bridgepkg.BridgeRoute, ) (*bridgepkg.BridgeRoute, error) { if s.UpsertRouteFn != nil { return s.UpsertRouteFn(ctx, route) } - return nil, bridgepkg.ErrBridgeRouteNotFound + return &route, nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/testutil/bridge_stub.go` around lines 139 - 157, The stub defaults for write/create ops are wrong: update StubBridgeService.ResolveOrCreateRoute to return the provided route pointer, a creation indicator, and nil error (e.g., return &route, true, nil) instead of ErrBridgeRouteNotFound, and update StubBridgeService.UpsertRoute to return the provided route pointer and nil error (e.g., return &route, nil); this aligns these methods' zero-stub behavior with other write-op defaults in the file.internal/api/spec/spec.go-4507-4515 (1)
4507-4515:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDeep-copy nested container values in
cloneSpecValue.At Line 4509, only the top-level
map[string]anyis copied. Nested maps/slices remain shared and can still mutate registry-backed data returned byOperations().Proposed fix
func cloneSpecValue(value any) any { switch typed := value.(type) { case map[string]any: - cloned := make(map[string]any, len(typed)) - maps.Copy(cloned, typed) + cloned := make(map[string]any, len(typed)) + for key, item := range typed { + cloned[key] = cloneSpecValue(item) + } return cloned + case []any: + cloned := make([]any, len(typed)) + for index, item := range typed { + cloned[index] = cloneSpecValue(item) + } + return cloned default: return value } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/spec/spec.go` around lines 4507 - 4515, cloneSpecValue currently shallow-copies only the top-level map which leaves nested maps/slices shared; update cloneSpecValue to perform a recursive deep copy: when value is map[string]any, create a new map and for each k,v set cloned[k] = cloneSpecValue(v); when value is []any, create a new slice and append cloneSpecValue(elem) for each element; for all other types return the value as-is. Keep the function name cloneSpecValue and its switch-based type branching so it replaces the shallow maps.Copy with recursive handling of maps and slices to fully decouple returned data from the registry.
🧹 Nitpick comments (11)
internal/codegen/openapits/generate.go (1)
24-28: ⚡ Quick winAdd compile-time interface assertion for
execRunner.Per coding guidelines, use compile-time interface assertions to verify that implementations satisfy interfaces. This catches mismatches at compile time rather than runtime.
Proposed fix
type execRunner struct{} + +var _ commandRunner = execRunner{} func (execRunner) Run(ctx context.Context, name string, args ...string) error { return runCommand(ctx, name, args...) }As per coding guidelines: "Use compile-time interface assertions in Go: assign nil to _ to verify interface implementation at compile time."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/codegen/openapits/generate.go` around lines 24 - 28, Add a compile-time interface assertion for execRunner by locating the interface type that declares Run(ctx context.Context, name string, args ...string) error (the interface used where execRunner is passed) and add a nil assignment to _ to verify implementation, e.g. assert that execRunner implements that interface with a statement using (*execRunner)(nil); place this assertion near the execRunner type definition (which contains Run and calls runCommand) so mismatches are caught at compile time.internal/agentidentity/identity_test.go (1)
347-350: ⚡ Quick winBroaden copied-field assertions in snapshot conversion test.
Line [347]-[350] validates only a subset of mapped fields, so regressions in
Name,AgentName,Provider,Channel,Type, orUpdatedAtcan slip through.As per coding guidelines, "Ensure tests verify behavior outcomes, not just function calls."Proposed assertion tightening
- if got.ID != info.ID || got.WorkspacePath != info.Workspace || got.State != info.State || - !got.CreatedAt.Equal(now) { + if got.ID != info.ID || + got.Name != info.Name || + got.AgentName != info.AgentName || + got.Provider != info.Provider || + got.WorkspaceID != info.WorkspaceID || + got.WorkspacePath != info.Workspace || + got.Channel != info.Channel || + got.Type != info.Type || + got.State != info.State || + !got.CreatedAt.Equal(now) || + !got.UpdatedAt.Equal(now) { t.Fatalf("SessionSnapshotFromInfo() = %#v, want fields copied from session.Info", got) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/agentidentity/identity_test.go` around lines 347 - 350, The test for SessionSnapshotFromInfo only asserts a subset of fields; update the assertion to verify all mapped fields from session.Info are copied (at minimum: ID, Name, AgentName, Provider, Channel, Type, WorkspacePath, State, CreatedAt, UpdatedAt). Locate the test that calls SessionSnapshotFromInfo() and either compare each of those fields individually (using time.Equal for timestamps) or perform a full struct comparison (e.g., reflect.DeepEqual) between the expected snapshot and got, and update the t.Fatalf message to include which field mismatched for easier debugging.internal/cli/format.go (1)
192-207: ⚡ Quick winWrap the new render errors with context.
These branches now surface first-class errors, but they return raw
fmt.Fprintf/Flushfailures unchanged. Add operation context here so callers can tell whether title emission, row formatting, or flush failed.💡 Suggested change
- if _, err := fmt.Fprintf(&buffer, "%s\n", title); err != nil { - return "", err + if _, err := fmt.Fprintf(&buffer, "%s\n", title); err != nil { + return "", fmt.Errorf("cli: write section title: %w", err) } - if _, err := fmt.Fprintf(&buffer, "%s\n", strings.Repeat("=", len(title))); err != nil { - return "", err + if _, err := fmt.Fprintf(&buffer, "%s\n", strings.Repeat("=", len(title))); err != nil { + return "", fmt.Errorf("cli: write section underline: %w", err) } } writer := tabwriter.NewWriter(&buffer, 0, 0, 2, ' ', 0) for _, row := range rows { - if _, err := fmt.Fprintf(writer, "%s:\t%s\n", row.Label, row.Value); err != nil { - return "", err + if _, err := fmt.Fprintf(writer, "%s:\t%s\n", row.Label, row.Value); err != nil { + return "", fmt.Errorf("cli: write section row: %w", err) } } if err := writer.Flush(); err != nil { - return "", err + return "", fmt.Errorf("cli: flush section output: %w", err) }As per coding guidelines, "Use
%wfor error wrapping anderrors.Is/errors.Asfor error comparison in Go code".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/cli/format.go` around lines 192 - 207, Wrap each error returned from the fmt.Fprintf and writer.Flush calls with contextual messages using fmt.Errorf("%s: %w", ...) so callers can distinguish failures emitting the title, emitting the underline, formatting a specific row, or flushing the tabwriter; specifically, wrap the errors from the calls to fmt.Fprintf(&buffer, "%s\n", title) and fmt.Fprintf(&buffer, "%s\n", strings.Repeat("=", len(title))) with messages like "emit title: %w" / "emit underline: %w", wrap the error inside the loop from fmt.Fprintf(writer, "%s:\t%s\n", row.Label, row.Value) with a message including the row.Label (e.g. "format row %q: %w"), and wrap writer.Flush() with "flush writer: %w" so callers can use errors.Is/errors.As to inspect underlying errors.internal/acp/client_integration_test.go (1)
16-36: ⚡ Quick winMark the independent subtests parallel.
These subtests each build their own temp dir, driver, and helper process, so they can call
t.Parallel(). Keeping them serial slows the integration suite and skips the default test-concurrency discipline this repo asks for. The network guardrails case can stay serial because it usest.Setenv.As per coding guidelines, "Default to
t.Parallelin Go tests unless there is a specific reason to disable it (opt-out witht.Setenv)".Also applies to: 40-67, 71-104, 108-179, 183-223
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/acp/client_integration_test.go` around lines 16 - 36, Add t.Parallel() as the first statement inside each independent t.Run subtest closure (e.g., the "Should return prompt events through helper ACP process" subtest) so the subtests run concurrently; do this for the other independent subtests referenced (the blocks covering lines 40-67, 71-104, 108-179, 183-223) but do NOT add t.Parallel() to tests that use t.Setenv or otherwise require serial execution (the network guardrails case).internal/api/httpapi/middleware_refac_test.go (1)
32-43: ⚡ Quick winWrap each table entry in its own subtest.
This loop flattens five scenarios into one failure domain, so the first mismatch aborts coverage for the rest and misses the repo’s required per-case
t.Run("Should ...")structure.As per coding guidelines, "Use `t.Run('Should ...')` pattern for Go test subtests instead of flat test structures".♻️ Suggested structure
- for _, tt := range tests { - got := canonicalHost(tt.raw) - if got != tt.want { - t.Fatalf("%s: canonicalHost(%q) = %q, want %q", tt.name, tt.raw, got, tt.want) - } - if isLoopbackHost(got) != tt.loopback { - t.Fatalf("%s: isLoopbackHost(%q) = %v, want %v", tt.name, got, isLoopbackHost(got), tt.loopback) - } - if isWildcardHost(got) != tt.wildcard { - t.Fatalf("%s: isWildcardHost(%q) = %v, want %v", tt.name, got, isWildcardHost(got), tt.wildcard) - } - } + for _, tt := range tests { + tt := tt + t.Run("Should normalize "+tt.name, func(t *testing.T) { + t.Parallel() + + got := canonicalHost(tt.raw) + if got != tt.want { + t.Fatalf("canonicalHost(%q) = %q, want %q", tt.raw, got, tt.want) + } + if isLoopbackHost(got) != tt.loopback { + t.Fatalf("isLoopbackHost(%q) = %v, want %v", got, isLoopbackHost(got), tt.loopback) + } + if isWildcardHost(got) != tt.wildcard { + t.Fatalf("isWildcardHost(%q) = %v, want %v", got, isWildcardHost(got), tt.wildcard) + } + }) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/httpapi/middleware_refac_test.go` around lines 32 - 43, Convert the flat loop over tests into per-case subtests using t.Run(tt.name, func(t *testing.T) { ... }), so for each test case (from the tests table) call canonicalHost(tt.raw) and assert equality, then assert isLoopbackHost(got) and isWildcardHost(got) inside that subtest; use tt.name (or a "Should ..." string built from tt.name) as the subtest name and keep the existing failure messages/assertions but scoped to the subtest so failures don't abort other cases.internal/bundles/ids.go (1)
9-28: 💤 Low value
strings.TrimSpacecalled twice per part — consider pre-trimming to a local slice.The size loop (lines 11–16) and the append loop (lines 18–23) both call
strings.TrimSpaceon every element. Since this is a deterministic ID utility it isn't a hot-path concern, but trimming once into a local slice makes the intent clearer and removes the redundancy.♻️ Suggested refactor
func stableID(prefix string, parts ...string) string { + trimmed := make([]string, len(parts)) + for i, p := range parts { + trimmed[i] = strings.TrimSpace(p) + } size := 0 - for idx, part := range parts { + for idx, part := range trimmed { if idx > 0 { size++ } - size += len(strings.TrimSpace(part)) + size += len(part) } payload := make([]byte, 0, size) - for idx, part := range parts { + for idx, part := range trimmed { if idx > 0 { payload = append(payload, '\n') } - payload = append(payload, strings.TrimSpace(part)...) + payload = append(payload, part...) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/bundles/ids.go` around lines 9 - 28, In stableID, strings.TrimSpace is being called twice for each element (in the size calculation and again when building payload); fix this by creating a local trimmedParts slice (from the incoming parts) where each element is trimmed once, then use trimmedParts in both the size loop and the payload append loop so capacity calculation and payload construction use the pre-trimmed values while preserving the existing logic that inserts '\n' between parts and then computes sha256.Sum256 and hex-encodes into encoded before returning prefix+"_"+string(encoded[:]).internal/cli/daemon_wait_test.go (1)
41-61: ⚡ Quick winWrap modified tests in
t.Run("Should ...")subtests per project guidelines.
TestWaitForDaemonStartReturnsStatusWhenDaemonBecomesReady(lines 41–61) andTestRunDaemonDetachedReturnsReadyStatus(lines 260–286) both have changed lines in this PR but still lack the requiredt.Run("Should ...")subtest wrapper. As per coding guidelines, "MUST uset.Run('Should ...')pattern for ALL test cases."♻️ Example for TestWaitForDaemonStartReturnsStatusWhenDaemonBecomesReady
func TestWaitForDaemonStartReturnsStatusWhenDaemonBecomesReady(t *testing.T) { - t.Parallel() - - child := &stubDaemonProcess{done: make(chan struct{})} - // ...rest of body... + t.Run("Should return ready status when the daemon becomes ready", func(t *testing.T) { + t.Parallel() + + child := &stubDaemonProcess{done: make(chan struct{})} + // ...rest of body... + }) }Apply the same pattern to
TestRunDaemonDetachedReturnsReadyStatus.As per coding guidelines, "Use
agh-test-conventionsskill before writing or editing any*_test.gofile. Covers:t.Run('Should ...')subtests."Also applies to: 260-286
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/cli/daemon_wait_test.go` around lines 41 - 61, Wrap the bodies of the modified tests in a t.Run subtest using the "Should ..." naming pattern per guidelines: for TestWaitForDaemonStartReturnsStatusWhenDaemonBecomesReady, enclose the existing test code (including t.Parallel(), creation of child and deps, deps.pollInterval/startTimeout, the call to waitForDaemonStart and child.complete, and assertions) inside t.Run("Should return status when daemon becomes ready", func(t *testing.T){ ... }), and do the same for TestRunDaemonDetachedReturnsReadyStatus with an appropriate "Should ..." description; ensure you keep t.Parallel() and all existing setup/assertions inside the subtest so behavior of waitForDaemonStart and any calls like child.complete remain unchanged.internal/api/udsapi/bridges_test.go (2)
56-70: ⚡ Quick winAssert
response.Bridge.Status == BridgeStatusStartingto cover the contract change.The PR's stated behavioral change is that
statusis no longer client-supplied but is now derived from theenabledflag (mapped toBridgeStatusStarting). The stub at line 40 echoesreq.Statusback verbatim — after removingstatusfrom the request JSON the handler must be setting it before callingCreateInstance. Neither the stub assertions (lines 19-32) nor the response assertions (lines 65-70) verify that this mapping actually occurred, leaving the key guarantee of this change untested.♻️ Suggested assertion addition
if got, want := string(response.Bridge.ProviderConfig), `{"mode":"bot","tenant":"acme"}`; got != want { t.Fatalf("response.Bridge.ProviderConfig = %s, want %s", got, want) } +if got, want := response.Bridge.Status, bridgepkg.BridgeStatusStarting; got != want { + t.Fatalf("response.Bridge.Status = %q, want %q (derived from enabled=true)", got, want) +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/udsapi/bridges_test.go` around lines 56 - 70, Add an assertion to verify the handler maps enabled->status by checking response.Bridge.Status == bridgepkg.BridgeStatusStarting (i.e., assert response.Bridge.Status equals BridgeStatusStarting), and update the test stub assertions that echo req.Status (the CreateInstance stub) to expect the server-set BridgeStatusStarting rather than echoing the client value so the mapping is actually exercised.
13-71: ⚡ Quick winWrap
TestCreateBridgeHandlerReturnsPersistedPayloadin at.Run("Should ...")subtest.As per coding guidelines, "Use
t.Run('Should ...')subtests witht.Parallelas default" and "MUST uset.Run('Should ...')pattern for ALL test cases." Since line 56 was modified, this is a good opportunity to align with the mandate.♻️ Proposed structure
func TestCreateBridgeHandlerReturnsPersistedPayload(t *testing.T) { - t.Parallel() - - homePaths := newTestHomePaths(t) - // ... + t.Run("Should persist and return the bridge payload with status derived from enabled", func(t *testing.T) { + t.Parallel() + + homePaths := newTestHomePaths(t) + // ... rest of test body ... + }) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/udsapi/bridges_test.go` around lines 13 - 71, The test TestCreateBridgeHandlerReturnsPersistedPayload must be converted into a t.Run subtest to follow the "Should ..." pattern; wrap the existing test body in t.Run("Should create bridge and return persisted payload", func(t *testing.T) { t.Parallel(); ... }) and move the current contents (calls to newTestHomePaths, stubBridgeService/CreateInstanceFn, newTestRouter, performRequest, decodeJSONResponse and assertions) into that subtest, keeping any outer t.Parallel() only if you intend the parent to also run in parallel (prefer calling t.Parallel() inside the new subtest to follow the guideline). Ensure function name TestCreateBridgeHandlerReturnsPersistedPayload remains and only its internals are wrapped by the new t.Run subtest.internal/api/testutil/bridge_stub.go (1)
26-32: ⚡ Quick win
*TaskSubscription*Fnfields drop theBridgeprefix that the actual methods carry.Every other
*Fnfield in this struct is named after its method verbatim (CreateInstanceFn→CreateInstance,ResolveOrCreateRouteFn→ResolveOrCreateRoute, etc.). Four fields break the pattern:
Field Method PutTaskSubscriptionFnPutBridgeTaskSubscriptionGetTaskSubscriptionFnGetBridgeTaskSubscriptionListTaskSubscriptionsFnListBridgeTaskSubscriptionsDeleteTaskSubscriptionFnDeleteBridgeTaskSubscriptionTest authors following the convention will instinctively look for
PutBridgeTaskSubscriptionFnand not find it.♻️ Rename to match method names
- PutTaskSubscriptionFn func(context.Context, bridgepkg.BridgeTaskSubscription) error - GetTaskSubscriptionFn func(context.Context, string) (bridgepkg.BridgeTaskSubscription, error) - ListTaskSubscriptionsFn func(context.Context, bridgepkg.BridgeTaskSubscriptionQuery) ( + PutBridgeTaskSubscriptionFn func(context.Context, bridgepkg.BridgeTaskSubscription) error + GetBridgeTaskSubscriptionFn func(context.Context, string) (bridgepkg.BridgeTaskSubscription, error) + ListBridgeTaskSubscriptionsFn func(context.Context, bridgepkg.BridgeTaskSubscriptionQuery) ( []bridgepkg.BridgeTaskSubscription, error, ) - DeleteTaskSubscriptionFn func(context.Context, string) error + DeleteBridgeTaskSubscriptionFn func(context.Context, string) errorThen update the four method bodies to reference the renamed fields accordingly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/testutil/bridge_stub.go` around lines 26 - 32, Rename the four function fields to include the Bridge prefix so they match the real method names: change PutTaskSubscriptionFn → PutBridgeTaskSubscriptionFn, GetTaskSubscriptionFn → GetBridgeTaskSubscriptionFn, ListTaskSubscriptionsFn → ListBridgeTaskSubscriptionsFn, and DeleteTaskSubscriptionFn → DeleteBridgeTaskSubscriptionFn; then update their corresponding stub methods PutBridgeTaskSubscription, GetBridgeTaskSubscription, ListBridgeTaskSubscriptions, and DeleteBridgeTaskSubscription to call the renamed fields (e.g., replace references to PutTaskSubscriptionFn with PutBridgeTaskSubscriptionFn, etc.) so naming is consistent with other *Fn fields and actual method names.internal/cli/bridge_test.go (1)
152-176: ⚡ Quick winUse a
t.Run("Should ...")subtest for the new status-flag rejection case.Line 152 adds a flat test case; this repo’s test convention prefers
Should ...subtests for individual scenarios.Proposed adjustment
func TestBridgeCreateRejectsOperationalStatusFlag(t *testing.T) { - t.Parallel() - - deps := newTestDeps(t, &stubClient{ - createBridgeFn: func(context.Context, CreateBridgeRequest) (BridgeRecord, error) { - t.Fatal("CreateBridge() should not be called when operational status flag is provided") - return BridgeRecord{}, nil - }, - }) - - _, _, err := executeRootCommand( - t, - deps, - "bridge", "create", - "--scope", "global", - "--platform", "telegram", - "--extension", "ext-telegram", - "--display-name", "Support", - "--enabled=false", - "--status", "ready", - ) - if err == nil || !strings.Contains(err.Error(), "unknown flag: --status") { - t.Fatalf("bridge create error = %v, want unknown status flag", err) - } + t.Run("Should reject --status as an unknown flag", func(t *testing.T) { + t.Parallel() + + deps := newTestDeps(t, &stubClient{ + createBridgeFn: func(context.Context, CreateBridgeRequest) (BridgeRecord, error) { + t.Fatal("CreateBridge() should not be called when operational status flag is provided") + return BridgeRecord{}, nil + }, + }) + + _, _, err := executeRootCommand( + t, + deps, + "bridge", "create", + "--scope", "global", + "--platform", "telegram", + "--extension", "ext-telegram", + "--display-name", "Support", + "--enabled=false", + "--status", "ready", + ) + if err == nil || !strings.Contains(err.Error(), "unknown flag: --status") { + t.Fatalf("bridge create error = %v, want unknown status flag", err) + } + }) }As per coding guidelines,
**/*_test.go: Uset.Run('Should ...')pattern for Go test subtests instead of flat test structures.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/cli/bridge_test.go` around lines 152 - 176, Wrap the existing TestBridgeCreateRejectsOperationalStatusFlag body in a t.Run subtest named e.g. "Should reject operational status flag": convert TestBridgeCreateRejectsOperationalStatusFlag to call t.Run("Should reject operational status flag", func(t *testing.T) { t.Parallel(); /* existing body */ }), ensuring t.Parallel() is invoked inside the subtest, and keep the calls to newTestDeps, stubClient.createBridgeFn, executeRootCommand and the error assertion unchanged so the behavior and assertions remain identical.
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (5)
internal/api/testutil/apitest_test.go (1)
69-109: ⚡ Quick winConvert the two request scenarios to a table-driven subtest.
This block has multiple scenarios (
withBody/withoutBody) in one flow; table-driven subtests would reduce duplication and make future cases easier to add.As per coding guidelines, "Use table-driven test layout for Go tests with multiple scenarios."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/testutil/apitest_test.go` around lines 69 - 109, The test currently duplicates two request flows (withBody/withoutBody) in one t.Run; refactor it into a table-driven test by defining a slice of scenarios (fields: name, method, body, headers, wantBody, wantStatus) and then range over them using t.Run(s.name, func(t *testing.T) { t.Parallel(); ... }), invoking PerformRequestWithHeaders for each case and asserting response Code and Body.String() against s.wantStatus and s.wantBody; keep the same handler (the http.HandlerFunc that writes Content-Type and X-Trace) and preserve all existing assertions but move them inside each subtest so new scenarios can be added without duplicating logic.internal/bridges/json_equal_test.go (1)
5-18: 💤 Low valueConsider expanding test coverage with additional cases.
The test correctly validates numeric equivalence with
t.Parallel()andt.Run("Should...")pattern. However, for more robust coverage, consider adding cases for:
- Unequal numbers returning
false- Mixed types (number vs string) returning
false- Edge cases like very large numbers or scientific notation differences
♻️ Suggested additional test cases
func TestSemanticJSONEqualTreatsEquivalentNumbersAsEqual(t *testing.T) { t.Parallel() t.Run("Should treat numerically equivalent literals as equal", func(t *testing.T) { t.Parallel() if !semanticJSONEqual( []byte(`{"value":1,"nested":[{"ratio":1e1}]}`), []byte(`{"value":1.0,"nested":[{"ratio":10.0}]}`), ) { t.Fatal("semanticJSONEqual() = false, want true for equivalent numeric values") } }) + + t.Run("Should treat different numeric values as unequal", func(t *testing.T) { + t.Parallel() + + if semanticJSONEqual( + []byte(`{"value":1}`), + []byte(`{"value":2}`), + ) { + t.Fatal("semanticJSONEqual() = true, want false for different numeric values") + } + }) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/bridges/json_equal_test.go` around lines 5 - 18, Add more subtests to TestSemanticJSONEqualTreatsEquivalentNumbersAsEqual to cover negative cases and edge cases: add tests that assert semanticJSONEqual returns false for unequal numeric values (e.g., 1 vs 2), for mixed types (e.g., number vs string like 1 vs "1"), and for large/exotic numeric formats (very large integers, differing scientific notation like 1e308 vs 1e307) to ensure semanticJSONEqual handles precision/representation correctly; use t.Run subtests or a table-driven loop within the same test and call semanticJSONEqual for each case with explicit t.Fatal/t.Errorf on mismatch.internal/api/core/perf_bench_test.go (1)
140-200: 💤 Low value
BenchmarkPromptStreamEncoderEmit: encoder construction is insideb.Loop(), folding its allocations into every per-iteration measurement.
NewPromptStreamEncoder(now)is called at line 194 inside theb.Loop()body, sob.ReportAllocs()will attribute the encoder's construction allocations to each "emit" iteration. The benchmark name implies the intent is to measure theEmitpath. One of the explicit advantages oftesting.B.Loopis that it automatically excludes setup and cleanup code from benchmark timing — constructing the encoder beforeb.Loop()would leverage that property and isolate only theEmitpath.♻️ Proposed refactor to move encoder construction outside the measured loop
- writer := &benchmarkFlushWriter{} - now := func() time.Time { - return time.Date(2026, 4, 17, 12, 0, 0, 0, time.UTC) - } + writer := &benchmarkFlushWriter{} + now := func() time.Time { + return time.Date(2026, 4, 17, 12, 0, 0, 0, time.UTC) + } + encoder := NewPromptStreamEncoder(now) for b.Loop() { writer.Reset() - encoder := NewPromptStreamEncoder(now) for _, event := range events { if err := encoder.Emit(writer, event); err != nil { b.Fatalf("PromptStreamEncoder.Emit() error: %v", err) } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/core/perf_bench_test.go` around lines 140 - 200, The benchmark BenchmarkPromptStreamEncoderEmit is constructing the encoder inside b.Loop(), which folds NewPromptStreamEncoder allocations into each measured iteration; move the call to NewPromptStreamEncoder(now) outside the b.Loop() so the encoder is created once before timing starts, then inside the loop only call encoder.Emit(writer, event) (and keep writer.Reset() per iteration); ensure references to NewPromptStreamEncoder, encoder, and encoder.Emit remain consistent after the refactor.internal/bundles/lookup.go (1)
47-52: 💤 Low valueFallback scan is dead code for non-empty keys and creates an inconsistency for empty keys.
Because
newBundleRecordKeynormalises identically at both index-build time and lookup time, any record reachable by the fallback scan would already be present inlookup.exact. The fallback is therefore unreachable for any non-empty(extensionName, bundleName)pair.For the empty-key case there is a subtle inconsistency: records whose
extensionNameorbundleNameis empty are explicitly excluded from the index (lines 24–26), but the fallback scan would still surface them if the caller passes empty strings. If those records are intentionally invalid, the fallback should mirror that exclusion; if they are valid, they should be indexed.♻️ Proposed cleanup
func findBundleResourceRecordIndexed( lookup bundleRecordLookup, extensionName string, bundleName string, ) (resources.Record[BundleResourceSpec], bool) { key := newBundleRecordKey(extensionName, bundleName) idx, ok := lookup.exact[key] if ok { return lookup.records[idx], true } - for _, candidate := range lookup.records { - if strings.EqualFold(strings.TrimSpace(candidate.Spec.ExtensionName), key.extensionName) && - strings.EqualFold(strings.TrimSpace(candidate.Spec.Bundle.Name), key.bundleName) { - return candidate, true - } - } return resources.Record[BundleResourceSpec]{}, false }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/bundles/lookup.go` around lines 47 - 52, The fallback linear scan over lookup.records is dead/unreachable for normalized non-empty keys and inconsistent for empty keys; remove the loop that iterates lookup.records and instead rely solely on lookup.exact for lookups (using the same newBundleRecordKey normalization), or if empty-key lookups must be supported, change the index-building logic (where records are excluded) to include records with empty extensionName/bundleName so index and lookup behavior match; update references in this file to only consult lookup.exact (and newBundleRecordKey) to ensure consistent behavior.internal/automation/model/template_test.go (1)
118-122: ⚡ Quick winUse
errors.Isto assert the sentinel instead of substring matching.
ParseTriggerPromptTemplatereturnserrTriggerPromptTemplateRequiredunwrapped (line 16 oftemplate.go), and the test file is inpackage model, so it can reference the unexported sentinel directly. A substring match on"required"would silently pass if the sentinel message is renamed or if a completely different error happens to contain"required".♻️ Proposed fix
- { - name: "Should reject empty prompts", - prompt: " ", - want: []string{"required"}, - },Replace with a separate subtest that uses
errors.Is:t.Run("Should reject empty prompts", func(t *testing.T) { t.Parallel() _, err := ParseTriggerPromptTemplate(" ") if !errors.Is(err, errTriggerPromptTemplateRequired) { t.Fatalf("ParseTriggerPromptTemplate() error = %v, want errTriggerPromptTemplateRequired", err) } })As per coding guidelines, "Use
errors.Isanderrors.Asonly for error type checking in Go — do not use other error assertion patterns."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/automation/model/template_test.go` around lines 118 - 122, Replace the substring-based assertion for empty prompts with an errors.Is check: call ParseTriggerPromptTemplate(" "), capture the returned error, and assert errors.Is(err, errTriggerPromptTemplateRequired) (referencing ParseTriggerPromptTemplate and the sentinel errTriggerPromptTemplateRequired) so the test fails only when a different error occurs; make this a separate subtest with t.Parallel() consistent with other cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/agentidentity/errors.go`:
- Around line 104-112: The MarshalErrorJSONL function currently returns a JSON
object but does not terminate it with a newline, breaking JSONL consumers;
update MarshalErrorJSONL to append a trailing '\n' to the marshalled bytes
(i.e., call json.Marshal on the struct as now, then ensure the returned []byte
ends with a single newline) so that the output is a valid JSONL frame; reference
MarshalErrorJSONL and ErrorPayloadFor when making the change.
In `@internal/api/testutil/sse.go`:
- Around line 19-20: The test currently logs raw SSE payload by passing
record.Data into t.Fatalf which may leak secrets; change the failure message in
the SSE test helper so it does NOT include the raw payload — e.g., on
json.Unmarshal error for record.Data, log the error plus safe metadata only
(length, first N bytes, or a redacted JSON with sensitive keys removed) instead
of string(record.Data); implement redaction by unmarshaling into a generic map,
removing known sensitive keys like "agh_claim_*", "claim_token",
"mcp_auth_token", OAuth/PKCE keys, and secret bindings, then include the
redacted representation or just non-sensitive diagnostics in the t.Fatalf
message where json.Unmarshal(record.Data, dest) is handled.
In `@internal/automation/manager_refac_test.go`:
- Around line 35-47: The test registers cancelMerged with t.Cleanup but not
cancelRuntime, so if the t.Fatal check after mergedRuntimeContext returns nil
fires, cancelRuntime is never called; add t.Cleanup(cancelRuntime) immediately
after creating cancelRuntime (i.e., after runtimeCtx, cancelRuntime :=
context.WithCancel(...)) so that cancelRuntime is always run on test teardown;
update the subtest that calls
mergedRuntimeContext(nilContextForMergedRuntimeTest(), runtimeCtx) to mirror the
other subtest's cleanup pattern and keep the existing calls to cancelMerged and
waitForContextDone.
In `@internal/automation/model/template_test.go`:
- Around line 8-108: Add two new acceptance subtests for
ValidateTriggerPromptTemplate in template_test.go covering the new guards: one
subtest calling ValidateTriggerPromptTemplate with empty string "" and another
with whitespace " " (both should return nil), and a third subtest calling it
with a plain-text prompt like "plain text" (no template delimiters) which should
also return nil; place these entries in the
TestValidateTriggerPromptTemplateAcceptsSupportedReferences table (or add them
as individual t.Run cases) and follow the existing t.Parallel and error-checking
pattern used in other cases, referencing ValidateTriggerPromptTemplate to locate
where to add them.
In `@internal/automation/trigger_filter.go`:
- Around line 49-61: compileTriggerFilter silently drops invalid keys which can
turn an all-invalid filter into a match-all; update compileTriggerFilter to
fail-closed by detecting unsupported paths from triggerFilterEntryFromPath and
returning an error (change signature to compileTriggerFilter(...)(triggerFilter,
error)) or, if changing signatures is undesirable, return a triggerFilter
flagged as invalid that causes matches() to always return false; ensure you
surface the invalid key names in the returned error or store them on the
triggerFilter so callers can log/handle them instead of getting a silent
match-all.
In `@internal/bridges/resource_projection.go`:
- Around line 310-327: The decodeSemanticJSON function has redundant/unreachable
branches after the second decoder.Decode call; simplify by treating
decoder.Decode(&extra) as: if it returns io.EOF then return value, nil; if it
returns a non-nil error return that error; otherwise (successful decode means
there is an extra value) return the "bridges: JSON payload contains multiple
values" error. Update decodeSemanticJSON to remove the extra json.RawMessage nil
check and return the multiple-values error whenever the second Decode succeeds,
keeping errors.Is(err, io.EOF) to detect the single-value case.
In `@internal/bridgesdk/runtime_refac_test.go`:
- Around line 79-83: The test's mocked Shutdown handler increments shutdownCalls
and returns an error on the first call, but the assertion only checks that some
error occurred; update the mock to return a sentinel error variable (e.g., var
errProviderShutdown = errors.New("provider shutdown failed")) from the Shutdown
closure (referencing Shutdown and shutdownCalls) and change the assertion to use
errors.Is(returnedErr, errProviderShutdown) so the test specifically verifies
that the first shutdown failure is the expected sentinel error.
In `@internal/bridgesdk/runtime.go`:
- Around line 299-303: The code sets r.session and r.initializing=false
unconditionally after r.config.Initialize returns; re-check the request context
(ctx.Err()) before committing those changes so a canceled context doesn't leave
the runtime marked initialized. After r.config.Initialize returns (and before
assigning r.session or setting r.initializing), if ctx.Err() != nil return that
error (or a wrapped context cancellation) without mutating r.session or
r.initializing; ensure the mutex handling around r.mu.Lock()/defer r.mu.Unlock()
still prevents races and that any early return clears r.initializing if it was
set earlier.
---
Nitpick comments:
In `@internal/api/core/perf_bench_test.go`:
- Around line 140-200: The benchmark BenchmarkPromptStreamEncoderEmit is
constructing the encoder inside b.Loop(), which folds NewPromptStreamEncoder
allocations into each measured iteration; move the call to
NewPromptStreamEncoder(now) outside the b.Loop() so the encoder is created once
before timing starts, then inside the loop only call encoder.Emit(writer, event)
(and keep writer.Reset() per iteration); ensure references to
NewPromptStreamEncoder, encoder, and encoder.Emit remain consistent after the
refactor.
In `@internal/api/testutil/apitest_test.go`:
- Around line 69-109: The test currently duplicates two request flows
(withBody/withoutBody) in one t.Run; refactor it into a table-driven test by
defining a slice of scenarios (fields: name, method, body, headers, wantBody,
wantStatus) and then range over them using t.Run(s.name, func(t *testing.T) {
t.Parallel(); ... }), invoking PerformRequestWithHeaders for each case and
asserting response Code and Body.String() against s.wantStatus and s.wantBody;
keep the same handler (the http.HandlerFunc that writes Content-Type and
X-Trace) and preserve all existing assertions but move them inside each subtest
so new scenarios can be added without duplicating logic.
In `@internal/automation/model/template_test.go`:
- Around line 118-122: Replace the substring-based assertion for empty prompts
with an errors.Is check: call ParseTriggerPromptTemplate(" "), capture the
returned error, and assert errors.Is(err, errTriggerPromptTemplateRequired)
(referencing ParseTriggerPromptTemplate and the sentinel
errTriggerPromptTemplateRequired) so the test fails only when a different error
occurs; make this a separate subtest with t.Parallel() consistent with other
cases.
In `@internal/bridges/json_equal_test.go`:
- Around line 5-18: Add more subtests to
TestSemanticJSONEqualTreatsEquivalentNumbersAsEqual to cover negative cases and
edge cases: add tests that assert semanticJSONEqual returns false for unequal
numeric values (e.g., 1 vs 2), for mixed types (e.g., number vs string like 1 vs
"1"), and for large/exotic numeric formats (very large integers, differing
scientific notation like 1e308 vs 1e307) to ensure semanticJSONEqual handles
precision/representation correctly; use t.Run subtests or a table-driven loop
within the same test and call semanticJSONEqual for each case with explicit
t.Fatal/t.Errorf on mismatch.
In `@internal/bundles/lookup.go`:
- Around line 47-52: The fallback linear scan over lookup.records is
dead/unreachable for normalized non-empty keys and inconsistent for empty keys;
remove the loop that iterates lookup.records and instead rely solely on
lookup.exact for lookups (using the same newBundleRecordKey normalization), or
if empty-key lookups must be supported, change the index-building logic (where
records are excluded) to include records with empty extensionName/bundleName so
index and lookup behavior match; update references in this file to only consult
lookup.exact (and newBundleRecordKey) to ensure consistent behavior.
🪄 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: 3165bf5b-c8c3-49d1-b0c2-0a91940e3ad1
⛔ Files ignored due to path filters (43)
.compozy/tasks/refacs/reviews-001/issue_001.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_002.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_003.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_004.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_005.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_006.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_007.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_008.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_009.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_010.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_011.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_012.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_013.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_014.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_015.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_016.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_017.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_018.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_019.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_020.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_021.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_022.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_023.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_024.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_025.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_026.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_027.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_028.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_029.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_030.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_031.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_032.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_033.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_034.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_035.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_036.mdis excluded by!**/*.mdextensions/bridges/teams/extension.tomlis excluded by!**/*.tomlopenapi/agh.jsonis excluded by!**/*.jsonpackages/site/content/runtime/cli-reference/bridge/create.mdxis excluded by!**/*.mdxsdk/typescript/src/generated/contracts.tsis excluded by!**/generated/**web/e2e/fixtures/__tests__/runtime-seed.test.tsis excluded by!**/fixtures/**web/e2e/fixtures/runtime-seed.tsis excluded by!**/fixtures/**web/src/generated/agh-openapi.d.tsis excluded by!**/generated/**
📒 Files selected for processing (219)
internal/acp/acp_bench_test.gointernal/acp/client_integration_test.gointernal/acp/handlers.gointernal/acp/handlers_test.gointernal/acp/process_tree_constants_test.gointernal/acp/process_tree_test.gointernal/acp/terminal.gointernal/agentidentity/credentials.gointernal/agentidentity/errors.gointernal/agentidentity/identity.gointernal/agentidentity/identity_test.gointernal/agentidentity/snapshot.gointernal/api/contract/agents.gointernal/api/contract/authored_context.gointernal/api/contract/bridges.gointernal/api/contract/bridges_test.gointernal/api/contract/contract.gointernal/api/contract/json_safety.gointernal/api/contract/json_safety_bench_test.gointernal/api/core/bridges.gointernal/api/core/bridges_test.gointernal/api/core/coverage_helpers_test.gointernal/api/core/network_details.gointernal/api/core/network_test.gointernal/api/core/perf_bench_test.gointernal/api/core/prompt_stream.gointernal/api/core/sse.gointernal/api/core/tools_test.gointernal/api/httpapi/bridges_integration_test.gointernal/api/httpapi/bridges_test.gointernal/api/httpapi/handlers.gointernal/api/httpapi/handlers_test.gointernal/api/httpapi/middleware.gointernal/api/httpapi/middleware_refac_test.gointernal/api/httpapi/routes.gointernal/api/httpapi/routes_refac_test.gointernal/api/httpapi/server.gointernal/api/httpapi/static.gointernal/api/spec/authored_context.gointernal/api/spec/operations_refac_test.gointernal/api/spec/spec.gointernal/api/spec/spec_test.gointernal/api/testutil/apitest.gointernal/api/testutil/apitest_test.gointernal/api/testutil/automation_stub.gointernal/api/testutil/bridge_stub.gointernal/api/testutil/config.gointernal/api/testutil/home_helpers.gointernal/api/testutil/http_helpers.gointernal/api/testutil/logger.gointernal/api/testutil/network_stub.gointernal/api/testutil/observer_stub.gointernal/api/testutil/resource_stub.gointernal/api/testutil/session_fixtures.gointernal/api/testutil/session_stub.gointernal/api/testutil/skills_stub.gointernal/api/testutil/sse.gointernal/api/testutil/task_stub.gointernal/api/testutil/workspace_stub.gointernal/api/udsapi/bridges_integration_test.gointernal/api/udsapi/bridges_test.gointernal/api/udsapi/extensions.gointernal/api/udsapi/extensions_additional_test.gointernal/api/udsapi/helpers_test.gointernal/api/udsapi/hosted_mcp.gointernal/api/udsapi/hosted_mcp_test.gointernal/api/udsapi/memory.gointernal/api/udsapi/server.gointernal/api/udsapi/server_env_test.gointernal/api/udsapi/server_test.gointernal/api/udsapi/workspaces.gointernal/automation/manager.gointernal/automation/manager_refac_test.gointernal/automation/model/template.gointernal/automation/model/template_bench_test.gointernal/automation/model/template_test.gointernal/automation/model/validate.gointernal/automation/model/validate_test.gointernal/automation/trigger.gointernal/automation/trigger_filter.gointernal/automation/trigger_refac_test.gointernal/bridges/delivery_broker.gointernal/bridges/delivery_projection_refac_test.gointernal/bridges/delivery_types.gointernal/bridges/json_equal_bench_test.gointernal/bridges/json_equal_test.gointernal/bridges/managed_sync.gointernal/bridges/managed_sync_test.gointernal/bridges/registry.gointernal/bridges/registry_refac_test.gointernal/bridges/resource.gointernal/bridges/resource_projection.gointernal/bridges/resource_projection_bench_test.gointernal/bridges/resource_projection_refac_test.gointernal/bridges/resource_test.gointernal/bridges/types.gointernal/bridgesdk/batching.gointernal/bridgesdk/batching_refac_test.gointernal/bridgesdk/cache.gointernal/bridgesdk/errors.gointernal/bridgesdk/errors_refac_test.gointernal/bridgesdk/errors_test.gointernal/bridgesdk/hostapi.gointernal/bridgesdk/hostapi_refac_test.gointernal/bridgesdk/peer.gointernal/bridgesdk/peer_refac_test.gointernal/bridgesdk/runtime.gointernal/bridgesdk/runtime_refac_test.gointernal/bridgesdk/webhook.gointernal/bridgesdk/webhook_refac_test.gointernal/bundles/clone.gointernal/bundles/ids.gointernal/bundles/lookup.gointernal/bundles/model/model.gointernal/bundles/model/model_test.gointernal/bundles/resource_projection.gointernal/bundles/resource_store.gointernal/bundles/service.gointernal/bundles/service_refac_test.gointernal/bundles/service_test.gointernal/cli/bridge.gointernal/cli/bridge_test.gointernal/cli/cli_integration_test.gointernal/cli/client.gointernal/cli/client_test.gointernal/cli/daemon.gointernal/cli/daemon_wait_refac_test.gointernal/cli/daemon_wait_test.gointernal/cli/docpost/docpost.gointernal/cli/docpost/docpost_refac_test.gointernal/cli/docpost/docpost_test.gointernal/cli/format.gointernal/cli/format_test.gointernal/cli/json_flags.gointernal/cli/json_flags_test.gointernal/cli/network.gointernal/cli/session.gointernal/cli/task.gointernal/cli/update_command_test.gointernal/codegen/openapits/generate.gointernal/codegen/openapits/generate_test.gointernal/codegen/sdkts/generate.gointernal/codegen/sdkts/generate_test.gointernal/codegen/sdkts/perf_bench_test.gointernal/config/agent_edit.gointernal/config/automation.gointernal/config/capabilities.gointernal/config/config.gointernal/config/config_refac_test.gointernal/config/dotenv.gointernal/config/dotenv_test.gointernal/config/file_io.gointernal/config/hooks.gointernal/config/mcpjson.gointernal/config/mcpjson_test.gointernal/config/mcpjson_write.gointernal/config/merge.gointernal/config/perf_bench_test.gointernal/config/persistence.gointernal/config/persistence_test.gointernal/config/provider.gointernal/coordinator/coordinator.gointernal/coordinator/coordinator_bench_test.gointernal/coordinator/coordinator_test.gointernal/daemon/agent_skill_resources.gointernal/daemon/agent_skill_resources_refac_test.gointernal/daemon/boot.gointernal/daemon/bridges.gointernal/daemon/bridges_test.gointernal/daemon/daemon.gointernal/daemon/daemon_bridge_extension_integration_test.gointernal/daemon/daemon_integration_test.gointernal/daemon/daemon_network_collaboration_integration_test.gointernal/daemon/daemon_nightly_combined_integration_test.gointernal/daemon/harness_context_integration_test.gointernal/daemon/info.gointernal/daemon/lock.gointernal/daemon/perf_bench_test.gointernal/daemon/skills_watcher_refac_test.gointernal/daemon/tool_mcp_resources.gointernal/daemon/tool_mcp_resources_integration_test.gointernal/daemon/tool_mcp_resources_test.gointernal/diagnostics/redact.gointernal/diagnostics/redact_bench_test.gointernal/diagnostics/redact_test.gointernal/e2elane/command_wiring_test.gointernal/e2elane/lanes_test.gointernal/extension/contract/host_api.gointernal/extension/contract/host_api_test.gointernal/extension/contract/sdk.gointernal/extension/contract/sdk_test.gointernal/extension/host_api.gointernal/extension/host_api_tasks.gointernal/extension/install_managed.gointernal/extension/manager.gointernal/extension/manager_refac_test.gointernal/extension/perf_bench_test.gointernal/extension/registry.gointernal/extension/teams_provider_integration_test.gointernal/extension/telegram_provider_integration_test.gointernal/extension/tool_provider.gointernal/network/envelope_integration_test.gointernal/network/manager.gointernal/network/manager_test.gointernal/network/router_integration_test.gointernal/procutil/detached.gointernal/procutil/detached_unix.gointernal/procutil/detached_windows.gointernal/testutil/e2e/runtime_harness.gointernal/testutil/e2e/runtime_harness_helpers_test.gomagefile.gomagefile_lane_binary_test.gomagefile_test.goweb/e2e/__tests__/network.spec.tsweb/src/routes/_app/__tests__/-bridges.test.tsxweb/src/systems/bridges/adapters/__tests__/bridges-api.test.tsweb/src/systems/bridges/hooks/__tests__/use-bridge-actions.test.tsxweb/src/systems/bridges/lib/__tests__/bridge-drafts.test.tsweb/src/systems/bridges/lib/bridge-drafts.ts
💤 Files with no reviewable changes (8)
- internal/api/udsapi/workspaces.go
- internal/api/contract/contract.go
- internal/bridges/delivery_types.go
- internal/api/udsapi/memory.go
- internal/bridges/managed_sync.go
- internal/api/spec/spec_test.go
- internal/agentidentity/identity.go
- internal/bridges/managed_sync_test.go
✅ Files skipped from review due to trivial changes (32)
- internal/api/udsapi/bridges_integration_test.go
- internal/api/testutil/logger.go
- internal/api/httpapi/bridges_test.go
- internal/api/udsapi/hosted_mcp_test.go
- internal/bridges/json_equal_bench_test.go
- internal/api/spec/authored_context.go
- internal/agentidentity/credentials.go
- internal/api/udsapi/extensions.go
- internal/api/udsapi/server_env_test.go
- internal/api/testutil/session_fixtures.go
- internal/api/testutil/config.go
- internal/bundles/ids.go
- internal/api/contract/authored_context.go
- internal/automation/model/template_bench_test.go
- internal/api/testutil/home_helpers.go
- internal/automation/model/validate_test.go
- internal/agentidentity/snapshot.go
- internal/api/testutil/http_helpers.go
- internal/api/contract/json_safety.go
- internal/bridges/registry_refac_test.go
- internal/bundles/clone.go
- internal/acp/process_tree_constants_test.go
- internal/api/testutil/resource_stub.go
- internal/api/httpapi/routes_refac_test.go
- internal/api/testutil/automation_stub.go
- internal/api/testutil/observer_stub.go
- internal/api/testutil/skills_stub.go
- internal/api/testutil/session_stub.go
- internal/bundles/model/model_test.go
- internal/api/udsapi/extensions_additional_test.go
- internal/api/testutil/network_stub.go
- internal/api/testutil/apitest.go
🚧 Files skipped from review as they are similar to previous changes (19)
- internal/api/httpapi/handlers.go
- internal/bridges/types.go
- internal/bridgesdk/hostapi_refac_test.go
- internal/api/udsapi/helpers_test.go
- internal/automation/manager.go
- internal/bridgesdk/batching_refac_test.go
- internal/acp/acp_bench_test.go
- internal/api/httpapi/middleware_refac_test.go
- internal/api/udsapi/hosted_mcp.go
- internal/bridgesdk/peer.go
- internal/api/core/tools_test.go
- internal/api/testutil/workspace_stub.go
- internal/agentidentity/identity_test.go
- internal/api/core/coverage_helpers_test.go
- internal/bundles/resource_store.go
- internal/api/core/prompt_stream.go
- internal/automation/trigger.go
- internal/api/testutil/task_stub.go
- internal/api/testutil/bridge_stub.go
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/agentidentity/errors.go`:
- Around line 37-43: Add a compile-time interface assertion to lock the contract
that *Error implements the error interface: after the Error type declaration
(the struct named Error that has Code, Message, Action, Err) add a var
declaration asserting the implementation (e.g. var _ error = (*Error)(nil)) so
the compiler fails if Error no longer satisfies the Error() method.
In `@internal/api/testutil/apitest_test.go`:
- Around line 183-194: The test reveals an unexported sentinel error
errStubNetworkServiceWaitInboxNotImplemented which prevents external packages
from using errors.Is like they can with ErrStubWorkspaceServiceNotImplemented;
to fix, export the sentinel in network_stub.go by renaming
errStubNetworkServiceWaitInboxNotImplemented to
ErrStubNetworkServiceWaitInboxNotImplemented (update any references in
StubNetworkService, WaitInbox and tests) so external tests can do typed
errors.Is checks; ensure the exported name is used anywhere WaitInboxFn or
StubNetworkService refer to the sentinel.
In `@internal/automation/manager_refac_test.go`:
- Around line 55-56: The test currently passes cancelMerged directly to
t.Cleanup which will panic if cancelMerged is nil; update the call site around
mergedRuntimeContext so you protect against nil by wrapping cancelMerged in a
closure that checks for nil before calling (e.g., after calling
mergedRuntimeContext(nilParent, nilRuntime) use t.Cleanup(func() { if
cancelMerged != nil { cancelMerged() } }) ), or alternatively assert
cancelMerged is non-nil with a test helper (e.g., require.NotNil) before
registering cleanup; reference mergedRuntimeContext and cancelMerged to locate
the change.
In `@internal/bridgesdk/runtime_refac_test.go`:
- Around line 54-60: The fixed 250ms timeout in the select that waits on the
done channel can cause flaky failures under t.Parallel(); change the timeout to
derive its budget from the test deadline or use a forgiving fallback. In the
test around the select that reads from done and calls handleInitialize(),
compute a timeout by checking t.Deadline() (use time.Until(t.Deadline()) and cap
it to a reasonable minimum/maximum) or create a larger default timeout if no
deadline is set, then replace the fixed 250ms case <-time.After(...) with a
time.After using that derived duration (or alternatively implement a handshake
via an additional channel/ctx with timeout) so the test only fails on real
stalls.
- Around line 67-112: The test only covers idle→failure→retry→success; add a new
subtest in runtime_refac_test.go that exercises the runtimeShutdownRunning and
post-success idempotent paths by using a blocking Shutdown callback (e.g., have
Shutdown in RuntimeConfig block on a channel while incrementing shutdownCalls),
call runtime.handleShutdown concurrently from a goroutine and assert the second
concurrent call immediately returns the "shutdown running" RPC error (match
error text or type the code emits for runtimeShutdownRunning), then release the
blocker to let Shutdown succeed and make a third handleShutdown call and assert
shutdownCalls did not increment further and the third call returns success;
reference the RuntimeConfig Shutdown callback, runtime.handleShutdown, and
shutdownCalls to locate and implement these assertions.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 454de824-0374-4958-ba5e-a3f27e64db53
⛔ Files ignored due to path filters (13)
.compozy/tasks/refacs/reviews-002/issue_001.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_002.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_003.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_004.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_005.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_006.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_007.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_008.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_009.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_010.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_011.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_012.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_013.mdis excluded by!**/*.md
📒 Files selected for processing (14)
internal/agentidentity/errors.gointernal/agentidentity/identity_test.gointernal/api/core/perf_bench_test.gointernal/api/testutil/apitest_test.gointernal/api/testutil/sse.gointernal/automation/manager_refac_test.gointernal/automation/model/template_test.gointernal/bridges/json_equal_test.gointernal/bridges/resource_projection.gointernal/bridgesdk/runtime.gointernal/bridgesdk/runtime_refac_test.gointernal/bundles/lookup.gointernal/bundles/service_test.gopackages/site/lib/__tests__/runtime-hub-quality.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (2)
internal/api/udsapi/server_test.go (1)
350-354: ⚡ Quick winUse a bounded context in the cleanup shutdown.
context.Background()can hang this test indefinitely if shutdown regresses. The other shutdown paths in this file already usecontext.WithTimeout, so this cleanup should too for the same reason.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/udsapi/server_test.go` around lines 350 - 354, The cleanup uses context.Background() which can hang; change the t.Cleanup shutdown to create a bounded context (use context.WithTimeout) and pass its ctx to server.Shutdown, defer cancel() inside the cleanup closure, and handle the Shutdown error as before; locate the t.Cleanup block that calls server.Shutdown and replace the unbounded context with a timed context (e.g., 5s) while keeping the existing error check for server.Shutdown.internal/api/contract/bridges_test.go (1)
66-101: ⚡ Quick winAdd a disabled-create case for the new status derivation branch.
This assertion only covers
Enabled=true, so a regression in the newEnabled=false -> BridgeStatusDisabledbranch would currently slip through. A companion case here would keep both outcomes ofbridgeCreateInitialStatuspinned down.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/contract/bridges_test.go` around lines 66 - 101, Add a companion test case to TestCreateBridgeRequestPreservesNormalizedFieldsAndDefaults that verifies the Disabled branch of bridgeCreateInitialStatus: construct a contract.CreateBridgeRequest identical to the existing one but with Enabled: false, call req.ToCreateInstanceRequest(), assert no error, and check mapped.Status == bridgepkg.BridgeStatusDisabled (and preserve any normalized fields as needed); use the same helper symbols (CreateBridgeRequest, ToCreateInstanceRequest, bridgeCreateInitialStatus, BridgeStatusDisabled) so the new case pins down the Enabled=false outcome.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/api/contract/bridges_test.go`:
- Around line 66-101: Add a companion test case to
TestCreateBridgeRequestPreservesNormalizedFieldsAndDefaults that verifies the
Disabled branch of bridgeCreateInitialStatus: construct a
contract.CreateBridgeRequest identical to the existing one but with Enabled:
false, call req.ToCreateInstanceRequest(), assert no error, and check
mapped.Status == bridgepkg.BridgeStatusDisabled (and preserve any normalized
fields as needed); use the same helper symbols (CreateBridgeRequest,
ToCreateInstanceRequest, bridgeCreateInitialStatus, BridgeStatusDisabled) so the
new case pins down the Enabled=false outcome.
In `@internal/api/udsapi/server_test.go`:
- Around line 350-354: The cleanup uses context.Background() which can hang;
change the t.Cleanup shutdown to create a bounded context (use
context.WithTimeout) and pass its ctx to server.Shutdown, defer cancel() inside
the cleanup closure, and handle the Shutdown error as before; locate the
t.Cleanup block that calls server.Shutdown and replace the unbounded context
with a timed context (e.g., 5s) while keeping the existing error check for
server.Shutdown.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d2a592a5-00f7-4353-9a6c-fe1141df79d8
⛔ Files ignored due to path filters (50)
.compozy/tasks/refacs/reviews-001/issue_001.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_002.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_003.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_004.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_005.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_006.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_007.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_008.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_009.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_010.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_011.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_012.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_013.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_014.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_015.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_016.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_017.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_018.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_019.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_020.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_021.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_022.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_023.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_024.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_025.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_026.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_027.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_028.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_029.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_030.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_031.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_032.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_033.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_034.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_035.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_036.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_001.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_002.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_003.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_004.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_005.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_006.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_007.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_008.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_009.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_010.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_011.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_012.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_013.mdis excluded by!**/*.mdextensions/bridges/teams/extension.tomlis excluded by!**/*.toml
📒 Files selected for processing (141)
internal/acp/acp_bench_test.gointernal/acp/client_integration_test.gointernal/acp/handlers.gointernal/acp/handlers_test.gointernal/acp/process_tree_constants_test.gointernal/acp/process_tree_test.gointernal/acp/terminal.gointernal/agentidentity/credentials.gointernal/agentidentity/errors.gointernal/agentidentity/identity.gointernal/agentidentity/identity_test.gointernal/agentidentity/snapshot.gointernal/api/contract/agents.gointernal/api/contract/authored_context.gointernal/api/contract/bridges.gointernal/api/contract/bridges_test.gointernal/api/contract/contract.gointernal/api/contract/json_safety.gointernal/api/contract/json_safety_bench_test.gointernal/api/core/bridges.gointernal/api/core/bridges_test.gointernal/api/core/coverage_helpers_test.gointernal/api/core/network_details.gointernal/api/core/network_test.gointernal/api/core/perf_bench_test.gointernal/api/core/prompt_stream.gointernal/api/core/sse.gointernal/api/core/tools_test.gointernal/api/httpapi/bridges_integration_test.gointernal/api/httpapi/bridges_test.gointernal/api/httpapi/handlers.gointernal/api/httpapi/handlers_test.gointernal/api/httpapi/middleware.gointernal/api/httpapi/middleware_refac_test.gointernal/api/httpapi/routes.gointernal/api/httpapi/routes_refac_test.gointernal/api/httpapi/server.gointernal/api/httpapi/static.gointernal/api/spec/authored_context.gointernal/api/spec/operations_refac_test.gointernal/api/spec/spec.gointernal/api/spec/spec_test.gointernal/api/testutil/apitest.gointernal/api/testutil/apitest_test.gointernal/api/testutil/automation_stub.gointernal/api/testutil/bridge_stub.gointernal/api/testutil/config.gointernal/api/testutil/home_helpers.gointernal/api/testutil/http_helpers.gointernal/api/testutil/logger.gointernal/api/testutil/network_stub.gointernal/api/testutil/observer_stub.gointernal/api/testutil/resource_stub.gointernal/api/testutil/session_fixtures.gointernal/api/testutil/session_stub.gointernal/api/testutil/skills_stub.gointernal/api/testutil/sse.gointernal/api/testutil/task_stub.gointernal/api/testutil/workspace_stub.gointernal/api/udsapi/bridges_integration_test.gointernal/api/udsapi/bridges_test.gointernal/api/udsapi/extensions.gointernal/api/udsapi/extensions_additional_test.gointernal/api/udsapi/helpers_test.gointernal/api/udsapi/hosted_mcp.gointernal/api/udsapi/hosted_mcp_test.gointernal/api/udsapi/memory.gointernal/api/udsapi/server.gointernal/api/udsapi/server_env_test.gointernal/api/udsapi/server_test.gointernal/api/udsapi/workspaces.gointernal/automation/manager.gointernal/automation/manager_refac_test.gointernal/automation/model/template.gointernal/automation/model/template_bench_test.gointernal/automation/model/template_test.gointernal/automation/model/validate.gointernal/automation/model/validate_test.gointernal/automation/trigger.gointernal/automation/trigger_filter.gointernal/automation/trigger_refac_test.gointernal/bridges/delivery_broker.gointernal/bridges/delivery_projection_refac_test.gointernal/bridges/delivery_types.gointernal/bridges/json_equal_bench_test.gointernal/bridges/json_equal_test.gointernal/bridges/managed_sync.gointernal/bridges/managed_sync_test.gointernal/bridges/registry.gointernal/bridges/registry_refac_test.gointernal/bridges/resource.gointernal/bridges/resource_projection.gointernal/bridges/resource_projection_bench_test.gointernal/bridges/resource_projection_refac_test.gointernal/bridges/resource_test.gointernal/bridges/types.gointernal/bridgesdk/batching.gointernal/bridgesdk/batching_refac_test.gointernal/bridgesdk/cache.gointernal/bridgesdk/errors.gointernal/bridgesdk/errors_refac_test.gointernal/bridgesdk/errors_test.gointernal/bridgesdk/hostapi.gointernal/bridgesdk/hostapi_refac_test.gointernal/bridgesdk/peer.gointernal/bridgesdk/peer_refac_test.gointernal/bridgesdk/runtime.gointernal/bridgesdk/runtime_refac_test.gointernal/bridgesdk/webhook.gointernal/bridgesdk/webhook_refac_test.gointernal/bundles/clone.gointernal/bundles/ids.gointernal/bundles/lookup.gointernal/bundles/model/model.gointernal/bundles/model/model_test.gointernal/bundles/resource_projection.gointernal/bundles/resource_store.gointernal/bundles/service.gointernal/bundles/service_refac_test.gointernal/bundles/service_test.gointernal/cli/bridge.gointernal/cli/bridge_test.gointernal/cli/cli_integration_test.gointernal/cli/client.gointernal/cli/client_test.gointernal/cli/daemon.gointernal/cli/daemon_wait_refac_test.gointernal/cli/daemon_wait_test.gointernal/cli/docpost/docpost.gointernal/cli/docpost/docpost_refac_test.gointernal/cli/docpost/docpost_test.gointernal/cli/format.gointernal/cli/format_test.gointernal/cli/json_flags.gointernal/cli/json_flags_test.gointernal/cli/network.gointernal/cli/session.gointernal/cli/task.gointernal/cli/update_command_test.gointernal/codegen/openapits/generate.gointernal/codegen/openapits/generate_test.go
💤 Files with no reviewable changes (8)
- internal/api/udsapi/memory.go
- internal/api/udsapi/workspaces.go
- internal/api/contract/contract.go
- internal/bridges/delivery_types.go
- internal/bridges/managed_sync.go
- internal/api/spec/spec_test.go
- internal/bridges/managed_sync_test.go
- internal/agentidentity/identity.go
✅ Files skipped from review due to trivial changes (28)
- internal/api/udsapi/bridges_integration_test.go
- internal/api/testutil/logger.go
- internal/acp/process_tree_constants_test.go
- internal/agentidentity/credentials.go
- internal/automation/model/template_bench_test.go
- internal/bridges/json_equal_bench_test.go
- internal/api/httpapi/bridges_test.go
- internal/bridges/resource_projection_refac_test.go
- internal/bridges/json_equal_test.go
- internal/api/testutil/session_fixtures.go
- internal/api/udsapi/server_env_test.go
- internal/bridgesdk/hostapi_refac_test.go
- internal/api/testutil/apitest.go
- internal/api/testutil/http_helpers.go
- internal/api/testutil/home_helpers.go
- internal/agentidentity/snapshot.go
- internal/api/contract/json_safety_bench_test.go
- internal/api/testutil/resource_stub.go
- internal/api/testutil/config.go
- internal/api/httpapi/routes_refac_test.go
- internal/api/contract/json_safety.go
- internal/api/testutil/sse.go
- internal/api/testutil/workspace_stub.go
- internal/agentidentity/errors.go
- internal/automation/trigger_filter.go
- internal/bridges/registry_refac_test.go
- internal/api/testutil/bridge_stub.go
- internal/api/testutil/skills_stub.go
🚧 Files skipped from review as they are similar to previous changes (26)
- internal/bridges/resource.go
- internal/api/udsapi/helpers_test.go
- internal/bridges/types.go
- internal/api/udsapi/hosted_mcp.go
- internal/api/httpapi/bridges_integration_test.go
- internal/bridgesdk/batching_refac_test.go
- internal/bridges/registry.go
- internal/api/core/tools_test.go
- internal/api/core/bridges.go
- internal/automation/model/template.go
- internal/api/core/coverage_helpers_test.go
- internal/api/httpapi/routes.go
- internal/api/core/prompt_stream.go
- internal/api/udsapi/extensions_additional_test.go
- internal/bridgesdk/hostapi.go
- internal/api/testutil/automation_stub.go
- internal/bridges/resource_projection.go
- internal/bridges/resource_test.go
- internal/acp/handlers.go
- internal/bridgesdk/batching.go
- internal/automation/manager_refac_test.go
- internal/acp/client_integration_test.go
- internal/api/testutil/network_stub.go
- internal/api/testutil/task_stub.go
- internal/api/testutil/apitest_test.go
- internal/acp/terminal.go
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
internal/api/core/bridges_test.go (2)
536-536: ⚡ Quick winRename the new parent subtest to the
Should ...pattern.Line 536 currently uses
t.Run("create rejects client-owned operational state fields", ...). Please rename tot.Run("Should reject client-owned operational state fields", ...)for consistency with repo test conventions.As per coding guidelines, "
MUST use t.Run(\"Should...\") pattern for ALL test cases."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/core/bridges_test.go` at line 536, The subtest name string passed to t.Run currently reads "create rejects client-owned operational state fields"; update it to follow the repo convention by changing that call to t.Run("Should reject client-owned operational state fields", ...) so the test uses the required "Should ..." pattern (locate the t.Run invocation in the test function that defines the subtest for rejecting client-owned operational state fields).
130-130: ⚡ Quick winAssert derived status explicitly after removing
statusfrom create payload.Line 130 removes client-owned
status, but this test path still doesn’t assert that the handler setsreq.Statustobridgepkg.BridgeStatusStartingbefore callingCreateInstanceFn. Adding that assertion will lock the contract and prevent silent regressions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/core/bridges_test.go` at line 130, The test removed client-owned status from the create payload but doesn’t assert that the handler derives and sets req.Status to bridgepkg.BridgeStatusStarting before invoking CreateInstanceFn; update the test for the handler path that exercises CreateInstanceFn to explicitly assert req.Status == bridgepkg.BridgeStatusStarting (or equivalent enum) was set on the request passed into CreateInstanceFn (e.g., by inspecting the captured request argument in the CreateInstanceFn mock/spying closure) so the contract is locked and regressions are prevented.internal/bridges/registry_refac_test.go (1)
46-48: ⚡ Quick winConsider rebinding loop variables for clarity in parallel table subtests.
Loop variables in range loops are automatically scoped per iteration on Go 1.22+, so
tc := tcrebinding is not required for correctness on Go 1.25.5. However, the rebinding pattern remains a common clarity convention for parallel subtests and improves portability to earlier Go versions if maintained as a coding standard.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/bridges/registry_refac_test.go` around lines 46 - 48, The table-driven test launches parallel subtests using t.Run and t.Parallel with the loop variable tc; add an explicit rebinding (tc := tc) immediately inside the loop before calling t.Run to make the variable capture explicit and improve readability/portability (update the loop around the cases iteration where t.Run(tc.name, func(t *testing.T) { ... }) is invoked).internal/api/httpapi/routes_refac_test.go (1)
99-106: ⚡ Quick winAdd a compile-time interface assertion for the resolver adapter.
httpapiCoordinatorConfigResolverFuncimplementscore.CoordinatorConfigResolverbut lacks a compile-time assertion to lock that contract. Add:var _ core.CoordinatorConfigResolver = httpapiCoordinatorConfigResolverFunc(nil)This requires importing
"github.com/pedronauck/agh/internal/api/core".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/httpapi/routes_refac_test.go` around lines 99 - 106, Add a compile-time interface assertion that httpapiCoordinatorConfigResolverFunc implements core.CoordinatorConfigResolver by adding a var declaration like: var _ core.CoordinatorConfigResolver = httpapiCoordinatorConfigResolverFunc(nil); also add the import for "github.com/pedronauck/agh/internal/api/core" so the core package is available. This should be placed alongside the httpapiCoordinatorConfigResolverFunc type and its ResolveCoordinatorConfig method to lock the contract at compile time.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/acp/terminal.go`:
- Around line 726-737: The function isAllowedNetworkTerminalArgv currently only
allows "send", "peers", "channels", "status", and "inbox"; update this allowlist
in isAllowedNetworkTerminalArgv (which checks argv[0] == "agh" and argv[1] ==
networkCommandName) to also permit the documented network subcommands "threads",
"directs", and "work" so that all eight registered network commands are accepted
during network turns.
In `@internal/agentidentity/identity_test.go`:
- Around line 241-244: The "Should reject nil context" test case is ambiguous
because both ctx and lookup are nil; set a non-nil stub lookup so the test only
exercises the nil-context path: in the test case named "Should reject nil
context" provide a concrete lookup implementation (e.g., a minimal stub or
LookupFunc that returns ErrIdentityLookupUnavailable or a controlled response)
instead of nil so that the error originates from passing a nil context to the
function under test and the assertion against ErrIdentityLookupUnavailable is
deterministic.
In `@internal/api/testutil/apitest_test.go`:
- Around line 69-77: The handler closure that calls t.Fatalf is defined once
outside the table-driven loop, so failures inside it get reported against the
outer test; move the http.HandlerFunc definition into each table-driven subtest
(inside the t.Run for each tt) so it captures the correct *testing.T for that
iteration, i.e. define the handler inside the per-case t.Run (just before
calling PerformRequestWithHeaders and assertions) so any w.Write errors call
t.Fatalf on the subtest's t and failures are attributed to the correct test
case.
In `@internal/bridges/resource_test.go`:
- Around line 408-410: The test currently calls err.Error() without checking err
in the nil-plan subtests; update those assertions in
internal/bridges/resource_test.go to guard err first (e.g., if err == nil {
t.Fatalf("ApplyResourceState(typed nil) returned nil, want error") }) and only
then compare err.Error() to the expected string; do the same change for the
other subtest block that asserts the error (the second occurrence around lines
426-428) to avoid panics when ApplyResourceState returns nil.
---
Nitpick comments:
In `@internal/api/core/bridges_test.go`:
- Line 536: The subtest name string passed to t.Run currently reads "create
rejects client-owned operational state fields"; update it to follow the repo
convention by changing that call to t.Run("Should reject client-owned
operational state fields", ...) so the test uses the required "Should ..."
pattern (locate the t.Run invocation in the test function that defines the
subtest for rejecting client-owned operational state fields).
- Line 130: The test removed client-owned status from the create payload but
doesn’t assert that the handler derives and sets req.Status to
bridgepkg.BridgeStatusStarting before invoking CreateInstanceFn; update the test
for the handler path that exercises CreateInstanceFn to explicitly assert
req.Status == bridgepkg.BridgeStatusStarting (or equivalent enum) was set on the
request passed into CreateInstanceFn (e.g., by inspecting the captured request
argument in the CreateInstanceFn mock/spying closure) so the contract is locked
and regressions are prevented.
In `@internal/api/httpapi/routes_refac_test.go`:
- Around line 99-106: Add a compile-time interface assertion that
httpapiCoordinatorConfigResolverFunc implements core.CoordinatorConfigResolver
by adding a var declaration like: var _ core.CoordinatorConfigResolver =
httpapiCoordinatorConfigResolverFunc(nil); also add the import for
"github.com/pedronauck/agh/internal/api/core" so the core package is available.
This should be placed alongside the httpapiCoordinatorConfigResolverFunc type
and its ResolveCoordinatorConfig method to lock the contract at compile time.
In `@internal/bridges/registry_refac_test.go`:
- Around line 46-48: The table-driven test launches parallel subtests using
t.Run and t.Parallel with the loop variable tc; add an explicit rebinding (tc :=
tc) immediately inside the loop before calling t.Run to make the variable
capture explicit and improve readability/portability (update the loop around the
cases iteration where t.Run(tc.name, func(t *testing.T) { ... }) is invoked).
🪄 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: 02252431-1050-4221-9a2a-e9604289d787
⛔ Files ignored due to path filters (56)
.compozy/tasks/refacs/reviews-001/issue_001.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_002.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_003.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_004.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_005.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_006.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_007.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_008.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_009.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_010.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_011.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_012.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_013.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_014.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_015.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_016.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_017.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_018.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_019.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_020.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_021.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_022.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_023.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_024.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_025.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_026.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_027.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_028.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_029.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_030.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_031.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_032.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_033.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_034.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_035.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-001/issue_036.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_001.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_002.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_003.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_004.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_005.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_006.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_007.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_008.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_009.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_010.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_011.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_012.mdis excluded by!**/*.md.compozy/tasks/refacs/reviews-002/issue_013.mdis excluded by!**/*.mdextensions/bridges/teams/extension.tomlis excluded by!**/*.tomlopenapi/agh.jsonis excluded by!**/*.jsonpackages/site/content/runtime/cli-reference/bridge/create.mdxis excluded by!**/*.mdxsdk/typescript/src/generated/contracts.tsis excluded by!**/generated/**web/e2e/fixtures/__tests__/runtime-seed.test.tsis excluded by!**/fixtures/**web/e2e/fixtures/runtime-seed.tsis excluded by!**/fixtures/**web/src/generated/agh-openapi.d.tsis excluded by!**/generated/**
📒 Files selected for processing (219)
internal/acp/acp_bench_test.gointernal/acp/client_integration_test.gointernal/acp/handlers.gointernal/acp/handlers_test.gointernal/acp/process_tree_constants_test.gointernal/acp/process_tree_test.gointernal/acp/terminal.gointernal/agentidentity/credentials.gointernal/agentidentity/errors.gointernal/agentidentity/identity.gointernal/agentidentity/identity_test.gointernal/agentidentity/snapshot.gointernal/api/contract/agents.gointernal/api/contract/authored_context.gointernal/api/contract/bridges.gointernal/api/contract/bridges_test.gointernal/api/contract/contract.gointernal/api/contract/json_safety.gointernal/api/contract/json_safety_bench_test.gointernal/api/core/bridges.gointernal/api/core/bridges_test.gointernal/api/core/coverage_helpers_test.gointernal/api/core/network_details.gointernal/api/core/network_test.gointernal/api/core/perf_bench_test.gointernal/api/core/prompt_stream.gointernal/api/core/sse.gointernal/api/core/tools_test.gointernal/api/httpapi/bridges_integration_test.gointernal/api/httpapi/bridges_test.gointernal/api/httpapi/handlers.gointernal/api/httpapi/handlers_test.gointernal/api/httpapi/middleware.gointernal/api/httpapi/middleware_refac_test.gointernal/api/httpapi/routes.gointernal/api/httpapi/routes_refac_test.gointernal/api/httpapi/server.gointernal/api/httpapi/static.gointernal/api/spec/authored_context.gointernal/api/spec/operations_refac_test.gointernal/api/spec/spec.gointernal/api/spec/spec_test.gointernal/api/testutil/apitest.gointernal/api/testutil/apitest_test.gointernal/api/testutil/automation_stub.gointernal/api/testutil/bridge_stub.gointernal/api/testutil/config.gointernal/api/testutil/home_helpers.gointernal/api/testutil/http_helpers.gointernal/api/testutil/logger.gointernal/api/testutil/network_stub.gointernal/api/testutil/observer_stub.gointernal/api/testutil/resource_stub.gointernal/api/testutil/session_fixtures.gointernal/api/testutil/session_stub.gointernal/api/testutil/skills_stub.gointernal/api/testutil/sse.gointernal/api/testutil/task_stub.gointernal/api/testutil/workspace_stub.gointernal/api/udsapi/bridges_integration_test.gointernal/api/udsapi/bridges_test.gointernal/api/udsapi/extensions.gointernal/api/udsapi/extensions_additional_test.gointernal/api/udsapi/helpers_test.gointernal/api/udsapi/hosted_mcp.gointernal/api/udsapi/hosted_mcp_test.gointernal/api/udsapi/memory.gointernal/api/udsapi/server.gointernal/api/udsapi/server_env_test.gointernal/api/udsapi/server_test.gointernal/api/udsapi/workspaces.gointernal/automation/manager.gointernal/automation/manager_refac_test.gointernal/automation/model/template.gointernal/automation/model/template_bench_test.gointernal/automation/model/template_test.gointernal/automation/model/validate.gointernal/automation/model/validate_test.gointernal/automation/trigger.gointernal/automation/trigger_filter.gointernal/automation/trigger_refac_test.gointernal/bridges/delivery_broker.gointernal/bridges/delivery_projection_refac_test.gointernal/bridges/delivery_types.gointernal/bridges/json_equal_bench_test.gointernal/bridges/json_equal_test.gointernal/bridges/managed_sync.gointernal/bridges/managed_sync_test.gointernal/bridges/registry.gointernal/bridges/registry_refac_test.gointernal/bridges/resource.gointernal/bridges/resource_projection.gointernal/bridges/resource_projection_bench_test.gointernal/bridges/resource_projection_refac_test.gointernal/bridges/resource_test.gointernal/bridges/types.gointernal/bridgesdk/batching.gointernal/bridgesdk/batching_refac_test.gointernal/bridgesdk/cache.gointernal/bridgesdk/errors.gointernal/bridgesdk/errors_refac_test.gointernal/bridgesdk/errors_test.gointernal/bridgesdk/hostapi.gointernal/bridgesdk/hostapi_refac_test.gointernal/bridgesdk/peer.gointernal/bridgesdk/peer_refac_test.gointernal/bridgesdk/runtime.gointernal/bridgesdk/runtime_refac_test.gointernal/bridgesdk/webhook.gointernal/bridgesdk/webhook_refac_test.gointernal/bundles/clone.gointernal/bundles/ids.gointernal/bundles/lookup.gointernal/bundles/model/model.gointernal/bundles/model/model_test.gointernal/bundles/resource_projection.gointernal/bundles/resource_store.gointernal/bundles/service.gointernal/bundles/service_refac_test.gointernal/bundles/service_test.gointernal/cli/bridge.gointernal/cli/bridge_test.gointernal/cli/cli_integration_test.gointernal/cli/client.gointernal/cli/client_test.gointernal/cli/daemon.gointernal/cli/daemon_wait_refac_test.gointernal/cli/daemon_wait_test.gointernal/cli/docpost/docpost.gointernal/cli/docpost/docpost_refac_test.gointernal/cli/docpost/docpost_test.gointernal/cli/format.gointernal/cli/format_test.gointernal/cli/json_flags.gointernal/cli/json_flags_test.gointernal/cli/network.gointernal/cli/session.gointernal/cli/task.gointernal/cli/update_command_test.gointernal/codegen/openapits/generate.gointernal/codegen/openapits/generate_test.gointernal/codegen/sdkts/generate.gointernal/codegen/sdkts/generate_test.gointernal/codegen/sdkts/perf_bench_test.gointernal/config/agent_edit.gointernal/config/automation.gointernal/config/capabilities.gointernal/config/config.gointernal/config/config_refac_test.gointernal/config/dotenv.gointernal/config/dotenv_test.gointernal/config/file_io.gointernal/config/hooks.gointernal/config/mcpjson.gointernal/config/mcpjson_test.gointernal/config/mcpjson_write.gointernal/config/merge.gointernal/config/perf_bench_test.gointernal/config/persistence.gointernal/config/persistence_test.gointernal/config/provider.gointernal/coordinator/coordinator.gointernal/coordinator/coordinator_bench_test.gointernal/coordinator/coordinator_test.gointernal/daemon/agent_skill_resources.gointernal/daemon/agent_skill_resources_refac_test.gointernal/daemon/boot.gointernal/daemon/bridges.gointernal/daemon/bridges_test.gointernal/daemon/daemon.gointernal/daemon/daemon_bridge_extension_integration_test.gointernal/daemon/daemon_integration_test.gointernal/daemon/daemon_network_collaboration_integration_test.gointernal/daemon/daemon_nightly_combined_integration_test.gointernal/daemon/harness_context_integration_test.gointernal/daemon/info.gointernal/daemon/lock.gointernal/daemon/perf_bench_test.gointernal/daemon/skills_watcher_refac_test.gointernal/daemon/tool_mcp_resources.gointernal/daemon/tool_mcp_resources_integration_test.gointernal/daemon/tool_mcp_resources_test.gointernal/diagnostics/redact.gointernal/diagnostics/redact_bench_test.gointernal/diagnostics/redact_test.gointernal/e2elane/command_wiring_test.gointernal/e2elane/lanes_test.gointernal/extension/contract/host_api.gointernal/extension/contract/host_api_test.gointernal/extension/contract/sdk.gointernal/extension/contract/sdk_test.gointernal/extension/host_api.gointernal/extension/host_api_tasks.gointernal/extension/install_managed.gointernal/extension/manager.gointernal/extension/manager_refac_test.gointernal/extension/perf_bench_test.gointernal/extension/registry.gointernal/extension/teams_provider_integration_test.gointernal/extension/telegram_provider_integration_test.gointernal/extension/tool_provider.gointernal/network/envelope_integration_test.gointernal/network/manager.gointernal/network/manager_test.gointernal/network/router_integration_test.gointernal/procutil/detached.gointernal/procutil/detached_unix.gointernal/procutil/detached_windows.gointernal/testutil/e2e/runtime_harness.gointernal/testutil/e2e/runtime_harness_helpers_test.gomagefile.gomagefile_lane_binary_test.gomagefile_test.goweb/e2e/__tests__/network.spec.tsweb/src/routes/_app/__tests__/-bridges.test.tsxweb/src/systems/bridges/adapters/__tests__/bridges-api.test.tsweb/src/systems/bridges/hooks/__tests__/use-bridge-actions.test.tsxweb/src/systems/bridges/lib/__tests__/bridge-drafts.test.tsweb/src/systems/bridges/lib/bridge-drafts.ts
💤 Files with no reviewable changes (8)
- internal/api/udsapi/workspaces.go
- internal/api/contract/contract.go
- internal/api/udsapi/memory.go
- internal/bridges/delivery_types.go
- internal/bridges/managed_sync.go
- internal/api/spec/spec_test.go
- internal/bridges/managed_sync_test.go
- internal/agentidentity/identity.go
✅ Files skipped from review due to trivial changes (33)
- internal/api/testutil/apitest.go
- internal/acp/process_tree_constants_test.go
- internal/bridges/json_equal_bench_test.go
- internal/api/httpapi/bridges_test.go
- internal/api/udsapi/hosted_mcp_test.go
- internal/api/testutil/logger.go
- internal/bridges/resource_projection_refac_test.go
- internal/api/testutil/config.go
- internal/api/udsapi/server_env_test.go
- internal/bridges/json_equal_test.go
- internal/api/contract/json_safety.go
- internal/api/testutil/session_fixtures.go
- internal/api/testutil/home_helpers.go
- internal/agentidentity/credentials.go
- internal/api/testutil/http_helpers.go
- internal/automation/trigger_refac_test.go
- internal/automation/manager_refac_test.go
- internal/api/contract/json_safety_bench_test.go
- internal/api/testutil/resource_stub.go
- internal/api/testutil/sse.go
- internal/agentidentity/snapshot.go
- internal/api/testutil/automation_stub.go
- internal/api/testutil/workspace_stub.go
- internal/api/testutil/observer_stub.go
- internal/api/contract/bridges_test.go
- internal/api/testutil/task_stub.go
- internal/bridges/resource_projection_bench_test.go
- internal/agentidentity/errors.go
- internal/api/core/coverage_helpers_test.go
- internal/api/testutil/skills_stub.go
- internal/automation/model/template_test.go
- internal/api/testutil/network_stub.go
- internal/automation/trigger_filter.go
🚧 Files skipped from review as they are similar to previous changes (27)
- internal/api/udsapi/extensions.go
- internal/bridges/resource.go
- internal/api/udsapi/bridges_integration_test.go
- internal/api/spec/authored_context.go
- internal/bridgesdk/errors_test.go
- internal/api/httpapi/handlers.go
- internal/api/httpapi/middleware.go
- internal/bridges/registry.go
- internal/bridgesdk/hostapi_refac_test.go
- internal/automation/manager.go
- internal/bridgesdk/cache.go
- internal/api/udsapi/hosted_mcp.go
- internal/api/core/bridges.go
- internal/bridges/types.go
- internal/bridgesdk/batching_refac_test.go
- internal/automation/model/template_bench_test.go
- internal/api/contract/bridges.go
- internal/acp/handlers.go
- internal/bridgesdk/runtime.go
- internal/api/core/sse.go
- internal/api/core/prompt_stream.go
- internal/bridges/resource_projection.go
- internal/api/udsapi/server_test.go
- internal/api/spec/spec.go
- internal/api/udsapi/server.go
- internal/api/testutil/session_stub.go
- internal/bridges/delivery_projection_refac_test.go
This was referenced May 23, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Merged
pedronauck
added a commit
that referenced
this pull request
May 26, 2026
## Release v0.0.1 This PR prepares the release of version v0.0.1. ### Changelog ## 0.0.1 - 2026-05-26 ### Other Changes - Lessons learned ### ♻️ Refactoring - Project structure (#7) - Kb improvements (#12) - Rename spaces to channels (#17) - Add extensions gaps (#21) - Improve tool calls ui (#22) - Remove web app header - Module improvements (#29) - Memory improvements (#35) - Storybook for web and ui (#38) - Enable AGH network by default for new installs (#57) - Hermes adjustments (#69) - Badges design (#84) - Storybook scenario and logos gallery - Migrate typescript tests (#114) - Internal go packages (#120) - Ui patterns (#127) - Improve e2e tests (#130) - Ui redesign - Workspace isolation across runtime surfaces (#145) - Prod ready applies (#162) - Tool card ui (#164) - Alpha on logo - Prod ready features (#167) - Thread sheet (#202) ### 🎉 Features - Implement config foundation packages - Implement sqlite store package - Add ACP client package - Add session lifecycle manager - Implement observe package - Add daemon composition root - Add uds api server - Implement cli package - Add http api server - Add system design - Add foundation types, schemas, and layout shell for web client - Add daemon health polling and agent sidebar systems for web client - Add session system CRUD, streaming core, and session store for web client - Add chat view, messages, and composer tests for web client - Add tool cards and renderers for web client - Add file-backed memory store core - Scaffold memory session seams - Add memory dream consolidation service - Wire memory assembler into daemon - Add memory api and cli - New skills system (#1) - Add workspace entity (#5) - Add new skill capabilities (#8) - Web ui v2 (#9) - Improve hooks system (#10) - Session resilience (#11) - Add extensability (#13) - Add automation (#16) - Add channels (#14) - Add network implementation (#15) - Add network, bridges and automations web pages (#18) - Ext registry (#20) - Add core tasks (#19) - Bridge adapters (#23) - Add site (#26) - Add ext refac and sandbox (#25) - Settings ui (#37) - Tasks ui (#36) - Harness improvements (#44) - Agent capabilities (#49) - Redesign ui (#48) - Unify capability (#53) - Redesign network workspace (#59) - Add task deletion and split session delete from stop (#58) - Session provider selection (#60) - Production grade adjustments (#66) - Autonomous system (#75) - Add agent session route (#80) - Tools registry (#85) - Agents soul (#88) - Add network threads (#105) - Orchestration improvements (#106) - Memory v2 (#108) - Agent categories (#113) - Providers model (#118) - Add canonical AGH bundled skill (#143) - Onboarding and improvements (#198) - Onboarding and improvements (#201) ### 🐛 Bug Fixes - Review round - Review rounds - Resolve memory extensibility review batch - Embed web into daemon - Defaults agents - Acp integration (#4) - Lint errors - Prd folder - Remove orphan web actions and dead surfaces (#55) - Qa testing and fixes (#73) - New review rounds (#82) - Security audit (#90) - Release qa round (#95) - Add missing tools (#141) - New qa round (#147) - Advanced qa round (#149) - Homebrew tap - Final review round (#151) - Daemon healthy - Reasoning models (#158) - Lint errors (#160) - Review round (#168) - Release adjustments (#171) - Stabilize release ci fixtures - Stabilize release integration gate - Stabilize release verify gates - Stabilize release integration flows - Stabilize release verify gates - Stabilize main verify shutdown - Ignore stale acpmock cancel - Marketplace search focus and filtering (#193) - Website video - Workspace command select ### 📚 Documentation - Update agents.md - Update prd - Update skills - Update compozy tasks - Update compozy - Update compozy - Add new skills - Archive prd - Update prds - Update rfc - Update prds - Update prds - Add automation prd - Channels prd - Update prd - Update prd - New prds - Archive prds - Bridges adapters prd - Sandbox prd - Update - Archive prd - Update - Add new prd - New design - Update prd - Archive prds - Update prds - Tasks-ui prd tasks - Update prd - Update design docs - Agent capabilities prd - Improve site docs - Remove old design references - Udpate - Autonomous prd - Update skills - Blog design - Agent sould prd - Final qa plan - Update - Remove codex ledgers from gitignore - Remove not needed files - Udpate ledger - Update cy-codex-loop skill - Orchestration improves prd - Update prds - Orch improvs prd - Memv2 prd - Providers model prd - Update refacs prd - New design proposal - Update rules - Update skills - New blog posts (#173) - Format docs - Remove old design files - Remove old - Skeeper update ### 📦 Build System - Initial structure - Commitlint - Frontend base structure - Update vscode settings - Add subagents - Coderabbit - Prd and tooling - Bun lock - Lint tooling - Copy.md and tooling adjusts - Add repoclone rc - Upgrade skeeper to v0.2.0 - Update go.mod - Adopt task artifacts into skeeper - Sync codex plans with skeeper - Skeeper lock - Skeeper lock - New skills - Skeeper lock - Skeeper lock - Skeeper lock - Update deps and go - Regenerate daytona sidecar assets for go 1.26.3 - Fix cliff - Ignore docs on fmt - Build web assets before goreleaser - Extend release dry-run timeout ### 🔧 CI/CD - Lint errors - Fint release pr - Fix goreleaser ### 🧪 Testing - Add e2e tests (#27) - Qa rounds (#78) - Improve test suite (#138) - Harden daemon-served restart reloads - Harden daemon-served readiness waits - Stabilize dashboard focus assertion - Stabilize release integration gates - Stabilize release e2e markers - Stabilize release e2e flows Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This was referenced May 26, 2026
Closed
Merged
pedronauck
added a commit
that referenced
this pull request
May 26, 2026
## Release v0.0.1 This PR prepares the release of version v0.0.1. ### Changelog ## 0.0.1 - 2026-05-26 ### Other Changes - Lessons learned ### ♻️ Refactoring - Project structure (#7) - Kb improvements (#12) - Rename spaces to channels (#17) - Add extensions gaps (#21) - Improve tool calls ui (#22) - Remove web app header - Module improvements (#29) - Memory improvements (#35) - Storybook for web and ui (#38) - Enable AGH network by default for new installs (#57) - Hermes adjustments (#69) - Badges design (#84) - Storybook scenario and logos gallery - Migrate typescript tests (#114) - Internal go packages (#120) - Ui patterns (#127) - Improve e2e tests (#130) - Ui redesign - Workspace isolation across runtime surfaces (#145) - Prod ready applies (#162) - Tool card ui (#164) - Alpha on logo - Prod ready features (#167) - Thread sheet (#202) ### 🎉 Features - Implement config foundation packages - Implement sqlite store package - Add ACP client package - Add session lifecycle manager - Implement observe package - Add daemon composition root - Add uds api server - Implement cli package - Add http api server - Add system design - Add foundation types, schemas, and layout shell for web client - Add daemon health polling and agent sidebar systems for web client - Add session system CRUD, streaming core, and session store for web client - Add chat view, messages, and composer tests for web client - Add tool cards and renderers for web client - Add file-backed memory store core - Scaffold memory session seams - Add memory dream consolidation service - Wire memory assembler into daemon - Add memory api and cli - New skills system (#1) - Add workspace entity (#5) - Add new skill capabilities (#8) - Web ui v2 (#9) - Improve hooks system (#10) - Session resilience (#11) - Add extensability (#13) - Add automation (#16) - Add channels (#14) - Add network implementation (#15) - Add network, bridges and automations web pages (#18) - Ext registry (#20) - Add core tasks (#19) - Bridge adapters (#23) - Add site (#26) - Add ext refac and sandbox (#25) - Settings ui (#37) - Tasks ui (#36) - Harness improvements (#44) - Agent capabilities (#49) - Redesign ui (#48) - Unify capability (#53) - Redesign network workspace (#59) - Add task deletion and split session delete from stop (#58) - Session provider selection (#60) - Production grade adjustments (#66) - Autonomous system (#75) - Add agent session route (#80) - Tools registry (#85) - Agents soul (#88) - Add network threads (#105) - Orchestration improvements (#106) - Memory v2 (#108) - Agent categories (#113) - Providers model (#118) - Add canonical AGH bundled skill (#143) - Onboarding and improvements (#198) - Onboarding and improvements (#201) ### 🐛 Bug Fixes - Review round - Review rounds - Resolve memory extensibility review batch - Embed web into daemon - Defaults agents - Acp integration (#4) - Lint errors - Prd folder - Remove orphan web actions and dead surfaces (#55) - Qa testing and fixes (#73) - New review rounds (#82) - Security audit (#90) - Release qa round (#95) - Add missing tools (#141) - New qa round (#147) - Advanced qa round (#149) - Homebrew tap - Final review round (#151) - Daemon healthy - Reasoning models (#158) - Lint errors (#160) - Review round (#168) - Release adjustments (#171) - Stabilize release ci fixtures - Stabilize release integration gate - Stabilize release verify gates - Stabilize release integration flows - Stabilize release verify gates - Stabilize main verify shutdown - Ignore stale acpmock cancel - Marketplace search focus and filtering (#193) - Website video - Workspace command select ### 📚 Documentation - Update agents.md - Update prd - Update skills - Update compozy tasks - Update compozy - Update compozy - Add new skills - Archive prd - Update prds - Update rfc - Update prds - Update prds - Add automation prd - Channels prd - Update prd - Update prd - New prds - Archive prds - Bridges adapters prd - Sandbox prd - Update - Archive prd - Update - Add new prd - New design - Update prd - Archive prds - Update prds - Tasks-ui prd tasks - Update prd - Update design docs - Agent capabilities prd - Improve site docs - Remove old design references - Udpate - Autonomous prd - Update skills - Blog design - Agent sould prd - Final qa plan - Update - Remove codex ledgers from gitignore - Remove not needed files - Udpate ledger - Update cy-codex-loop skill - Orchestration improves prd - Update prds - Orch improvs prd - Memv2 prd - Providers model prd - Update refacs prd - New design proposal - Update rules - Update skills - New blog posts (#173) - Format docs - Remove old design files - Remove old - Skeeper update ### 📦 Build System - Initial structure - Commitlint - Frontend base structure - Update vscode settings - Add subagents - Coderabbit - Prd and tooling - Bun lock - Lint tooling - Copy.md and tooling adjusts - Add repoclone rc - Upgrade skeeper to v0.2.0 - Update go.mod - Adopt task artifacts into skeeper - Sync codex plans with skeeper - Skeeper lock - Skeeper lock - New skills - Skeeper lock - Skeeper lock - Skeeper lock - Update deps and go - Regenerate daytona sidecar assets for go 1.26.3 - Fix cliff - Ignore docs on fmt - Build web assets before goreleaser - Extend release dry-run timeout ### 🔧 CI/CD - Lint errors - Fint release pr - Fix goreleaser - Fix release ### 🧪 Testing - Add e2e tests (#27) - Qa rounds (#78) - Improve test suite (#138) - Harden daemon-served restart reloads - Harden daemon-served readiness waits - Stabilize dashboard focus assertion - Stabilize release integration gates - Stabilize release e2e markers - Stabilize release e2e flows Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Merged
pedronauck
added a commit
that referenced
this pull request
May 26, 2026
## Release v0.0.2 This PR prepares the release of version v0.0.2. ### Changelog ## 0.0.2 - 2026-05-26 ### Other Changes - Lessons learned ### ♻️ Refactoring - Project structure (#7) - Kb improvements (#12) - Rename spaces to channels (#17) - Add extensions gaps (#21) - Improve tool calls ui (#22) - Remove web app header - Module improvements (#29) - Memory improvements (#35) - Storybook for web and ui (#38) - Enable AGH network by default for new installs (#57) - Hermes adjustments (#69) - Badges design (#84) - Storybook scenario and logos gallery - Migrate typescript tests (#114) - Internal go packages (#120) - Ui patterns (#127) - Improve e2e tests (#130) - Ui redesign - Workspace isolation across runtime surfaces (#145) - Prod ready applies (#162) - Tool card ui (#164) - Alpha on logo - Prod ready features (#167) - Thread sheet (#202) ### 🎉 Features - Implement config foundation packages - Implement sqlite store package - Add ACP client package - Add session lifecycle manager - Implement observe package - Add daemon composition root - Add uds api server - Implement cli package - Add http api server - Add system design - Add foundation types, schemas, and layout shell for web client - Add daemon health polling and agent sidebar systems for web client - Add session system CRUD, streaming core, and session store for web client - Add chat view, messages, and composer tests for web client - Add tool cards and renderers for web client - Add file-backed memory store core - Scaffold memory session seams - Add memory dream consolidation service - Wire memory assembler into daemon - Add memory api and cli - New skills system (#1) - Add workspace entity (#5) - Add new skill capabilities (#8) - Web ui v2 (#9) - Improve hooks system (#10) - Session resilience (#11) - Add extensability (#13) - Add automation (#16) - Add channels (#14) - Add network implementation (#15) - Add network, bridges and automations web pages (#18) - Ext registry (#20) - Add core tasks (#19) - Bridge adapters (#23) - Add site (#26) - Add ext refac and sandbox (#25) - Settings ui (#37) - Tasks ui (#36) - Harness improvements (#44) - Agent capabilities (#49) - Redesign ui (#48) - Unify capability (#53) - Redesign network workspace (#59) - Add task deletion and split session delete from stop (#58) - Session provider selection (#60) - Production grade adjustments (#66) - Autonomous system (#75) - Add agent session route (#80) - Tools registry (#85) - Agents soul (#88) - Add network threads (#105) - Orchestration improvements (#106) - Memory v2 (#108) - Agent categories (#113) - Providers model (#118) - Add canonical AGH bundled skill (#143) - Onboarding and improvements (#198) - Onboarding and improvements (#201) ### 🐛 Bug Fixes - Review round - Review rounds - Resolve memory extensibility review batch - Embed web into daemon - Defaults agents - Acp integration (#4) - Lint errors - Prd folder - Remove orphan web actions and dead surfaces (#55) - Qa testing and fixes (#73) - New review rounds (#82) - Security audit (#90) - Release qa round (#95) - Add missing tools (#141) - New qa round (#147) - Advanced qa round (#149) - Homebrew tap - Final review round (#151) - Daemon healthy - Reasoning models (#158) - Lint errors (#160) - Review round (#168) - Release adjustments (#171) - Stabilize release ci fixtures - Stabilize release integration gate - Stabilize release verify gates - Stabilize release integration flows - Stabilize release verify gates - Stabilize main verify shutdown - Ignore stale acpmock cancel - Marketplace search focus and filtering (#193) - Website video - Workspace command select ### 📚 Documentation - Update agents.md - Update prd - Update skills - Update compozy tasks - Update compozy - Update compozy - Add new skills - Archive prd - Update prds - Update rfc - Update prds - Update prds - Add automation prd - Channels prd - Update prd - Update prd - New prds - Archive prds - Bridges adapters prd - Sandbox prd - Update - Archive prd - Update - Add new prd - New design - Update prd - Archive prds - Update prds - Tasks-ui prd tasks - Update prd - Update design docs - Agent capabilities prd - Improve site docs - Remove old design references - Udpate - Autonomous prd - Update skills - Blog design - Agent sould prd - Final qa plan - Update - Remove codex ledgers from gitignore - Remove not needed files - Udpate ledger - Update cy-codex-loop skill - Orchestration improves prd - Update prds - Orch improvs prd - Memv2 prd - Providers model prd - Update refacs prd - New design proposal - Update rules - Update skills - New blog posts (#173) - Format docs - Remove old design files - Remove old - Skeeper update ### 📦 Build System - Initial structure - Commitlint - Frontend base structure - Update vscode settings - Add subagents - Coderabbit - Prd and tooling - Bun lock - Lint tooling - Copy.md and tooling adjusts - Add repoclone rc - Upgrade skeeper to v0.2.0 - Update go.mod - Adopt task artifacts into skeeper - Sync codex plans with skeeper - Skeeper lock - Skeeper lock - New skills - Skeeper lock - Skeeper lock - Skeeper lock - Update deps and go - Regenerate daytona sidecar assets for go 1.26.3 - Fix cliff - Ignore docs on fmt - Build web assets before goreleaser - Extend release dry-run timeout ### 🔧 CI/CD - Lint errors - Fint release pr - Fix goreleaser - Fix release - Fix release process ### 🧪 Testing - Add e2e tests (#27) - Qa rounds (#78) - Improve test suite (#138) - Harden daemon-served restart reloads - Harden daemon-served readiness waits - Stabilize dashboard focus assertion - Stabilize release integration gates - Stabilize release e2e markers - Stabilize release e2e flows - Improve suite speed Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This was referenced May 26, 2026
Closed
Closed
Merged
pedronauck
added a commit
that referenced
this pull request
May 27, 2026
## Release v0.0.2 This PR prepares the release of version v0.0.2. ### Changelog ## 0.0.2 - 2026-05-26 ### Other Changes - Lessons learned ### ♻️ Refactoring - Project structure (#7) - Kb improvements (#12) - Rename spaces to channels (#17) - Add extensions gaps (#21) - Improve tool calls ui (#22) - Remove web app header - Module improvements (#29) - Memory improvements (#35) - Storybook for web and ui (#38) - Enable AGH network by default for new installs (#57) - Hermes adjustments (#69) - Badges design (#84) - Storybook scenario and logos gallery - Migrate typescript tests (#114) - Internal go packages (#120) - Ui patterns (#127) - Improve e2e tests (#130) - Ui redesign - Workspace isolation across runtime surfaces (#145) - Prod ready applies (#162) - Tool card ui (#164) - Alpha on logo - Prod ready features (#167) - Thread sheet (#202) ### 🎉 Features - Implement config foundation packages - Implement sqlite store package - Add ACP client package - Add session lifecycle manager - Implement observe package - Add daemon composition root - Add uds api server - Implement cli package - Add http api server - Add system design - Add foundation types, schemas, and layout shell for web client - Add daemon health polling and agent sidebar systems for web client - Add session system CRUD, streaming core, and session store for web client - Add chat view, messages, and composer tests for web client - Add tool cards and renderers for web client - Add file-backed memory store core - Scaffold memory session seams - Add memory dream consolidation service - Wire memory assembler into daemon - Add memory api and cli - New skills system (#1) - Add workspace entity (#5) - Add new skill capabilities (#8) - Web ui v2 (#9) - Improve hooks system (#10) - Session resilience (#11) - Add extensability (#13) - Add automation (#16) - Add channels (#14) - Add network implementation (#15) - Add network, bridges and automations web pages (#18) - Ext registry (#20) - Add core tasks (#19) - Bridge adapters (#23) - Add site (#26) - Add ext refac and sandbox (#25) - Settings ui (#37) - Tasks ui (#36) - Harness improvements (#44) - Agent capabilities (#49) - Redesign ui (#48) - Unify capability (#53) - Redesign network workspace (#59) - Add task deletion and split session delete from stop (#58) - Session provider selection (#60) - Production grade adjustments (#66) - Autonomous system (#75) - Add agent session route (#80) - Tools registry (#85) - Agents soul (#88) - Add network threads (#105) - Orchestration improvements (#106) - Memory v2 (#108) - Agent categories (#113) - Providers model (#118) - Add canonical AGH bundled skill (#143) - Onboarding and improvements (#198) - Onboarding and improvements (#201) ### 🐛 Bug Fixes - Review round - Review rounds - Resolve memory extensibility review batch - Embed web into daemon - Defaults agents - Acp integration (#4) - Lint errors - Prd folder - Remove orphan web actions and dead surfaces (#55) - Qa testing and fixes (#73) - New review rounds (#82) - Security audit (#90) - Release qa round (#95) - Add missing tools (#141) - New qa round (#147) - Advanced qa round (#149) - Homebrew tap - Final review round (#151) - Daemon healthy - Reasoning models (#158) - Lint errors (#160) - Review round (#168) - Release adjustments (#171) - Stabilize release ci fixtures - Stabilize release integration gate - Stabilize release verify gates - Stabilize release integration flows - Stabilize release verify gates - Stabilize main verify shutdown - Ignore stale acpmock cancel - Marketplace search focus and filtering (#193) - Website video - Workspace command select ### 📚 Documentation - Update agents.md - Update prd - Update skills - Update compozy tasks - Update compozy - Update compozy - Add new skills - Archive prd - Update prds - Update rfc - Update prds - Update prds - Add automation prd - Channels prd - Update prd - Update prd - New prds - Archive prds - Bridges adapters prd - Sandbox prd - Update - Archive prd - Update - Add new prd - New design - Update prd - Archive prds - Update prds - Tasks-ui prd tasks - Update prd - Update design docs - Agent capabilities prd - Improve site docs - Remove old design references - Udpate - Autonomous prd - Update skills - Blog design - Agent sould prd - Final qa plan - Update - Remove codex ledgers from gitignore - Remove not needed files - Udpate ledger - Update cy-codex-loop skill - Orchestration improves prd - Update prds - Orch improvs prd - Memv2 prd - Providers model prd - Update refacs prd - New design proposal - Update rules - Update skills - New blog posts (#173) - Format docs - Remove old design files - Remove old - Skeeper update ### 📦 Build System - Initial structure - Commitlint - Frontend base structure - Update vscode settings - Add subagents - Coderabbit - Prd and tooling - Bun lock - Lint tooling - Copy.md and tooling adjusts - Add repoclone rc - Upgrade skeeper to v0.2.0 - Update go.mod - Adopt task artifacts into skeeper - Sync codex plans with skeeper - Skeeper lock - Skeeper lock - New skills - Skeeper lock - Skeeper lock - Skeeper lock - Update deps and go - Regenerate daytona sidecar assets for go 1.26.3 - Fix cliff - Ignore docs on fmt - Build web assets before goreleaser - Extend release dry-run timeout ### 🔧 CI/CD - Lint errors - Fint release pr - Fix goreleaser - Fix release - Fix release process - Fix release sync - Decouple release dry-run npm auth - Persist web assets git auth ### 🧪 Testing - Add e2e tests (#27) - Qa rounds (#78) - Improve test suite (#138) - Harden daemon-served restart reloads - Harden daemon-served readiness waits - Stabilize dashboard focus assertion - Stabilize release integration gates - Stabilize release e2e markers - Stabilize release e2e flows - Improve suite speed <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated web assets dependency to a newer version for improved stability and performance. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/compozy/agh/pull/211?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
](https://app.coderabbit.ai/change-stack/compozy/agh/pull/211?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
This was referenced May 27, 2026
Closed
Closed
Merged
pedronauck
added a commit
that referenced
this pull request
May 27, 2026
## Release v0.0.2 This PR prepares the release of version v0.0.2. ### Changelog ## 0.0.2 - 2026-05-27 ### Other Changes - Lessons learned ### ♻️ Refactoring - Project structure (#7) - Kb improvements (#12) - Rename spaces to channels (#17) - Add extensions gaps (#21) - Improve tool calls ui (#22) - Remove web app header - Module improvements (#29) - Memory improvements (#35) - Storybook for web and ui (#38) - Enable AGH network by default for new installs (#57) - Hermes adjustments (#69) - Badges design (#84) - Storybook scenario and logos gallery - Migrate typescript tests (#114) - Internal go packages (#120) - Ui patterns (#127) - Improve e2e tests (#130) - Ui redesign - Workspace isolation across runtime surfaces (#145) - Prod ready applies (#162) - Tool card ui (#164) - Alpha on logo - Prod ready features (#167) - Thread sheet (#202) ### 🎉 Features - Implement config foundation packages - Implement sqlite store package - Add ACP client package - Add session lifecycle manager - Implement observe package - Add daemon composition root - Add uds api server - Implement cli package - Add http api server - Add system design - Add foundation types, schemas, and layout shell for web client - Add daemon health polling and agent sidebar systems for web client - Add session system CRUD, streaming core, and session store for web client - Add chat view, messages, and composer tests for web client - Add tool cards and renderers for web client - Add file-backed memory store core - Scaffold memory session seams - Add memory dream consolidation service - Wire memory assembler into daemon - Add memory api and cli - New skills system (#1) - Add workspace entity (#5) - Add new skill capabilities (#8) - Web ui v2 (#9) - Improve hooks system (#10) - Session resilience (#11) - Add extensability (#13) - Add automation (#16) - Add channels (#14) - Add network implementation (#15) - Add network, bridges and automations web pages (#18) - Ext registry (#20) - Add core tasks (#19) - Bridge adapters (#23) - Add site (#26) - Add ext refac and sandbox (#25) - Settings ui (#37) - Tasks ui (#36) - Harness improvements (#44) - Agent capabilities (#49) - Redesign ui (#48) - Unify capability (#53) - Redesign network workspace (#59) - Add task deletion and split session delete from stop (#58) - Session provider selection (#60) - Production grade adjustments (#66) - Autonomous system (#75) - Add agent session route (#80) - Tools registry (#85) - Agents soul (#88) - Add network threads (#105) - Orchestration improvements (#106) - Memory v2 (#108) - Agent categories (#113) - Providers model (#118) - Add canonical AGH bundled skill (#143) - Onboarding and improvements (#198) - Onboarding and improvements (#201) ### 🐛 Bug Fixes - Review round - Review rounds - Resolve memory extensibility review batch - Embed web into daemon - Defaults agents - Acp integration (#4) - Lint errors - Prd folder - Remove orphan web actions and dead surfaces (#55) - Qa testing and fixes (#73) - New review rounds (#82) - Security audit (#90) - Release qa round (#95) - Add missing tools (#141) - New qa round (#147) - Advanced qa round (#149) - Homebrew tap - Final review round (#151) - Daemon healthy - Reasoning models (#158) - Lint errors (#160) - Review round (#168) - Release adjustments (#171) - Stabilize release ci fixtures - Stabilize release integration gate - Stabilize release verify gates - Stabilize release integration flows - Stabilize release verify gates - Stabilize main verify shutdown - Ignore stale acpmock cancel - Marketplace search focus and filtering (#193) - Website video - Workspace command select ### 📚 Documentation - Update agents.md - Update prd - Update skills - Update compozy tasks - Update compozy - Update compozy - Add new skills - Archive prd - Update prds - Update rfc - Update prds - Update prds - Add automation prd - Channels prd - Update prd - Update prd - New prds - Archive prds - Bridges adapters prd - Sandbox prd - Update - Archive prd - Update - Add new prd - New design - Update prd - Archive prds - Update prds - Tasks-ui prd tasks - Update prd - Update design docs - Agent capabilities prd - Improve site docs - Remove old design references - Udpate - Autonomous prd - Update skills - Blog design - Agent sould prd - Final qa plan - Update - Remove codex ledgers from gitignore - Remove not needed files - Udpate ledger - Update cy-codex-loop skill - Orchestration improves prd - Update prds - Orch improvs prd - Memv2 prd - Providers model prd - Update refacs prd - New design proposal - Update rules - Update skills - New blog posts (#173) - Format docs - Remove old design files - Remove old - Skeeper update ### 📦 Build System - Initial structure - Commitlint - Frontend base structure - Update vscode settings - Add subagents - Coderabbit - Prd and tooling - Bun lock - Lint tooling - Copy.md and tooling adjusts - Add repoclone rc - Upgrade skeeper to v0.2.0 - Update go.mod - Adopt task artifacts into skeeper - Sync codex plans with skeeper - Skeeper lock - Skeeper lock - New skills - Skeeper lock - Skeeper lock - Skeeper lock - Update deps and go - Regenerate daytona sidecar assets for go 1.26.3 - Fix cliff - Ignore docs on fmt - Build web assets before goreleaser - Extend release dry-run timeout - Fix release dry-run token contract ### 🔧 CI/CD - Lint errors - Fint release pr - Fix goreleaser - Fix release - Fix release process - Fix release sync - Decouple release dry-run npm auth - Persist web assets git auth - Require npm auth before release merge ### 🧪 Testing - Add e2e tests (#27) - Qa rounds (#78) - Improve test suite (#138) - Harden daemon-served restart reloads - Harden daemon-served readiness waits - Stabilize dashboard focus assertion - Stabilize release integration gates - Stabilize release e2e markers - Stabilize release e2e flows - Improve suite speed <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated dependencies to latest versions. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/compozy/agh/pull/214?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
](https://app.coderabbit.ai/change-stack/compozy/agh/pull/214?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Tests