feat(rewrite): add Phase 3 agent directory - #397
Conversation
|
Warning Review limit reached
Next review available in: 29 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesAgent domain and persistence
Browser dashboard
Validation and documentation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
greenfield/src/browser/auth/useAuthenticationAction.ts (1)
30-37: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winKeep the original error when the fallback reset fails.
resetAuthenticatedBrowserCachecallscollections.reset(), andreset()rejects with aTypeErrorafter the registry is cleaned up. If line 34 rejects, that rejection escapes thecatchblock andthrow erroron line 36 never runs. The user then sees the reset failure message instead of the original operation failure.🛡️ Proposed fix
} catch (error: unknown) { try { await refreshAuthenticationStatus(); } catch { - await resetAuthenticatedBrowserCache(queryClient, collections); + try { + await resetAuthenticatedBrowserCache(queryClient, collections); + } catch { + // Preserve the original operation failure. + } } throw error; }🤖 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 `@greenfield/src/browser/auth/useAuthenticationAction.ts` around lines 30 - 37, Preserve the original operation error in the catch path of the authentication action: ensure failures from resetAuthenticatedBrowserCache do not escape or replace the error ultimately rethrown by the outer catch. Update the fallback handling around refreshAuthenticationStatus and resetAuthenticatedBrowserCache so the original error is always thrown.
🧹 Nitpick comments (5)
greenfield/src/browser/data/dashboardCollections.ts (2)
54-63: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRecreate the collections even when the previous cleanup fails.
If
cleanupAgentCollections(previousAgents)rejects on line 60, line 61 never runs. The registry then keeps servingpreviousAgents, whose sub-collections may already be cleaned up. Consumers read a half-torn-down registry after an authentication boundary reset.Move the recreation into a
finallyblock so the registry always exposes fresh collections.♻️ Proposed refactor
async reset(): Promise<void> { await enqueue(async () => { if (cleaned) { throw new TypeError("Dashboard collections are cleaned up"); } const previousAgents = agents; - await cleanupAgentCollections(previousAgents); - agents = createAgentCollections(queryClient, trpcClient); + try { + await cleanupAgentCollections(previousAgents); + } finally { + agents = createAgentCollections(queryClient, trpcClient); + } }); },🤖 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 `@greenfield/src/browser/data/dashboardCollections.ts` around lines 54 - 63, Update reset() so createAgentCollections always replaces agents in a finally block after cleanupAgentCollections(previousAgents), while preserving the cleaned-state check and propagating cleanup errors.
16-21: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
Promise.allleaves the second cleanup rejection unobserved.If
definitions.cleanup()rejects,Promise.allrejects immediately. A later rejection fromstatuses.cleanup()becomes an unhandled rejection. UsePromise.allSettledand then report the failures.🤖 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 `@greenfield/src/browser/data/dashboardCollections.ts` around lines 16 - 21, Update cleanupAgentCollections to use Promise.allSettled for definitions.cleanup() and statuses.cleanup(), then inspect the settled results and report any rejected cleanup operations through the existing error-reporting mechanism.greenfield/src/browser/api/useRealtimeQueryInvalidation.ts (1)
30-36: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winHandle rejections from
refreshQueries.Line 34 discards the promise with
void. IfrefreshQueriesrejects, the rejection is unhandled and reaches the global handler. Add a.catchso a transient invalidation failure does not surface as an unhandled rejection.🛡️ Proposed fix
refreshTimer = setTimeout(() => { refreshTimer = undefined; - void refreshQueries(queryClient); + void refreshQueries(queryClient).catch(() => undefined); }, refreshDelayMs);🤖 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 `@greenfield/src/browser/api/useRealtimeQueryInvalidation.ts` around lines 30 - 36, Update the setTimeout callback in scheduleRefresh to attach a catch handler to the promise returned by refreshQueries(queryClient), while preserving the existing refreshTimer reset and scheduling behavior. Ensure any rejection is handled locally so it does not become an unhandled promise rejection.greenfield/src/contracts/agentModel.ts (1)
95-95: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winReuse one shared
agentTimestampSchemaacross agent contracts.
agentModel.tsandagents.tseach declare separateagentTimestampSchemaconstants, so changes to the agent timestamp validation can drift between status and cursor schemas. Export oneagentTimestampSchemaand import it where needed.🤖 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 `@greenfield/src/contracts/agentModel.ts` at line 95, Consolidate the duplicate agentTimestampSchema declarations by exporting the existing shared schema from its defining module and importing it in the other agent contract module. Update both status and cursor schema usage to reference this single agentTimestampSchema, preserving the existing validation message and behavior.greenfield/src/contracts/agents.test.ts (1)
75-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the limit bound and the cursor-consistency check.
The test name states "defaults and bounds", but the body only checks the default limit and the ordering check. Two contract rules in
agents.tsstay untested:agentTaskHistoryLimitSchemarejects a limit aboveagentTaskHistoryPageMaximum, andagentTaskHistoryCursorIsConsistentrejects anextCursorthat does not match the final row.♻️ Proposed additional assertions
expect(v.parse(listAgentTaskHistoryInputSchema, {})).toEqual({ limit: agentTaskHistoryPageDefault, }); + expect( + v.safeParse(listAgentTaskHistoryInputSchema, { + limit: agentTaskHistoryPageMaximum + 1, + }).success + ).toBeFalse(); + expect( + v.safeParse(listAgentTaskHistoryResultSchema, { + nextCursor: { id: secondRunId, startedAtMs: 2000 }, + runs: [ + { + agentId: "main", + id: firstRunId, + lastActivityAtMs: 1000, + startedAtMs: 1000, + status: "active", + task: "Newer task", + }, + ], + }).success + ).toBeFalse();
agentTaskHistoryPageMaximummust be added to the import list at line 5.🤖 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 `@greenfield/src/contracts/agents.test.ts` around lines 75 - 102, Extend the “defaults and bounds newest-first task history” test to assert that agentTaskHistoryLimitSchema rejects a limit above agentTaskHistoryPageMaximum, and that agentTaskHistoryCursorIsConsistent rejects a nextCursor inconsistent with the final run. Add agentTaskHistoryPageMaximum to the existing imports and preserve the current default and ordering assertions.
🤖 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 `@greenfield/src/browser/agents/AgentsRoute.tsx`:
- Around line 48-54: Update the refresh function to attach a rejection handler
to the Promise.all aggregate created from the three refetch calls, routing
failures to the existing error-reporting mechanism such as console.error.
Preserve the current refetch behavior and query-driven rendered error state.
In `@greenfield/src/browser/agents/AgentStatusGrid.tsx`:
- Around line 77-83: Update the agent card Heading in AgentStatusGrid to use
level 3 instead of level 2, preserving the existing id, size, and displayName
rendering so the cards are subordinate to the “Current status” section heading.
In `@greenfield/src/server/domains/agents/routes.ts`:
- Around line 19-32: The runAgentEffect function currently checks the rejected
Effect wrapper instead of the underlying typed failure, so AgentNotFoundError is
not mapped correctly. Update runAgentEffect to inspect or handle the Effect
cause before testing AgentNotFoundError, using an appropriate Effect mechanism
such as catchTag, runPromiseExit with cause extraction, or equivalent, while
preserving the existing NOT_FOUND TRPCError mapping.
In `@greenfield/src/server/domains/agents/service.test.ts`:
- Around line 225-239: Add await to the rejects assertions in
greenfield/src/server/domains/agents/service.test.ts lines 225-239 and 268-270.
Ensure both fail-closed tests wait for the AgentNotFoundError and unknown-agent
history error assertions before continuing to row-count checks or teardown.
In `@greenfield/src/server/domains/agents/service.ts`:
- Around line 132-149: Update listStatuses to construct its statuses in the
canonical ascending agentId order required by listAgentStatusesResultSchema,
rather than the dashboardAgentConfiguration.agents order; keep active-run lookup
and status generation unchanged, and ensure the resulting IDs satisfy the schema
contract.
---
Outside diff comments:
In `@greenfield/src/browser/auth/useAuthenticationAction.ts`:
- Around line 30-37: Preserve the original operation error in the catch path of
the authentication action: ensure failures from resetAuthenticatedBrowserCache
do not escape or replace the error ultimately rethrown by the outer catch.
Update the fallback handling around refreshAuthenticationStatus and
resetAuthenticatedBrowserCache so the original error is always thrown.
---
Nitpick comments:
In `@greenfield/src/browser/api/useRealtimeQueryInvalidation.ts`:
- Around line 30-36: Update the setTimeout callback in scheduleRefresh to attach
a catch handler to the promise returned by refreshQueries(queryClient), while
preserving the existing refreshTimer reset and scheduling behavior. Ensure any
rejection is handled locally so it does not become an unhandled promise
rejection.
In `@greenfield/src/browser/data/dashboardCollections.ts`:
- Around line 54-63: Update reset() so createAgentCollections always replaces
agents in a finally block after cleanupAgentCollections(previousAgents), while
preserving the cleaned-state check and propagating cleanup errors.
- Around line 16-21: Update cleanupAgentCollections to use Promise.allSettled
for definitions.cleanup() and statuses.cleanup(), then inspect the settled
results and report any rejected cleanup operations through the existing
error-reporting mechanism.
In `@greenfield/src/contracts/agentModel.ts`:
- Line 95: Consolidate the duplicate agentTimestampSchema declarations by
exporting the existing shared schema from its defining module and importing it
in the other agent contract module. Update both status and cursor schema usage
to reference this single agentTimestampSchema, preserving the existing
validation message and behavior.
In `@greenfield/src/contracts/agents.test.ts`:
- Around line 75-102: Extend the “defaults and bounds newest-first task history”
test to assert that agentTaskHistoryLimitSchema rejects a limit above
agentTaskHistoryPageMaximum, and that agentTaskHistoryCursorIsConsistent rejects
a nextCursor inconsistent with the final run. Add agentTaskHistoryPageMaximum to
the existing imports and preserve the current default and ordering assertions.
🪄 Autofix
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: CHILL
Plan: Pro Plus
Run ID: 8e1a31b4-d8ba-4437-840a-32da38af9242
⛔ Files ignored due to path filters (20)
greenfield/docs/generated/procedures.mdis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/agents.getConfiguration.input.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/agents.getConfiguration.output.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/agents.getStatus.input.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/agents.getStatus.output.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/agents.listStatuses.input.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/agents.listStatuses.output.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/agents.listTaskHistory.input.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/agents.listTaskHistory.output.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/agents.updateMetadata.input.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/agents.updateMetadata.output.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/automationSecurity.createPrincipal.input.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/automationSecurity.createPrincipal.output.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/automationSecurity.disablePrincipal.output.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/automationSecurity.listPrincipals.output.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/automationSecurity.replaceCapabilities.input.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/automationSecurity.replaceCapabilities.output.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/events.stream.input.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/events.stream.output.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/securityAudit.listEvents.output.schema.jsonis excluded by!**/generated/**and included by**/*
📒 Files selected for processing (83)
greenfield/.gitignoregreenfield/docs/architecture/greenfield-rewrite/application-architecture.mdgreenfield/docs/architecture/greenfield-rewrite/data-and-security.mdgreenfield/docs/architecture/greenfield-rewrite/progress.mdgreenfield/migrations/20260804022252_dashboard-foundation/migration.sqlgreenfield/migrations/20260804022252_dashboard-foundation/snapshot.jsongreenfield/scripts/delivery/productionReleaseActivation.test.tsgreenfield/scripts/documentation/artifacts.test.tsgreenfield/scripts/documentation/jsonSchema.test.tsgreenfield/scripts/documentation/jsonSchema.tsgreenfield/scripts/documentation/markdown.tsgreenfield/src/app/dashboardServer.tsgreenfield/src/app/server.tsgreenfield/src/app/trpcHttpHandler.tsgreenfield/src/browser/agents/AgentHistoryTable.tsxgreenfield/src/browser/agents/AgentStatusGrid.tsxgreenfield/src/browser/agents/AgentsRoute.test.tsxgreenfield/src/browser/agents/AgentsRoute.tsxgreenfield/src/browser/agents/agentCollections.tsgreenfield/src/browser/agents/agentQueries.tsgreenfield/src/browser/agents/useAgentCollectionQueryState.tsgreenfield/src/browser/agents/useAgentRealtimeInvalidation.tsgreenfield/src/browser/api/trpcClient.tsgreenfield/src/browser/api/useRealtimeQueryInvalidation.tsgreenfield/src/browser/application.test.tsxgreenfield/src/browser/application.tsxgreenfield/src/browser/auth/AuthenticatedSessionActivity.test.tsxgreenfield/src/browser/auth/AuthenticatedSessionActivity.tsxgreenfield/src/browser/auth/AuthenticationBoundary.test.tsxgreenfield/src/browser/auth/LoginRoute.test.tsxgreenfield/src/browser/auth/authQueries.tsgreenfield/src/browser/auth/useAuthenticationAction.tsgreenfield/src/browser/data/dashboardCollections.tsgreenfield/src/browser/data/dashboardCollectionsContext.tsxgreenfield/src/browser/data/dashboardCollectionsContextValue.tsgreenfield/src/browser/layout/DashboardShell.tsxgreenfield/src/browser/lib/dashboardRoutes.tsgreenfield/src/browser/router.tsxgreenfield/src/browser/routes/agents.lazy.tsxgreenfield/src/browser/security/AccountSecurityRoute.test.tsxgreenfield/src/browser/security/SessionManagementSection.tsxgreenfield/src/browser/tasks/TaskBoardRoute.test.tsxgreenfield/src/browser/tasks/useTaskRealtimeInvalidation.test.tsxgreenfield/src/browser/tasks/useTaskRealtimeInvalidation.tsgreenfield/src/browser/ui/DataTable.tsxgreenfield/src/browser/ui/PageHeader.tsxgreenfield/src/contracts/agentModel.test.tsgreenfield/src/contracts/agentModel.tsgreenfield/src/contracts/agentRealtime.tsgreenfield/src/contracts/agents.test.tsgreenfield/src/contracts/agents.tsgreenfield/src/contracts/contractRegistry.tsgreenfield/src/contracts/events.test.tsgreenfield/src/contracts/events.tsgreenfield/src/contracts/security.tsgreenfield/src/server/database/migrations/agentTaskRunsSchema.test.tsgreenfield/src/server/database/migrations/migrationGraph.test.tsgreenfield/src/server/database/migrations/securityIdentitySchema.automation.test.tsgreenfield/src/server/database/schema/agentTaskRuns.tsgreenfield/src/server/database/schema/automationPrincipalCapabilities.tsgreenfield/src/server/database/schema/drizzleSchema.tsgreenfield/src/server/database/validation/agentTaskRuns.tsgreenfield/src/server/domains/agents/directory.tsgreenfield/src/server/domains/agents/errors.tsgreenfield/src/server/domains/agents/procedures.test.tsgreenfield/src/server/domains/agents/procedures.tsgreenfield/src/server/domains/agents/repository.tsgreenfield/src/server/domains/agents/routes.tsgreenfield/src/server/domains/agents/service.test.tsgreenfield/src/server/domains/agents/service.tsgreenfield/src/server/domains/agents/testSupport/agentService.tsgreenfield/src/server/domains/agents/testSupport/service.tsgreenfield/src/server/domains/security/requestAuthenticationSession.test.tsgreenfield/src/server/test/support/requestContext.tsgreenfield/src/server/trpc/appRouter.tsgreenfield/src/server/trpc/context.test.tsgreenfield/src/server/trpc/context.tsgreenfield/src/server/trpc/procedureErrorPolicy.tsgreenfield/src/shared/databaseMigrationManifest.tsgreenfield/src/test/parity/fixtures/frontend-routes.jsongreenfield/src/test/parity/fixtures/greenfield-contracts.jsongreenfield/src/test/parity/fixtures/legacy-endpoints.jsongreenfield/src/test/parity/parityInventory.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Analyze JavaScript and TypeScript
- GitHub Check: dashboard-checks
🧰 Additional context used
🪛 ast-grep (0.45.0)
greenfield/src/browser/agents/agentQueries.ts
[error] 19-26: Avoid SQL injection
Context: client.query(
"agents.listTaskHistory",
{
...(pageParam === undefined ? {} : { cursor: pageParam }),
limit: 50,
},
{ signal }
)
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(sql-injection-typescript)
🔇 Additional comments (97)
greenfield/src/app/dashboardServer.ts (1)
6-7: LGTM!Also applies to: 77-77, 248-262
greenfield/src/app/server.ts (1)
5-5: LGTM!Also applies to: 127-127, 174-174
greenfield/src/app/trpcHttpHandler.ts (1)
3-3: LGTM!Also applies to: 32-32, 207-207
greenfield/src/browser/agents/useAgentRealtimeInvalidation.ts (1)
1-16: LGTM!greenfield/src/browser/layout/DashboardShell.tsx (1)
3-3: LGTM!Also applies to: 19-19
greenfield/src/browser/lib/dashboardRoutes.ts (1)
5-5: LGTM!greenfield/src/browser/routes/agents.lazy.tsx (1)
1-14: LGTM!greenfield/src/browser/security/SessionManagementSection.tsx (1)
10-10: LGTM!Also applies to: 69-69, 78-88
greenfield/src/browser/tasks/useTaskRealtimeInvalidation.test.tsx (1)
53-62: LGTM!Also applies to: 168-204
greenfield/src/browser/tasks/useTaskRealtimeInvalidation.ts (1)
2-2: LGTM!Also applies to: 12-17
greenfield/src/browser/ui/DataTable.tsx (1)
88-88: LGTM!greenfield/src/browser/ui/PageHeader.tsx (1)
7-7: LGTM!Also applies to: 17-33
greenfield/src/browser/application.test.tsx (1)
9-9: LGTM!Also applies to: 58-71, 91-92
greenfield/src/browser/application.tsx (1)
14-18: LGTM!Also applies to: 31-36, 50-50, 60-69, 82-82
greenfield/src/browser/auth/AuthenticatedSessionActivity.tsx (1)
8-8: LGTM!Also applies to: 64-64, 96-96, 150-150
greenfield/src/browser/auth/AuthenticationBoundary.test.tsx (1)
13-13: LGTM!Also applies to: 84-97, 138-139
greenfield/src/browser/auth/LoginRoute.test.tsx (1)
16-19: LGTM!Also applies to: 89-90, 104-118, 131-135
greenfield/src/browser/auth/authQueries.ts (1)
5-5: LGTM!Also applies to: 26-54
greenfield/src/browser/agents/agentQueries.ts (1)
1-37: LGTM!greenfield/src/browser/agents/useAgentCollectionQueryState.ts (1)
1-28: LGTM!greenfield/src/browser/api/trpcClient.ts (1)
71-74: LGTM!greenfield/src/browser/router.tsx (1)
24-27: LGTM!Also applies to: 36-36
greenfield/src/browser/security/AccountSecurityRoute.test.tsx (1)
28-31: LGTM!Also applies to: 259-260, 271-285, 313-317
greenfield/src/browser/auth/AuthenticatedSessionActivity.test.tsx (1)
10-11: LGTM!Also applies to: 58-65, 105-105, 131-138, 160-160, 184-191, 211-211
greenfield/src/browser/data/dashboardCollectionsContext.tsx (1)
1-21: LGTM!greenfield/src/browser/data/dashboardCollectionsContextValue.ts (1)
1-20: LGTM!greenfield/src/browser/agents/AgentHistoryTable.tsx (2)
94-125: LGTM!
1-18: 🩺 Stability & AvailabilityNo change needed for the
@tanstack/react-tablev9 usage here.
tableFeatures({}), the two-type-parametercreateColumnHelper, anduseTablematch v9 semantics, and the core row model is included automatically fortable.getRowModel().> Likely an incorrect or invalid review comment.greenfield/src/browser/agents/AgentsRoute.test.tsx (1)
51-173: LGTM!Also applies to: 175-350
greenfield/src/browser/agents/AgentsRoute.tsx (1)
29-46: LGTM!Also applies to: 56-109
greenfield/src/browser/agents/agentCollections.ts (1)
19-60: LGTM!greenfield/src/browser/api/useRealtimeQueryInvalidation.ts (2)
37-55: LGTM!
56-63: 🩺 Stability & AvailabilityCurrent consumers pass stable
refreshQueriesfunctions.greenfield/src/browser/tasks/TaskBoardRoute.test.tsx (1)
24-27: LGTM!Also applies to: 189-220
greenfield/src/server/domains/agents/testSupport/agentService.ts (1)
1-18: LGTM!Also applies to: 20-27, 29-37, 39-61, 64-71, 73-73
greenfield/src/server/domains/agents/testSupport/service.ts (1)
1-10: LGTM!Also applies to: 12-21, 23-49
greenfield/src/server/domains/security/requestAuthenticationSession.test.ts (1)
11-11: LGTM!Also applies to: 89-89
greenfield/src/server/test/support/requestContext.ts (1)
8-9: LGTM!Also applies to: 385-385, 404-404, 497-497, 513-513
greenfield/src/test/parity/fixtures/greenfield-contracts.json (1)
65-84: LGTM!greenfield/src/test/parity/fixtures/legacy-endpoints.json (1)
159-159: LGTM!Also applies to: 172-172, 185-185, 198-199, 1990-1990
greenfield/src/test/parity/parityInventory.test.ts (1)
56-56: LGTM!greenfield/src/server/trpc/appRouter.ts (1)
1-1: LGTM!Also applies to: 29-29, 41-41
greenfield/src/server/trpc/context.test.ts (1)
3-3: LGTM!Also applies to: 35-35, 111-111, 138-138
greenfield/src/server/trpc/context.ts (1)
2-2: LGTM!Also applies to: 27-27, 46-46, 76-76
greenfield/src/test/parity/fixtures/frontend-routes.json (1)
36-36: LGTM!greenfield/src/shared/databaseMigrationManifest.ts (1)
16-18: 🗄️ Data Integrity & IntegrationManifest hashes match the committed migration artifacts.
greenfield/src/server/trpc/procedureErrorPolicy.ts (1)
28-37: 🗄️ Data Integrity & IntegrationNo change needed.
The agent procedure errors match the procedure contracts exactly, including
agents.updateMetadata.greenfield/.gitignore (1)
15-17: LGTM!greenfield/docs/architecture/greenfield-rewrite/application-architecture.md (1)
390-390: LGTM!Also applies to: 605-609
greenfield/docs/architecture/greenfield-rewrite/data-and-security.md (1)
125-143: LGTM!Also applies to: 195-195, 224-227
greenfield/docs/architecture/greenfield-rewrite/progress.md (1)
15-15: LGTM!Also applies to: 823-843
greenfield/migrations/20260804022252_dashboard-foundation/migration.sql (1)
142-142: LGTM!Also applies to: 485-542
greenfield/migrations/20260804022252_dashboard-foundation/snapshot.json (1)
4-10: LGTM!Also applies to: 111-230, 2646-2654, 2862-2915, 3888-3929, 4213-4213
greenfield/scripts/delivery/productionReleaseActivation.test.ts (1)
2-2: LGTM!Also applies to: 57-57
greenfield/scripts/documentation/artifacts.test.ts (1)
3-6: LGTM!Also applies to: 74-82, 136-136
greenfield/scripts/documentation/jsonSchema.test.ts (1)
11-12: LGTM!Also applies to: 105-114, 237-256
greenfield/scripts/documentation/jsonSchema.ts (1)
4-15: LGTM!Also applies to: 98-125, 428-428
greenfield/scripts/documentation/markdown.ts (1)
29-40: LGTM!greenfield/src/contracts/security.ts (1)
95-96: LGTM!greenfield/src/contracts/agentModel.test.ts (1)
14-62: LGTM!greenfield/src/contracts/agentModel.ts (1)
116-129: LGTM!Also applies to: 158-187
greenfield/src/server/database/migrations/agentTaskRunsSchema.test.ts (2)
74-125: LGTM!
35-68: 🗄️ Data Integrity & IntegrationNo changes needed.
The migration defines the
agent_task_runsmonotonic, immutable-completion, and delete-blocking triggers, while the Drizzle table is responsible for columns, checks, indexes, and validation schema.greenfield/src/server/database/migrations/migrationGraph.test.ts (1)
33-33: LGTM!greenfield/src/server/database/migrations/securityIdentitySchema.automation.test.ts (1)
3-3: LGTM!Also applies to: 84-93, 154-157
greenfield/src/server/database/schema/agentTaskRuns.ts (1)
19-86: LGTM!greenfield/src/server/database/schema/automationPrincipalCapabilities.ts (1)
21-21: LGTM!greenfield/src/server/database/schema/drizzleSchema.ts (1)
6-6: LGTM!greenfield/src/server/database/validation/agentTaskRuns.ts (2)
23-54: LGTM!Also applies to: 71-99
56-69: 🗄️ Data Integrity & IntegrationNo change needed.
nonnegativeDateSchemais a refinement callback, so Drizzle applies it before wrapping nullable date columns;completedAtretains itsnullablehandling.> Likely an incorrect or invalid review comment.greenfield/src/server/domains/agents/directory.ts (1)
9-60: LGTM!greenfield/src/server/domains/agents/routes.ts (1)
34-76: LGTM!greenfield/src/server/domains/agents/service.test.ts (1)
17-219: LGTM!greenfield/src/contracts/agentRealtime.ts (3)
7-21: LGTM!
24-30: LGTM!
33-40: LGTM!greenfield/src/contracts/agents.test.ts (1)
18-73: LGTM!Also applies to: 104-122
greenfield/src/contracts/agents.ts (6)
18-51: LGTM!
58-73: LGTM!
75-108: LGTM!
115-133: LGTM!
136-161: LGTM!
164-243: LGTM!greenfield/src/contracts/contractRegistry.ts (1)
2-2: LGTM!Also applies to: 19-19
greenfield/src/contracts/events.test.ts (1)
20-20: LGTM!greenfield/src/contracts/events.ts (1)
7-11: LGTM!Also applies to: 28-28, 43-57, 90-90
greenfield/src/server/domains/agents/errors.ts (1)
1-13: LGTM!greenfield/src/server/domains/agents/procedures.test.ts (1)
19-49: LGTM!Also applies to: 51-92, 94-110
greenfield/src/server/domains/agents/procedures.ts (1)
1-8: LGTM!greenfield/src/server/domains/agents/repository.ts (5)
23-95: LGTM!
104-123: LGTM!
125-140: LGTM!
142-249: LGTM!
258-304: LGTM!greenfield/src/server/domains/agents/service.ts (3)
151-171: LGTM!
237-295: LGTM!
297-394: LGTM!
## Summary - add complete-snapshot monitoring ingestion plus immutable report, incident, and Dashboard-notification catalogs - expose 13 validated tRPC procedures with capability and principal-kind boundaries, typed expected errors, production request-context wiring, and a real HTTP composition proof - add exact monitoring transport budgets, atomic realtime events, bounded catalog mutations, generated contract artifacts, parity inventory, and Phase 3 progress evidence - harden idempotent producer replay, clock-regression ordering, retention overflow, report-linked deletion, and database admission behavior - address review findings with default keyset indexes, composed body-profile limits, shared principal/filter policies, explicit wake-failure observability, incident-generation validation, and targeted cascade/nullability regressions ## Behavior and regression coverage - automation principals can submit one normalized complete monitoring snapshot and produce reports, incident lifecycle state, notifications, and realtime events in one immediate transaction - authenticated callers can list/load incidents and reports; scoped sessions or automations may upsert reports, while notification producers and browser-session acknowledgement actions retain exact principal-kind boundaries - report and notification producer replays are exact and idempotent; divergent immutable IDs return `CONFLICT` without partial writes or duplicate events - notification producers must reference the current persisted incident generation; mismatches return `NOT_FOUND` without a notification row or realtime event - incident-generation notifications retain the authenticated forward link `/incidents?incidentId=…` with `reportId: null`, because a persistent generation can span multiple report observations; the hidden reader lands in the immediately stacked browser slice before cutover - monitoring snapshot ingestion and report upsert use an exact non-batchable 640 KiB transport profile while semantic aggregate limits remain 512 KiB; unknown or malformed procedure paths keep the 64 KiB default - mixed ordinary request-body profiles compose by their largest allowance, while authentication and WebAuthn retain their restrictive namespace ceilings - unfiltered and cursor-based report, notification, and incident lists use matching composite keyset indexes without temporary sorting - notification bulk work and report-linked cascades are bounded; linked rows use one compact `snapshot-required` invalidation, and oversized report deletion returns `PRECONDITION_FAILED` - mutation time remains monotonic across clock regression, and producer timestamps that cannot leave realtime-retention room return declared `BAD_REQUEST` failures without writes, events, or wakeups - post-commit realtime wake failures are logged safely while SQLite remains authoritative and adaptive polling recovers the missed wake - generated procedure/realtime schemas remain fail-closed for runtime-only Valibot refinements and document all five snapshot-capable realtime topics ## Verification - [x] Repository lint: `cd greenfield && bun run lint` - [x] Repository formatting: `cd greenfield && bun run format:check` - [ ] Frontend build: `bun run build:frontend` — not applicable to this isolated greenfield slice; browser artifacts were built by `cd greenfield && bun run build:release` - [ ] Frontend tests/coverage: `bun run test:frontend:coverage` — legacy frontend is outside this slice; full greenfield browser coverage ran below - [ ] Backend build: `bun run build:backend` — not applicable to this isolated greenfield slice; process artifacts were built by `cd greenfield && bun run build:release` - [ ] Backend tests/coverage: `bun run test:backend:coverage` — legacy backend is outside this slice; full greenfield coverage ran below - [x] Focused regression tests: latest catalog/service/procedure review suite (19/19), plus the complete greenfield suite below - [ ] Manual UI/API smoke check, if relevant — inactive pre-cutover slice with no new browser workflow; real HTTP composition and release-build tests cover the executable boundary Additional greenfield gates: - `bun run typecheck` - `bun run check:boundaries` - `bun run docs:check` - `bun run db:check` - `bun run test:coverage` — 1,474 tests, 0 failures, 92.25% line coverage (37,004/40,112) - `bun run build:release` — clean-source release for `488425875a815b1806f77a0ba75342937c2d8e13` ## Risk checklist - [x] No secrets, tokens, `.env` files, database dumps, or runtime state committed - [x] Auth, Gateway, terminal, file, Docker, or settings changes were reviewed carefully - [x] New/changed API routes enforce the expected authentication and validation - [x] Migrations or data-shape changes include a rollout/rollback note, if relevant - [x] Runtime/reconnect behavior preserves ordering, idempotency, and recovery - [x] No visible UI change in this slice The migration edits update the unpublished fresh-database greenfield baseline only. This slice remains inactive until the final supervised cutover; rollback before cutover is to revert the slice and regenerate the fresh baseline and release artifacts. ## Deployment / operations - [x] No deploy/restart needed - [ ] Deploy/restart needed after merge: none; this stack remains inactive until supervised cutover - [ ] Config/secrets changes needed: none - [x] Rollback path verified: revert this PR before cutover and regenerate the unpublished fresh-database baseline/release artifacts ## Notes for reviewers - focus on immediate-transaction atomicity, exact replay handling, bounded deletion, and monotonic event timing - review the qualified 640 KiB request profile together with the 512 KiB semantic budgets and fail-closed unknown-path behavior - producer timestamp overflow is a catalog-specific validation failure; incompatible clock/retention configuration fails fast before the service is exposed - stack base: #397 at locked head `47fac9196081c19deb1c16034f09d63e92aa33f2`
Summary
agents:read/agents:writecapabilities, durable task-run history, atomic realtime events, and the complete validated tRPC surface/agentswith TanStack DB, Query, Table, and Virtual projections for configuration, current status, and keyset-paginated historyBehavior and regression coverage
/agentsrenders reviewed roles, current task status, and keyset-paginated history with corrected heading hierarchyVerification
cd greenfield && bun run lintcd greenfield && bun run format:checkbun run build:frontend— not applicable to this isolated greenfield slice; the browser artifact was built bycd greenfield && bun run build:releasebun run test:frontend:coverage— legacy frontend is outside this slice; greenfield browser coverage ran belowbun run build:backend— not applicable to this isolated greenfield slice; process artifacts were built bycd greenfield && bun run build:releasebun run test:backend:coverage— legacy backend is outside this slice; full greenfield coverage ran belowAdditional greenfield gates:
bun run typecheckbun run check:boundariesbun run docs:checkbun run db:checkbun run test:coverage— 1,382 Bun tests + 66 browser tests, 0 failures, 92.06% line coveragebun run build:release— clean-source release for47fac9196081c19deb1c16034f09d63e92aa33f2Risk checklist
.envfiles, database dumps, or runtime state committedThe migration edits update the unpublished fresh-database greenfield baseline only. This slice remains inactive until the final supervised cutover; rollback before cutover is to revert the slice and rebuild the fresh baseline.
Visible UI change: the new
/agentsroute shows five agent cards, live current-task state, and a virtualized task-history table. Agent card headings are level 3 beneath the page's “Current status” section.Deployment / operations
Notes for reviewers
effect@4.0.0-beta.104preserves the exact typed error instance throughEffect.runPromise; the real procedure regression verifies unknown agents map toNOT_FOUND68aa8523f30db5a52369eb173c596441229ef269