feat: concurrent-agent sweep — docs, migrate CLI, engine registry, workspace app flow#710
Conversation
…rkspace app flow, dispatch updates
✅ Deploy Preview for agent-native-design ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for agent-native-images ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for agent-native-voice ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for agent-native-meeting-notes ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for agent-native-scheduling ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…elper for per-app resource visibility
❌ Deploy Preview for agent-native-fw failed. Why did it fail? →
|
✅ Deploy Preview for nutritrack-daily-calories canceled.
|
✅ Deploy Preview for agent-native-content canceled.
|
✅ Deploy Preview for agent-native-starter canceled.
|
✅ Deploy Preview for agent-native-forms canceled.
|
✅ Deploy Preview for agent-native-calendar canceled.
|
✅ Deploy Preview for agent-native-slides canceled.
|
✅ Deploy Preview for agent-native-dispatch canceled.
|
✅ Deploy Preview for agent-native-macros canceled.
|
✅ Deploy Preview for agent-native-issues canceled.
|
✅ Deploy Preview for agent-native-videos canceled.
|
✅ Deploy Preview for agent-native-mail canceled.
|
✅ Deploy Preview for agent-native-recruiting canceled.
|
…at; sweep concurrent dispatch/brain/docs work + prettier
…s; agent-chat-plugin + dispatch resource-store refinements
There was a problem hiding this comment.
Builder reviewed your changes and found 2 potential issues 🔴
Review Details
Incremental Code Review Summary
This incremental review focuses on verifying the status of issues from the previous review and checking for new regressions in the latest commits.
Risk Level: Standard — Backend/CLI changes with workspace sync logic and Dream proposal integration.
Status of Previous Issues
✅ FIXED: Missing db variable in applyDreamProposalDirect
- The function now properly declares
const db = getDb();at line 1577 - Database updates will execute correctly
Critical Issues Requiring Fix Before Merge
🔴 HIGH #1: PUT response not checked in syncResourcesToApp
- File:
packages/dispatch/src/server/lib/workspace-resources-store.ts(lines 1004-1027) - Problem: When recovering from a 409 conflict, the code sends a PUT request to update an existing resource but never checks if it succeeded. The code unconditionally marks the resource as synced and updates the
syncedAttimestamp regardless of whether the PUT succeeds or fails. - Impact: Resources can silently fail to sync while the UI and audit trail report success. Stale app resources persist undetected.
- Fix Required: Check the PUT response (
putRes.ok) before marking the resource as synced. Only advancesyncedAtand add tosyncedPathswhen the update actually succeeds.
🔴 HIGH #2: --last flag parsed but never used in resolveWorkbenchDir
- File:
packages/core/src/cli/migrate.ts(lines 178-179 parse, but 1077-1081 ignore it) - Problem: The
--lastflag is parsed and documented in help text (resume --last,status --last,ui --last,stop --last), butresolveWorkbenchDir()completely ignores it. The function falls back to the defaultDEFAULT_APP_NAMEregardless of the flag. - Impact: The advertised CLI workflow is broken. Users trying to resume their last migration with
--lastwill get "No Migration Workbench found" errors because the command still looks only in the default directory. - Fix Required: Implement
--lastfunctionality by either finding the most recently created workbench directory, or remove the flag from parsing and documentation until it's fully implemented.
Architecture Notes
✅ Good patterns:
- New Dream proposal workflow with proper async handling
- Workspace scope isolation working correctly in org context
- Helper functions (withTimeout, mapWithConcurrency) are well-implemented
- Changeset files properly included for publishable packages
Browser Testing
🧪 Skipped — PR is pure backend/CLI with no UI changes. All modifications are to dispatch logic, resource sync, and CLI argument handling.
| await fetch( | ||
| `${agent.url}/_agent-native/resources/${existing.id}`, | ||
| { | ||
| method: "PUT", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ content: resource.content }), | ||
| body: JSON.stringify({ | ||
| content: resource.content, | ||
| metadata: { | ||
| source: DISPATCH_RESOURCE_METADATA_SOURCE, | ||
| resourceId: resource.id, | ||
| kind: resource.kind, | ||
| name: resource.name, | ||
| description: resource.description, | ||
| updatedAt: resource.updatedAt, | ||
| }, | ||
| }), | ||
| }, | ||
| ); |
There was a problem hiding this comment.
🔴 PUT response not checked before marking resource synced
The code awaits the PUT request but never checks if it succeeded. Even if the update returns 4xx/5xx, the code still marks the resource as synced (line 1027) and updates syncedAt. Check putRes.ok and only mark as synced if the update actually succeeded.
Or react with 🚀 to auto-fix.
| function resolveWorkbenchDir(opts: MigrateCliOptions): string { | ||
| if (opts.workbench) return path.resolve(process.cwd(), opts.workbench); |
There was a problem hiding this comment.
🔴 last flag is parsed but never used
The parseMigrateArgs function accepts --last (line 178), and help text advertises resume --last, but resolveWorkbenchDir ignores opts.last and always falls back to DEFAULT_APP_NAME. Either implement finding the most recent workbench when --last is true, or remove the flag from parsing.
Or react with 🚀 to auto-fix.
… docs, new-app flow
There was a problem hiding this comment.
Builder reviewed your changes and found 1 potential issue 🔴
Review Details
Incremental Code Review Summary
This incremental review verifies the status of 2 previously flagged issues and checks for new regressions in the latest commit (8bc6f85: feat: sweep concurrent-agent work).
Risk Level: Standard — Backend/CLI changes affecting resource sync and migration CLI workflow.
Status of Previously Flagged Issues
❌ STILL NOT FIXED: PUT response not verified in syncResourcesToApp
- File:
packages/dispatch/src/server/lib/workspace-resources-store.ts(lines 1057-1074) - Status: Issue remains unchanged. The PUT request is made but response is never checked before marking resource as synced.
- Impact: Resources can silently fail to sync while audit trail reports success, corrupting sync state.
❌ STILL NOT FIXED: --last flag parsed but not used
- File:
packages/core/src/cli/migrate.ts(lines 1278-1279) - Status: Core issue remains:
resolveWorkbenchDir()never checksopts.last. However, the symptom is partially masked by new code that checks run records before calling this function. - Impact: The documented
--lastflag doesn't actually work as intended.
NEW BUG FOUND IN LATEST COMMIT
🔴 HIGH: migrate status ignores --app-surface when any prior run exists
- File:
packages/core/src/cli/migrate.ts(line 478) - Problem: The condition
if (last || !opts.appSurface)causes the function to display Code Agents status whenever any prior run exists, completely ignoring an explicit--app-surfaceflag. This breaks the documented ability to inspect legacy workbench status. - Impact: Users cannot access the legacy workbench status view on machines that have recorded migration runs, making that entire surface unreachable.
- Fix Required: Change to
if (last && !opts.appSurface)so an explicit--app-surfacerequest takes precedence.
Summary
The PR still has 2 high-severity blocking issues (PUT response verification and --last flag) that were previously reported and remain unfixed. Additionally, a new regression was introduced in the CLI status command that breaks the legacy workbench surface for users with recorded runs.
🧪 Browser Testing: Skipped — Pure backend/CLI changes, no UI impact.
| function printMigrationStatus(opts: MigrateCliOptions): void { | ||
| const last = getLastCodeAgentRunRecord("migrate"); | ||
| const runs = listCodeAgentRunRecords("migrate"); | ||
| if (last || !opts.appSurface) { |
There was a problem hiding this comment.
🔴 migrate status --app-surface flag ignored when any prior run exists
The condition if (last || !opts.appSurface) should be if (last && !opts.appSurface) so an explicit --app-surface request is not overridden. Currently, any existing run record causes the Code Agents view to be shown regardless of the user's request for the legacy workbench surface.
Or react with 🚀 to auto-fix.
There was a problem hiding this comment.
Builder reviewed your changes — no new findings
Review Details
Incremental Code Review Summary
This incremental review checks the latest commit (e49d25c: style: prettier-format) and the status of the 3 previously flagged blocking issues.
Risk Level: Standard — Formatting commit with no functional changes to the flagged areas.
Status of Previously Flagged Issues
❌ Still unfixed (3 blocking issues):
-
PUT response not verified in syncResourcesToApp — workspace-resources-store.ts:1057-1074
- The PUT request result is never checked before marking resource as synced
- Resources can silently fail to sync while audit trail reports success
-
--last flag parsed but never used — migrate.ts:1278-1279
- The flag is parsed but
resolveWorkbenchDir()ignores it - Documented workflow (
migrate resume --last) doesn't work
- The flag is parsed but
-
migrate status --app-surface flag ignored — migrate.ts:478
- Logic
if (last || !opts.appSurface)overrides explicit --app-surface request - Users cannot access legacy workbench status when run records exist
- Logic
Latest Commit Review
The latest commit (e49d25c) is a prettier formatting pass on unrelated files (templates/brain). No functional changes to the flagged areas. No new bugs introduced.
Assessment
The PR remains blocking until all 3 high-severity issues are fixed. No progress has been made on any of the issues since they were first flagged. The review comments remain open on the PR documenting each issue and the required fixes.
🧪 Browser Testing: Skipped — No UI changes, backend/CLI only.
…overload (CI Typecheck fix)
…es panel + editor, chat threads, migrate CLI, dispatch updates
… under CI frozen-lockfile (was spawnSync status:null ENOENT)
…mbedding SDK docs, brain lib
…ms-store, embedding SDK docs, core routes
…rce-store refactor, brain search
…ons catalog module
…ions store, dispatch integrations UI, code-agent auto-mode
… code-agents-ui + desktop IPC + code template
…pecheck; sweep code-agents-ui + cli + docs
…ace-connections docs, dispatch
…egrations UI, docs (mcp-clients/embedding-sdk/workspace)
… (agent-teams/embedding-sdk/mcp-clients)
…ooks; action-discovery refinements; composer-geometry QA smoke
Concurrent-agent work sweep across docs content, migrate CLI, agent engine registry, workspace app flow, and dispatch. PR opened early so CI + review agents start; local prep verification in progress.