feat(deploy-2): teams-as-environments deploy, owner-scoped task identity, push-driven surfaces - #1764
Merged
Conversation
Rod-Christensen
requested review from
jmaionchi and
stepmikhaylov
as code owners
July 31, 2026 07:24
Contributor
🤖 Internal: Discord sync markerAuto-managed by the Discord notification workflow. Stores the linked Discord message ID and forum thread ID. Do not edit or delete. |
Contributor
|
Note Reviews pausedUse the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds immutable team-scoped deployments, scoped task and log identity, deployment UI surfaces, shell API renames, shared UI updates, and a tools webhook endpoint. ChangesDeployment platform
Shell and shared UI
Tools endpoint
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 48
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/ai/src/ai/modules/task/task_server.py (1)
1234-1252: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove or align the extra
Taskkwargs.
Task.__init__accepts**kwargs, so this call is valid, butTask.__init__does not declarerun_kindortriggerat the call-site. Add those parameters onTask.__init__if these values should be part of the task config, or remove them from this call.🤖 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 `@packages/ai/src/ai/modules/task/task_server.py` around lines 1234 - 1252, Align the Task construction with Task.__init__: either explicitly declare and handle run_kind and trigger in Task.__init__ as task configuration values, or remove both kwargs from this Task call if they are not supported configuration. Keep the chosen behavior consistent between the constructor signature and the call site.apps/vscode/src/providers/ProjectProvider.ts (1)
294-307: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRemove the global monitor only when the last editor closes.
stopMonitoringruns on every panel dispose (Line 872). The project-scoped registration is per project, so removing it is correct. The{ token: '*' }registration with['deploy', 'task']is shared by all open editors. When one editor closes, the remaining editors lose deployment invalidations and task lifecycle events. The where-live running badges and the push-driven drawer refresh then stop working until a reconnect.Track the global registration with a reference count, or drop it only when no other editor state remains.
🐛 Proposed fix
private async stopMonitoring(panel: vscode.WebviewPanel): Promise<void> { const editorState = this.editorStates.get(panel); if (!editorState || !editorState.projectId) return; try { const client = this.connectionManager.getClient(); if (client) { await client.removeMonitor({ projectId: editorState.projectId, source: '*' }, ['summary', 'flow', 'output', 'task']); - await client.removeMonitor({ token: '*' }, ['deploy', 'task']); + // The global registration is shared by every open editor — + // drop it only when this panel is the last one. + const others = [...this.editorStates.values()].filter((s) => s !== editorState && !s.isDisposed); + if (others.length === 0) { + await client.removeMonitor({ token: '*' }, ['deploy', 'task']); + } } } catch (error) { this.logger.error(`Stopping monitoring for project ${editorState.projectId}: ${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 `@apps/vscode/src/providers/ProjectProvider.ts` around lines 294 - 307, Update stopMonitoring so the shared { token: '*' } monitor is removed only after the closing panel leaves no other active editor states; continue removing the project-scoped monitor for the closing editor. Track active editor usage with a reference count or inspect editorStates after removing the panel, and preserve the existing deploy/task monitor removal behavior when the last editor closes.apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsx (1)
225-252: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMirror
statusUpdateanddashboardSnapshotwrites into the refs.
handleTaskEventfolds fromactiveTasksRef.currentandunknownTasksRef.current. These two handlers update state only. The effects at Lines 125-130 re-anchor the refs after the next render. A task event that arrives before that render folds from stale collections, so accumulated errors and warnings can be lost onrestart, and a snapshot-seeded task can be missed. Update the refs in the same handler, the same wayhandleTaskEventdoes.🐛 Proposed fix
case 'statusUpdate': { // Update errors/warnings for a specific source const statusKey = `${msg.projectId}.${msg.sourceId}`; setActiveTasks((prev) => { const next = new Map(prev); const existing = next.get(statusKey) ?? { running: false, errors: [], warnings: [] }; next.set(statusKey, { ...existing, errors: msg.errors, warnings: msg.warnings }); + activeTasksRef.current = next; return next; }); break; } @@ setActiveTasks(taskMap); setUnknownTasks(unknown); + activeTasksRef.current = taskMap; + unknownTasksRef.current = unknown; break;🤖 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 `@apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsx` around lines 225 - 252, Update the statusUpdate and dashboardSnapshot branches in the message handler to mirror every activeTasks and unknownTasks state change into activeTasksRef.current and unknownTasksRef.current immediately, using the same collection updates as handleTaskEvent. Ensure subsequent events before the next render fold from the newly written refs, preserving accumulated errors, warnings, and snapshot-seeded tasks.
🤖 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 `@apps/vscode/package.json`:
- Line 175: Update the useState<SettingsData> initializer in SettingsWebview.tsx
so pipelineTraceLevel starts as "full", matching the package configuration
default; retain the settingsLoaded overwrite behavior and confirm that full
tracing is intended for development runs.
In `@apps/vscode/src/providers/ProjectProvider.ts`:
- Around line 943-947: Update the logsession:open case to look up and close any
existing stream for message.sessionId before replacing it in sessions. Then
store the newly opened stream as currently done, preserving the existing
project, source, and teamId options.
In `@apps/vscode/src/providers/views/deployTypes.ts`:
- Around line 274-282: Update the documentation comment for the deployment:fetch
message type to remove the stale reference to the drawer’s own poll and describe
that re-fetches are triggered through the push-driven scheduleDeploymentRefetch
path. Preserve the existing descriptions of drawer-open and post-mutation
refreshes.
In `@apps/vscode/src/providers/views/Project/ProjectWebview.tsx`:
- Around line 355-359: Update the deployment:error flow to carry and validate
sourceId: add sourceId to the deployment:error variant in deployment types,
populate it in fetchAndPushDeployment, and require it to match
openDeploymentRef.current alongside teamId before calling setDeploymentError in
ProjectWebview.
In `@apps/vscode/src/providers/views/Settings/DeploySettings.tsx`:
- Line 120: Handle the removed team ID settings during extension configuration
migration: either restore rocketride.development.teamId and
rocketride.deployment.teamId in the VS Code package configuration as deprecation
stubs with messages stating that teams are no longer selected in settings, or
remove those stale keys during extension activation. Ensure the toggle handler
around onSettingsChange no longer leaves deployment.teamId orphaned in existing
user settings.
In `@apps/vscode/src/shared/util/deployMapping.ts`:
- Around line 131-138: The deployment mappers use invalid 'active' defaults for
missing states. In apps/vscode/src/shared/util/deployMapping.ts lines 131-138,
update the mapper returning the deployment row to use a valid default such as
'enabled' or omit rows without a state; apply the same change to the mapper
using deployment.state at lines 149-157, keeping both behaviors consistent.
In `@packages/ai/src/ai/account/deployment_backend.py`:
- Around line 554-556: Update iter_enabled’s org listing to catch StorageError
around list_entries('orgs/', recursive=False, include_files=False), matching the
_project_ids handling; when the orgs tree is absent, treat it as empty and
continue yielding no organizations instead of failing the scheduler feed.
- Around line 175-198: Prevent concurrent publishes in mutate from overwriting
an existing artifact path: update the _store.write_file call in mutate to use an
exclusive write-if-absent operation, propagating a collision failure so
_mutate_meta retries with a new version. Preserve the registry’s
sha256-to-artifact integrity and never-overwritten invariant.
In `@packages/ai/src/ai/common/sandbox.py`:
- Around line 184-185: Update the warnings context around compile_restricted to
use warnings.filterwarnings with category=SyntaxWarning and a message pattern
matching only the intended diagnostic, rather than ignoring all SyntaxWarning
instances. Preserve the existing suppression behavior for that specific message
while allowing unrelated syntax warnings through.
- Around line 184-185: Make the SyntaxWarning suppression in execute_sandboxed
thread-safe by protecting the warnings.catch_warnings() block with a shared
lock, or remove the local context and configure the filter once during process
startup. Ensure concurrent sandbox executions cannot mutate global warning
filters unsafely.
In `@packages/ai/src/ai/modules/task/commands/cmd_deploy.py`:
- Around line 265-271: Reorder the validation guards in the pipeline handling
flow so the isinstance/non-empty check runs before the pipeline.name check.
Ensure missing or empty pipeline values raise the existing “pipeline is required
and must be an object” error, while valid dictionaries without a name still
raise the pipeline.name validation error; keep the project_id validation after
these checks.
In `@packages/ai/src/ai/modules/task/run_log.py`:
- Around line 155-177: Update scope_paths for deploy runs to validate team_id
before constructing the Team store prefix. Reject IDs containing “/” or “\”,
equal to “.” or “..”, or beginning with “@”, “=”, or “.”, while preserving the
existing missing-team ValueError and valid return tuple.
In `@packages/ai/src/ai/modules/task/task_engine.py`:
- Around line 104-118: Update cap_trace_payload in
packages/ai/src/ai/modules/task/task_engine.py (lines 104-118) to build the
truncation marker and progressively shrink preview until json.dumps(marker) is
within CONST_TRACE_PAYLOAD_CAP, accounting for escaping of quotes and
backslashes rather than sizing only raw. In
packages/ai/tests/ai/modules/task/test_task_engine.py (lines 951-962), add an
oversized trace fixture containing many quotes and backslashes and retain the
serialized-length assertion to verify the cap for escaped content.
In `@packages/ai/src/ai/modules/task/task_scheduler.py`:
- Around line 346-365: Extract the duplicated deploy-event construction from
_notify_deploy_changed and DeployCommands._notify_deploy_changed into one shared
helper, such as broadcast_deploy_changed, preserving the EVENT_TYPE.DEPLOY type,
apaevt_deploy name, body keys, and org_id scope. Update both callers to use the
helper while retaining each caller’s existing error logging and best-effort
behavior.
In `@packages/ai/tests/ai/modules/task/commands/test_cmd_deploy.py`:
- Around line 84-89: Update the test harness in _make_conn so
server.broadcast_server_event is an AsyncMock, allowing awaited deploy
notifications to execute instead of being swallowed. Add an async mutation test
such as test_deploy_broadcasts_invalidation_event that invokes _deploy_deploy
and asserts the EVENT_TYPE.DEPLOY payload body and org_id arguments.
In `@packages/ai/tests/ai/modules/task/commands/test_cmd_task.py`:
- Around line 166-177: Update
test_on_execute_accepts_team_id_matching_session_team and
test_on_execute_run_kind_cannot_be_spoofed_via_dap to monkeypatch
account_mod.get_merged_env with an AsyncMock returning an empty dictionary
before invoking TaskCommands.on_execute, matching the neighboring tests and
removing reliance on ambient account or .env state.
In `@packages/ai/tests/ai/modules/task/test_task_engine.py`:
- Around line 951-962: Update
test_cap_trace_payload_truncates_oversized_payloads to use an oversized payload
containing many quotes and backslashes, such as repeated objects with an escaped
quote/backslash value, instead of the plain z string. Keep the existing
assertions so the test verifies cap_trace_payload bounds the serialized marker
even when preview content requires JSON escaping.
In `@packages/ai/tests/ai/modules/task/test_task_identity.py`:
- Around line 51-66: Extract the task/public token content construction from
start_task into a shared digest-builder helper, then update both start_task and
the test _digest function to call it. Ensure the helper owns the production
field names and kind values, while preserving the existing generate_token(None,
...) behavior and digest outputs.
In `@packages/ai/tests/ai/modules/task/test_task_scheduler.py`:
- Around line 70-97: Update the scheduler fixture’s server stub so
broadcast_server_event is an AsyncMock, allowing _start_run to await it
successfully; then in the happy-path run test, assert the captured event’s
event, body.action, and body.teamId fields match the DEPLOY contract.
In `@packages/client-python/src/rocketride/deploy.py`:
- Around line 246-249: Update the run method docstring in the deployment client
to remove the claim that the caller is the billing-attribution actor; describe
manual deploy runs as carrying no human identity and being actor-independent, or
accurately document the server-recorded attribution if one exists. Ensure the
published Python reference reflects this behavior.
- Line 191: Update the filter example in the deployment method documentation
near the filters parameter to use a valid deployment state, such as enabled,
instead of active; keep the example format and surrounding documentation
unchanged.
In `@packages/client-python/src/rocketride/log_stream.py`:
- Around line 139-154: Remove the orphaned LogRunKind type and its export from
rocketride.types, since LogStream.__init__ no longer accepts or produces a
run-kind value. Update any remaining consumers or documentation references to
use the current team_id-based stream scope instead.
In `@packages/client-python/src/rocketride/types/deploy.py`:
- Around line 78-86: Update the nullable field annotations in the relevant
TypedDict, including ttl, traceLevel, and lastRunAt, to use Optional[...] so
type checkers accept explicit None values while preserving their existing
non-null types and total=False absent-key behavior.
In `@packages/client-python/tests/test_deploy.py`:
- Around line 176-181: Update the get_task_token call in the deploy test to pass
team_id=TEAM, ensuring it resolves the deploy task created by deploy.run for
TEAM rather than the user’s default task.
In `@packages/client-typescript/docs/guide/methods/deploy.md`:
- Around line 37-51: Update the deployment method and endpoint tables in the
guide to document deploy.run(), deploy.artifact(), and deploy.setSourceConfig(),
including each method’s parameters and corresponding rrext_deploy subcommand.
Preserve the existing table structure and descriptions while ensuring all three
public APIs are discoverable.
In `@packages/client-typescript/docs/guide/methods/log.md`:
- Around line 4-7: Update the Python examples for open_event_stream() and
chapters() to remove the obsolete positional 'dev' runKind argument; omit the
third argument for the dev stream and pass team_id='team-prod' by keyword for
the deploy stream, including the additional example around the referenced
chapters usage.
In `@packages/client-typescript/src/client/client.ts`:
- Around line 1899-1905: Update the monitor-key serialization and parsing pair,
including the logic near _monitorStringToKey, to encode projectId, source,
pipeId, and teamId without ambiguity—either use structured data or symmetrically
escape every delimiter. Preserve reconnect round-trips when project or source
IDs contain “@”, and add a test covering that case.
In `@packages/client-typescript/src/client/deploy.ts`:
- Around line 103-109: Update the publish method parameter in the deploy client
to require a name by narrowing its type to PipelineConfig & { name: string };
leave PipelineConfig unchanged so non-deployment APIs such as use() continue
allowing optional names.
In `@packages/client-typescript/tests/deploy.test.ts`:
- Around line 184-192: The task-token lookup in the deploy test should resolve
the run using the same team owner as deploy.run(project, 'webhook_1', TEAM).
Update the client.getTaskToken call to pass the appropriate team identity, then
keep the token assertion and termination flow unchanged.
In `@packages/server/engine-lib/engLib/store/pipeline/pipeline_config.cpp`:
- Around line 349-359: Update the source-marking traversal in the pipeline chain
construction to recursively visit the full control closure, not only direct
entries in comp.controls. For every source-controlled component, mark it
visited, then traverse its output lane targets and control attachments using the
existing DFS behavior before appending pipeline.chain, so data descendants and
nested control components match generatePipelineStack().
In `@packages/shared-ui/src/components/deploy-panel/DeploymentView.tsx`:
- Around line 99-108: Remove the duplicated sourceConfig and
onSourceConfigChange declarations from IDeploymentViewProps, retaining one
definition of each with their existing types and documentation so
DeploymentView.tsx compiles without duplicate identifiers.
- Around line 208-267: Remove the duplicated definitions of configRow,
configLabel, configSelect, configHint, and configCheck in the style object S,
retaining one complete block with the existing values.
In `@packages/shared-ui/src/components/deploy-panel/DeployPanel.tsx`:
- Around line 515-541: Filter removed deployments from the where-live grid by
deriving a liveDeployments list alongside liveByVersion and mapping that list
instead of deployments. In the state-dot handler, allow toggling only for live
states, while preserving the existing enabled/disabled behavior for eligible
deployments.
- Around line 402-418: The run function leaves error state active while
pendingDeploy or pickerVersion keeps the confirmation dialog open, but the
dialog does not display it. Update the ConfirmDialog rendering paths for
pendingDeploy and pickerVersion to surface the current error inside the open
dialog, while preserving the existing setError behavior in run and clearing it
on subsequent attempts.
In `@packages/shared-ui/src/components/deploy-panel/index.ts`:
- Around line 15-21: Update the deploy-panel barrel exports in the module index
to include the missing public symbols: export IDeploymentRecordData from
DeploymentRecordPanel, and export SchedulePreviewResult plus describeTtl from
SchedulePanel. Preserve the existing exports and use the symbols’ defining
modules rather than requiring consumers to use deep imports.
In `@packages/shared-ui/src/components/deploy-panel/SchedulePanel.tsx`:
- Around line 286-305: Move the orphaned cron-description docblock so it is
directly above the describeCron function, leaving the separate describeTtl
documentation immediately above describeTtl. Preserve both descriptions and
their existing function behavior.
- Around line 223-227: Update the weekly schedule handling in the
cron-generation logic and the Save gate to require at least one selected day
when state.kind is "weekly"; do not fall back to "*" for an empty state.days
set. Align summaryWording with this validation so an empty weekly selection
cannot be saved or represented as a daily schedule.
In
`@packages/shared-ui/src/components/deploy-panel/TeamDeploymentRecordPanel.tsx`:
- Around line 278-295: Update the timeline-loading effect around fetchTimeline
and schedules to start all per-source fetches concurrently with Promise.all
rather than awaiting inside the loop. Preserve per-source failure isolation by
converting each rejected fetch to an empty or omitted timeline, then build the
Map from the completed results and retain the existing cancelled guard before
setTimelines.
In `@packages/shared-ui/src/components/deploy-panel/types.ts`:
- Around line 86-89: Update TeamDeploymentRow’s inherited-field omission to
exclude runningSources alongside schedules, matching the ProjectView.tsx
derivation that represents running state through TeamDeploymentSource.running.
Keep the sources field and other inherited properties unchanged.
In `@packages/shared-ui/src/modules/project/components/SourcePanel.tsx`:
- Around line 237-265: Update the timeline polling useEffect that invokes
fetchTimelineRef to include projectId in its dependency array, matching the DVR
session effect’s stream-binding dependencies so project changes immediately
refresh chapters for the active session.
- Around line 284-287: Update the destructuring in the SourcePanel component
around useTaskEvents so the unused rangeEvents binding is prefixed with an
underscore, preserving the returned value while satisfying the unused-variable
rule.
In `@packages/shared-ui/src/modules/project/hooks/useTaskEvents.ts`:
- Line 671: Preserve the absent-key contract for TaskChapter.traceLevel: in
packages/shared-ui/src/modules/project/hooks/useTaskEvents.ts:671, conditionally
spread traceLevel only when first.body.traceLevel is a string; in
packages/shared-ui/src/modules/project/components/SourcePanel.tsx:324-342, apply
the same conditional spread to the local begin-announcement chapter instead of
writing null. In
packages/shared-ui/src/modules/project/components/StatusPane.tsx:271-275, retain
the existing 'traceLevel' in chapter consumer check without direct changes.
In `@packages/shared-ui/src/modules/project/ProjectView.tsx`:
- Around line 523-527: Update renderDocPanel so its unused subView parameter is
prefixed with an underscore, preserving the existing call sites and behavior.
In `@packages/shared-ui/src/modules/server/components/ActivityPanel.tsx`:
- Around line 121-132: Handle unmatched TaskEvent.action values in both
getTaskEventDisplay in
packages/shared-ui/src/modules/server/components/ActivityPanel.tsx (lines
121-132) and the task-source branch of getTickerSummary in
packages/shared-ui/src/modules/server/components/OverviewPanel.tsx (lines
232-245). Add explicit task fallback handling that returns a defined tone,
label, and message in getTaskEventDisplay, and prevents getTickerSummary from
falling through to DashboardEvent casting; preserve existing behavior for begin,
end, restart, and running.
In `@packages/shared-ui/src/modules/sidebar/taskFold.ts`:
- Around line 38-39: Add teamId?: string to the tasks row type in
TaskLifecycleEvent, define a shared named row type, and update
foldDeployRunState and foldProjectDeployRuns to use it instead of duplicating
inline casts. Preserve the existing fields and behavior.
- Around line 66-236: Add unit tests for foldTaskEvent, foldDeployRunState, and
foldProjectDeployRuns. Cover deploy rows being excluded from dev-task results,
the running snapshot rebuilding active and unknown state, and correct
team/project scoping for deploy lifecycle events and snapshots, including
unrelated events being ignored. Verify the pure functions return expected fresh
state without mutating inputs.
- Around line 98-114: The bulk running snapshot in the task fold should preserve
existing errors and warnings for tasks that remain active. Update the `running`
case’s `nextActive.set` logic to reuse the existing entry for each
`${t.projectId}.${t.source}` when present, while still marking it running; only
initialize empty error and warning arrays for newly seen tasks.
In `@pyproject.toml`:
- Around line 93-100: Update the filterwarnings entry in pyproject.toml to match
the complete Starlette deprecation message, including the recommendation to
install httpx2, or target StarletteDeprecationWarning specifically. Keep the
suppression scoped to this warning and preserve the existing test dependency
configuration.
---
Outside diff comments:
In `@apps/vscode/src/providers/ProjectProvider.ts`:
- Around line 294-307: Update stopMonitoring so the shared { token: '*' }
monitor is removed only after the closing panel leaves no other active editor
states; continue removing the project-scoped monitor for the closing editor.
Track active editor usage with a reference count or inspect editorStates after
removing the panel, and preserve the existing deploy/task monitor removal
behavior when the last editor closes.
In `@apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsx`:
- Around line 225-252: Update the statusUpdate and dashboardSnapshot branches in
the message handler to mirror every activeTasks and unknownTasks state change
into activeTasksRef.current and unknownTasksRef.current immediately, using the
same collection updates as handleTaskEvent. Ensure subsequent events before the
next render fold from the newly written refs, preserving accumulated errors,
warnings, and snapshot-seeded tasks.
In `@packages/ai/src/ai/modules/task/task_server.py`:
- Around line 1234-1252: Align the Task construction with Task.__init__: either
explicitly declare and handle run_kind and trigger in Task.__init__ as task
configuration values, or remove both kwargs from this Task call if they are not
supported configuration. Keep the chosen behavior consistent between the
constructor signature and the call site.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: f67cae12-e466-4aac-96e7-71218796e10a
📒 Files selected for processing (156)
apps/shell-ui/scripts/freeze-shell-api.jsapps/shell-ui/scripts/tasks.jsapps/shell-ui/src/api.tsapps/shell-ui/src/components/layout/OverlayManager.tsxapps/shell-ui/src/components/layout/Sidebar.tsxapps/shell-ui/src/hooks/iframeBridgeProtocol.tsapps/shell-ui/src/hooks/useIframeBridge.tsapps/shell-ui/src/index.tsapps/shell-ui/src/lib/Documents.tsxapps/shell-ui/src/providers/AccountProvider.tsxapps/shell-ui/src/providers/EnvironmentProvider.tsxapps/shell-ui/src/providers/SettingsProvider.tsxapps/vscode/package.jsonapps/vscode/src/config.tsapps/vscode/src/extension.tsapps/vscode/src/providers/ProjectProvider.tsapps/vscode/src/providers/SettingsProvider.tsapps/vscode/src/providers/SidebarProvider.tsapps/vscode/src/providers/WelcomeProvider.tsapps/vscode/src/providers/shared/connection-message-handler.tsapps/vscode/src/providers/views/Account/index.tsxapps/vscode/src/providers/views/Environment/index.tsxapps/vscode/src/providers/views/Monitor/MonitorWebview.tsxapps/vscode/src/providers/views/Monitor/index.tsxapps/vscode/src/providers/views/Project/ProjectWebview.tsxapps/vscode/src/providers/views/Project/index.tsxapps/vscode/src/providers/views/Settings/ConnectionSettings.tsxapps/vscode/src/providers/views/Settings/DeploySettings.tsxapps/vscode/src/providers/views/Settings/SettingsWebview.tsxapps/vscode/src/providers/views/Settings/index.tsxapps/vscode/src/providers/views/Sidebar/SidebarWebview.tsxapps/vscode/src/providers/views/Welcome/WelcomeWebview.tsxapps/vscode/src/providers/views/components/ConnectionConfig.tsxapps/vscode/src/providers/views/components/panels/CloudPanel.tsxapps/vscode/src/providers/views/deployTypes.tsapps/vscode/src/providers/views/logTypes.tsapps/vscode/src/providers/views/setup.react.tsapps/vscode/src/providers/views/types.tsapps/vscode/src/shared/util/deployMapping.tsnodes/src/nodes/webhook/IEndpoint.pynodes/src/nodes/webhook/services.tools.jsonnodes/src/nodes/webhook/tools/README.mdnodes/test/test_db_arango.pynodes/test/test_tool_v0.pynodes/test/tool_filesystem/test_live_anchor.pynodes/test/tool_google_workspace/test_google_client.pypackages/ai/src/ai/account/__init__.pypackages/ai/src/ai/account/base.pypackages/ai/src/ai/account/deployment_backend.pypackages/ai/src/ai/account/deployment_store.pypackages/ai/src/ai/account/models.pypackages/ai/src/ai/common/sandbox.pypackages/ai/src/ai/modules/task/commands/cmd_debug.pypackages/ai/src/ai/modules/task/commands/cmd_deploy.pypackages/ai/src/ai/modules/task/commands/cmd_log.pypackages/ai/src/ai/modules/task/commands/cmd_misc.pypackages/ai/src/ai/modules/task/commands/cmd_monitor.pypackages/ai/src/ai/modules/task/commands/cmd_task.pypackages/ai/src/ai/modules/task/run_log.pypackages/ai/src/ai/modules/task/task_engine.pypackages/ai/src/ai/modules/task/task_scheduler.pypackages/ai/src/ai/modules/task/task_server.pypackages/ai/src/ai/modules/task/task_server_facade.pypackages/ai/src/ai/modules/task_http/task_data.pypackages/ai/src/ai/node.pypackages/ai/tests/ai/account/test_deployment_backend.pypackages/ai/tests/ai/account/test_deployment_store.pypackages/ai/tests/ai/account/test_store.pypackages/ai/tests/ai/modules/task/commands/test_cmd_debug.pypackages/ai/tests/ai/modules/task/commands/test_cmd_deploy.pypackages/ai/tests/ai/modules/task/commands/test_cmd_log.pypackages/ai/tests/ai/modules/task/commands/test_cmd_misc.pypackages/ai/tests/ai/modules/task/commands/test_cmd_monitor.pypackages/ai/tests/ai/modules/task/commands/test_cmd_task.pypackages/ai/tests/ai/modules/task/test_log_stream.pypackages/ai/tests/ai/modules/task/test_run_log.pypackages/ai/tests/ai/modules/task/test_run_log_team.pypackages/ai/tests/ai/modules/task/test_task_engine.pypackages/ai/tests/ai/modules/task/test_task_identity.pypackages/ai/tests/ai/modules/task/test_task_scheduler.pypackages/ai/tests/ai/modules/task/test_task_server.pypackages/ai/tests/ai/modules/task/test_task_server_facade.pypackages/client-python/docs/index.mdpackages/client-python/docs/log.mdpackages/client-python/src/rocketride/client.pypackages/client-python/src/rocketride/deploy.pypackages/client-python/src/rocketride/log.pypackages/client-python/src/rocketride/log_stream.pypackages/client-python/src/rocketride/mixins/events.pypackages/client-python/src/rocketride/mixins/execution.pypackages/client-python/src/rocketride/types/__init__.pypackages/client-python/src/rocketride/types/deploy.pypackages/client-python/src/rocketride/types/events.pypackages/client-python/src/rocketride/types/log.pypackages/client-python/tests/test_deploy.pypackages/client-python/tests/test_monitor_keys.pypackages/client-typescript/docs/guide/methods/deploy.mdpackages/client-typescript/docs/guide/methods/log.mdpackages/client-typescript/src/client/client.tspackages/client-typescript/src/client/deploy.tspackages/client-typescript/src/client/log-stream.tspackages/client-typescript/src/client/log.tspackages/client-typescript/src/client/types/deploy.tspackages/client-typescript/src/client/types/events.tspackages/client-typescript/src/client/types/log.tspackages/client-typescript/src/client/types/pipeline.tspackages/client-typescript/tests/deploy.test.tspackages/client-typescript/tests/monitor-keys.test.tspackages/server/docs/observability.mdpackages/server/engine-lib/engLib/store/pipeline/pipeline_config.cpppackages/server/engine-lib/engLib/store/stack.cpppackages/shared-ui/src/components/canvas/CanvasPanel.tsxpackages/shared-ui/src/components/canvas/index.tsxpackages/shared-ui/src/components/data-grid/DataGrid.tsxpackages/shared-ui/src/components/data-grid/tabulator-theme.csspackages/shared-ui/src/components/deploy-panel/DeployPanel.tsxpackages/shared-ui/src/components/deploy-panel/DeploymentRecordPanel.tsxpackages/shared-ui/src/components/deploy-panel/DeploymentView.tsxpackages/shared-ui/src/components/deploy-panel/SchedulePanel.tsxpackages/shared-ui/src/components/deploy-panel/TeamDeploymentRecordPanel.tsxpackages/shared-ui/src/components/deploy-panel/VersionRecordPanel.tsxpackages/shared-ui/src/components/deploy-panel/index.tspackages/shared-ui/src/components/deploy-panel/types.tspackages/shared-ui/src/components/detail-panel/DetailPanel.tsxpackages/shared-ui/src/components/sidebar-menu/SidebarMenu.tsxpackages/shared-ui/src/components/tab-control/TabControl.tsxpackages/shared-ui/src/components/tab-control/ViewMenuBadge.tsxpackages/shared-ui/src/components/tab-panel/TabPanel.tsxpackages/shared-ui/src/index.tspackages/shared-ui/src/modules/account/AccountView.tsxpackages/shared-ui/src/modules/account/components/ProfilePanel.tsxpackages/shared-ui/src/modules/billing/components/BillingDashboard.tsxpackages/shared-ui/src/modules/environment/EnvironmentView.tsxpackages/shared-ui/src/modules/project/ProjectView.tsxpackages/shared-ui/src/modules/project/components/SourcePanel.tsxpackages/shared-ui/src/modules/project/components/StatusPane.tsxpackages/shared-ui/src/modules/project/hooks/useTaskEvents.tspackages/shared-ui/src/modules/project/index.tsxpackages/shared-ui/src/modules/project/types.tspackages/shared-ui/src/modules/project/utils.tspackages/shared-ui/src/modules/server/MonitorView.tsxpackages/shared-ui/src/modules/server/components/ActivityPanel.tsxpackages/shared-ui/src/modules/server/components/ConnectionRecordPanel.tsxpackages/shared-ui/src/modules/server/components/ConnectionsPanel.tsxpackages/shared-ui/src/modules/server/components/OverviewGrid.tsxpackages/shared-ui/src/modules/server/components/OverviewPanel.tsxpackages/shared-ui/src/modules/server/components/TaskRecordPanel.tsxpackages/shared-ui/src/modules/server/components/TasksPanel.tsxpackages/shared-ui/src/modules/server/components/index.tspackages/shared-ui/src/modules/server/util/formatters.tspackages/shared-ui/src/modules/sidebar/SidebarView.tsxpackages/shared-ui/src/modules/sidebar/taskFold.tspackages/shared-ui/src/themes/styles.tspackages/shared-ui/src/types/viewMenu.tspackages/shell-api/versions/v0.d.tspyproject.toml
💤 Files with no reviewable changes (7)
- apps/vscode/src/providers/views/Settings/ConnectionSettings.tsx
- apps/vscode/src/providers/SettingsProvider.ts
- apps/vscode/src/providers/shared/connection-message-handler.ts
- apps/vscode/src/providers/WelcomeProvider.ts
- packages/ai/tests/ai/account/test_deployment_store.py
- packages/ai/src/ai/node.py
- packages/ai/src/ai/account/deployment_store.py
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 48
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/ai/src/ai/modules/task/task_server.py (1)
1234-1252: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove or align the extra
Taskkwargs.
Task.__init__accepts**kwargs, so this call is valid, butTask.__init__does not declarerun_kindortriggerat the call-site. Add those parameters onTask.__init__if these values should be part of the task config, or remove them from this call.🤖 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 `@packages/ai/src/ai/modules/task/task_server.py` around lines 1234 - 1252, Align the Task construction with Task.__init__: either explicitly declare and handle run_kind and trigger in Task.__init__ as task configuration values, or remove both kwargs from this Task call if they are not supported configuration. Keep the chosen behavior consistent between the constructor signature and the call site.apps/vscode/src/providers/ProjectProvider.ts (1)
294-307: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRemove the global monitor only when the last editor closes.
stopMonitoringruns on every panel dispose (Line 872). The project-scoped registration is per project, so removing it is correct. The{ token: '*' }registration with['deploy', 'task']is shared by all open editors. When one editor closes, the remaining editors lose deployment invalidations and task lifecycle events. The where-live running badges and the push-driven drawer refresh then stop working until a reconnect.Track the global registration with a reference count, or drop it only when no other editor state remains.
🐛 Proposed fix
private async stopMonitoring(panel: vscode.WebviewPanel): Promise<void> { const editorState = this.editorStates.get(panel); if (!editorState || !editorState.projectId) return; try { const client = this.connectionManager.getClient(); if (client) { await client.removeMonitor({ projectId: editorState.projectId, source: '*' }, ['summary', 'flow', 'output', 'task']); - await client.removeMonitor({ token: '*' }, ['deploy', 'task']); + // The global registration is shared by every open editor — + // drop it only when this panel is the last one. + const others = [...this.editorStates.values()].filter((s) => s !== editorState && !s.isDisposed); + if (others.length === 0) { + await client.removeMonitor({ token: '*' }, ['deploy', 'task']); + } } } catch (error) { this.logger.error(`Stopping monitoring for project ${editorState.projectId}: ${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 `@apps/vscode/src/providers/ProjectProvider.ts` around lines 294 - 307, Update stopMonitoring so the shared { token: '*' } monitor is removed only after the closing panel leaves no other active editor states; continue removing the project-scoped monitor for the closing editor. Track active editor usage with a reference count or inspect editorStates after removing the panel, and preserve the existing deploy/task monitor removal behavior when the last editor closes.apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsx (1)
225-252: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMirror
statusUpdateanddashboardSnapshotwrites into the refs.
handleTaskEventfolds fromactiveTasksRef.currentandunknownTasksRef.current. These two handlers update state only. The effects at Lines 125-130 re-anchor the refs after the next render. A task event that arrives before that render folds from stale collections, so accumulated errors and warnings can be lost onrestart, and a snapshot-seeded task can be missed. Update the refs in the same handler, the same wayhandleTaskEventdoes.🐛 Proposed fix
case 'statusUpdate': { // Update errors/warnings for a specific source const statusKey = `${msg.projectId}.${msg.sourceId}`; setActiveTasks((prev) => { const next = new Map(prev); const existing = next.get(statusKey) ?? { running: false, errors: [], warnings: [] }; next.set(statusKey, { ...existing, errors: msg.errors, warnings: msg.warnings }); + activeTasksRef.current = next; return next; }); break; } @@ setActiveTasks(taskMap); setUnknownTasks(unknown); + activeTasksRef.current = taskMap; + unknownTasksRef.current = unknown; break;🤖 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 `@apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsx` around lines 225 - 252, Update the statusUpdate and dashboardSnapshot branches in the message handler to mirror every activeTasks and unknownTasks state change into activeTasksRef.current and unknownTasksRef.current immediately, using the same collection updates as handleTaskEvent. Ensure subsequent events before the next render fold from the newly written refs, preserving accumulated errors, warnings, and snapshot-seeded tasks.
🤖 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 `@apps/vscode/package.json`:
- Line 175: Update the useState<SettingsData> initializer in SettingsWebview.tsx
so pipelineTraceLevel starts as "full", matching the package configuration
default; retain the settingsLoaded overwrite behavior and confirm that full
tracing is intended for development runs.
In `@apps/vscode/src/providers/ProjectProvider.ts`:
- Around line 943-947: Update the logsession:open case to look up and close any
existing stream for message.sessionId before replacing it in sessions. Then
store the newly opened stream as currently done, preserving the existing
project, source, and teamId options.
In `@apps/vscode/src/providers/views/deployTypes.ts`:
- Around line 274-282: Update the documentation comment for the deployment:fetch
message type to remove the stale reference to the drawer’s own poll and describe
that re-fetches are triggered through the push-driven scheduleDeploymentRefetch
path. Preserve the existing descriptions of drawer-open and post-mutation
refreshes.
In `@apps/vscode/src/providers/views/Project/ProjectWebview.tsx`:
- Around line 355-359: Update the deployment:error flow to carry and validate
sourceId: add sourceId to the deployment:error variant in deployment types,
populate it in fetchAndPushDeployment, and require it to match
openDeploymentRef.current alongside teamId before calling setDeploymentError in
ProjectWebview.
In `@apps/vscode/src/providers/views/Settings/DeploySettings.tsx`:
- Line 120: Handle the removed team ID settings during extension configuration
migration: either restore rocketride.development.teamId and
rocketride.deployment.teamId in the VS Code package configuration as deprecation
stubs with messages stating that teams are no longer selected in settings, or
remove those stale keys during extension activation. Ensure the toggle handler
around onSettingsChange no longer leaves deployment.teamId orphaned in existing
user settings.
In `@apps/vscode/src/shared/util/deployMapping.ts`:
- Around line 131-138: The deployment mappers use invalid 'active' defaults for
missing states. In apps/vscode/src/shared/util/deployMapping.ts lines 131-138,
update the mapper returning the deployment row to use a valid default such as
'enabled' or omit rows without a state; apply the same change to the mapper
using deployment.state at lines 149-157, keeping both behaviors consistent.
In `@packages/ai/src/ai/account/deployment_backend.py`:
- Around line 554-556: Update iter_enabled’s org listing to catch StorageError
around list_entries('orgs/', recursive=False, include_files=False), matching the
_project_ids handling; when the orgs tree is absent, treat it as empty and
continue yielding no organizations instead of failing the scheduler feed.
- Around line 175-198: Prevent concurrent publishes in mutate from overwriting
an existing artifact path: update the _store.write_file call in mutate to use an
exclusive write-if-absent operation, propagating a collision failure so
_mutate_meta retries with a new version. Preserve the registry’s
sha256-to-artifact integrity and never-overwritten invariant.
In `@packages/ai/src/ai/common/sandbox.py`:
- Around line 184-185: Update the warnings context around compile_restricted to
use warnings.filterwarnings with category=SyntaxWarning and a message pattern
matching only the intended diagnostic, rather than ignoring all SyntaxWarning
instances. Preserve the existing suppression behavior for that specific message
while allowing unrelated syntax warnings through.
- Around line 184-185: Make the SyntaxWarning suppression in execute_sandboxed
thread-safe by protecting the warnings.catch_warnings() block with a shared
lock, or remove the local context and configure the filter once during process
startup. Ensure concurrent sandbox executions cannot mutate global warning
filters unsafely.
In `@packages/ai/src/ai/modules/task/commands/cmd_deploy.py`:
- Around line 265-271: Reorder the validation guards in the pipeline handling
flow so the isinstance/non-empty check runs before the pipeline.name check.
Ensure missing or empty pipeline values raise the existing “pipeline is required
and must be an object” error, while valid dictionaries without a name still
raise the pipeline.name validation error; keep the project_id validation after
these checks.
In `@packages/ai/src/ai/modules/task/run_log.py`:
- Around line 155-177: Update scope_paths for deploy runs to validate team_id
before constructing the Team store prefix. Reject IDs containing “/” or “\”,
equal to “.” or “..”, or beginning with “@”, “=”, or “.”, while preserving the
existing missing-team ValueError and valid return tuple.
In `@packages/ai/src/ai/modules/task/task_engine.py`:
- Around line 104-118: Update cap_trace_payload in
packages/ai/src/ai/modules/task/task_engine.py (lines 104-118) to build the
truncation marker and progressively shrink preview until json.dumps(marker) is
within CONST_TRACE_PAYLOAD_CAP, accounting for escaping of quotes and
backslashes rather than sizing only raw. In
packages/ai/tests/ai/modules/task/test_task_engine.py (lines 951-962), add an
oversized trace fixture containing many quotes and backslashes and retain the
serialized-length assertion to verify the cap for escaped content.
In `@packages/ai/src/ai/modules/task/task_scheduler.py`:
- Around line 346-365: Extract the duplicated deploy-event construction from
_notify_deploy_changed and DeployCommands._notify_deploy_changed into one shared
helper, such as broadcast_deploy_changed, preserving the EVENT_TYPE.DEPLOY type,
apaevt_deploy name, body keys, and org_id scope. Update both callers to use the
helper while retaining each caller’s existing error logging and best-effort
behavior.
In `@packages/ai/tests/ai/modules/task/commands/test_cmd_deploy.py`:
- Around line 84-89: Update the test harness in _make_conn so
server.broadcast_server_event is an AsyncMock, allowing awaited deploy
notifications to execute instead of being swallowed. Add an async mutation test
such as test_deploy_broadcasts_invalidation_event that invokes _deploy_deploy
and asserts the EVENT_TYPE.DEPLOY payload body and org_id arguments.
In `@packages/ai/tests/ai/modules/task/commands/test_cmd_task.py`:
- Around line 166-177: Update
test_on_execute_accepts_team_id_matching_session_team and
test_on_execute_run_kind_cannot_be_spoofed_via_dap to monkeypatch
account_mod.get_merged_env with an AsyncMock returning an empty dictionary
before invoking TaskCommands.on_execute, matching the neighboring tests and
removing reliance on ambient account or .env state.
In `@packages/ai/tests/ai/modules/task/test_task_engine.py`:
- Around line 951-962: Update
test_cap_trace_payload_truncates_oversized_payloads to use an oversized payload
containing many quotes and backslashes, such as repeated objects with an escaped
quote/backslash value, instead of the plain z string. Keep the existing
assertions so the test verifies cap_trace_payload bounds the serialized marker
even when preview content requires JSON escaping.
In `@packages/ai/tests/ai/modules/task/test_task_identity.py`:
- Around line 51-66: Extract the task/public token content construction from
start_task into a shared digest-builder helper, then update both start_task and
the test _digest function to call it. Ensure the helper owns the production
field names and kind values, while preserving the existing generate_token(None,
...) behavior and digest outputs.
In `@packages/ai/tests/ai/modules/task/test_task_scheduler.py`:
- Around line 70-97: Update the scheduler fixture’s server stub so
broadcast_server_event is an AsyncMock, allowing _start_run to await it
successfully; then in the happy-path run test, assert the captured event’s
event, body.action, and body.teamId fields match the DEPLOY contract.
In `@packages/client-python/src/rocketride/deploy.py`:
- Around line 246-249: Update the run method docstring in the deployment client
to remove the claim that the caller is the billing-attribution actor; describe
manual deploy runs as carrying no human identity and being actor-independent, or
accurately document the server-recorded attribution if one exists. Ensure the
published Python reference reflects this behavior.
- Line 191: Update the filter example in the deployment method documentation
near the filters parameter to use a valid deployment state, such as enabled,
instead of active; keep the example format and surrounding documentation
unchanged.
In `@packages/client-python/src/rocketride/log_stream.py`:
- Around line 139-154: Remove the orphaned LogRunKind type and its export from
rocketride.types, since LogStream.__init__ no longer accepts or produces a
run-kind value. Update any remaining consumers or documentation references to
use the current team_id-based stream scope instead.
In `@packages/client-python/src/rocketride/types/deploy.py`:
- Around line 78-86: Update the nullable field annotations in the relevant
TypedDict, including ttl, traceLevel, and lastRunAt, to use Optional[...] so
type checkers accept explicit None values while preserving their existing
non-null types and total=False absent-key behavior.
In `@packages/client-python/tests/test_deploy.py`:
- Around line 176-181: Update the get_task_token call in the deploy test to pass
team_id=TEAM, ensuring it resolves the deploy task created by deploy.run for
TEAM rather than the user’s default task.
In `@packages/client-typescript/docs/guide/methods/deploy.md`:
- Around line 37-51: Update the deployment method and endpoint tables in the
guide to document deploy.run(), deploy.artifact(), and deploy.setSourceConfig(),
including each method’s parameters and corresponding rrext_deploy subcommand.
Preserve the existing table structure and descriptions while ensuring all three
public APIs are discoverable.
In `@packages/client-typescript/docs/guide/methods/log.md`:
- Around line 4-7: Update the Python examples for open_event_stream() and
chapters() to remove the obsolete positional 'dev' runKind argument; omit the
third argument for the dev stream and pass team_id='team-prod' by keyword for
the deploy stream, including the additional example around the referenced
chapters usage.
In `@packages/client-typescript/src/client/client.ts`:
- Around line 1899-1905: Update the monitor-key serialization and parsing pair,
including the logic near _monitorStringToKey, to encode projectId, source,
pipeId, and teamId without ambiguity—either use structured data or symmetrically
escape every delimiter. Preserve reconnect round-trips when project or source
IDs contain “@”, and add a test covering that case.
In `@packages/client-typescript/src/client/deploy.ts`:
- Around line 103-109: Update the publish method parameter in the deploy client
to require a name by narrowing its type to PipelineConfig & { name: string };
leave PipelineConfig unchanged so non-deployment APIs such as use() continue
allowing optional names.
In `@packages/client-typescript/tests/deploy.test.ts`:
- Around line 184-192: The task-token lookup in the deploy test should resolve
the run using the same team owner as deploy.run(project, 'webhook_1', TEAM).
Update the client.getTaskToken call to pass the appropriate team identity, then
keep the token assertion and termination flow unchanged.
In `@packages/server/engine-lib/engLib/store/pipeline/pipeline_config.cpp`:
- Around line 349-359: Update the source-marking traversal in the pipeline chain
construction to recursively visit the full control closure, not only direct
entries in comp.controls. For every source-controlled component, mark it
visited, then traverse its output lane targets and control attachments using the
existing DFS behavior before appending pipeline.chain, so data descendants and
nested control components match generatePipelineStack().
In `@packages/shared-ui/src/components/deploy-panel/DeploymentView.tsx`:
- Around line 99-108: Remove the duplicated sourceConfig and
onSourceConfigChange declarations from IDeploymentViewProps, retaining one
definition of each with their existing types and documentation so
DeploymentView.tsx compiles without duplicate identifiers.
- Around line 208-267: Remove the duplicated definitions of configRow,
configLabel, configSelect, configHint, and configCheck in the style object S,
retaining one complete block with the existing values.
In `@packages/shared-ui/src/components/deploy-panel/DeployPanel.tsx`:
- Around line 515-541: Filter removed deployments from the where-live grid by
deriving a liveDeployments list alongside liveByVersion and mapping that list
instead of deployments. In the state-dot handler, allow toggling only for live
states, while preserving the existing enabled/disabled behavior for eligible
deployments.
- Around line 402-418: The run function leaves error state active while
pendingDeploy or pickerVersion keeps the confirmation dialog open, but the
dialog does not display it. Update the ConfirmDialog rendering paths for
pendingDeploy and pickerVersion to surface the current error inside the open
dialog, while preserving the existing setError behavior in run and clearing it
on subsequent attempts.
In `@packages/shared-ui/src/components/deploy-panel/index.ts`:
- Around line 15-21: Update the deploy-panel barrel exports in the module index
to include the missing public symbols: export IDeploymentRecordData from
DeploymentRecordPanel, and export SchedulePreviewResult plus describeTtl from
SchedulePanel. Preserve the existing exports and use the symbols’ defining
modules rather than requiring consumers to use deep imports.
In `@packages/shared-ui/src/components/deploy-panel/SchedulePanel.tsx`:
- Around line 286-305: Move the orphaned cron-description docblock so it is
directly above the describeCron function, leaving the separate describeTtl
documentation immediately above describeTtl. Preserve both descriptions and
their existing function behavior.
- Around line 223-227: Update the weekly schedule handling in the
cron-generation logic and the Save gate to require at least one selected day
when state.kind is "weekly"; do not fall back to "*" for an empty state.days
set. Align summaryWording with this validation so an empty weekly selection
cannot be saved or represented as a daily schedule.
In
`@packages/shared-ui/src/components/deploy-panel/TeamDeploymentRecordPanel.tsx`:
- Around line 278-295: Update the timeline-loading effect around fetchTimeline
and schedules to start all per-source fetches concurrently with Promise.all
rather than awaiting inside the loop. Preserve per-source failure isolation by
converting each rejected fetch to an empty or omitted timeline, then build the
Map from the completed results and retain the existing cancelled guard before
setTimelines.
In `@packages/shared-ui/src/components/deploy-panel/types.ts`:
- Around line 86-89: Update TeamDeploymentRow’s inherited-field omission to
exclude runningSources alongside schedules, matching the ProjectView.tsx
derivation that represents running state through TeamDeploymentSource.running.
Keep the sources field and other inherited properties unchanged.
In `@packages/shared-ui/src/modules/project/components/SourcePanel.tsx`:
- Around line 237-265: Update the timeline polling useEffect that invokes
fetchTimelineRef to include projectId in its dependency array, matching the DVR
session effect’s stream-binding dependencies so project changes immediately
refresh chapters for the active session.
- Around line 284-287: Update the destructuring in the SourcePanel component
around useTaskEvents so the unused rangeEvents binding is prefixed with an
underscore, preserving the returned value while satisfying the unused-variable
rule.
In `@packages/shared-ui/src/modules/project/hooks/useTaskEvents.ts`:
- Line 671: Preserve the absent-key contract for TaskChapter.traceLevel: in
packages/shared-ui/src/modules/project/hooks/useTaskEvents.ts:671, conditionally
spread traceLevel only when first.body.traceLevel is a string; in
packages/shared-ui/src/modules/project/components/SourcePanel.tsx:324-342, apply
the same conditional spread to the local begin-announcement chapter instead of
writing null. In
packages/shared-ui/src/modules/project/components/StatusPane.tsx:271-275, retain
the existing 'traceLevel' in chapter consumer check without direct changes.
In `@packages/shared-ui/src/modules/project/ProjectView.tsx`:
- Around line 523-527: Update renderDocPanel so its unused subView parameter is
prefixed with an underscore, preserving the existing call sites and behavior.
In `@packages/shared-ui/src/modules/server/components/ActivityPanel.tsx`:
- Around line 121-132: Handle unmatched TaskEvent.action values in both
getTaskEventDisplay in
packages/shared-ui/src/modules/server/components/ActivityPanel.tsx (lines
121-132) and the task-source branch of getTickerSummary in
packages/shared-ui/src/modules/server/components/OverviewPanel.tsx (lines
232-245). Add explicit task fallback handling that returns a defined tone,
label, and message in getTaskEventDisplay, and prevents getTickerSummary from
falling through to DashboardEvent casting; preserve existing behavior for begin,
end, restart, and running.
In `@packages/shared-ui/src/modules/sidebar/taskFold.ts`:
- Around line 38-39: Add teamId?: string to the tasks row type in
TaskLifecycleEvent, define a shared named row type, and update
foldDeployRunState and foldProjectDeployRuns to use it instead of duplicating
inline casts. Preserve the existing fields and behavior.
- Around line 66-236: Add unit tests for foldTaskEvent, foldDeployRunState, and
foldProjectDeployRuns. Cover deploy rows being excluded from dev-task results,
the running snapshot rebuilding active and unknown state, and correct
team/project scoping for deploy lifecycle events and snapshots, including
unrelated events being ignored. Verify the pure functions return expected fresh
state without mutating inputs.
- Around line 98-114: The bulk running snapshot in the task fold should preserve
existing errors and warnings for tasks that remain active. Update the `running`
case’s `nextActive.set` logic to reuse the existing entry for each
`${t.projectId}.${t.source}` when present, while still marking it running; only
initialize empty error and warning arrays for newly seen tasks.
In `@pyproject.toml`:
- Around line 93-100: Update the filterwarnings entry in pyproject.toml to match
the complete Starlette deprecation message, including the recommendation to
install httpx2, or target StarletteDeprecationWarning specifically. Keep the
suppression scoped to this warning and preserve the existing test dependency
configuration.
---
Outside diff comments:
In `@apps/vscode/src/providers/ProjectProvider.ts`:
- Around line 294-307: Update stopMonitoring so the shared { token: '*' }
monitor is removed only after the closing panel leaves no other active editor
states; continue removing the project-scoped monitor for the closing editor.
Track active editor usage with a reference count or inspect editorStates after
removing the panel, and preserve the existing deploy/task monitor removal
behavior when the last editor closes.
In `@apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsx`:
- Around line 225-252: Update the statusUpdate and dashboardSnapshot branches in
the message handler to mirror every activeTasks and unknownTasks state change
into activeTasksRef.current and unknownTasksRef.current immediately, using the
same collection updates as handleTaskEvent. Ensure subsequent events before the
next render fold from the newly written refs, preserving accumulated errors,
warnings, and snapshot-seeded tasks.
In `@packages/ai/src/ai/modules/task/task_server.py`:
- Around line 1234-1252: Align the Task construction with Task.__init__: either
explicitly declare and handle run_kind and trigger in Task.__init__ as task
configuration values, or remove both kwargs from this Task call if they are not
supported configuration. Keep the chosen behavior consistent between the
constructor signature and the call site.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: f67cae12-e466-4aac-96e7-71218796e10a
📒 Files selected for processing (156)
apps/shell-ui/scripts/freeze-shell-api.jsapps/shell-ui/scripts/tasks.jsapps/shell-ui/src/api.tsapps/shell-ui/src/components/layout/OverlayManager.tsxapps/shell-ui/src/components/layout/Sidebar.tsxapps/shell-ui/src/hooks/iframeBridgeProtocol.tsapps/shell-ui/src/hooks/useIframeBridge.tsapps/shell-ui/src/index.tsapps/shell-ui/src/lib/Documents.tsxapps/shell-ui/src/providers/AccountProvider.tsxapps/shell-ui/src/providers/EnvironmentProvider.tsxapps/shell-ui/src/providers/SettingsProvider.tsxapps/vscode/package.jsonapps/vscode/src/config.tsapps/vscode/src/extension.tsapps/vscode/src/providers/ProjectProvider.tsapps/vscode/src/providers/SettingsProvider.tsapps/vscode/src/providers/SidebarProvider.tsapps/vscode/src/providers/WelcomeProvider.tsapps/vscode/src/providers/shared/connection-message-handler.tsapps/vscode/src/providers/views/Account/index.tsxapps/vscode/src/providers/views/Environment/index.tsxapps/vscode/src/providers/views/Monitor/MonitorWebview.tsxapps/vscode/src/providers/views/Monitor/index.tsxapps/vscode/src/providers/views/Project/ProjectWebview.tsxapps/vscode/src/providers/views/Project/index.tsxapps/vscode/src/providers/views/Settings/ConnectionSettings.tsxapps/vscode/src/providers/views/Settings/DeploySettings.tsxapps/vscode/src/providers/views/Settings/SettingsWebview.tsxapps/vscode/src/providers/views/Settings/index.tsxapps/vscode/src/providers/views/Sidebar/SidebarWebview.tsxapps/vscode/src/providers/views/Welcome/WelcomeWebview.tsxapps/vscode/src/providers/views/components/ConnectionConfig.tsxapps/vscode/src/providers/views/components/panels/CloudPanel.tsxapps/vscode/src/providers/views/deployTypes.tsapps/vscode/src/providers/views/logTypes.tsapps/vscode/src/providers/views/setup.react.tsapps/vscode/src/providers/views/types.tsapps/vscode/src/shared/util/deployMapping.tsnodes/src/nodes/webhook/IEndpoint.pynodes/src/nodes/webhook/services.tools.jsonnodes/src/nodes/webhook/tools/README.mdnodes/test/test_db_arango.pynodes/test/test_tool_v0.pynodes/test/tool_filesystem/test_live_anchor.pynodes/test/tool_google_workspace/test_google_client.pypackages/ai/src/ai/account/__init__.pypackages/ai/src/ai/account/base.pypackages/ai/src/ai/account/deployment_backend.pypackages/ai/src/ai/account/deployment_store.pypackages/ai/src/ai/account/models.pypackages/ai/src/ai/common/sandbox.pypackages/ai/src/ai/modules/task/commands/cmd_debug.pypackages/ai/src/ai/modules/task/commands/cmd_deploy.pypackages/ai/src/ai/modules/task/commands/cmd_log.pypackages/ai/src/ai/modules/task/commands/cmd_misc.pypackages/ai/src/ai/modules/task/commands/cmd_monitor.pypackages/ai/src/ai/modules/task/commands/cmd_task.pypackages/ai/src/ai/modules/task/run_log.pypackages/ai/src/ai/modules/task/task_engine.pypackages/ai/src/ai/modules/task/task_scheduler.pypackages/ai/src/ai/modules/task/task_server.pypackages/ai/src/ai/modules/task/task_server_facade.pypackages/ai/src/ai/modules/task_http/task_data.pypackages/ai/src/ai/node.pypackages/ai/tests/ai/account/test_deployment_backend.pypackages/ai/tests/ai/account/test_deployment_store.pypackages/ai/tests/ai/account/test_store.pypackages/ai/tests/ai/modules/task/commands/test_cmd_debug.pypackages/ai/tests/ai/modules/task/commands/test_cmd_deploy.pypackages/ai/tests/ai/modules/task/commands/test_cmd_log.pypackages/ai/tests/ai/modules/task/commands/test_cmd_misc.pypackages/ai/tests/ai/modules/task/commands/test_cmd_monitor.pypackages/ai/tests/ai/modules/task/commands/test_cmd_task.pypackages/ai/tests/ai/modules/task/test_log_stream.pypackages/ai/tests/ai/modules/task/test_run_log.pypackages/ai/tests/ai/modules/task/test_run_log_team.pypackages/ai/tests/ai/modules/task/test_task_engine.pypackages/ai/tests/ai/modules/task/test_task_identity.pypackages/ai/tests/ai/modules/task/test_task_scheduler.pypackages/ai/tests/ai/modules/task/test_task_server.pypackages/ai/tests/ai/modules/task/test_task_server_facade.pypackages/client-python/docs/index.mdpackages/client-python/docs/log.mdpackages/client-python/src/rocketride/client.pypackages/client-python/src/rocketride/deploy.pypackages/client-python/src/rocketride/log.pypackages/client-python/src/rocketride/log_stream.pypackages/client-python/src/rocketride/mixins/events.pypackages/client-python/src/rocketride/mixins/execution.pypackages/client-python/src/rocketride/types/__init__.pypackages/client-python/src/rocketride/types/deploy.pypackages/client-python/src/rocketride/types/events.pypackages/client-python/src/rocketride/types/log.pypackages/client-python/tests/test_deploy.pypackages/client-python/tests/test_monitor_keys.pypackages/client-typescript/docs/guide/methods/deploy.mdpackages/client-typescript/docs/guide/methods/log.mdpackages/client-typescript/src/client/client.tspackages/client-typescript/src/client/deploy.tspackages/client-typescript/src/client/log-stream.tspackages/client-typescript/src/client/log.tspackages/client-typescript/src/client/types/deploy.tspackages/client-typescript/src/client/types/events.tspackages/client-typescript/src/client/types/log.tspackages/client-typescript/src/client/types/pipeline.tspackages/client-typescript/tests/deploy.test.tspackages/client-typescript/tests/monitor-keys.test.tspackages/server/docs/observability.mdpackages/server/engine-lib/engLib/store/pipeline/pipeline_config.cpppackages/server/engine-lib/engLib/store/stack.cpppackages/shared-ui/src/components/canvas/CanvasPanel.tsxpackages/shared-ui/src/components/canvas/index.tsxpackages/shared-ui/src/components/data-grid/DataGrid.tsxpackages/shared-ui/src/components/data-grid/tabulator-theme.csspackages/shared-ui/src/components/deploy-panel/DeployPanel.tsxpackages/shared-ui/src/components/deploy-panel/DeploymentRecordPanel.tsxpackages/shared-ui/src/components/deploy-panel/DeploymentView.tsxpackages/shared-ui/src/components/deploy-panel/SchedulePanel.tsxpackages/shared-ui/src/components/deploy-panel/TeamDeploymentRecordPanel.tsxpackages/shared-ui/src/components/deploy-panel/VersionRecordPanel.tsxpackages/shared-ui/src/components/deploy-panel/index.tspackages/shared-ui/src/components/deploy-panel/types.tspackages/shared-ui/src/components/detail-panel/DetailPanel.tsxpackages/shared-ui/src/components/sidebar-menu/SidebarMenu.tsxpackages/shared-ui/src/components/tab-control/TabControl.tsxpackages/shared-ui/src/components/tab-control/ViewMenuBadge.tsxpackages/shared-ui/src/components/tab-panel/TabPanel.tsxpackages/shared-ui/src/index.tspackages/shared-ui/src/modules/account/AccountView.tsxpackages/shared-ui/src/modules/account/components/ProfilePanel.tsxpackages/shared-ui/src/modules/billing/components/BillingDashboard.tsxpackages/shared-ui/src/modules/environment/EnvironmentView.tsxpackages/shared-ui/src/modules/project/ProjectView.tsxpackages/shared-ui/src/modules/project/components/SourcePanel.tsxpackages/shared-ui/src/modules/project/components/StatusPane.tsxpackages/shared-ui/src/modules/project/hooks/useTaskEvents.tspackages/shared-ui/src/modules/project/index.tsxpackages/shared-ui/src/modules/project/types.tspackages/shared-ui/src/modules/project/utils.tspackages/shared-ui/src/modules/server/MonitorView.tsxpackages/shared-ui/src/modules/server/components/ActivityPanel.tsxpackages/shared-ui/src/modules/server/components/ConnectionRecordPanel.tsxpackages/shared-ui/src/modules/server/components/ConnectionsPanel.tsxpackages/shared-ui/src/modules/server/components/OverviewGrid.tsxpackages/shared-ui/src/modules/server/components/OverviewPanel.tsxpackages/shared-ui/src/modules/server/components/TaskRecordPanel.tsxpackages/shared-ui/src/modules/server/components/TasksPanel.tsxpackages/shared-ui/src/modules/server/components/index.tspackages/shared-ui/src/modules/server/util/formatters.tspackages/shared-ui/src/modules/sidebar/SidebarView.tsxpackages/shared-ui/src/modules/sidebar/taskFold.tspackages/shared-ui/src/themes/styles.tspackages/shared-ui/src/types/viewMenu.tspackages/shell-api/versions/v0.d.tspyproject.toml
💤 Files with no reviewable changes (7)
- apps/vscode/src/providers/views/Settings/ConnectionSettings.tsx
- apps/vscode/src/providers/SettingsProvider.ts
- apps/vscode/src/providers/shared/connection-message-handler.ts
- apps/vscode/src/providers/WelcomeProvider.ts
- packages/ai/tests/ai/account/test_deployment_store.py
- packages/ai/src/ai/node.py
- packages/ai/src/ai/account/deployment_store.py
🛑 Comments failed to post (3)
packages/shared-ui/src/modules/project/components/SourcePanel.tsx (2)
237-265: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add
projectIdto the timeline effect dependencies.The DVR session effect at line 282 re-creates on
[source.id, runKind, projectId, openSession === null], but this timeline poll keys only on[source.id, runKind]. ThefetchTimelineclosure is stream-bound, so aprojectIdchange rebinds the session while the chapter poll keeps serving the previous stream's chapters. The activity bar then draws chapters that do not belong to the active session until the next 30-second tick.🐛 Proposed fix
- }, [source.id, runKind]); + }, [source.id, runKind, projectId]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.useEffect(() => { let cancelled = false; const refresh = async () => { if (!fetchTimelineRef.current) return; try { const next = await fetchTimelineRef.current(); if (cancelled) return; setTimeline((prev) => { if (!prev) return next; // A locally synthesized chapter and the server's chapter for // the SAME run carry different begin seqs (task-begin event // vs run-begin marker) — treat a fetched chapter within 2s // of a local begin as the same run, or the bar draws two // overlapping blocks. const localNewer = prev.chapters.filter((c) => !next.chapters.some((f) => f.beginSeq === c.beginSeq || Math.abs(f.beginTime - c.beginTime) < 2)); if (localNewer.length === 0) return next; return { ...next, completed: false, chapters: [...next.chapters, ...localNewer] }; }); } catch { // A never-logged stream (or transient error) keeps the last value. } }; void refresh(); const timer = setInterval(refresh, TIMELINE_REFRESH_MS); return () => { cancelled = true; clearInterval(timer); }; }, [source.id, runKind, projectId]);🤖 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 `@packages/shared-ui/src/modules/project/components/SourcePanel.tsx` around lines 237 - 265, Update the timeline polling useEffect that invokes fetchTimelineRef to include projectId in its dependency array, matching the DVR session effect’s stream-binding dependencies so project changes immediately refresh chapters for the active session.
284-287: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Prefix the unused
rangeEventsbinding.
rangeEventsis destructured fromuseTaskEventsand never read in this component. Rename the binding so it satisfies the unused-variable rule, or drop it from the destructure.♻️ Proposed fix
- const { statusAt, trackEvents, rangeEvents, chartSeries, trackStats, ingestLive, player, controller } = useTaskEvents({ + const { statusAt, trackEvents, chartSeries, trackStats, ingestLive, player, controller } = useTaskEvents({As per path instructions "Unused variables must be prefixed with underscore (_)".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const { statusAt, trackEvents, chartSeries, trackStats, ingestLive, player, controller } = useTaskEvents({ session, timeline, });🤖 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 `@packages/shared-ui/src/modules/project/components/SourcePanel.tsx` around lines 284 - 287, Update the destructuring in the SourcePanel component around useTaskEvents so the unused rangeEvents binding is prefixed with an underscore, preserving the returned value while satisfying the unused-variable rule.Source: Path instructions
packages/shared-ui/src/modules/server/components/ActivityPanel.tsx (1)
121-132: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Non-exhaustive
TaskEvent.actionswitches in two files. BothgetTaskEventDisplayandgetTickerSummaryswitch overTaskEvent.action('begin'/'end'/'restart'/'running') with nodefaultcase, unlike their siblingDashboardEventbranches in the same two files, which both handle the unmatched case explicitly.
packages/shared-ui/src/modules/server/components/ActivityPanel.tsx#L121-L132: add adefaultbranch togetTaskEventDisplayso an unmatched action returns a defined tone/label/message instead ofundefined.packages/shared-ui/src/modules/server/components/OverviewPanel.tsx#L232-L245: add a fallback inside the task-source branch ofgetTickerSummaryso an unmatched action does not fall through to castingevent.bodyasDashboardEvent.📍 Affects 2 files
packages/shared-ui/src/modules/server/components/ActivityPanel.tsx#L121-L132(this comment)packages/shared-ui/src/modules/server/components/OverviewPanel.tsx#L232-L245🤖 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 `@packages/shared-ui/src/modules/server/components/ActivityPanel.tsx` around lines 121 - 132, Handle unmatched TaskEvent.action values in both getTaskEventDisplay in packages/shared-ui/src/modules/server/components/ActivityPanel.tsx (lines 121-132) and the task-source branch of getTickerSummary in packages/shared-ui/src/modules/server/components/OverviewPanel.tsx (lines 232-245). Add explicit task fallback handling that returns a defined tone, label, and message in getTaskEventDisplay, and prevents getTickerSummary from falling through to DashboardEvent casting; preserve existing behavior for begin, end, restart, and running.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/shared-ui/src/components/deploy-panel/DeploymentView.tsx (1)
397-403: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared deployment-action verb map into one location.
The same verb-label object literal (
publish,deploy,rollback,enable,disable,errored,remove,pause,resume) is duplicated in two files. This commit had to edit both copies to addrollbackanderrored, which is direct evidence that keeping them separate risks the two panels drifting out of sync.
packages/shared-ui/src/components/deploy-panel/DeploymentView.tsx#L397-L403: import a sharedDEPLOY_ACTION_VERBSconstant (or a smallformatDeployAction(row)helper) instead of the inline map.packages/shared-ui/src/components/deploy-panel/TeamDeploymentRecordPanel.tsx#L340-L346: replace the duplicate inline map with the same shared constant/helper.🤖 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 `@packages/shared-ui/src/components/deploy-panel/DeploymentView.tsx` around lines 397 - 403, The deployment-action verb map is duplicated across both panels and should have one shared source. In packages/shared-ui/src/components/deploy-panel/DeploymentView.tsx lines 397-403, extract or import a shared DEPLOY_ACTION_VERBS constant (or formatting helper) and use it instead of the inline map; apply the same replacement in packages/shared-ui/src/components/deploy-panel/TeamDeploymentRecordPanel.tsx lines 340-346, preserving all existing action mappings and fallback behavior.
🤖 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 `@packages/shared-ui/src/modules/project/ProjectView.tsx`:
- Line 26: Update the fetchDeployArtifact contract used by ProjectView and
DeployPanel so it returns the canvas project payload matching
IVersionRecordPanelProps['pipeline'], rather than Promise<unknown>. Reuse the
existing payload type or introduce a shared artifact-return type, and propagate
that type through the related prop definitions without weakening the DeployPanel
pipeline contract.
---
Outside diff comments:
In `@packages/shared-ui/src/components/deploy-panel/DeploymentView.tsx`:
- Around line 397-403: The deployment-action verb map is duplicated across both
panels and should have one shared source. In
packages/shared-ui/src/components/deploy-panel/DeploymentView.tsx lines 397-403,
extract or import a shared DEPLOY_ACTION_VERBS constant (or formatting helper)
and use it instead of the inline map; apply the same replacement in
packages/shared-ui/src/components/deploy-panel/TeamDeploymentRecordPanel.tsx
lines 340-346, preserving all existing action mappings and fallback 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: ASSERTIVE
Plan: Pro Plus
Run ID: 569c00da-44a2-47ce-9b59-a449033536f1
📒 Files selected for processing (4)
packages/client-typescript/tests/deploy.test.tspackages/shared-ui/src/components/deploy-panel/DeploymentView.tsxpackages/shared-ui/src/components/deploy-panel/TeamDeploymentRecordPanel.tsxpackages/shared-ui/src/modules/project/ProjectView.tsx
Rod-Christensen
added a commit
that referenced
this pull request
Jul 31, 2026
) 33 of the 44 review findings verified against the tree and fixed; the rest are disputed with reasons on the PR or deliberately deferred (CAS artifact race, C++ chain traversal, monitor-key escaping, fold unit tests). Grouped by why each mattered: Event-contract coverage (the most important find): the stub servers in test_cmd_deploy and test_task_scheduler exposed broadcast_server_event as a plain MagicMock attribute — the notifier's await raised TypeError and its best-effort catch swallowed it, so the apaevt_deploy push that replaced the deploy surfaces' polling had ZERO test coverage; any body regression would have shipped silently. Both harnesses now use AsyncMock and pin the org-scoped envelope + identity-only body. The notifier itself is also now built in exactly one place (new deploy_events.broadcast_deploy_changed) with cmd_deploy and the scheduler delegating, because two hand-copied builders of a client cache-invalidation contract WILL drift when a field is added. Honest-marker integrity: cap_trace_payload sized its preview on the raw slice, but the preview holds already-serialized JSON — escaping on re-serialization (quotes, backslashes, \uXXXX control chars) could inflate a "capped" 1MB marker to ~2MB, defeating the cap exactly for the object-heavy traces it exists for. The marker is now sized as it travels, trimmed proportionally until it fits; the new test uses an escape-heavy fixture (the old all-'z' fixture was the one input shape that could not expose this). Dev-view fidelity: foldTaskEvent's bulk 'running' snapshot rebuilt every active task with empty errors/warnings, wiping accumulated indicators until the next statusUpdate — it now reuses the tracked entry (same rule as begin/restart). Both live-chapter producers normalized a missing traceLevel stamp to null, but the consumer contract reserves ABSENT for "no stamp" (pre-stamp markers, non-marker fallback events) and reads null as "tracing off" — a live run could dim the report card and claim tracing was disabled. Conditional spreads preserve the absent key. Deploy-surface UX: a rejected pointer move kept the confirm dialog open while the error line was suppressed under any open dialog — the failure now renders inside the dialog (and the team picker). The where-live grid rendered soft-removed deployments whose still-live state dot would RE-ENABLE them on click — removed rows are filtered at one place shared with the version badges. Weekly schedules with zero days selected silently stored a DAILY cron ('* * *' fallback) — Save now gates on a non-empty day set, same as the Advanced gate. Correctness fringes: deployMapping defaulted an unstamped state to 'active', a value outside the enabled/disabled/errored/removed vocabulary this branch introduced — now 'enabled'. iter_enabled lacked the fresh-store StorageError guard its sibling _project_ids has, so a first boot with no orgs/ tree crashed the scheduler feed instead of yielding nothing. publish validated pipeline.name before validating pipeline itself, so {} produced a misleading error. scope_paths now rejects path-unsafe team ids before embedding them in the store's '=<id>' grammar (defense in depth — upstream membership checks already gate, but the path builder should not trust its callers). VS Code's logsession:open leaked the previous stream when a session id was reused — it closes it first. SDK/doc sync (the docs are generated from these): both run() docstrings still claimed the caller is the billing-attribution actor — stale against this branch's actor-free deploy model (billing goes to org+team; who fired lives only in audit history). The 'active' filter example named a rejected state. Python's schedule TypedDict declared non-optional ttl/traceLevel/lastRunAt while the wire delivers None (total=False only covers absence) — now `| None`, matching the TS declarations. deploy.md was missing run()/artifact()/setSourceConfig() entirely; the log guides still showed the removed positional 'dev' argument in eight examples. The Python stop-path test now passes team_id=TEAM — the exact mirror of the jest fix (dea4333): an unscoped lookup resolves the caller's dev run and finds nothing. Settings hygiene: the four teamId keys retired by the dev-team model (8edd607) got deprecationMessage stubs following the existing migration-stub pattern, so lingering user settings.json entries render as deprecated instead of unknown; the SettingsWebview seed aligned to package.json's 'full' trace default. Plus mechanical tidy-ups: teamId added to the shared fold's row type (drops four inline casts), runningSources omitted from the derived row type (the derivation folds it into per-source running), unused subView parameter dropped, orphaned describeCron docblock reattached, stale "drawer's own poll" comment updated, missing barrel exports (IDeploymentRecordData, SchedulePreviewResult, describeTtl), record timelines fetched with Promise.all instead of N serial round trips, and get_merged_env stubbed in the two on_execute tests that reached the ambient account backend. Verified: full ai suite 1752 passed (was 1739), ruff clean, shell:check green, vscode tsc + build green, rocket-ui and shell-ui builds green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/vscode/src/providers/views/deployTypes.ts (1)
1-124: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument the deployment webview contract in
apps/vscode/docs/.
apps/vscode/src/providers/views/deployTypes.tsexposes deploy lifecycle DTOs and message protocols such asDeployLifecycleHostToWebview,DeploymentLoadPayload,DeploymentHostToWebview, andDeploymentWebviewToHost. Add this protocol surface to the existing webview contract doc inapps/vscode/docs/google-oauth.mdor updateapps/vscode/docs/usage.mdso VS Code extension surface changes are documented as required.🤖 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 `@apps/vscode/src/providers/views/deployTypes.ts` around lines 1 - 124, Document the deployment webview protocol exposed by deployTypes.ts in the existing VS Code webview contract documentation, preferably google-oauth.md or usage.md. Include the lifecycle DTOs and message protocols, including DeployLifecycleHostToWebview, DeploymentLoadPayload, DeploymentHostToWebview, and DeploymentWebviewToHost, while keeping the documentation aligned with the current exported contract.Source: Coding guidelines
♻️ Duplicate comments (1)
packages/ai/src/ai/account/deployment_backend.py (1)
175-198: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftConcurrent publishers can still corrupt the artifact registry.
mutateinpublish()writes the artifact file before the CAS commit ofmeta.json(line 191). Two concurrent publishers can compute the sameversion, and the second write silently overwrites the first artifact's bytes while the registry keeps the first publisher'ssha256.artifact()then raises a permanent "sha256 mismatch" for that version.The same window affects the non-atomic create path in
_mutate_meta: whencas_version is None(line 641-645), the write uses plainwrite_file, not a CAS write. Two concurrent first-time creators can both succeed, and the later write silently discards the earlier publisher's registry entry.This was raised in a prior review of this file and remains unresolved in the current code.
🛡️ One possible fix: commit the meta first, then write the artifact only after the version is uniquely allocated
async def mutate(meta: Dict[str, Any]) -> Dict[str, Any]: """Allocate the next version and append the registry entry.""" version = 1 + max((v['version'] for v in meta['versions']), default=0) entry = { ... 'artifactPath': artifact_path(org_id, project_id, version), } - # Write the artifact BEFORE committing the meta: an orphaned - # artifact file is harmless; a registry entry without its - # artifact would be a broken deploy target. - await self._store.write_file(entry['artifactPath'], data) meta['versions'].append(entry) ... return entry - return await self._mutate_meta(org_id, project_id, mutate) + entry = await self._mutate_meta(org_id, project_id, mutate) + # The version is now committed and unique: no other publisher can + # target this path, so the artifact write can never overwrite. + await self._store.write_file(entry['artifactPath'], data) + return entryThis still leaves a crash-window gap (a registry entry without its artifact). If that is unacceptable, keep the current order and make the artifact write an exclusive create so a colliding version write fails and forces a retry.
Also applies to: 616-652
🤖 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 `@packages/ai/src/ai/account/deployment_backend.py` around lines 175 - 198, Make publication concurrency-safe in publish()’s mutate callback and _mutate_meta: ensure version allocation and initial meta creation use atomic conditional writes with retry, including when cas_version is None. Prevent artifactPath writes from overwriting an existing artifact by using exclusive creation (or commit metadata first), and retry the publish when allocation or artifact creation collides so each registry version retains its matching artifact and sha256.
🤖 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 `@packages/client-typescript/docs/guide/methods/log.md`:
- Around line 54-55: Update the ownership documentation around
client.log.open_event_stream to distinguish user-owned development streams from
team-owned deployment streams. Explain that passing team_id targets a team
deployment continuum and requires authorization for that team, while preserving
the existing guidance for authenticated users’ own streams.
---
Outside diff comments:
In `@apps/vscode/src/providers/views/deployTypes.ts`:
- Around line 1-124: Document the deployment webview protocol exposed by
deployTypes.ts in the existing VS Code webview contract documentation,
preferably google-oauth.md or usage.md. Include the lifecycle DTOs and message
protocols, including DeployLifecycleHostToWebview, DeploymentLoadPayload,
DeploymentHostToWebview, and DeploymentWebviewToHost, while keeping the
documentation aligned with the current exported contract.
---
Duplicate comments:
In `@packages/ai/src/ai/account/deployment_backend.py`:
- Around line 175-198: Make publication concurrency-safe in publish()’s mutate
callback and _mutate_meta: ensure version allocation and initial meta creation
use atomic conditional writes with retry, including when cas_version is None.
Prevent artifactPath writes from overwriting an existing artifact by using
exclusive creation (or commit metadata first), and retry the publish when
allocation or artifact creation collides so each registry version retains its
matching artifact and sha256.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 0995fb14-f194-4496-b158-063146276a24
📒 Files selected for processing (32)
apps/vscode/package.jsonapps/vscode/src/providers/ProjectProvider.tsapps/vscode/src/providers/views/Settings/SettingsWebview.tsxapps/vscode/src/providers/views/deployTypes.tsapps/vscode/src/shared/util/deployMapping.tspackages/ai/src/ai/account/deployment_backend.pypackages/ai/src/ai/modules/task/commands/cmd_deploy.pypackages/ai/src/ai/modules/task/deploy_events.pypackages/ai/src/ai/modules/task/run_log.pypackages/ai/src/ai/modules/task/task_engine.pypackages/ai/src/ai/modules/task/task_scheduler.pypackages/ai/tests/ai/modules/task/commands/test_cmd_deploy.pypackages/ai/tests/ai/modules/task/commands/test_cmd_task.pypackages/ai/tests/ai/modules/task/test_run_log_team.pypackages/ai/tests/ai/modules/task/test_task_engine.pypackages/ai/tests/ai/modules/task/test_task_scheduler.pypackages/client-python/docs/log.mdpackages/client-python/src/rocketride/deploy.pypackages/client-python/src/rocketride/types/deploy.pypackages/client-python/tests/test_deploy.pypackages/client-typescript/docs/guide/methods/deploy.mdpackages/client-typescript/docs/guide/methods/log.mdpackages/client-typescript/src/client/deploy.tspackages/shared-ui/src/components/deploy-panel/DeployPanel.tsxpackages/shared-ui/src/components/deploy-panel/SchedulePanel.tsxpackages/shared-ui/src/components/deploy-panel/TeamDeploymentRecordPanel.tsxpackages/shared-ui/src/components/deploy-panel/index.tspackages/shared-ui/src/components/deploy-panel/types.tspackages/shared-ui/src/modules/project/ProjectView.tsxpackages/shared-ui/src/modules/project/components/SourcePanel.tsxpackages/shared-ui/src/modules/project/hooks/useTaskEvents.tspackages/shared-ui/src/modules/sidebar/taskFold.ts
Rod-Christensen
added a commit
that referenced
this pull request
Jul 31, 2026
Second and final review-fix pass — everything still open is now either
fixed here or answered on the thread with reasoning. Grouped by why:
Race-proof artifact registry (both backends): a CAS retry could
overwrite a committed artifact — two publishers racing on the same
version number both wrote v000N.json; the loser retried as v(N+1) and
left the winner's registry entry pointing at overwritten bytes,
breaking the sha256-verified immutability invariant. The artifact
filename now carries the content hash (v000N-<sha8>.json): racers
write DIFFERENT files, a version+hash collision implies identical
bytes, and readers are untouched because they always used the entry's
STORED artifactPath (verified: no other consumer derives the path).
Existing artifacts keep loading — their stored paths still resolve; no
migration. Same fix applied to the SaaS Db backend (paired saas
commit), which shares the path builder and had the identical race on
its unique-constraint retry loop.
Unambiguous monitor-key registry encoding (both SDKs): the
delimiter-joined key string could not round-trip free-text ids — a
source like 'chat@legacy' decoded as source 'chat' + teamId 'legacy',
and reconnect round-trips EVERY subscription through this pair, so the
misparse silently rewrote the subscription scope after a reconnect.
Project keys now use a JSON-array encoding (private registry state,
never wire); round-trip tests gain delimiter-hostile cases in both
SDKs.
publish() requires a name at compile time: the server rejects nameless
publishes (artifacts are immutable; pipelineName renders everywhere),
but PipelineConfig.name is optional, so the error only surfaced at
runtime. The TS signature narrows to PipelineConfig & { name: string }
(name stays optional for non-deployment APIs); both host call sites
now BUILD the narrowed shape via spread instead of mutating/ternary
(which would not narrow statically); the Python docstring documents
the requirement (no compile-time lever there).
deployment:error record guard: the message carried only teamId, so a
stale source-record fetch failing after the user switched records
within the same team painted its error onto the wrong drawer. The
message now stamps the addressed sourceId and the webview applies the
same record guard deployment:load already had.
fetchDeployArtifact typed, cast dropped: the host prop was
(version) => Promise<unknown> and ProjectView cast it into
DeployPanel's shape — hiding real drift. The prop now IS DeployPanel's
own fetchArtifact type, so contract drift surfaces at compile time.
Shared fold unit tests (new taskFold.test.ts): the folds are the ONE
dev/deploy classification both hosts delegate to — a regression breaks
both UIs at once. Pins deploy filtering on every foldTaskEvent path
(top-level AND per-snapshot-row), indicators surviving the bulk
rebuild, and team/project scoping of both deploy-run folds.
Smaller items: the sandbox SyntaxWarning suppression narrows from the
whole category to RestrictedPython's specific "Prints, but never reads
'printed' variable" message, so a script provoking any OTHER
SyntaxWarning still surfaces it; log.md's access-scoping paragraph
stops claiming all access is user-scoped (team-scoped deploy reads
resolve permissions against the TARGET team — membership is the right,
foreign/unknown teams read as a permission miss).
Answered on-thread, not changed: the deploy.test.ts teamId dispute
(the scoped lookup resolves exactly the run deploy.run created),
test_task_identity's independent digest re-implementation (sharing the
builder would make the pin tautological), the pyproject filter (valid
prefix regex against the real message), LogRunKind (published type;
retirement rides the runKind deprecation batch). The pipeline_config
chain traversal is CONFIRMED real against walkControl's fixpoint and
stays open as an engine follow-up — no C++ toolchain here to compile
or test an engine change responsibly.
Verified: full ai suite green, deployment-backend + saas Db backend +
monitor-key suites green (both SDKs), shared-ui fold tests green,
shell:check green, vscode tsc + build, rocket-ui/shell-ui/saas builds
green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/client-python/src/rocketride/deploy.py (1)
112-131: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider a typed guard for the required
nameonpublish().The docstring states
pipeline['name']is REQUIRED and server-enforced. The TypeScript client enforces this at compile time by narrowing toPipelineConfig & { name: string }(seepackages/client-typescript/src/client/deploy.ts, lines 96-106). The Pythonpublish()signature still accepts a plainpipeline: PipelineConfig, so a caller only discovers a missingnamefrom a server error at runtime, not from a type checker.Define a
TypedDictsubtype that makesnamerequired, and use it for thepipelineparameter, to give static-type-checked callers the same guardrail the TypeScript SDK has.♻️ Proposed direction
+class PublishablePipelineConfig(PipelineConfig): + name: Required[str] # requires `from typing import Required` (3.11+) or `typing_extensions` + async def publish( self, - pipeline: PipelineConfig, + pipeline: PublishablePipelineConfig, *, comment: str | None = None, deploy_to: str | None = None, ) -> PublishResult:Verify
PipelineConfig's actual definition (TypedDict vs plain dict) and the Python version target before applying, sinceRequired[...]needs Python 3.11+ ortyping_extensions.🤖 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 `@packages/client-python/src/rocketride/deploy.py` around lines 112 - 131, Define a TypedDict subtype of PipelineConfig with name marked as required, using the project’s supported typing approach (stdlib or typing_extensions), and update publish() to accept that subtype for pipeline. Preserve the existing pipeline structure and behavior while ensuring static type checkers require name for publish callers.packages/ai/src/ai/account/deployment_backend.py (2)
456-462: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winSkip persistence for a no-op
mark_run.When the deployment or source is absent, the callback at Line [458]–[460] leaves
metaunchanged._mutate_meta()still writes the object. Ifmeta.jsonis absent, this creates an empty registry for a project that has no deployment. That record then pollutes project discovery.Return a private no-op sentinel from the callback and make
_mutate_meta()skip persistence for that sentinel.Proposed fix
async def mutate(meta: Dict[str, Any]) -> None: dep = meta['deployments'].get(team_id) if dep and source_id in dep.get('schedules', {}): dep['schedules'][source_id]['lastRunAt'] = time.time() + else: + return _NO_CHANGE result = await mutate(meta) +if result is _NO_CHANGE: + return None🤖 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 `@packages/ai/src/ai/account/deployment_backend.py` around lines 456 - 462, Update the mark_run callback mutate and _mutate_meta flow so a missing deployment or source returns a private no-op sentinel, and _mutate_meta skips writing metadata when it receives that sentinel. Preserve the existing timestamp update and persistence behavior when the deployment and source schedule exist.
612-626: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDistinguish unavailable storage from missing metadata.
StorageErrormeans either not found or read failure for these APIs._read_meta()and_mutate_meta()currently treat all read failures as missing and create/seed a replacementmeta.json. Keep transient store failures out of_read_meta(), and reject/correct corruption separately from true not-found state before CAS writes in_mutate_meta().🤖 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 `@packages/ai/src/ai/account/deployment_backend.py` around lines 612 - 626, The _read_meta method currently treats every StorageError as missing metadata, allowing transient storage failures to trigger replacement metadata creation. Distinguish not-found errors from other StorageError cases, returning None only for genuine absence and propagating or preserving read failures; update _mutate_meta to validate this distinction and reject corrupted or unavailable metadata before performing CAS writes, while retaining replacement creation only for true not-found state.packages/client-typescript/src/client/client.ts (1)
1210-1235: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate the TypeScript API reference to include
teamId.
restart()andgetTaskToken()now acceptteamId, butdocs/agents/ROCKETRIDE_typescript_API.mdstill documents them withoutteamId.packages/client-typescript/docs/has noclient-typescript:docs-generatescript, so regenerate or update these reference signatures in the published docs.🤖 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 `@packages/client-typescript/src/client/client.ts` around lines 1210 - 1235, Update the published TypeScript API reference for restart() and getTaskToken() to document their optional teamId parameter, matching the current method signatures in the client implementation. Since no docs generation script exists, edit the corresponding reference signatures and parameter descriptions directly in the API documentation.Source: Path instructions
🤖 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 `@apps/vscode/src/providers/ProjectProvider.ts`:
- Around line 809-823: Update the pipeline name derivation in the parsed
pipeline construction to strip .pipe and .pipe.json suffixes case-insensitively,
matching the existing fileName logic and preserving the current basename
fallback behavior.
In `@packages/ai/src/ai/account/deployment_backend.py`:
- Around line 29-33: Update the artifact path construction in artifact_path to
use the complete sha256 digest instead of an eight-character prefix, and revise
the surrounding path documentation from <sha8> to the full digest format. Add a
regression test covering two payloads with the same eight-character prefix but
different full SHA-256 values, ensuring they produce distinct artifact paths and
remain loadable.
---
Outside diff comments:
In `@packages/ai/src/ai/account/deployment_backend.py`:
- Around line 456-462: Update the mark_run callback mutate and _mutate_meta flow
so a missing deployment or source returns a private no-op sentinel, and
_mutate_meta skips writing metadata when it receives that sentinel. Preserve the
existing timestamp update and persistence behavior when the deployment and
source schedule exist.
- Around line 612-626: The _read_meta method currently treats every StorageError
as missing metadata, allowing transient storage failures to trigger replacement
metadata creation. Distinguish not-found errors from other StorageError cases,
returning None only for genuine absence and propagating or preserving read
failures; update _mutate_meta to validate this distinction and reject corrupted
or unavailable metadata before performing CAS writes, while retaining
replacement creation only for true not-found state.
In `@packages/client-python/src/rocketride/deploy.py`:
- Around line 112-131: Define a TypedDict subtype of PipelineConfig with name
marked as required, using the project’s supported typing approach (stdlib or
typing_extensions), and update publish() to accept that subtype for pipeline.
Preserve the existing pipeline structure and behavior while ensuring static type
checkers require name for publish callers.
In `@packages/client-typescript/src/client/client.ts`:
- Around line 1210-1235: Update the published TypeScript API reference for
restart() and getTaskToken() to document their optional teamId parameter,
matching the current method signatures in the client implementation. Since no
docs generation script exists, edit the corresponding reference signatures and
parameter descriptions directly in the API documentation.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 1168f41f-39e2-4a3d-a28b-a3c160fca57d
📒 Files selected for processing (15)
apps/vscode/src/providers/ProjectProvider.tsapps/vscode/src/providers/views/Project/ProjectWebview.tsxapps/vscode/src/providers/views/deployTypes.tspackages/ai/src/ai/account/deployment_backend.pypackages/ai/src/ai/common/sandbox.pypackages/ai/tests/ai/account/test_deployment_backend.pypackages/client-python/src/rocketride/deploy.pypackages/client-python/src/rocketride/mixins/events.pypackages/client-python/tests/test_monitor_keys.pypackages/client-typescript/docs/guide/methods/log.mdpackages/client-typescript/src/client/client.tspackages/client-typescript/src/client/deploy.tspackages/client-typescript/tests/monitor-keys.test.tspackages/shared-ui/src/modules/project/ProjectView.tsxpackages/shared-ui/src/modules/sidebar/taskFold.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
packages/shared-ui/src/modules/project/components/SourcePanel.tsx (2)
417-436: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winReject chapter export when playback fails.
walker.play(...)returnsPromise<void>, but the export Promise can still resolve after the 700 ms settle timer despite a failedplay/pump. Await or catchwalker.play(...)so failures do not return partial events and leave an unhandled rejection.🤖 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 `@packages/shared-ui/src/modules/project/components/SourcePanel.tsx` around lines 417 - 436, Update the Promise wrapper around walker.play in the SourcePanel export flow so the returned playback promise is awaited or caught, rejecting the export when playback or pump fails instead of resolving with partial events. Preserve the existing 700 ms settle behavior for successful playback and ensure failures do not become unhandled rejections.
246-253: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not merge chapters by the two-second timestamp fallback.
beginSeqis the run’s permanent chapter identity. Concurrent streams can start within two seconds, soMath.abs(f.beginTime - c.beginTime) < 2can drop a distinct local chapter from the merged timeline. UsebeginSeqfor matching; use time only whenbeginSeqis absent and the local timestamp is the only stable key available.🤖 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 `@packages/shared-ui/src/modules/project/components/SourcePanel.tsx` around lines 246 - 253, Update the chapter merge logic in SourcePanel to match chapters by beginSeq only when both chapters have beginSeq values; use the beginTime comparison only when beginSeq is absent and the local chapter’s timestamp is the available stable key. Remove the unconditional two-second fallback so distinct concurrent runs are preserved, while retaining the existing completed and chapter-append behavior.packages/ai/src/ai/modules/task/task_engine.py (3)
2221-2238: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidate
stop_task(reason)before recording the run outcome.
Task.stop_task()accepts any string and line 1033 recordsokwhen_stop_reason == 'ttl'on a cancelled task. A request such asstop_task(reason='ttl')can make an explicit user stop logged as a successful TTL completion.Reject unsupported values before setting
_stop_reason. Keep the TTL termination path internal to the TTL scheduler.Proposed validation
async def stop_task(self, reason: str = 'user') -> None: + if reason not in {'user', 'ttl'}: + raise ValueError(f'Unsupported stop reason: {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 `@packages/ai/src/ai/modules/task/task_engine.py` around lines 2221 - 2238, Validate the reason argument at the start of Task.stop_task before acquiring termination state or assigning _stop_reason, accepting only the supported explicit-user stop value for external callers and preventing callers from supplying 'ttl'. Preserve the internal TTL scheduler path by using its existing internal mechanism to record a TTL termination without exposing that reason through stop_task(reason).
1229-1237: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winStamp every body identity field independently.
Preserve
project_id/sourceidempotence, but stamprunKind,teamId, anduserIdfrom_run_kind,team_id, andclient_idon every dict body. The currentrunKindguard skips identity stamping for bodies that already haverunKind, and body values should not be assumed correct.🤖 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 `@packages/ai/src/ai/modules/task/task_engine.py` around lines 1229 - 1237, Update the dict-body identity stamping logic in the task engine to assign runKind, teamId, and userId independently on every body from _run_kind, team_id, and client_id, without guarding on an existing runKind value. Preserve the existing idempotent project_id/source behavior.
2107-2134: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winKeep deploy run-begin metadata actor-independent.
RunLogWriter.open(user=...)writesuserinto therun-beginlifecycle event viauser=user. For deploy runs, the log store is team-scoped, so passself.team_id, a deploy-specific runner identity, or an explicit no-actor value here instead ofself.client_id.🤖 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 `@packages/ai/src/ai/modules/task/task_engine.py` around lines 2107 - 2134, The RunLogWriter.open invocation must avoid recording the developer client as the actor for deploy run-begin events. Update the user argument in the run-log initialization flow to use a deploy-appropriate team, runner, or explicit no-actor identity while preserving self.client_id for non-deploy runs.apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsx (1)
119-130: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winSynchronize every task mirror before the state setter.
updatesetsunknownTasksat line 185, butunknownTasksRef.currentremains stale until the effect runs. AtaskEventthat arrives in the same tick can fold with the old unknown-task list and replace the server snapshot with stale unknown entries. Apply the same pattern toentriesRefbeforesetEntriesinentriesUpdate.Suggested fix
if (msg.data.unknownTasks) { + unknownTasksRef.current = msg.data.unknownTasks; setUnknownTasks(msg.data.unknownTasks); } case 'entriesUpdate': + entriesRef.current = msg.entries; setEntries(msg.entries);🤖 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 `@apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsx` around lines 119 - 130, Update the task and entries update handlers, including entriesUpdate, so each corresponding mirror ref is assigned the new collection immediately before its state setter runs; ensure unknownTasksRef.current is updated before setUnknownTasks and entriesRef.current before setEntries, preserving the synchronous fold behavior used by activeTasksRef and unknownTasksRef.
🤖 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 `@apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsx`:
- Around line 250-253: Filter deployment runs when seeding dashboard snapshots
so they do not enter the development task maps. Update the snapshot DTO to
expose each task’s run classification, then adjust the snapshot loop in
SidebarWebview to skip tasks with runKind equal to deploy, or route seeding
through the shared foldTaskEvent classifier; preserve existing handling for
development tasks.
In `@packages/ai/src/ai/modules/task/task_engine.py`:
- Around line 279-280: Validate run_kind and trigger during task creation before
any storage-scoping logic uses them, allowing only run_kind values “dev” or
“deploy” and trigger values “manual” or “schedule”. Preserve the existing
default values, and ensure run_kind “deploy” remains restricted to trusted
in-process dispatch rather than externally accepted creation paths.
In `@packages/shared-ui/src/modules/project/components/SourcePanel.tsx`:
- Around line 265-267: Update SourcePanel’s stream identity handling to use a
stable key combining projectId, source.id, and runKind instead of
source.id/runKind alone. When this stream key changes, reset timeline,
runActive, the live-feed cursor, and openTrace, while preserving the existing
chapter refresh behavior tied to projectId.
---
Outside diff comments:
In `@apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsx`:
- Around line 119-130: Update the task and entries update handlers, including
entriesUpdate, so each corresponding mirror ref is assigned the new collection
immediately before its state setter runs; ensure unknownTasksRef.current is
updated before setUnknownTasks and entriesRef.current before setEntries,
preserving the synchronous fold behavior used by activeTasksRef and
unknownTasksRef.
In `@packages/ai/src/ai/modules/task/task_engine.py`:
- Around line 2221-2238: Validate the reason argument at the start of
Task.stop_task before acquiring termination state or assigning _stop_reason,
accepting only the supported explicit-user stop value for external callers and
preventing callers from supplying 'ttl'. Preserve the internal TTL scheduler
path by using its existing internal mechanism to record a TTL termination
without exposing that reason through stop_task(reason).
- Around line 1229-1237: Update the dict-body identity stamping logic in the
task engine to assign runKind, teamId, and userId independently on every body
from _run_kind, team_id, and client_id, without guarding on an existing runKind
value. Preserve the existing idempotent project_id/source behavior.
- Around line 2107-2134: The RunLogWriter.open invocation must avoid recording
the developer client as the actor for deploy run-begin events. Update the user
argument in the run-log initialization flow to use a deploy-appropriate team,
runner, or explicit no-actor identity while preserving self.client_id for
non-deploy runs.
In `@packages/shared-ui/src/modules/project/components/SourcePanel.tsx`:
- Around line 417-436: Update the Promise wrapper around walker.play in the
SourcePanel export flow so the returned playback promise is awaited or caught,
rejecting the export when playback or pump fails instead of resolving with
partial events. Preserve the existing 700 ms settle behavior for successful
playback and ensure failures do not become unhandled rejections.
- Around line 246-253: Update the chapter merge logic in SourcePanel to match
chapters by beginSeq only when both chapters have beginSeq values; use the
beginTime comparison only when beginSeq is absent and the local chapter’s
timestamp is the available stable key. Remove the unconditional two-second
fallback so distinct concurrent runs are preserved, while retaining the existing
completed and chapter-append 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: ASSERTIVE
Plan: Pro Plus
Run ID: 842b3b7e-70a8-4945-b876-660f1b93f157
📒 Files selected for processing (6)
apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsxpackages/ai/src/ai/common/sandbox.pypackages/ai/src/ai/modules/task/task_engine.pypackages/shared-ui/src/modules/project/components/SourcePanel.tsxpackages/shared-ui/src/modules/server/components/ActivityPanel.tsxpackages/shared-ui/src/modules/server/components/OverviewPanel.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
packages/shared-ui/src/modules/project/ProjectView.tsx (2)
323-354: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftBuild deployment rows from the immutable artifact sources.
teamDeploymentRowsderivessourceIdsfrom the current editable pipeline and schedule records. After the pipeline changes, the UI can show sources that are not in the deployed version and omit sources that still exist in that immutable artifact.Carry source IDs from each deployed artifact in
TeamDeployment, or fetch the immutable artifact before building these rows. Use the current pipeline only to resolve display names.🤖 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 `@packages/shared-ui/src/modules/project/ProjectView.tsx` around lines 323 - 354, Update teamDeploymentRows to derive each deployment’s sourceIds from the immutable deployed artifact exposed by TeamDeployment, while still including schedule-record IDs as needed; use the current sources collection only for sourceName lookup and never as the source-ID authority.
574-578: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGate the DeployPanel on the lifecycle fetch, not on deploy callbacks.
fetchDeployLifecyclecurrently gates the whole page. The read-only render path is now broken by addingonDeployPublish && onDeployVersion, so a host with lifecycle access but no publish/deploy capability shows “Deployment lifecycle is not available in this host yet” instead of the lifecycle view. RenderDeployPanelwhenfetchDeployLifecycleis present, and passonPublish/onDeployonly when the host provides them.🤖 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 `@packages/shared-ui/src/modules/project/ProjectView.tsx` around lines 574 - 578, Update the DeployPanel conditional in ProjectView to render whenever fetchDeployLifecycle is available, without requiring onDeployPublish or onDeployVersion. Pass onPublish and onDeploy only when their respective callbacks exist, preserving the read-only lifecycle view for hosts without publish/deploy capabilities.packages/shared-ui/src/modules/project/components/SourcePanel.tsx (1)
417-435: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winPropagate chapter reconstruction failures.
TaskEventSession.playreturns a Promise, butvoid walker.play(...)discards rejection. If the session disconnects or reconstruction fails, the timeout can resolvefetchChapterEventswith a partial event list while the rejection becomes unhandled. Reject the outer Promise and let the caller report the failed download.Suggested fix
- await new Promise<void>((resolve) => { + await new Promise<void>((resolve, reject) => { ... - void walker.play(undefined, 0, ({ event }) => { + void walker.play(undefined, 0, ({ event }) => { ... - }); + }).catch(reject); });🤖 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 `@packages/shared-ui/src/modules/project/components/SourcePanel.tsx` around lines 417 - 435, Update the Promise wrapper around walker.play in the chapter event reconstruction flow to catch rejected TaskEventSession.play calls and reject the outer Promise instead of resolving with partial events. Preserve the existing timeout and event collection behavior for successful playback, and ensure fetchChapterEvents receives the reconstruction failure for caller-side reporting.packages/ai/src/ai/modules/task/task_engine.py (2)
1238-1246: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winStamp all task identity fields from the validated task.
The condition checks only
runKind. If an incoming event already containsrunKind, missingteamIdanduserIdremain absent, and a stale or invalidrunKindis preserved. This breaks the task-event identity contract and can misclassify events in downstream filters. Assign all three fields from the task identity, or fill each field independently if existing trusted values must be preserved.Suggested fix
- if isinstance(body, dict) and 'runKind' not in body: - body['runKind'] = self._run_kind - body['teamId'] = self.team_id - body['userId'] = self.client_id + if isinstance(body, dict): + body['runKind'] = self._run_kind + body['teamId'] = self.team_id + body['userId'] = self.client_id🤖 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 `@packages/ai/src/ai/modules/task/task_engine.py` around lines 1238 - 1246, Update the event identity stamping logic around _run_kind, team_id, and client_id so runKind, teamId, and userId are each populated from the validated task identity. Do not let an existing or invalid runKind prevent missing fields from being stamped; assign all three fields consistently, or independently preserve only existing values explicitly considered trusted.
2230-2247: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject unsupported termination reasons.
The method documentation defines
userandttl, butstop_taskstores any string._terminatedtreats only the exact valuettlas successful, so a typo is recorded as a cancelled run. Validatereasonbefore assigning_stop_reason, and add tests for invalid values.Suggested fix
async def stop_task(self, reason: str = 'user') -> None: + if reason not in ('user', 'ttl'): + raise ValueError(f'invalid stop reason: {reason!r}') try:🤖 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 `@packages/ai/src/ai/modules/task/task_engine.py` around lines 2230 - 2247, The stop_task method accepts any string for the reason parameter but only documents 'user' and 'ttl' as valid values. Add validation in the stop_task method to reject unsupported reason values before assigning to _stop_reason, ensuring only the documented reasons are permitted and preventing typos from being silently stored as incorrect termination outcomes.packages/ai/src/ai/modules/task/commands/cmd_misc.py (1)
670-679: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject incomplete owner-scoped monitor keys.
owner_keyemitsp.{owner}.{project}.{source}. The wildcard form emitted here isp.{owner}.{project}.*. With the currentlen(parts) == 2branch,p.owner.projectis incorrectly labeled asproject.*even though it has no source or.*segment. Treat keys with fewer than three segments, or empty required segments, asTask monitor.The supplied
owner_keycontract uses an explicit source segment and an explicit wildcard segment.Proposed fix
- if len(parts) < 2: + if len(parts) < 3 or any(not segment for segment in parts[:3]): return 'Task monitor' ... - if len(parts) == 2 or (len(parts) == 3 and parts[2] == '*'): + if len(parts) == 3 and parts[2] == '*': return f'{project_label}.*'🤖 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 `@packages/ai/src/ai/modules/task/commands/cmd_misc.py` around lines 670 - 679, Update the monitor-key parsing around the parts split to return “Task monitor” when the key has fewer than three segments or any required owner, project, or source segment is empty. Remove the len(parts) == 2 labeling path, while preserving project label resolution and the existing wildcard formatting only for valid explicit source or .* segments.apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsx (1)
121-132: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate every synchronous mirror before its state setter.
updateandentriesUpdatecallsetEntries/setUnknownTaskswhile the refs still hold the previous values. If ataskEventarrives before the mirror effects run,handleTaskEventcan fold against staleentriesRef/unknownTasksRef. Assign each ref before calling its corresponding setter.🤖 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 `@apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsx` around lines 121 - 132, Update the synchronous mirror logic used by update and entriesUpdate so each corresponding ref (including entriesRef and unknownTasksRef) is assigned the newly computed collection before invoking setEntries or setUnknownTasks. Ensure handleTaskEvent can immediately fold against the latest values without waiting for mirror effects, while preserving the existing state updates.packages/shared-ui/src/components/deploy-panel/DeploymentView.tsx (1)
328-346: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winScope deploy timeline and live-event state by the deployment record.
DeploymentViewfetches timelines only bysourceId, while the drawer key includesdeployment.version; switching deployments can keep old version timelines while the new fetch is pending. Track a stable deployment/source scope and clear/replace timeline buckets when it changes.
liveBySourcegroupsliveEventsonly bybody.sourceeven though task live events are project- and team-scoped; group or filter by the deployment identity (for examplebody.project_id === projectId) and pass that scope to the source panels.🤖 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 `@packages/shared-ui/src/components/deploy-panel/DeploymentView.tsx` around lines 328 - 346, Scope DeploymentView’s timeline and live-event state by the deployment identity, including deployment.version and the relevant project/team identifiers, rather than sourceId alone. Update the useEffect around fetchTimeline and the timelines state to clear or replace buckets whenever the deployment/source scope changes, preventing stale timelines during fetches. Update liveBySource and the source-panel props to group or filter liveEvents by the current deployment scope, including the project identity.Source: Learnings
🤖 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 `@apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsx`:
- Around line 246-258: Update the dashboard snapshot rebuild in the task-map
seeding logic to preserve diagnostics for tasks already present in
activeTasksRef.current. Reuse the shared foldTaskEvent behavior or copy each
existing task’s errors and warnings into the new entry, while keeping empty
arrays for newly seen tasks and preserving the existing deploy filtering and
unknown-task handling.
In `@packages/ai/src/ai/modules/task/task_engine.py`:
- Around line 296-300: Align the task engine’s documented `trigger` contract
with the values accepted by its constructor and `RunLogWriter.open`: either
document and test the empty-string trigger for interactive development, or
update the boundary validation to reject `''`; apply the same change to the
corresponding later constructor documentation and validation.
---
Outside diff comments:
In `@apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsx`:
- Around line 121-132: Update the synchronous mirror logic used by update and
entriesUpdate so each corresponding ref (including entriesRef and
unknownTasksRef) is assigned the newly computed collection before invoking
setEntries or setUnknownTasks. Ensure handleTaskEvent can immediately fold
against the latest values without waiting for mirror effects, while preserving
the existing state updates.
In `@packages/ai/src/ai/modules/task/commands/cmd_misc.py`:
- Around line 670-679: Update the monitor-key parsing around the parts split to
return “Task monitor” when the key has fewer than three segments or any required
owner, project, or source segment is empty. Remove the len(parts) == 2 labeling
path, while preserving project label resolution and the existing wildcard
formatting only for valid explicit source or .* segments.
In `@packages/ai/src/ai/modules/task/task_engine.py`:
- Around line 1238-1246: Update the event identity stamping logic around
_run_kind, team_id, and client_id so runKind, teamId, and userId are each
populated from the validated task identity. Do not let an existing or invalid
runKind prevent missing fields from being stamped; assign all three fields
consistently, or independently preserve only existing values explicitly
considered trusted.
- Around line 2230-2247: The stop_task method accepts any string for the reason
parameter but only documents 'user' and 'ttl' as valid values. Add validation in
the stop_task method to reject unsupported reason values before assigning to
_stop_reason, ensuring only the documented reasons are permitted and preventing
typos from being silently stored as incorrect termination outcomes.
In `@packages/shared-ui/src/components/deploy-panel/DeploymentView.tsx`:
- Around line 328-346: Scope DeploymentView’s timeline and live-event state by
the deployment identity, including deployment.version and the relevant
project/team identifiers, rather than sourceId alone. Update the useEffect
around fetchTimeline and the timelines state to clear or replace buckets
whenever the deployment/source scope changes, preventing stale timelines during
fetches. Update liveBySource and the source-panel props to group or filter
liveEvents by the current deployment scope, including the project identity.
In `@packages/shared-ui/src/modules/project/components/SourcePanel.tsx`:
- Around line 417-435: Update the Promise wrapper around walker.play in the
chapter event reconstruction flow to catch rejected TaskEventSession.play calls
and reject the outer Promise instead of resolving with partial events. Preserve
the existing timeout and event collection behavior for successful playback, and
ensure fetchChapterEvents receives the reconstruction failure for caller-side
reporting.
In `@packages/shared-ui/src/modules/project/ProjectView.tsx`:
- Around line 323-354: Update teamDeploymentRows to derive each deployment’s
sourceIds from the immutable deployed artifact exposed by TeamDeployment, while
still including schedule-record IDs as needed; use the current sources
collection only for sourceName lookup and never as the source-ID authority.
- Around line 574-578: Update the DeployPanel conditional in ProjectView to
render whenever fetchDeployLifecycle is available, without requiring
onDeployPublish or onDeployVersion. Pass onPublish and onDeploy only when their
respective callbacks exist, preserving the read-only lifecycle view for hosts
without publish/deploy capabilities.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: d9261a21-d74d-4f87-847f-f09baee1c1f3
📒 Files selected for processing (11)
apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsxpackages/ai/src/ai/common/sandbox.pypackages/ai/src/ai/modules/task/commands/cmd_misc.pypackages/ai/src/ai/modules/task/task_engine.pypackages/ai/tests/ai/modules/task/commands/test_cmd_misc.pypackages/ai/tests/ai/modules/task/test_task_engine.pypackages/shared-ui/src/components/deploy-panel/DeploymentView.tsxpackages/shared-ui/src/modules/project/ProjectView.tsxpackages/shared-ui/src/modules/project/components/SourcePanel.tsxpackages/shared-ui/src/modules/server/components/ActivityPanel.tsxpackages/shared-ui/src/modules/server/components/OverviewPanel.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@apps/vscode/docs/deployment-webview.md`:
- Around line 25-27: Rewrite the shared protocol conventions in
deployment-webview.md to match the message tables and ProjectProvider handlers:
document that deploy:fetch and deployment:fetch lack requestId; deploy:data,
deployment:load, and deployment:error do not echo requestId; previewResult and
validateResult carry requestId and result without teamId; shell:connectionChange
has no teamId; and fetch responses use teamId with optional sourceId while
preview and validation responses use request correlation only.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: d1ccc1f4-0018-4df0-aa9d-e498bd51e69d
📒 Files selected for processing (5)
apps/vscode/docs/deployment-webview.mdpackages/client-typescript/docs/guide/methods/log.mdpackages/shared-ui/src/components/deploy-panel/DeploymentView.tsxpackages/shared-ui/src/components/deploy-panel/TeamDeploymentRecordPanel.tsxpackages/shared-ui/src/components/deploy-panel/types.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsx (3)
106-133: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSynchronize
unknownTasksRefbefore every state update.
handleTaskEventreadsunknownTasksRef.currentbefore React renders. Theupdatebranch at Line 187 callssetUnknownTasks(...)without updating that ref. If anupdatemessage is followed by ataskEventbefore the effect runs, the fold reads the old list and can restore stale unknown tasks. Assign the ref before the setter.Proposed fix
- if (msg.data.unknownTasks) setUnknownTasks(msg.data.unknownTasks); + if (msg.data.unknownTasks) { + unknownTasksRef.current = msg.data.unknownTasks; + setUnknownTasks(msg.data.unknownTasks); + }🤖 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 `@apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsx` around lines 106 - 133, Update the unknown-task update flow in handleTaskEvent so unknownTasksRef.current is assigned the next unknown-task list before calling setUnknownTasks. Preserve the existing state update behavior while ensuring any immediately following taskEvent reads the latest list rather than the pre-update value.
246-263: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReuse existing unknown-task metadata during snapshot rebuilds.
The shared
foldTaskEventcontract reuses an existing unknown entry during bulkrunningsnapshots. This loop creates a new object for every unknown row. Whent.nameis absent or differs, a dashboard snapshot can replace the existing display name with the source ID. ReuseunknownTasksRef.current.find(...)before creating a new entry.Proposed fix
if (!isKnownTask(t.projectId, t.source)) { - unknown.push({ projectId: t.projectId, sourceId: t.source, displayName: t.name || t.source, projectLabel: t.projectId.substring(0, 8) }); + const existing = unknownTasksRef.current.find((ut) => ut.projectId === t.projectId && ut.sourceId === t.source); + unknown.push(existing ?? { projectId: t.projectId, sourceId: t.source, displayName: t.name || t.source, projectLabel: t.projectId.substring(0, 8) }); }🤖 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 `@apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsx` around lines 246 - 263, The snapshot rebuild in the task-processing loop should preserve existing unknown-task metadata instead of always creating a new entry. Before pushing to unknown, reuse the matching entry from unknownTasksRef.current by projectId and sourceId; only create a new entry with the fallback displayName and projectLabel when no existing entry is found.
368-388: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winDocument the removed team-selection behavior.
This change removes the Development and Deployment team selectors and makes both entries open Settings. The supplied
apps/vscode/docs/deployment-webview.mddocuments protocol messages, but it does not document this user-visible sidebar behavior. Add the behavior to an appropriate document underapps/vscode/docs/.As per path instructions, “VS Code extension surface changes must be documented in
apps/vscode/docs/.”🤖 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 `@apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsx` around lines 368 - 388, Document the sidebar behavior represented by the Development and Deployment items in the appropriate file under apps/vscode/docs/: both entries no longer select teams and instead open their respective Settings sections, while retaining their status/progress display. Update the existing VS Code extension documentation rather than introducing unrelated changes.Source: Path instructions
packages/ai/src/ai/modules/task/task_engine.py (2)
2232-2249: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidate and preserve the first stop reason.
stop_task()currently accepts any string and always overwrites_stop_reason. A caller can pass'ttl'and make the cancelled-path run-log record anokoutcome with reasonttl, even when no TTL has expired. Also preserve the first reason when another stop request reaches the teardown log write.Proposed fix
+ if reason not in ('user', 'ttl'): + raise ValueError(f'invalid stop reason: {reason!r}') self._stop_requested = True - self._stop_reason = reason + if self._stop_reason is None: + self._stop_reason = 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 `@packages/ai/src/ai/modules/task/task_engine.py` around lines 2232 - 2249, Update stop_task to accept only the supported reasons, “user” and “ttl”, rejecting or normalizing any other value before updating state. Preserve the first stop reason by assigning _stop_reason only when no earlier stop reason exists, including when concurrent stop requests reach teardown logging; ensure cancellation cannot be reported as a successful TTL outcome unless the original reason was genuinely “ttl”.
2118-2139: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftKeep deployment identity team-scoped across logs and events.
Deploy storage, task identity, and run-log scope all use
teamId:'identity': { 'userId': self.client_id, 'teamId': self.team_id, 'orgId': self.org_id, }but deploy events/run logs still emit the initiating user. This contradicts team-owned deployment streams and can leak or reintroduce
client_idas the deployment owner.
packages/ai/src/ai/modules/task/task_engine.py#L2124-L2144: pass a team-scoped display identity toRunLogWriterandopen(user=...)forrun_kind == 'deploy', and clearuserIdin deploy run-begin events.packages/ai/src/ai/modules/task/task_engine.py#L1245-L1248: omit or clear bodyuserIdfor deploy events while keepingrunKindandteamId.🤖 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 `@packages/ai/src/ai/modules/task/task_engine.py` around lines 2118 - 2139, Keep deployment identity team-scoped in packages/ai/src/ai/modules/task/task_engine.py lines 2118-2139 by passing a team-scoped display identity to RunLogWriter and open(user=...) when run_kind == 'deploy', while preserving the existing user identity for dev runs. In packages/ai/src/ai/modules/task/task_engine.py lines 1240-1248, omit or clear body userId for deploy events while retaining runKind and teamId.
🤖 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 `@apps/vscode/docs/deployment-webview.md`:
- Around line 25-34: Update the correlation summary in the deployment webview
documentation to say result replies include requestId plus message-specific
fields, rather than claiming nothing else is present. Align the deployment:load
payload table with the record-drawer description by documenting its optional
sourceId field, or define that field within DeploymentLoadPayload.
---
Outside diff comments:
In `@apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsx`:
- Around line 106-133: Update the unknown-task update flow in handleTaskEvent so
unknownTasksRef.current is assigned the next unknown-task list before calling
setUnknownTasks. Preserve the existing state update behavior while ensuring any
immediately following taskEvent reads the latest list rather than the pre-update
value.
- Around line 246-263: The snapshot rebuild in the task-processing loop should
preserve existing unknown-task metadata instead of always creating a new entry.
Before pushing to unknown, reuse the matching entry from unknownTasksRef.current
by projectId and sourceId; only create a new entry with the fallback displayName
and projectLabel when no existing entry is found.
- Around line 368-388: Document the sidebar behavior represented by the
Development and Deployment items in the appropriate file under
apps/vscode/docs/: both entries no longer select teams and instead open their
respective Settings sections, while retaining their status/progress display.
Update the existing VS Code extension documentation rather than introducing
unrelated changes.
In `@packages/ai/src/ai/modules/task/task_engine.py`:
- Around line 2232-2249: Update stop_task to accept only the supported reasons,
“user” and “ttl”, rejecting or normalizing any other value before updating
state. Preserve the first stop reason by assigning _stop_reason only when no
earlier stop reason exists, including when concurrent stop requests reach
teardown logging; ensure cancellation cannot be reported as a successful TTL
outcome unless the original reason was genuinely “ttl”.
- Around line 2118-2139: Keep deployment identity team-scoped in
packages/ai/src/ai/modules/task/task_engine.py lines 2118-2139 by passing a
team-scoped display identity to RunLogWriter and open(user=...) when run_kind ==
'deploy', while preserving the existing user identity for dev runs. In
packages/ai/src/ai/modules/task/task_engine.py lines 1240-1248, omit or clear
body userId for deploy events while retaining runKind and teamId.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: d35e6df1-9083-4c57-bd9c-daa21c22143d
📒 Files selected for processing (3)
apps/vscode/docs/deployment-webview.mdapps/vscode/src/providers/views/Sidebar/SidebarWebview.tsxpackages/ai/src/ai/modules/task/task_engine.py
asclearuc
previously approved these changes
Jul 31, 2026
asclearuc
left a comment
asclearuc
left a comment
Collaborator
There was a problem hiding this comment.
Thanks @Rod-Christensen — this deploy-2 capstone is well staged.
Making the run's OWNER the identity (team for deploy, user for dev) and pushing that single idea through the token digest, the monitor keys, the run-log scope and the task lookup is the right call, and scope_paths / owner_key / foldTaskEvent each existing exactly once is what keeps a 159-file PR reviewable. The new coverage (test_task_identity.py, test_deployment_backend.py, test_run_log_team.py, the monitor-key round-trips) exercises the owner-scoped behavior well, and both new warning suppressions (pyproject.toml, sandbox.py) are narrowly scoped to one exact message each.
dsapandora
previously approved these changes
Jul 31, 2026
…rol wiring, proven by a live storage-anchor E2E
WHY this exists: applications need to call tools directly — file
storage operations, vector-store management, integrations driven by
application code rather than by an LLM. Until now the only way to host
a tool node was a full agent pipeline (admin-ui drags the entire chat
pipe around just to manage Qdrant), because tools could only attach to
an agent. This adds the first-class shape: a source endpoint whose only
job is to hold the pipeline open and own tool attachments, invoked with
client.tool() — no agent, no LLM, no data lanes.
* 'tools' endpoint (services.tools.json, webhook family). An
invoke-capable source ("invoke": {"tool"}), so the canvas offers it a
tool port and tool nodes wire to it with the exact control-edge
grammar they use for agents. It still runs the family web server —
that server IS the task's DAP data plane (client.pipe/client.tool
arrive through its 'data' route), so a data-less endpoint still needs
it; only the user-facing URL announcements are skipped.
* Engine: control edges FROM THE SOURCE now count as wiring
(stack.cpp generatePipelineStack/walkControl, sourceId passed as a
parameter). The inclusion walk added control-attached components only
when their consumer was already a stack member — true for agents
(always lane targets), never true for the source, which is the walk's
ROOT rather than a member. Components wired to the source were
silently dropped, so a tools pipeline instantiated nothing to invoke.
Notably, everything DOWNSTREAM of inclusion already anticipated this
shape: buildConnections maps control.from == source to
COMPONENT_SOURCE and the fromIndex == -1 branch binds source controls
onto the 'pipe' head filter's controller map. Only the inclusion test
had never learned the source spelling — this closes that gap, and is
strictly additive: pipelines without control.from == source edges take
a byte-identical path (2116 node unit tests + 1572 ai tests
unchanged). pipeline_config.cpp gets the same rule at its source-visit
so the validated 'chain' report agrees with what actually
instantiates.
* LIVE E2E for the storage anchor (nodes/test/tool_filesystem/
test_live_anchor.py). The tools endpoint finally makes this testable
without an agent: a real pipeline whose tool_filesystem node runs
inside the engine subprocess, exercising the full identity chain no
unit test covers end to end — task file ('identity' + 'storage'
blocks) -> rocketlib.getTask() -> Store.engine_file_store() ->
anchored writes in the real backing store. Three tests prove the
anchor contract in both directions (tool write visible to the session
fs API at the same plain path, session write visible to the tool,
list/stat lifecycle), and every test deletes what it created and
asserts it is gone — create/read/delete verified across both
identities. Run from the rocketride-server root (the saas root
brackets a Zitadel-auth server and the live tests skip):
./builder nodes:test --pytest-pattern=live_anchor
* Test hygiene found along the way:
- tool_google_workspace: the invalid-expiry broker test imported
google.auth.exceptions without the importorskip guard its siblings
carry, failing instead of skipping in envs where the mocks provide
googleapiclient but not google.auth.
- test_db_arango/test_tool_v0: the synthetic-package loaders passed
submodule_search_locations to spec_from_file_location, silently
turning module specs into PACKAGE specs (a package is its own
parent), so the pinned __package__ could never match __spec__.parent
— the source of the 8-per-worker DeprecationWarning wall under
Python 3.12, and a hard break on the future release that removes the
fallback. Plain module specs + letting module_from_spec derive
__package__ fix both files; warnings are gone.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…team pointers, credential-less scheduled dispatch Phase 3 of Deploy v5: deployments stop being user-scoped records with a stored login token and become TEAM deployments of immutable, org-registry pipeline versions — the backend the staged-rollout model (dev -> Staging -> Production as ordinary teams) and the v5 deployment screens are built on. WHY each piece exists: * Account module owns deployments (AccountBase deployments_* interface). OSS keeps deployments in files; SaaS will hold tens of thousands and needs indexed queries (per-team lists, org counts, one-query scheduler loads) — so the edition switch lives at the same virtualized seam as auth/billing/audit. Every default delegates through ONE hook (_deployment_backend()), so the SaaS edition overrides a single method. Callers (cmd_deploy, scheduler, future UI) never branch on edition, and clients stay edition-blind. * FileDeploymentBackend (deployment_backend.py) — the OSS implementation and the contract's reference semantics: PUBLISH writes an immutable artifact (orgs/<org>/files/.deployments/<pid>/v000N.json, sha256-locked and re-verified on every load — what was tested is provably what runs); DEPLOY points a team at a version (promotion and rollback are the same pointer move, rollback labeled honestly in history); removal is SOFT (state='removed' — audit trail and artifacts are never destroyed); every action records WHO with denormalized display/email so the audit survives user deletion (the enterprise requirement). meta.json updates are CAS-guarded so concurrent publishers/deployers serialize. Execution-time lesson pinned by a regression test: mutations must return the JOINED record — scheduler.sync() keys off projectId, and a raw internal dict here silently broke scheduling. * cmd_deploy rework: publish / deploy / list / get / versions / history / pause / resume / remove / schedule_set / preview. Team-permission checks live HERE, against the ADDRESSED team (task.monitor reads, task.control mutates, publish needs control somewhere) — the account module stays mechanical, mirroring how cmd_log gates the .logs system tree. preview is THE single cron evaluator (validate + next-N) so panel validation, "next:" lines, and future DVR ghosts can never disagree with what the scheduler actually fires. Old add/remove/status/update surface is gone with the model it served; SDKs re-land in Phase 5. * Scheduler keyed (team, project, source): the same project deployed by Staging and Production runs independently, and each SOURCE carries its own cron (a scheduled task IS project.source — the schedule's source becomes the pipeline entry point per run). At fire time the deployment is RE-READ so a pause/remove/pointer-move between ticks always beats the in-memory heap. Permission-shaped dispatch failures mark the deployment errored instead of retrying every tick. Legacy user-scoped records are NOT migrated (the credential model changed) — one startup warning tells owners to re-deploy. * Credential-less dispatch (start_server_task_as_team): scheduled runs no longer replay a stored user token — the server CONSTRUCTS the identity as the team (task permissions on that one team, nothing else). The env merge uses org+team layers only, so a deployment's configuration never depends on which human deployed it; the deploying user rides along as billing attribution, not authentication; nobody's departure can break production. run_kind='deploy'/trigger travel as trusted connection attributes read by on_execute — never DAP arguments — with a negative test proving a remote client cannot spoof a deploy run into the team continuum. * Event-body identity stamp: _forward_task_event now stamps teamId/ userId/runKind beside project_id/source on every event body. Two teams (or two devs) running the SAME project stop aliasing in watch UIs — client-side filtering works today, and the post-alb server-side subscription filters will match these exact fields. Deliberately the ONLY monitor-adjacent change: everything bus-shaped waits for feat/alb to land (cmd_monitor is rewritten there). Verified: 17-case contract suite (the behavioral spec the SaaS DB implementation mirrors), scheduler/cmd suites, spoof negatives — 1573 tests green, ruff clean. LIVE E2E against a real OSS server: publish -> deploy -> versions/history -> schedule -> scheduled TEAM dispatch fired -> the run wrote the tools_1.deploy.* continuum (dev/deploy separation live for the first time) -> lastRunAt stamped -> pause -> soft remove with surviving history. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…replay team runs
Phase 4 of teams-as-environments. Until now every run-log continuum lived
in the run OWNER's user tree, which is wrong for deployments: a deploy run
executes AS THE TEAM (the team is the environment), so its logs belong to
the team — any teammate with task.monitor should be able to watch, replay,
and DVR a scheduled run, and a user leaving the org must not take the
production run history with them.
What changed and why:
- run_log.py gains ONE scope helper, scope_paths(run_kind, client_id,
team_id) -> (store prefix, scope id): deploy -> ('@/Team/=<tid>/',
team_id), dev -> ('', client_id). Both RunLogWriter and RunLogReader
derive their control/segment paths, spool filenames, and registry keys
from this single function precisely so the two sides can never disagree
about where a stream lives. The prefix is the internal-identity scope
grammar resolved by the account FileStore to teams/<tid>/files/ — the
run-log subsystem stays the only legal writer of .logs content and no
new store plumbing is needed.
- A deploy run with an empty team_id is a hard ValueError, not a fallback
to the user tree: silently landing team output in a private namespace
would be a data-placement bug that only surfaces much later. Same rule
at the command layer: rrext_log now takes teamId, which is REQUIRED for
runKind=deploy and REJECTED for dev — both mismatches fail loudly as
contract errors instead of returning misleading empty results.
- Spool filenames and the WRITERS registry key off the SCOPE id (team id
for deploy, owner id for dev) so the same project deployed to Staging
and Production on one host can never collide. The reader's two
live-writer lookups were still keyed by client id — left that way, a
team-scoped reader would MISS the live team writer and delete
coordination / live-tail composition would silently degrade; they now
use the scope id too.
- task_engine passes team_id to the writer only when run_kind == 'deploy'
(the task file already carries the trusted teamId); the dev path is
byte-identical to before.
- cmd_log permission gate: team-scoped requests resolve task.monitor
(reads) / task.control (delete) against the ADDRESSED team via
verify_team_permission — membership is the read right, exactly the
deployment model — with uniform denial so a foreign team is
indistinguishable from a permission miss (no existence leak). Unscoped
dev requests keep the original default-team check.
- FileDeploymentBackend.history now breaks timestamp ties by append
order. Found via a real full-suite failure: rows were sorted on the
'at' float alone, and two mutations landing inside the clock's
resolution kept the OLDER row first (stable sort), so a rollback
recorded immediately after a deploy could be listed beneath it. The DB
backend already guarantees this via id.desc(); the file backend now
matches, pinned by a frozen-clock regression test.
Tests: test_run_log_team.py (scope contract; deploy segments land under
teams/<tid>/files/.logs with nothing leaked to the user tree; uuid-scoped
spool names still match the startup sweep regex; registry scope-keying;
unscoped deploy reader rejected) and commands/test_cmd_log.py (pairing
rules; teammate read passes with the default-team gate NOT consulted;
non-member denied uniformly; monitor-only member cannot delete). 1592
server tests green, ruff clean. Live E2E on a real server: scheduled team
dispatch wrote the continuum to teams/local/files/.logs, rrext_log
chapters+read with teamId returned the stream, both pairing negatives
rejected.
Known follow-ups: client SDKs/UI pass teamId in Phase 5/6 (until then a
deploy-stream read without teamId errors by design); pre-existing deploy
streams in user trees are orphaned (data intact, unreachable via the API).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e kind
teamId present addresses that team's DEPLOY continuum; absent addresses
the caller's own DEV stream. Since Phase 4 made that pairing a hard
bijection (teamId required for deploy, meaningless for dev), runKind
carried zero information on the wire — every legal request was fully
determined by teamId alone. A redundant argument is worse than none: it
creates the contradictory spellings we had to detect and reject.
Nothing has shipped to customers yet, so the argument is removed outright
rather than deprecated: _scope_for() derives (team_id, run_kind) from
teamId presence and runKind is ignored like any unknown argument. A stale
in-repo caller that still sends runKind='deploy' without teamId now reads
its (empty) dev stream instead of erroring — accepted transitional
behavior; the client SDKs and UI drop the argument next.
Run kind itself survives BELOW the wire: stream files are still
{source}.{dev|deploy}.* with per-kind retention — the server derives the
kind from scope instead of being told.
Live-verified: team-scoped chapters/read return the deploy continuum, the
unscoped spelling of the same identity returns the dev stream, and a
legacy runKind argument is confirmed ignored.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ges in the backend
The three rrext_deploy reads now speak the platform list convention
({rows,total,page,pageSize} + page/page_size/search/filters/sort), so the
deployment screens consume them through the same DataGrid contract as
every other list surface — written once against the final shape before
the SDKs are built.
WHERE paging happens differs by surface, deliberately:
- list / versions page at the COMMAND layer via the shared paginate_rows:
their row sets are bounded (the caller's team memberships; one
project's registry), so materialize-then-page is correct and keeps the
backends' interfaces simple.
- history pages in the BACKEND: the trail is append-only and unbounded by
design (enterprise audit), so deployments_history() takes the list-args
and returns the envelope itself. The OSS file backend applies
paginate_rows to the (small) materialized trail; the SaaS DB backend
runs the same contract as real SQL — ilike search, whitelisted filters
(at__gte/at__lte take epoch-second bounds, matching the row
representation), COUNT before ORDER/LIMIT/OFFSET — and never
materializes the full trail. Unknown filter keys are skipped on both
sides (never a SQL error or injection surface).
Every history row now carries 'seq', the stable append-order key (file
backend: trail position; DB: the row id). It is the default sort (seq
desc = newest first — timestamps can tie inside the clock's resolution,
seq cannot), the cross-page stability guarantee (no shuffled rows between
requests), and a ready-made row identity for the UI. The DB tiebreak
direction mirrors paginate_rows' ascending-tiebreak semantics exactly so
both backends order identically under client sorts; this also supersedes
the (at, append-index) tie-break with pure append order.
Contract pinned by mirrored tests on BOTH backends (page boundaries with
no overlap/gap, search, filters, unknown-key skip — the SaaS side against
real SQLite) plus cmd-layer tests proving history's list-args pass
through to the backend untouched. Live E2E green end to end on the
envelope shapes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… team; clients no longer pick a launch team Why: on the cloud the client-side team selection was never wired to launches — cloud dev runs silently landed on whatever the server-side default team happened to be, while the sidebar/settings/welcome team dropdowns persisted validated-but-unused config. The concept is now defined as the user's DEVELOPMENT TEAM: assigned on the profile screen, used for every dev/debug launch; deploy targets always name their team explicitly. Team context has exactly two unambiguous sources — the profile assignment for development, the deployment record for deploys — and no client ever chooses a team at launch again. - shell-api: teamId removed from the frozen use() contract and the base v0 regenerated from scratch (versions/ wiped + shell:freeze), per explicit sign-off — the append-only floors forbid removals through a normal freeze, so regeneration is the only path. - SDKs (TS + Python, kept in sync): use() no longer accepts a team; the run team is resolved server-side. - server: cmd_task/cmd_debug always run under account_info.defaultTeam (the profile dev team for client connections; the deployment's team for the trusted in-process dispatch, which synthesizes defaultTeam). A client-supplied teamId that differs is REJECTED rather than silently ignored so the caller is never surprised by which team a run was billed/authorized under. cmd_deploy requires an explicit teamId — no dev-team fallback, so changing a profile assignment can never silently re-target a deployment. - vscode: team selectors removed from Settings (dev + deploy), the Welcome screen, and the sidebar footer submenus; the development.teamId / deployment.teamId config keys, their migration entries, and the fetchTeams webview plumbing are deleted (dead config — stored values were never sent with a launch). - shared-ui ProfilePanel: "Set default"/"Default" relabeled to "Set as development team"/"Development team" to match the semantics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two warnings survived every run of the ai suite; both are silenced at
the correct layer rather than blanket-suppressed:
- sandbox: RestrictedPython's compile_restricted emits a SyntaxWarning
("Prints, but never reads 'printed' variable") for ANY sandboxed
script that prints — stdout is collected by PrintCollector, so the
hint is meaningless noise for every user script, not just tests.
Suppressed exactly that category around the compile call in the
sandbox itself, so runtime logs stay clean too.
- pytest: message-scoped filterwarnings ignore for starlette's
httpx-testclient deprecation, which fires at fastapi.testclient
IMPORT time in every suite using FastAPI's TestClient. The real fix
is upgrading the engine's test dependency to httpx2 — a constraints
decision recorded in the filter's comment as the follow-up.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… team-scoped logs
Phase 5 wave 1 of the deployment work: the TypeScript and Python SDKs
(kept in lockstep, as always) move to the server contract that landed on
this branch. Both SDKs still carried the PRE-teams deploy namespace
(add/status/update — subcommands the server no longer answers) and log
surfaces addressed by runKind, which is no longer a wire argument. This
is the client half of that contract, written once against the final
shapes so the UI work builds on a stable surface.
Deploy namespace (client.deploy), both SDKs:
- publish(pipeline, {comment, deployTo}) — snapshots the pipeline as an
IMMUTABLE, sha256-locked artifact version in the org registry. WHY a
publish/deploy split instead of the old add/update: what was tested is
provably what runs (the sha is verified on every load), and "update"
can never silently change what production executes.
- deploy(projectId, version, teamId) — points a TEAM at a version.
Teams are the environments; promotion and rollback are deliberately
this same pointer move, so there is no separate machinery to learn,
test, or get wrong. teamId is REQUIRED on every mutation (deploy/get/
pause/resume/remove/setSchedule) per the development-team decision:
deploy targets are always explicit, so changing a profile team
assignment can never silently re-target a deployment. teamId appears
as an OPTIONAL filter only on list/history.
- list/versions/history return the platform list envelope
({rows,total,page,pageSize}) with page/pageSize/search/filters/sort —
the same DataGrid contract as every other list surface (SDK pageSize
maps to the wire's page_size). history rows carry seq, the stable
append-order key, documented as the row identity.
- remove is a SOFT delete and says so: history and artifacts survive
forever (the enterprise audit requirement).
- preview(schedule, count) is THE single cron evaluator — clients never
parse cron, so what a UI shows can never disagree with what the
scheduler actually fires.
- Full type sets both sides (DeployArtifact, Deployment,
DeployHistoryEntry, PublishResult, SchedulePreview, envelopes),
exported at the package roots.
Log surfaces (client.log + the DVR LogEventStream), both SDKs:
- run_kind/runKind removed from every method and from LogStreamRef; the
scope IS the kind: team_id/teamId present addresses that team's
DEPLOY continuum (deploy runs execute as the team and log into its
tree — teammates with monitor rights watch/replay), absent addresses
the caller's own dev stream. WHY: since the pairing is a hard
bijection server-side, a run-kind argument carried zero information
and only created contradictory spellings to reject.
- LogEventStream carries the team scope through chapters/segment reads,
so DVR sessions over team deploy runs need no other change.
Execution-time find: the per-source schedule record's wire key is
'cron', not 'schedule' — the live integration tests caught the mismatch
and types/tests/docs now match the wire.
Verification: 12 Python + 10 TypeScript integration tests, rewritten
for the new contract and green against a live server (publish
immutability, rollback labeling + seq ordering, envelope paging, soft
remove with surviving history, schedule set/clear, preview); tsc
--noEmit, ruff, and prettier clean; a live SDK smoke read a real team
continuum through the new log signatures. Co-located docs updated in
the same change per the docs rule: python docs/index.md deploy section
+ docs/log.md, TS guide deploy.md (rewritten) + log.md + client.ts
getter examples.
Known follow-ups (UI fixup, next): test-ui's method catalog still lists
the retired deploy.add/status/update names as skipped entries;
shared-ui hosts still pass runKind-shaped refs to their adapters.
builder docs:build is currently blocked by an unrelated pre-existing
untracked node (tool_google_workspace) whose docs/ tree has no mount.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ne architecture campaign
Two intertwined bodies of work, landed together because the second grew
directly out of reviewing the first.
== 1. Deploy v5 surface completion (server + SDK + UI) ==
WHAT / WHY, server side:
- cmd_deploy: artifact subcommand (sha-verified pipeline JSON), run
subcommand (manual team dispatch, trigger='manual', honors schedule ttl,
refuses paused), publish now REQUIRES pipeline.name -- the registry renders
the name on every deploy surface, and unnamed artifacts rendered as GUIDs.
- Schedule run window (ttl): schedule_set stores/validates ttl seconds
('fixed window' in the UI); deployments_schedule_set carries it through
both backends. WHY: endpoint-style sources never terminate on their own --
the schedule record needs to say how long a run stays up.
- Dispatch ttl contract (task_server_facade): a deploy run ALWAYS sends ttl
in the execute arguments -- 0 = run until the pipeline exits, N = seconds.
WHY: omitting ttl silently applied the server's DEFAULT idle timeout
(15 min) -- a dev-task policy that would have idle-killed long deploy runs.
Pinned by the new test_task_server_facade wire-contract suite.
- cmd_monitor bulk 'running' snapshot entries + rrext_get_tasks descriptors
now carry the identity stamps (runKind/teamId). WHY: clients must
attribute deploy runs to DEPLOYMENTS, never the AD-HOC list; the per-event
stamp existed but the snapshot path bypassed it.
- SDKs (python + typescript, in lockstep): full deploy namespace
(publish/deploy/list/get/versions/history/pause/resume/remove/
set_schedule+ttl/preview/artifact/run); log surfaces speak teamId only --
the scope IS the kind, runKind is gone from the wire.
== 2. The architecture + naming campaign ==
Trigger: reviewing the deploy UI exposed invented names (DeployLifecycle-
"Bridge"/"Host"/"Provider" -- zero precedent for any of them), a ReactNode
slot no other View had, duplicated files, and three coexisting naming
schemes for the same role. Rule adopted (now in docs/README-app-styles.html):
never invent a name or folder shape without a sibling precedent; the
vocabulary below is that precedent, written down.
The settled ladder -- every surface, every host:
Provider -> View -> Panel -> Pane
*Provider absolute top level regardless of host: the data owner behind
the navigable client area. providers/ directory on EVERY host.
*View shared host-agnostic composition (strip + title + panel
placement); data-in/callbacks-out; the reason one UI runs
under the shell, in VS Code, and standalone.
*Panel one strip-switched drawing area, rooted in a TabPanel slot;
capability-fed (closures in, live rows as props) -- panels own
their fetch/refresh lifecycle.
*Pane a piece composing a Page/Panel; co-located with its Panel;
shared pieces live in the module's components/.
FILE NAME = COMPONENT NAME, always.
WHAT / WHY, piece by piece:
- Capability-fed DeployPanel; ReactNode slot deleted: ProjectView composes
its DEPLOY page itself from host closures (fetchDeployLifecycle presence
enables it) and derives publish gating from isDirty/isNew it already
held. Both hosts' filler adapter components deleted outright -- the whole
naming argument evaporated because the component no longer exists. VS
Code deploy messages ride ProjectWebview's ONE useMessaging handler (no
parallel window listener).
- Panel renames (platform-wide, incl. admin-ui consumers): Canvas.tsx ->
CanvasPanel.tsx -- the component inside was named Flow (THREE names for
one thing); DeployLifecyclePanel -> DeployPanel; SourceSection ->
SourcePanel; OverviewTab/ConnectionsGrid/TasksGrid/ActivityGrid ->
Overview/Connections/Tasks/ActivityPanel.
- Stock pair renamed to say what they switch: PageViewControl -> TabControl
(components/tab-control/), TabPanelContent -> TabPanel. Under the
vocabulary the strip switches PANELS; the old names said "page".
- shell-ui providers: views/*/*Page.tsx -> src/providers/*Provider.tsx
(Account, Settings, Environment); iframe bridge useShellEvents ->
useIframeBridge + ShellIframeProtocol.ts -> hooks/iframeBridgeProtocol.ts,
killing the one-letter trap with useShellEvent (an unrelated per-event
subscriber); shell-ui views/ directory is gone.
- shell-api v0 REGENERATED (pre-publication reset, design-owner waiver):
AccountPage/SettingsPage/useShellEvents were frozen shellApi members; the
renames are removals, so v0 was reset per the documented procedure. The
permanence rule is unchanged for post-publication.
- vscode structural fixes: deploy webview types routed through views/
types.ts like every other surface (deployTypes.ts remains host-only
plumbing for the extension tsconfig boundary, the environmentTypes
pattern); duplicated setup.react.ts hoisted to views/ (ReactFlow global
shim); Monitor webview gets the pinned ContentHeader (documentTitle) --
MonitorView grew the ProjectView-style host-gated header with per-page
subtitles; readonly deployment canvas drops its header (canvas pages are
full-bleed, the ProjectView design-page rule).
- SchedulePanel: verbs moved into the stock DetailPanel footer (Save
materializes on dirty, left of stationary Cancel -- the style-guide rule;
hand-rolled in-body footer deleted); schedule times render via the
browser locale (formatClock) like every other time in the product.
- Misc: DetailPanel Documents.setEditorLabelByUri (optional, constructor-
assigned -- contravariant-safe) for tab-title self-healing; grid row
padding tightened; account_service delete_team companion fix lives in the
paired saas commit.
The surface matrix after the campaign (cloud | vscode per surface):
Surface Provider (cloud) Provider (vscode) Shared View + Panels
--------------- ---------------------- ---------------------- --------------------------------
Pipeline Editor ProjectProvider.tsx ProjectProvider.ts ProjectView: CanvasPanel,
+ ProjectWebview SourcePanel xN, DeployPanel
Deployment tab DeploymentProvider.tsx DeploymentProvider.ts DeploymentView: tiles +
+ DeploymentWebview SchedulePanel, SourcePanel
(deploy), CanvasPanel RO
Monitor MonitorProvider.tsx MonitorProvider.ts MonitorView: Overview/Connections/
+ MonitorWebview Tasks/ActivityPanel
Sidebar SidebarProvider.tsx SidebarProvider.ts SidebarView: Explorer
+ SidebarWebview (PIPELINES|DEPLOYMENTS), AD-HOC
Account AccountProvider (s) AccountProvider.ts AccountView: 7 section panels
+ AccountWebview
Environment EnvironmentProvider(s) EnvironmentProvider.ts EnvironmentView
+ EnvironmentWebview
Settings SettingsProvider (s) SettingsProvider.ts NONE (unification pending)
+ SettingsWebview
Welcome -- WelcomeProvider.ts NONE
+ WelcomeWebview
Unknown-task readonly ProjectProv. StatusProvider.ts ProjectView (readonly)
App webviews WebviewProvider.tsx -- NONE (useIframeBridge +
iframeBridgeProtocol)
(s) = shell-ui src/providers/. Full matrix with messaging, types, and the
grid-persistence gaps: C:\tmp\ui-architecture-comparison.html; the durable
vocabulary + host anatomy documentation lives in the saas repo's
docs/README-app-styles.html (updated in the paired saas commit).
Verified: full server suite green incl. the new facade wire-contract
suite; SDK integration suites green against a live server; tsc clean on
shared-ui + vscode host; shell:freeze green on the regenerated v0; builds
green: shell-ui, vscode vsix, rocket-ui, admin-ui.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…mes, and the deploy drawer rework
The deploy-v5 screens went through live design review and the model under
them had three ambiguities baked in: "paused" meant two different things,
deployment timestamps were guesses, and a run that expired its configured
window screamed CANCELLED even though it did exactly what it was told.
This commit fixes the model and conforms the engine, SDKs, and UI to it.
Vocabulary: enabled/disabled vs paused
- A deployment cannot be "paused" — schedules belong to sources. The
deployment-level toggle is now enable/disable (the kill switch: a
disabled deployment runs NOTHING, including manual runs — no back-door
"just this once"), and the per-schedule flag is paused (one source's
schedule stops firing; its cron/ttl/settings are kept). Verbs:
enable/disable + schedule_pause/schedule_resume. The audit trail keeps
the old pause/resume words on rows written before the split — the
trail is immutable; history is never rewritten to match new language.
Honest run outcomes: ttl expiry is success, not cancellation
- The fixed run window exists for always-on sources (webhooks, watchers)
that never exit on their own. Reaping such a run at window end is the
system doing its job, so it now records outcome 'ok' with reason 'ttl'
(named after the knob the user set); only a human Stop records
'cancelled'. stop_task() carries a reason end-to-end: monitor ->
engine _stop_reason -> run-end marker, chapter, and terminal event.
Manual deploy runs have no window
- Run from the deploy screen dispatches ttl=None, which the trusted
facade sends as an explicit wire ttl:0 (no window). Rationale: the
window is a scheduler policy for unattended fires; a human-started run
must not die on its own — the user stops it. The facade ALWAYS sends
ttl explicitly because omitting it would silently apply the server's
default idle timeout, a dev-task policy that must never govern deploy
runs (pinned by the facade wire-contract tests).
Per-source execution settings ride the schedule row
- trace_level/debug_out live on the deployment schedule record rather
than a new table: the row doubles as the source's execution-config
record (cron '' = "manual", i.e. configured but unscheduled), so the
scheduler's fire-time re-read picks up schedule AND settings in one
read, and clearing a schedule keeps the row iff its settings are
non-default. Both the scheduler and manual Run dispatch
pipelineTraceLevel plus --trace=debugOut — the same execute contract
dev runs use, so a deploy run is not a special snowflake downstream.
Trace defaults to 'full' everywhere
- VS Code dev runs, cloud dev runs, and deploy runs all default
pipelineTraceLevel to 'full' (manifests + code fallbacks).
Observability is the product; turning it off is an explicit choice,
not the silent default. No "default (full)" sentinel option in any
dropdown — unset simply renders as full.
1MB trace/flow clip at the forward choke point
- cap_trace_payload() clamps a parsed trace to ~1MB at the single point
where apaevt_trace is parsed, so all three consumers of that body —
broadcast trace, derived flow, and the run-log continuum — share the
clamped structure and one oversized component payload cannot stall
the websocket. Over-cap bodies are CLIPPED, not shrunk: the preview
fills the cap (CAP - 1KB of marker headroom) with truncated/
originalBytes markers, so the user still gets a megabyte of real
data, not a token 64KB apology.
Chapters record their trace level
- Chapters now carry traceLevel so replay UIs can distinguish "tracing
was off for this run" from "no data yet". StatusPane grays the
trace-fed sections (same ghost treatment as the dead zone) and says
so honestly; chapters written before this stamp have no field and
keep the generic wording — absence of evidence is not an accusation.
Deploy UI: three drawers, one derivation
- All where-live derivation (genuine source detection via
config.mode === 'Source', name resolution, schedule joins) lives ONCE
in shared ProjectView; hosts pass raw facts (ids, cron strings,
timestamps) so cloud and VS Code cannot drift. The shared deploy
module moved to components/deploy-panel/ accordingly.
- Each click target now opens a drawer scoped to one identity:
- Source drawer (team/project/source): status tiles, Execution
settings form (Save/Cancel materialize only when dirty), history;
footer is [Pause|Resume][Run|Stop] only — no Remove/Disable/
Deploy, which are team-level acts.
- Team drawer: per-source status grid (last outcome, schedule, next
run, 7d runs + failures, running pill), "vN available" hint, and
the team verbs (Remove; Enable/Disable, Rollback, Deploy
version...). Built because the team header previously opened a
source-shaped drawer that answered none of the team's questions.
- Version drawer: readonly canvas of the immutable artifact — no
tabs, no verbs; you inspect a version, you don't operate it.
- Schedule editing consolidated on the DEPLOY page pills; the DEPLOY
page History card was removed as redundant with the team drawer.
- VS Code's standalone Deployment view/provider is deleted: drawers
render inside ProjectWebview via deployment:* messages (webview
drives post-mutation refresh; the host only acks), matching the
cloud architecture.
shell:regen is now a one-shot v0 contract reset
- --regen deletes the version floors and stubs the generated contract
files BEFORE preCheck, then re-mints v0. Previously a breaking
pre-freeze change wedged the freeze tooling against its own stale
floors and the reset had to be re-derived by hand every time.
SDKs (TypeScript + Python) migrated in lockstep: disable/enable,
pauseSchedule/resumeSchedule, setSourceConfig; Deployment.state/
deployedAt, DeploymentSchedule.paused/traceLevel/debugOut, LogChapter.
traceLevel; history action union widened (legacy words retained).
Tests: ai suite 1618 green. New pins: verb/state contract, disabled-
deployment run guard, schedule pause trio, source-config validation and
survival, facade ttl wire contract (always sent; 0 = no window; manual
run ignores the schedule window but honors its settings), scheduler
paused skip + dispatch settings, ttl stop reason, cap_trace_payload
boundaries, chapter traceLevel.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ven deploy surfaces
The same pipeline could not run in two teams, nor in development mode
while its deployment was live. Root cause: a task's identity was a
deterministic sha256 of {userId, project_id, source} — no team, no run
kind — so the second run hashed to the same tk_ token and start_task
refused it ("Pipeline is already running"). The same unscoped
(project, source) pair was the monitor key and the lookup key, so even
where runs could coexist, their events and lookups aliased.
1) Owner-scoped task identity
A task is now uniquely {owner}.{projectId}.{source}: the owner of a
dev run is its USER (dev = once per user, total), the owner of a
deploy run is its TEAM (once per team, actor-independent). The token
digests carry only the applicable owner field — the field NAME
(userId vs teamId) disambiguates the id spaces — plus a kind
discriminator so tk_ and pk_ digests can never collide (pk_ was
previously identical for two users running the same pair, making the
public-key auth scan nondeterministic). This mirrors the run-log
layer's shipped scoping (scope_paths: dev -> user, deploy -> team),
so storage and identity finally agree on what "the same run" means.
There is deliberately NO runKind in any client-facing contract:
teamId PRESENCE is the scope discriminator everywhere (lookups,
monitors, HTTP), matching the run-log API's "the scope IS the kind"
convention. get_task_control_by_project resolves by owner scope and
the legacy unscoped scan now refuses to guess between multiple
matches instead of returning an arbitrary run. The HTTP data path
resolves with the caller's identity (middleware attaches it) and
short-circuits pk_/tk_ credentials — previously any authenticated
principal could resolve any user's task token by path.
2) Actor-free deploy runs
The trusted team dispatch no longer carries a human identity at all
(scheduled AND manual runs): a deployment's behavior, billing, and
identity must never depend on which human deployed or fired it.
Billing debits attribute to org+team; who acted lives only in the
audit log and deployment history. Consequence fixed here: the
run-log writer anchored its store view at the run's user id, and an
internal-context store REQUIRES a concrete anchor — with the now-
empty deploy identity it raised, the engine swallowed the error by
design, and every deploy run persisted NOTHING (empty report cards,
no chapters, no teams/ tree on disk). The writer now anchors at the
run's owner (team for deploy), and a store-guard regression test
pins the failure mode.
3) Owner-scoped monitor keys
Monitor subscription keys are always owner-scoped:
p.{teamId|userId}.{projectId}.{source}. A project/source
subscription without teamId is bound server-side to the CALLER's own
dev runs; with teamId it addresses that team's deploy run. A dev run
and a deploy run of one pipeline can no longer alias in the
keyspace, and watching a teammate's dev run now requires its task
token (deliberate under the owner model). Both SDKs gained the
optional teamId on MonitorKey and — critically — symmetric changes
to BOTH key-string directions (encode and decode), because reconnect
replays subscriptions through that round-trip and an encoder-only
change would silently drop the team scope. The log streams now pass
their teamId to the live monitor, fixing deploy DVR sessions that
previously subscribed to the caller's dev scope and went silent.
4) One dev-view classification in shared code
Both UIs filtered the shared connection's event firehose per view
with hand-copied, diverging conditions: the VS Code sidebar's fork
was missing the deploy filter on bulk snapshots (deploy runs
re-entered the dev task list on every resync), and both hosts' live
feeds and the shared parseServerEvent filtered by project only, so
deploy events painted dev canvases and trace folds. The
classification now exists once in shared-ui: parseServerEvent takes
an explicit continuum scope (hosts parse the raw firehose as 'dev';
SourcePanel folds session events under its own runKind — session
events are already scope-correct and must not be dropped),
isDevLiveEvent/isTeamLiveEvent are the two membership tests, and
foldTaskEvent is the single apaevt_task lifecycle fold both sidebars
delegate to, fixing the fork by construction.
5) Push-driven deploy surfaces (no more polling)
The deployment drawer polled everything every 10s, the deployments
list every 15s, and the VS Code drawer refreshed on an interval —
"poll-based until the monitoring refactor lands push events". The
correctly-scoped delivery above is that prerequisite, so: a new
EVENT_TYPE.DEPLOY class (both SDK enums, in lockstep) carries
apaevt_deploy invalidation events broadcast org-scoped from every
deployment mutation (publish, deploy/rollback, enable/disable/
remove, schedule set/pause/resume, source config, manual run) and
from the scheduler (dispatch, errored). The body is identity only —
receivers re-fetch; the fetch stays the source of truth. Run state
folds from the task-event stream (foldDeployRunState per drawer,
foldProjectDeployRuns per project for the where-live running badges)
seeded by the server's catch-up snapshot on subscribe. The deploy
record's report cards get a minimal host-fed live feed (the dev
pages' liveEvents pattern, team-scoped) rather than the full
session-self-subscribe redesign — that is deliberately deferred to
the feat/alb monitoring rework to avoid building what it will
replace.
6) Where-live running badges
The DEPLOY page's "Where this project is live" rows now show a green
"running" pill on each live source and a roll-up on the team header,
driven by the same event fold — visibility that previously required
opening the drawer.
7) Nodes no longer print the startup banner
Every spawned node process printed the ASCII banner into the one
captured run log (a webhook run spawns two processes -> two banners
before the first useful line). The banner remains on the server's
own console.
Tests: new test_task_identity.py (digest/collision matrices — including
same-user-two-teams-dev collides BY DESIGN — owner-scoped lookup
semantics, key grammar), monitor key round-trip + reconnect tests in
both SDKs, store-guard regression, and the existing monitor/task/
deploy/scheduler suites updated to the owner-scoped world (1739+
passing). Docs: observability scope tables and the event-type registry
(BILLING was already missing) updated with the new scope convention.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… one planned test break
The PR's first full CI run surfaced two classes of failure:
1. shell:check (full tsc over shared-ui — the app bundlers transpile
without typechecking, so none of this failed locally):
- DeploymentView had its sourceConfig/onSourceConfigChange interface
members and the config* style entries duplicated wholesale
(TS2300/TS1117) — dropped the second copies.
- The history verb maps in DeploymentView and
TeamDeploymentRecordPanel didn't cover the full DeployHistoryRow
action union — added 'rollback' and 'errored' so every recorded
action renders a real verb instead of failing the index type.
- ProjectView passed its loosely-typed host closure
(fetchDeployArtifact -> Promise<unknown>) straight into
DeployPanel's concretely-typed fetchArtifact — cast at the one
composition point to the panel's own prop type rather than
rippling the concrete pipeline type into every host.
2. deploy.test.ts "run-now dispatches as the team and is stoppable":
the test resolved the run-now task with an UNSCOPED
getTaskToken({projectId, source}) — which, under owner-scoped
identity, correctly means "the caller's own dev run" and finds
nothing. The stop path now passes teamId, matching how the real UI
stop flows resolve deploy runs. This was the planned breakage class
for the identity change, not a regression.
Verified: shell:check green locally, client-typescript tsc clean, no
other test site uses an unscoped lookup against a deploy run.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
) 33 of the 44 review findings verified against the tree and fixed; the rest are disputed with reasons on the PR or deliberately deferred (CAS artifact race, C++ chain traversal, monitor-key escaping, fold unit tests). Grouped by why each mattered: Event-contract coverage (the most important find): the stub servers in test_cmd_deploy and test_task_scheduler exposed broadcast_server_event as a plain MagicMock attribute — the notifier's await raised TypeError and its best-effort catch swallowed it, so the apaevt_deploy push that replaced the deploy surfaces' polling had ZERO test coverage; any body regression would have shipped silently. Both harnesses now use AsyncMock and pin the org-scoped envelope + identity-only body. The notifier itself is also now built in exactly one place (new deploy_events.broadcast_deploy_changed) with cmd_deploy and the scheduler delegating, because two hand-copied builders of a client cache-invalidation contract WILL drift when a field is added. Honest-marker integrity: cap_trace_payload sized its preview on the raw slice, but the preview holds already-serialized JSON — escaping on re-serialization (quotes, backslashes, \uXXXX control chars) could inflate a "capped" 1MB marker to ~2MB, defeating the cap exactly for the object-heavy traces it exists for. The marker is now sized as it travels, trimmed proportionally until it fits; the new test uses an escape-heavy fixture (the old all-'z' fixture was the one input shape that could not expose this). Dev-view fidelity: foldTaskEvent's bulk 'running' snapshot rebuilt every active task with empty errors/warnings, wiping accumulated indicators until the next statusUpdate — it now reuses the tracked entry (same rule as begin/restart). Both live-chapter producers normalized a missing traceLevel stamp to null, but the consumer contract reserves ABSENT for "no stamp" (pre-stamp markers, non-marker fallback events) and reads null as "tracing off" — a live run could dim the report card and claim tracing was disabled. Conditional spreads preserve the absent key. Deploy-surface UX: a rejected pointer move kept the confirm dialog open while the error line was suppressed under any open dialog — the failure now renders inside the dialog (and the team picker). The where-live grid rendered soft-removed deployments whose still-live state dot would RE-ENABLE them on click — removed rows are filtered at one place shared with the version badges. Weekly schedules with zero days selected silently stored a DAILY cron ('* * *' fallback) — Save now gates on a non-empty day set, same as the Advanced gate. Correctness fringes: deployMapping defaulted an unstamped state to 'active', a value outside the enabled/disabled/errored/removed vocabulary this branch introduced — now 'enabled'. iter_enabled lacked the fresh-store StorageError guard its sibling _project_ids has, so a first boot with no orgs/ tree crashed the scheduler feed instead of yielding nothing. publish validated pipeline.name before validating pipeline itself, so {} produced a misleading error. scope_paths now rejects path-unsafe team ids before embedding them in the store's '=<id>' grammar (defense in depth — upstream membership checks already gate, but the path builder should not trust its callers). VS Code's logsession:open leaked the previous stream when a session id was reused — it closes it first. SDK/doc sync (the docs are generated from these): both run() docstrings still claimed the caller is the billing-attribution actor — stale against this branch's actor-free deploy model (billing goes to org+team; who fired lives only in audit history). The 'active' filter example named a rejected state. Python's schedule TypedDict declared non-optional ttl/traceLevel/lastRunAt while the wire delivers None (total=False only covers absence) — now `| None`, matching the TS declarations. deploy.md was missing run()/artifact()/setSourceConfig() entirely; the log guides still showed the removed positional 'dev' argument in eight examples. The Python stop-path test now passes team_id=TEAM — the exact mirror of the jest fix (dea4333): an unscoped lookup resolves the caller's dev run and finds nothing. Settings hygiene: the four teamId keys retired by the dev-team model (8edd607) got deprecationMessage stubs following the existing migration-stub pattern, so lingering user settings.json entries render as deprecated instead of unknown; the SettingsWebview seed aligned to package.json's 'full' trace default. Plus mechanical tidy-ups: teamId added to the shared fold's row type (drops four inline casts), runningSources omitted from the derived row type (the derivation folds it into per-source running), unused subView parameter dropped, orphaned describeCron docblock reattached, stale "drawer's own poll" comment updated, missing barrel exports (IDeploymentRecordData, SchedulePreviewResult, describeTtl), record timelines fetched with Promise.all instead of N serial round trips, and get_merged_env stubbed in the two on_execute tests that reached the ambient account backend. Verified: full ai suite 1752 passed (was 1739), ruff clean, shell:check green, vscode tsc + build green, rocket-ui and shell-ui builds green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Second and final review-fix pass — everything still open is now either
fixed here or answered on the thread with reasoning. Grouped by why:
Race-proof artifact registry (both backends): a CAS retry could
overwrite a committed artifact — two publishers racing on the same
version number both wrote v000N.json; the loser retried as v(N+1) and
left the winner's registry entry pointing at overwritten bytes,
breaking the sha256-verified immutability invariant. The artifact
filename now carries the content hash (v000N-<sha8>.json): racers
write DIFFERENT files, a version+hash collision implies identical
bytes, and readers are untouched because they always used the entry's
STORED artifactPath (verified: no other consumer derives the path).
Existing artifacts keep loading — their stored paths still resolve; no
migration. Same fix applied to the SaaS Db backend (paired saas
commit), which shares the path builder and had the identical race on
its unique-constraint retry loop.
Unambiguous monitor-key registry encoding (both SDKs): the
delimiter-joined key string could not round-trip free-text ids — a
source like 'chat@legacy' decoded as source 'chat' + teamId 'legacy',
and reconnect round-trips EVERY subscription through this pair, so the
misparse silently rewrote the subscription scope after a reconnect.
Project keys now use a JSON-array encoding (private registry state,
never wire); round-trip tests gain delimiter-hostile cases in both
SDKs.
publish() requires a name at compile time: the server rejects nameless
publishes (artifacts are immutable; pipelineName renders everywhere),
but PipelineConfig.name is optional, so the error only surfaced at
runtime. The TS signature narrows to PipelineConfig & { name: string }
(name stays optional for non-deployment APIs); both host call sites
now BUILD the narrowed shape via spread instead of mutating/ternary
(which would not narrow statically); the Python docstring documents
the requirement (no compile-time lever there).
deployment:error record guard: the message carried only teamId, so a
stale source-record fetch failing after the user switched records
within the same team painted its error onto the wrong drawer. The
message now stamps the addressed sourceId and the webview applies the
same record guard deployment:load already had.
fetchDeployArtifact typed, cast dropped: the host prop was
(version) => Promise<unknown> and ProjectView cast it into
DeployPanel's shape — hiding real drift. The prop now IS DeployPanel's
own fetchArtifact type, so contract drift surfaces at compile time.
Shared fold unit tests (new taskFold.test.ts): the folds are the ONE
dev/deploy classification both hosts delegate to — a regression breaks
both UIs at once. Pins deploy filtering on every foldTaskEvent path
(top-level AND per-snapshot-row), indicators surviving the bulk
rebuild, and team/project scoping of both deploy-run folds.
Smaller items: the sandbox SyntaxWarning suppression narrows from the
whole category to RestrictedPython's specific "Prints, but never reads
'printed' variable" message, so a script provoking any OTHER
SyntaxWarning still surfaces it; log.md's access-scoping paragraph
stops claiming all access is user-scoped (team-scoped deploy reads
resolve permissions against the TARGET team — membership is the right,
foreign/unknown teams read as a permission miss).
Answered on-thread, not changed: the deploy.test.ts teamId dispute
(the scoped lookup resolves exactly the run deploy.run created),
test_task_identity's independent digest re-implementation (sharing the
builder would make the pin tautological), the pyproject filter (valid
prefix regex against the real message), LogRunKind (published type;
retirement rides the runKind deprecation batch). The pipeline_config
chain traversal is CONFIRMED real against walkControl's fixpoint and
stays open as an engine follow-up — no C++ toolchain here to compile
or test an engine change responsibly.
Verified: full ai suite green, deployment-backend + saas Db backend +
monitor-key suites green (both SDKs), shared-ui fold tests green,
shell:check green, vscode tsc + build, rocket-ui/shell-ui/saas builds
green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… threads
CodeRabbit's review body carried findings that never appeared as inline
threads; verified each against the tree, fixed five, disputed one:
SidebarWebview sync-ref gap (real fold bug): handleTaskEvent advances
activeTasksRef/unknownTasksRef synchronously, but the statusUpdate and
dashboardSnapshot branches only called setState — the refs re-anchor a
render later, so a task event landing in the same tick folded from
STALE refs, dropping just-set errors/warnings or the snapshot seed.
Both branches now advance the refs first, same rule as the event fold.
Task constructor honesty: run_kind/trigger travelled via **kwargs and
leaked into DAPBase.__init__'s kwargs (they are task classification,
not DAP configuration). Promoted to declared keyword parameters with
documented semantics; the call site is unchanged and the values no
longer reach the transport layer.
Sandbox warnings thread-safety: warnings.catch_warnings() swaps the
interpreter-GLOBAL filter list, so concurrent execute_sandboxed calls
raced on it (one thread's restore can clobber another's filter
mid-compile). A module lock now serializes the (fast) compile step.
SourcePanel: the chapters-refresh effect gains projectId in its deps —
the DVR-session effect next to it already re-binds on project switch,
and the timeline must follow the same stream identity; the unused
rangeEvents destructure is dropped.
Activity surfaces tolerate unknown task actions: getTaskEventDisplay
returned undefined display fields and getTickerSummary fell through to
the dashboard-event cast when a task event carried an action outside
begin/end/restart/running (e.g. from a newer server). Both now render
a generic task line instead.
Disputed, no change: "stopMonitoring drops the shared {token:'*'}
monitor while other editors need it" — the SDK's monitor registry
refcounts per (key, type); each panel's paired add/remove decrements
and the server subscription drops only when the LAST panel closes.
The paired calls are exactly the refcounted contract.
Verified: full ai suite green, shared-ui tests green (41), shell:check
green, vscode tsc + build, rocket-ui/shell-ui/saas builds green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three new inline threads from the review of 35694c4, all verified real: Dashboard snapshot bypassed the dev/deploy classification: the shared foldTaskEvent filters deploy runs on every EVENT path, but the VS Code sidebar's initial dashboardSnapshot seed looped over raw dashboard rows and admitted every incomplete task — a deploy run visible to the caller would appear in the dev sidebar on load, and its later deploy-stamped events (filtered) would never remove it. The server's dashboard/list task rows now carry the runKind stamp (additive, same as rrext_get_tasks) and the snapshot seed skips deploy rows with the same rule the fold applies. rocket-ui has no dashboard seed path — its sidebar seeds from the monitor catch-up snapshot, which the shared fold already filters per-row. Stream-local DVR state now remounts on project switch: SourcePanel instances were keyed `${src.id}.${runKind}`, so two projects sharing a source id (every project's webhook_1) REUSED component state across a project switch — old chapters, traces, and feed cursors could bleed into the new project's panel. The key now carries the full stream identity (projectId.sourceId.runKind) in both mounts (ProjectView dev sections and the deploy record's runs page), so a project change remounts fresh state — strictly stronger than clearing fields one by one, and the standing key={id} rule for id-driven instances. Run classification is a CLOSED vocabulary, validated at the one construction choke point: run_kind/trigger gate storage anchors, run-log scoping, and token ownership downstream, but Task accepted any string. Task.__init__ now rejects run_kind outside {dev, deploy} and trigger outside {'', manual, schedule} ('' is the interactive-dev spelling; only the trusted dispatch stamps manual/schedule). The DAP spoofing path was already closed — this makes the invariant hold for every future caller, with a rejection test. Test stubs updated: the cmd_misc control stand-ins gain the run_kind attribute real controls always carry — without it the row builder's per-control catch silently skipped every row. Verified: full ai suite green (1754), shared-ui tests green, shell:check green, vscode tsc + build, rocket-ui/shell-ui/saas builds green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…edup) Re-applies the review fixes that were discarded in the working-tree reset before they could be committed: - log.md's openEventStream section now states the scope rule in prose (omit teamId = your own dev stream; teamId = that team's deploy continuum, requiring task.monitor membership on the TARGET team) — the section only carried it as a code comment. - New apps/vscode/docs/deployment-webview.md documents the deployment webview protocol exported by deployTypes.ts (both message protocols, the DTO-mirroring rule, requestId correlation, the push-driven refresh convention) — the co-located-doc rule for the deploy webview contract, following google-oauth.md's contract-table style. - DEPLOY_ACTION_VERBS: the history grids in DeploymentView and TeamDeploymentRecordPanel carried byte-identical action-to-verb maps — display vocabulary that WILL drift when an action is added. One shared constant now lives in the deploy-panel contract module (types.ts) with both formatters reading it. The fourth lost item (the store create-exclusive contract for the meta-creation race) is deliberately NOT restored — the API change was declined; the process-local-lock alternative is proposed separately. Verified: shell:check green, shared-ui tests green, rocket-ui/vscode/ shell-ui builds green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tocol doc accuracy Three findings from CodeRabbit's review of the previous commits: - The VS Code sidebar's dashboardSnapshot rebuild reset every tracked task's errors/warnings to empty — the shared fold's bulk 'running' rebuild preserves diagnostics for still-active tasks, and a reconnect/refresh snapshot must follow the same rule. The rebuild now reuses the previous entry's arrays from the sync ref. - Task.__init__'s Args documented trigger as 'manual' | 'schedule' while the validation (correctly) also accepts '' — the interactive- dev spelling only the trusted dispatch upgrades. The docstring now states the full contract instead of leaving '' undocumented. - deployment-webview.md's conventions overclaimed: fetch messages carry no requestId (their answer is a teamId/sourceId-scoped push that re-fires after mutations, so request correlation cannot apply), and the requestId-correlated replies plus shell:connectionChange carry no teamId. The conventions and the drawer intro now describe the two correlation styles exactly as the host implements them. Verified: task_engine suite green, vscode tsc + build green, ruff clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rod-Christensen
dismissed stale reviews from dsapandora and asclearuc
via
July 31, 2026 18:00
e705092
Rod-Christensen
force-pushed
the
feat/deploy-2
branch
from
July 31, 2026 18:00
4dd0e68 to
e705092
Compare
Rod-Christensen
enabled auto-merge (squash)
July 31, 2026 18:03
Contributor
There was a problem hiding this comment.
Actionable comments posted: 36
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
apps/vscode/src/providers/ProjectProvider.ts (2)
309-345: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRe-arm the open drawer's team monitor after a reconnect.
onConnectedClearStaleDatadrops_deployTaskMonitorsand relies on "the next drawer fetch" to re-register. An already-open deployment drawer does not senddeployment:fetchon a reconnect:ProjectWebview'sshell:connectionChangehandler clearsstatusMapandliveLogEventsonly. The drawer therefore keeps rendering, with no team-scopedtaskmonitor and no push refresh, until the user closes and reopens it.Post a refresh request to ready webviews after the reconnect, or re-register the last known (team, project) pairs before clearing the set.
🤖 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 `@apps/vscode/src/providers/ProjectProvider.ts` around lines 309 - 345, Update onConnectedClearStaleData to re-arm monitors for deployment drawers that are already open after reconnecting: preserve the known team/project pairs before clearing _deployTaskMonitors, then call ensureDeployTaskMonitor for each pair once the connection is available, or otherwise request a refresh from ready webviews. Ensure open drawers regain their team-scoped task monitor without requiring close-and-reopen.
283-303: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClosing one editor unsubscribes the remaining editors from the global monitors.
startMonitoringregisters{ token: '*' }for['deploy', 'task']once per panel, but that registration is connection-wide, not panel-scoped.stopMonitoringremoves it whenever any single panel disposes.With two pipeline editors open, closing the first removes the global registration. The second editor then stops receiving
apaevt_deployinvalidations and the'*'task lifecycle events. Its where-live badges freeze, and the deployment drawer's push refresh (scheduleDeploymentRefetchinapps/vscode/src/providers/views/Project/ProjectWebview.tsx) never fires again. No error surfaces.The project-scoped registration has the same problem for two editors on the same project, because its key is
{ projectId, source: '*' }.Reference-count the registrations, and remove a monitor only when the last editor that needs it disposes.
🛡️ Proposed guard for the global registration
private async stopMonitoring(panel: vscode.WebviewPanel): Promise<void> { const editorState = this.editorStates.get(panel); if (!editorState || !editorState.projectId) return; try { const client = this.connectionManager.getClient(); if (client) { - await client.removeMonitor({ projectId: editorState.projectId, source: '*' }, ['summary', 'flow', 'output', 'task']); - await client.removeMonitor({ token: '*' }, ['deploy', 'task']); + // Other panels may still need these connection-wide + // registrations; only the last one releases them. + const stillOpen = [...this.editorStates.entries()].filter(([other, state]) => other !== panel && !state.isDisposed); + if (!stillOpen.some(([, state]) => state.projectId === editorState.projectId)) { + await client.removeMonitor({ projectId: editorState.projectId, source: '*' }, ['summary', 'flow', 'output', 'task']); + } + if (stillOpen.length === 0) { + await client.removeMonitor({ token: '*' }, ['deploy', 'task']); + } } } catch (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 `@apps/vscode/src/providers/ProjectProvider.ts` around lines 283 - 303, Update startMonitoring and stopMonitoring to reference-count both the project-scoped monitor keyed by projectId/source and the connection-wide { token: '*' } monitor. Increment counts when an editor starts monitoring, and decrement on disposal; call removeMonitor only when the corresponding count reaches zero, while preserving independent counts for different projects and the global registration.apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsx (2)
187-187: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdvance
unknownTasksRefwhen the host pushesunknownTasks.Line 187 writes
unknownTasksstate without advancingunknownTasksRef. Every other mutation path in this handler advances the ref first:handleTaskEvent(Lines 167-170),statusUpdate(Lines 235-236), anddashboardSnapshot(Lines 262-263). The comment at Lines 121-124 states this rule explicitly.If a
taskEventarrives in the same tick as anupdatemessage,foldTaskEventreads the staleunknownTasksRef.currentand its result overwrites the host-supplied list. The re-anchoring effect at Lines 130-132 runs only after the next render, so it cannot prevent the lost update.🔧 Proposed fix
- if (msg.data.unknownTasks) setUnknownTasks(msg.data.unknownTasks); + if (msg.data.unknownTasks) { + // Advance the sync ref FIRST (same rule as handleTaskEvent) + // so a task event in the same tick folds from this list. + unknownTasksRef.current = msg.data.unknownTasks; + setUnknownTasks(msg.data.unknownTasks); + }🤖 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 `@apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsx` at line 187, Update the `unknownTasks` handling in the message handler to assign the host-provided list to `unknownTasksRef.current` before calling `setUnknownTasks`. Preserve the existing conditional behavior and ensure subsequent `handleTaskEvent` processing reads the newly supplied list within the same tick.
240-267: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRoute the dashboard snapshot through
foldTaskEventinstead of duplicating its rebuild.This block re-implements the shared fold's bulk
runningbranch (packages/shared-ui/src/modules/sidebar/taskFold.ts, Lines 99-121): the same deploy filter, the same diagnostics preservation, and the same unknown-task rebuild. The two copies have already diverged. The shared fold reuses an existingUnknownTaskobject when one matches (existing ?? { ... }); this copy always constructs a new object, so object identity changes on every snapshot and downstream memoization on those entries breaks.Build one
runningevent from the snapshot rows and fold it. The shared implementation already filtersrunKind === 'deploy'per row and preserves diagnostics, so the classification stays in one place.♻️ Proposed refactor
case 'dashboardSnapshot': { - // Seed active tasks + unknown tasks from dashboard (initial load) - const taskMap = new Map<string, ActiveTaskState>(); - const unknown: UnknownTask[] = []; - for (const t of msg.tasks) { - if (t.completed) continue; - // Same classification the shared fold applies on every event - // path: deploy runs never seed the dev sidebar. - if (t.runKind === 'deploy') continue; - const k = `${t.projectId}.${t.source}`; - // Preserve accumulated diagnostics for a task that was - // already tracked — the snapshot confirms it is still - // running, it does not reset its indicators (same rule as - // the shared fold's bulk 'running' rebuild). - const prev = activeTasksRef.current.get(k); - taskMap.set(k, { running: true, errors: prev?.errors ?? [], warnings: prev?.warnings ?? [] }); - if (!isKnownTask(t.projectId, t.source)) { - unknown.push({ projectId: t.projectId, sourceId: t.source, displayName: t.name || t.source, projectLabel: t.projectId.substring(0, 8) }); - } - } - // Advance the sync refs FIRST (same rule as handleTaskEvent) - // so an event in the same tick folds from the seeded state. - activeTasksRef.current = taskMap; - unknownTasksRef.current = unknown; - setActiveTasks(taskMap); - setUnknownTasks(unknown); + // A dashboard snapshot IS a bulk 'running' event — reuse the + // ONE shared fold so the deploy filter, the diagnostics + // preservation, and the unknown-task rebuild cannot drift. + handleTaskEvent({ + action: 'running', + projectId: '', + source: '', + tasks: msg.tasks.filter((t) => !t.completed), + }); break; }🤖 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 `@apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsx` around lines 240 - 267, Replace the duplicated dashboardSnapshot rebuild in the SidebarWebview message handler with a single running event containing the snapshot task rows, then pass it through foldTaskEvent. Remove the local deploy filtering, diagnostics preservation, unknown-task construction, and direct ref assignments so the shared fold owns classification and preserves existing UnknownTask object identity; retain the existing state updates using the fold result.apps/vscode/src/providers/views/setup.react.ts (1)
1-16: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winSet
window.React/window.ReactDOMin each webview entry’s setup path.
setup.react.tsis only imported byProject/index.tsx, butAccount,Environment,Settings, andMonitorentries bypass it. Either remove this file and duplicate its intent in every required entry setup, or make each webview entry import the setup explicitly before shared/UI components evaluate.🤖 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 `@apps/vscode/src/providers/views/setup.react.ts` around lines 1 - 16, Ensure every webview entry point, including Project, Account, Environment, Settings, and Monitor, executes the React/ReactDOM global initialization before importing or evaluating shared UI components. Prefer explicitly importing setup.react.ts at the start of each entry, or remove it and duplicate the same initialization in each setup path; do not leave Account, Environment, Settings, or Monitor bypassing this setup.packages/client-typescript/src/client/client.ts (1)
1260-1280: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
getTaskTokenthrows instead of resolvingundefinedwhen no task is running.The docstring states
getTaskToken"Returns undefined if no task is currently running for the given project/source."call()(same file, lines 2795-2818) throws anErrorwheneverresponse.success === false.getTaskTokendoes not catch this, so a "not found" response rejects the promise instead of resolvingundefined.The Python SDK's
get_task_token(packages/client-python/src/rocketride/mixins/execution.py) explicitly wraps the equivalent call intry/except RuntimeError: return Nonefor this exact scenario. AddingteamIdhere widens the "not found" surface (a team can have no deployed run for a project/source), so this gap now affects more call paths.🐛 Proposed fix to match the documented contract and Python SDK parity
async getTaskToken(options: { projectId: string; source: string; teamId?: string }): Promise<string | undefined> { - const body = await this.call('rrext_get_token', { - projectId: options.projectId, - source: options.source, - ...(options.teamId ? { teamId: options.teamId } : {}), - }); - return body?.token as string | undefined; + try { + const body = await this.call('rrext_get_token', { + projectId: options.projectId, + source: options.source, + ...(options.teamId ? { teamId: options.teamId } : {}), + }); + return body?.token as string | undefined; + } catch { + return undefined; + } }🤖 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 `@packages/client-typescript/src/client/client.ts` around lines 1260 - 1280, Update getTaskToken to catch the not-found error raised by call('rrext_get_token') and resolve undefined, preserving the existing token return behavior for successful responses and propagating unrelated errors.apps/vscode/src/config.ts (1)
449-456: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAdd a migration to retire per-group
teamIdsettings when saving.
rocketride.development.teamIdandrocketride.deployment.teamIdare declared as retired andapplyAllSettings()no longer writes them, but a saved settings snapshot can still carrydevelopment.teamIdanddeployment.teamId. Clear them in migration/storage cleanup so old files do not create stale team group settings.🤖 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 `@apps/vscode/src/config.ts` around lines 449 - 456, Update applyAllSettings() migration/storage cleanup to explicitly remove or clear development.teamId and deployment.teamId from saved settings snapshots, alongside the existing development and deployment group updates. Ensure both retired per-group teamId settings are cleaned when saving, preventing stale values from remaining in older settings files.
♻️ Duplicate comments (1)
packages/shared-ui/src/modules/project/components/SourcePanel.tsx (1)
237-267: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winStream-local state still survives a stream identity change.
The timeline effect and the session effect both key on
source.id,runKind, andprojectId, but no state is cleared when that key changes:
timelinekeeps the previous stream's chapters. The merge at Lines 244-254 then treats them aslocalNewerand re-attaches them to the new stream's fetched list.fedCountRefkeeps the previous cursor. The guard at Line 314 only resets on a shorter array, so a newliveEventsarray whose length is at least the old cursor silently skips its first events.runActiveandopenTracealso describe the previous stream.Reset all four when the stream key changes.
🛠️ Proposed reset on stream identity change
+ // One key for this section's stream identity — every stream-local piece + // of state resets when it changes. + const streamKey = `${projectId}.${source.id}.${runKind}`; + const streamKeyRef = useRef(streamKey); + if (streamKeyRef.current !== streamKey) { + streamKeyRef.current = streamKey; + fedCountRef.current = 0; + } + useEffect(() => { + setTimeline(null); + setRunActive(null); + setOpenTrace(null); + }, [streamKey]);Also applies to: 305-314
🤖 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 `@packages/shared-ui/src/modules/project/components/SourcePanel.tsx` around lines 237 - 267, Reset timeline, fedCountRef, runActive, and openTrace whenever the stream identity changes, using the existing source.id, runKind, and projectId dependency key. Add this reset to the relevant effect or equivalent keyed state-transition logic before processing the new stream, ensuring the timeline merge and fedCountRef guard cannot reuse prior-stream data.
🤖 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 `@apps/shell-ui/scripts/freeze-shell-api.js`:
- Around line 609-635: Update preCheck() so its TypeScript failure is throw-able
without changing normal-mode exit behavior. In the regenMode branch around
preCheck(), catch that failure and print a git restore instruction covering the
deleted version files and stubbed generated contract files, then terminate with
failure. Ensure the recovery message directs users to restore the generated tree
rather than rerun shell:regen.
In `@apps/shell-ui/src/components/layout/OverlayManager.tsx`:
- Around line 150-158: Update the overlay close button in OverlayManager to
include an accessible aria-label and title describing its close action, and mark
the dialog container as a modal dialog with the appropriate role and modal
attribute. Keep closeOverlay as the button action and preserve the existing
overlay content.
In `@apps/shell-ui/src/providers/EnvironmentProvider.tsx`:
- Around line 43-46: Update the team selection in the EnvironmentProvider
initialization to read the ConnectResult field authUser.defaultTeam instead of
authUser.defaultTeamId, while preserving the existing fallback to the first
organization team and the isTeamAdmin lookup.
In `@apps/vscode/src/config.ts`:
- Line 142: Align ConfigManager’s pipelineTraceLevel initialization with the
declared extension default by replacing the current 'full' values in the initial
config object and the config.get fallback with 'summary'. If 'full' is
intentional instead, update the package setting default and related extension
documentation consistently.
In `@apps/vscode/src/providers/ProjectProvider.ts`:
- Around line 894-925: Move the deploy lifecycle docblock to the sendDeployData
member and the deployment-drawer docblock to handleDeploymentMessage. Leave only
the logSessions comment immediately above the logSessions field so each
documentation block is associated with its intended member.
- Around line 1206-1218: Update the per-source preview logic in the nextRuns
block of the surrounding provider method to start all client.deploy.preview
calls concurrently and await their results together, rather than awaiting inside
the Object.entries loop. Preserve the existing filtering, valid-preview
handling, next-run assignment, and per-schedule error isolation.
- Around line 955-965: Validate message.method against an explicit allowlist of
the supported log-session methods (seek, getStatus, and getTrace) before
indexing or invoking session in the logsession:call handler. Reject unsupported
methods through the existing error response path, while preserving invocation
behavior for allowed methods.
In `@apps/vscode/src/providers/SidebarProvider.ts`:
- Around line 343-349: Update the apaevt_status_update handling in
SidebarProvider to use the shared isDevLiveEvent classifier instead of checking
event.body.runKind inline. Import and apply that helper for the dev/deploy
decision, preserving the existing behavior of excluding deploy status updates
from the dev path.
In `@apps/vscode/src/providers/views/Project/ProjectWebview.tsx`:
- Around line 53-55: Remove the stale polling-cadence JSDoc immediately above
DEPLOYMENT_REFETCH_COALESCE_MS, leaving only the comment describing the
push-triggered deployment re-fetch coalescing window.
In `@apps/vscode/src/shared/util/deployMapping.ts`:
- Around line 126-133: Update the deployments filter in the mapping flow to
exclude rows where dep.teamId is missing, before constructing the result with
teamId and teamName. Preserve the existing projectId and non-removed state
checks, matching the guard already used by resolveDeployTeams.
In `@packages/ai/src/ai/account/base.py`:
- Around line 339-346: Add a public Store accessor such as raw_store() that
returns the underlying store, then update _deployment_backend() to use it
instead of Store.instance()._store. Declare _deployments_backend on the account
base class and use that declared state for caching rather than relying on an
implicit getattr default.
In `@packages/ai/src/ai/account/deployment_backend.py`:
- Around line 653-664: The _mutate_meta create path currently uses unguarded
write_file when cas_version is None, allowing concurrent creators to overwrite
metadata. Replace it with the store’s atomic create-if-absent operation so a
lost creation raises VersionMismatchError and retries; also standardize the
absent-version value across store implementations so this guarded path behaves
consistently.
In `@packages/ai/src/ai/modules/task/commands/cmd_deploy.py`:
- Around line 585-618: Update _deploy_run to use the same active-run guard as
TaskScheduler for the matching team, project, and source before calling
start_server_task_as_team, refusing the manual run when a live task exists;
ensure any successfully returned manual-run token is registered in
TaskScheduler._active_tokens so subsequent scheduled ticks skip it.
- Around line 451-477: Validate sourceId against the deployment artifact’s
component IDs before persisting schedule data in _deploy_schedule_set, rejecting
unknown IDs with the command’s established user-facing validation error. Apply
the same artifact lookup and validation in _deploy_source_config, reusing the
shared deployment/artifact logic where appropriate, while preserving valid
source configuration and schedule behavior.
- Around line 650-659: The occurrence generation in the schedule validation flow
must remain inside the same try/except guard. Move the croniter construction and
occurrences list comprehension into the try block so any get_next failure is
caught as ValueError and returns the existing invalid response, while preserving
manual-schedule handling and successful output.
In `@packages/ai/src/ai/modules/task/commands/cmd_misc.py`:
- Around line 485-491: Move the project_key, project_wildcard_key, and
pipe_prefix assignments in the surrounding command logic to immediately before
the for cid, conn in conn_items loop, preserving their existing values and
leaving the loop’s connection-specific handling unchanged.
In `@packages/ai/src/ai/modules/task/task_engine.py`:
- Around line 1458-1460: Move the cap_trace_payload call from the unconditional
trace assignment to the if self._pipelineTraceLevel branch where the rebuilt
body is consumed. Preserve the raw trace until that point, then assign the
capped result (falling back to an empty mapping) before build_event and
downstream fan-out, while leaving tracing-disabled behavior free of
serialization.
In `@packages/ai/src/ai/modules/task/task_scheduler.py`:
- Around line 239-250: Update the scheduling path around _active_tokens and
_start_run to reserve entry.key before creating the asyncio task, preventing
subsequent ticks from dispatching the same schedule while startup is pending.
Ensure the reservation is cleared when _start_run fails, while preserving the
existing active-task overlap check and successful token assignment.
In `@packages/ai/src/ai/modules/task/task_server_facade.py`:
- Around line 127-134: The comment above the arguments assignment interleaves
the ttl explanation with trace-level details. Reorder the comment so the
complete ttl behavior, including “0 means no window,” “run until the pipeline
exits,” and the N-seconds fixed window, appears together; then place the
per-source execution settings explanation afterward, without changing the
arguments logic.
In `@packages/ai/tests/ai/account/test_deployment_backend.py`:
- Around line 395-409: Add a regression test alongside TestIterEnabled that
calls backend.iter_enabled() before any publish or deployment and asserts the
async iteration produces an empty list, preserving the fresh-store behavior when
the orgs/ tree is absent.
In `@packages/ai/tests/ai/modules/task/test_task_scheduler.py`:
- Around line 264-278: Replace the self-referential assertions in
TestOverlapGuard.test_active_previous_run_is_detectable with a call to the
scheduler’s extracted guard method, such as _is_previous_run_active(entry.key),
and assert it reports an active previous run. Implement that method from the
existing _active_tokens and _task_control lookup used by _run, then have _run
use it so the test exercises production guard logic.
In `@packages/client-python/src/rocketride/types/__init__.py`:
- Around line 159-171: Update the deployment exports in rocketride.types to
retain a deprecated DeploymentRecord alias for backward-compatible imports,
while removing the obsolete DeploymentRecord entry from the deployment import
block’s __all__ exports. Ensure both legacy imports and the current deployment
symbols remain available.
In `@packages/client-typescript/src/client/deploy.ts`:
- Around line 359-368: The TypeScript setSourceConfig method currently omits an
explicit null trace level; change its traceLevel condition to send the value
whenever it is not undefined. In packages/client-python/src/rocketride/deploy.py
lines 442-451, update the corresponding deploy method to use a sentinel default
so omitted trace_level remains distinguishable from None, and include traceLevel
whenever the caller supplies it, including None.
In `@packages/client-typescript/src/client/types/pipeline.ts`:
- Around line 88-91: Regenerate the /pipeline-reference documentation to include
the new optional PipelineConfig.name field and its schema details, ensuring the
generated output matches the updated .pipe definition before merging.
In `@packages/shared-ui/src/components/deploy-panel/DeploymentRecordPanel.tsx`:
- Around line 130-180: Reset staged verb state when the focused record identity
changes by adding an effect keyed to deployment.projectId, teamName, and
sourceId; clear stagedConfig, stopOpen, and error while preserving state for the
same record. Update the width calculation in the width useMemo so it recomputes
when a panel opening begins, rather than using the component-lifetime empty
dependency list.
In `@packages/shared-ui/src/components/deploy-panel/DeploymentView.tsx`:
- Around line 183-249: Remove the unused style definitions scheduleRow,
schedulePill, pickerRow, errorText, and previewList from the component’s styles
object. Keep all styles referenced by statusPage, runsPage, and designPage
unchanged, and do not alter schedule editing behavior elsewhere.
- Around line 378-408: Extract the shared STATE_COLOR map and historyColumns
definition into the shared module, exposing a createDeployHistoryColumns()
helper alongside the shared deploy types. Update
packages/shared-ui/src/components/deploy-panel/DeploymentView.tsx sites 378-408
and 280-285 to use the shared history-column helper and STATE_COLOR import, and
update
packages/shared-ui/src/components/deploy-panel/TeamDeploymentRecordPanel.tsx
sites 327-357 and 49-54 likewise; preserve the existing columns, formatter,
action lookup, and color values.
- Around line 433-435: Update the deployed-by metadata rendering in
DeploymentView to use formatDayTime for deployment.deployedAt instead of
formatTime, matching the formatting used by the sibling deployment panels.
In `@packages/shared-ui/src/components/deploy-panel/DeployPanel.tsx`:
- Line 452: Make every click-only interactive node in the deploy panel keyboard
accessible: the publish card, version cards, where-live group header,
kill-switch dot, schedule pill, and picker rows. Add button semantics,
focusability, and Enter/Space key activation matching each existing onClick
action; preserve existing stopPropagation behavior for nested controls in both
click and keyboard handlers.
- Line 516: Clear the stale error when any dialog is dismissed by updating the
relevant close/cancel handlers in DeployPanel, including the handlers around
lines 606, 609, 650, 652, and 689, to call closeDialogs(). Ensure publish,
promotion, and team-picker cancellation paths all reset error while preserving
their existing dialog-closing behavior.
In `@packages/shared-ui/src/components/deploy-panel/SchedulePanel.tsx`:
- Around line 402-411: Update the option-row helper and the “Run for” rows
around the referenced symbols to be keyboard-operable radio controls: add
role="radio", aria-checked, tabIndex={0}, Enter/Space activation, and a visible
focus style while preserving click behavior. Wrap each corresponding option
group in an appropriately labelled radiogroup. Update the weekday day-chip
controls similarly with role="checkbox", aria-checked, tabIndex={0}, Enter/Space
activation, and visible focus styling.
In `@packages/shared-ui/src/components/tab-panel/TabPanel.tsx`:
- Around line 68-80: Update TabPanel to assign each panel id="tabpanel-${id}"
and aria-labelledby="tab-${id}" alongside its existing role and visibility
attributes. Update the tab element rendered by TabControl to use the matching
id="tab-${entry.id}" and aria-controls="tabpanel-${entry.id}", preserving the
existing tab behavior.
In `@packages/shared-ui/src/index.ts`:
- Around line 44-45: Update the type export list in the shared-ui barrel to
include IOverviewPanelProps from ./modules/server/components alongside the
existing sibling panel prop types, while leaving the component exports
unchanged.
In `@packages/shared-ui/src/modules/project/components/SourcePanel.tsx`:
- Around line 428-435: Update the throwaway walker invocation in the event
collection flow to attach a rejection handler to walker.play, ensuring walker
failures settle the outer operation immediately with the rejection instead of
becoming unhandled and waiting for the settle timer. Preserve the existing event
callback behavior for successful playback.
In `@packages/shared-ui/src/modules/project/ProjectView.tsx`:
- Around line 483-493: Update the panels configuration in ProjectView so the
deploy panel is omitted when isReadonly is true, not merely hidden through the
tab entries or activeMode clamp. Ensure panels.deploy, including its DeployPanel
mount and refresh behavior, is only created for writable views while preserving
the existing deploy panel behavior otherwise.
In `@packages/shared-ui/src/modules/server/components/ConnectionsPanel.tsx`:
- Around line 76-88: Centralize the duplicated two-line cell styles by adding a
shared cellTitle/cellSub helper in defaults.ts, then remove the local entries
from domStyles in
packages/shared-ui/src/modules/server/components/ConnectionsPanel.tsx (lines
76-88) and packages/shared-ui/src/modules/server/components/TasksPanel.tsx
(lines 109-121), importing and using the shared helper in both components.
---
Outside diff comments:
In `@apps/vscode/src/config.ts`:
- Around line 449-456: Update applyAllSettings() migration/storage cleanup to
explicitly remove or clear development.teamId and deployment.teamId from saved
settings snapshots, alongside the existing development and deployment group
updates. Ensure both retired per-group teamId settings are cleaned when saving,
preventing stale values from remaining in older settings files.
In `@apps/vscode/src/providers/ProjectProvider.ts`:
- Around line 309-345: Update onConnectedClearStaleData to re-arm monitors for
deployment drawers that are already open after reconnecting: preserve the known
team/project pairs before clearing _deployTaskMonitors, then call
ensureDeployTaskMonitor for each pair once the connection is available, or
otherwise request a refresh from ready webviews. Ensure open drawers regain
their team-scoped task monitor without requiring close-and-reopen.
- Around line 283-303: Update startMonitoring and stopMonitoring to
reference-count both the project-scoped monitor keyed by projectId/source and
the connection-wide { token: '*' } monitor. Increment counts when an editor
starts monitoring, and decrement on disposal; call removeMonitor only when the
corresponding count reaches zero, while preserving independent counts for
different projects and the global registration.
In `@apps/vscode/src/providers/views/setup.react.ts`:
- Around line 1-16: Ensure every webview entry point, including Project,
Account, Environment, Settings, and Monitor, executes the React/ReactDOM global
initialization before importing or evaluating shared UI components. Prefer
explicitly importing setup.react.ts at the start of each entry, or remove it and
duplicate the same initialization in each setup path; do not leave Account,
Environment, Settings, or Monitor bypassing this setup.
In `@apps/vscode/src/providers/views/Sidebar/SidebarWebview.tsx`:
- Line 187: Update the `unknownTasks` handling in the message handler to assign
the host-provided list to `unknownTasksRef.current` before calling
`setUnknownTasks`. Preserve the existing conditional behavior and ensure
subsequent `handleTaskEvent` processing reads the newly supplied list within the
same tick.
- Around line 240-267: Replace the duplicated dashboardSnapshot rebuild in the
SidebarWebview message handler with a single running event containing the
snapshot task rows, then pass it through foldTaskEvent. Remove the local deploy
filtering, diagnostics preservation, unknown-task construction, and direct ref
assignments so the shared fold owns classification and preserves existing
UnknownTask object identity; retain the existing state updates using the fold
result.
In `@packages/client-typescript/src/client/client.ts`:
- Around line 1260-1280: Update getTaskToken to catch the not-found error raised
by call('rrext_get_token') and resolve undefined, preserving the existing token
return behavior for successful responses and propagating unrelated errors.
---
Duplicate comments:
In `@packages/shared-ui/src/modules/project/components/SourcePanel.tsx`:
- Around line 237-267: Reset timeline, fedCountRef, runActive, and openTrace
whenever the stream identity changes, using the existing source.id, runKind, and
projectId dependency key. Add this reset to the relevant effect or equivalent
keyed state-transition logic before processing the new stream, ensuring the
timeline merge and fedCountRef guard cannot reuse prior-stream data.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 0658d84f-5a8e-4bcf-b508-ef9e4bb44511
📒 Files selected for processing (159)
apps/shell-ui/scripts/freeze-shell-api.jsapps/shell-ui/scripts/tasks.jsapps/shell-ui/src/api.tsapps/shell-ui/src/components/layout/OverlayManager.tsxapps/shell-ui/src/components/layout/Sidebar.tsxapps/shell-ui/src/hooks/iframeBridgeProtocol.tsapps/shell-ui/src/hooks/useIframeBridge.tsapps/shell-ui/src/index.tsapps/shell-ui/src/lib/Documents.tsxapps/shell-ui/src/providers/AccountProvider.tsxapps/shell-ui/src/providers/EnvironmentProvider.tsxapps/shell-ui/src/providers/SettingsProvider.tsxapps/vscode/docs/deployment-webview.mdapps/vscode/package.jsonapps/vscode/src/config.tsapps/vscode/src/extension.tsapps/vscode/src/providers/ProjectProvider.tsapps/vscode/src/providers/SettingsProvider.tsapps/vscode/src/providers/SidebarProvider.tsapps/vscode/src/providers/WelcomeProvider.tsapps/vscode/src/providers/shared/connection-message-handler.tsapps/vscode/src/providers/views/Account/index.tsxapps/vscode/src/providers/views/Environment/index.tsxapps/vscode/src/providers/views/Monitor/MonitorWebview.tsxapps/vscode/src/providers/views/Monitor/index.tsxapps/vscode/src/providers/views/Project/ProjectWebview.tsxapps/vscode/src/providers/views/Project/index.tsxapps/vscode/src/providers/views/Settings/ConnectionSettings.tsxapps/vscode/src/providers/views/Settings/DeploySettings.tsxapps/vscode/src/providers/views/Settings/SettingsWebview.tsxapps/vscode/src/providers/views/Settings/index.tsxapps/vscode/src/providers/views/Sidebar/SidebarWebview.tsxapps/vscode/src/providers/views/Welcome/WelcomeWebview.tsxapps/vscode/src/providers/views/components/ConnectionConfig.tsxapps/vscode/src/providers/views/components/panels/CloudPanel.tsxapps/vscode/src/providers/views/deployTypes.tsapps/vscode/src/providers/views/logTypes.tsapps/vscode/src/providers/views/setup.react.tsapps/vscode/src/providers/views/types.tsapps/vscode/src/shared/util/deployMapping.tsnodes/src/nodes/webhook/IEndpoint.pynodes/src/nodes/webhook/services.tools.jsonnodes/src/nodes/webhook/tools/README.mdnodes/test/test_db_arango.pynodes/test/test_tool_v0.pynodes/test/tool_filesystem/test_live_anchor.pynodes/test/tool_google_workspace/test_google_client.pypackages/ai/src/ai/account/__init__.pypackages/ai/src/ai/account/base.pypackages/ai/src/ai/account/deployment_backend.pypackages/ai/src/ai/account/deployment_store.pypackages/ai/src/ai/account/models.pypackages/ai/src/ai/common/sandbox.pypackages/ai/src/ai/modules/task/commands/cmd_debug.pypackages/ai/src/ai/modules/task/commands/cmd_deploy.pypackages/ai/src/ai/modules/task/commands/cmd_log.pypackages/ai/src/ai/modules/task/commands/cmd_misc.pypackages/ai/src/ai/modules/task/commands/cmd_monitor.pypackages/ai/src/ai/modules/task/commands/cmd_task.pypackages/ai/src/ai/modules/task/deploy_events.pypackages/ai/src/ai/modules/task/run_log.pypackages/ai/src/ai/modules/task/task_engine.pypackages/ai/src/ai/modules/task/task_scheduler.pypackages/ai/src/ai/modules/task/task_server.pypackages/ai/src/ai/modules/task/task_server_facade.pypackages/ai/src/ai/modules/task_http/task_data.pypackages/ai/src/ai/node.pypackages/ai/tests/ai/account/test_deployment_backend.pypackages/ai/tests/ai/account/test_deployment_store.pypackages/ai/tests/ai/account/test_store.pypackages/ai/tests/ai/modules/task/commands/test_cmd_debug.pypackages/ai/tests/ai/modules/task/commands/test_cmd_deploy.pypackages/ai/tests/ai/modules/task/commands/test_cmd_log.pypackages/ai/tests/ai/modules/task/commands/test_cmd_misc.pypackages/ai/tests/ai/modules/task/commands/test_cmd_monitor.pypackages/ai/tests/ai/modules/task/commands/test_cmd_task.pypackages/ai/tests/ai/modules/task/test_log_stream.pypackages/ai/tests/ai/modules/task/test_run_log.pypackages/ai/tests/ai/modules/task/test_run_log_team.pypackages/ai/tests/ai/modules/task/test_task_engine.pypackages/ai/tests/ai/modules/task/test_task_identity.pypackages/ai/tests/ai/modules/task/test_task_scheduler.pypackages/ai/tests/ai/modules/task/test_task_server.pypackages/ai/tests/ai/modules/task/test_task_server_facade.pypackages/client-python/docs/index.mdpackages/client-python/docs/log.mdpackages/client-python/src/rocketride/client.pypackages/client-python/src/rocketride/deploy.pypackages/client-python/src/rocketride/log.pypackages/client-python/src/rocketride/log_stream.pypackages/client-python/src/rocketride/mixins/events.pypackages/client-python/src/rocketride/mixins/execution.pypackages/client-python/src/rocketride/types/__init__.pypackages/client-python/src/rocketride/types/deploy.pypackages/client-python/src/rocketride/types/events.pypackages/client-python/src/rocketride/types/log.pypackages/client-python/tests/test_deploy.pypackages/client-python/tests/test_monitor_keys.pypackages/client-typescript/docs/guide/methods/deploy.mdpackages/client-typescript/docs/guide/methods/log.mdpackages/client-typescript/src/client/client.tspackages/client-typescript/src/client/deploy.tspackages/client-typescript/src/client/log-stream.tspackages/client-typescript/src/client/log.tspackages/client-typescript/src/client/types/deploy.tspackages/client-typescript/src/client/types/events.tspackages/client-typescript/src/client/types/log.tspackages/client-typescript/src/client/types/pipeline.tspackages/client-typescript/tests/deploy.test.tspackages/client-typescript/tests/monitor-keys.test.tspackages/server/docs/observability.mdpackages/server/engine-lib/engLib/store/pipeline/pipeline_config.cpppackages/server/engine-lib/engLib/store/stack.cpppackages/shared-ui/src/components/canvas/CanvasPanel.tsxpackages/shared-ui/src/components/canvas/index.tsxpackages/shared-ui/src/components/data-grid/DataGrid.tsxpackages/shared-ui/src/components/data-grid/tabulator-theme.csspackages/shared-ui/src/components/deploy-panel/DeployPanel.tsxpackages/shared-ui/src/components/deploy-panel/DeploymentRecordPanel.tsxpackages/shared-ui/src/components/deploy-panel/DeploymentView.tsxpackages/shared-ui/src/components/deploy-panel/SchedulePanel.tsxpackages/shared-ui/src/components/deploy-panel/TeamDeploymentRecordPanel.tsxpackages/shared-ui/src/components/deploy-panel/VersionRecordPanel.tsxpackages/shared-ui/src/components/deploy-panel/index.tspackages/shared-ui/src/components/deploy-panel/types.tspackages/shared-ui/src/components/detail-panel/DetailPanel.tsxpackages/shared-ui/src/components/sidebar-menu/SidebarMenu.tsxpackages/shared-ui/src/components/tab-control/TabControl.tsxpackages/shared-ui/src/components/tab-control/ViewMenuBadge.tsxpackages/shared-ui/src/components/tab-panel/TabPanel.tsxpackages/shared-ui/src/index.tspackages/shared-ui/src/modules/account/AccountView.tsxpackages/shared-ui/src/modules/account/components/ProfilePanel.tsxpackages/shared-ui/src/modules/billing/components/BillingDashboard.tsxpackages/shared-ui/src/modules/environment/EnvironmentView.tsxpackages/shared-ui/src/modules/project/ProjectView.tsxpackages/shared-ui/src/modules/project/components/SourcePanel.tsxpackages/shared-ui/src/modules/project/components/StatusPane.tsxpackages/shared-ui/src/modules/project/hooks/useTaskEvents.tspackages/shared-ui/src/modules/project/index.tsxpackages/shared-ui/src/modules/project/types.tspackages/shared-ui/src/modules/project/utils.tspackages/shared-ui/src/modules/server/MonitorView.tsxpackages/shared-ui/src/modules/server/components/ActivityPanel.tsxpackages/shared-ui/src/modules/server/components/ConnectionRecordPanel.tsxpackages/shared-ui/src/modules/server/components/ConnectionsPanel.tsxpackages/shared-ui/src/modules/server/components/OverviewGrid.tsxpackages/shared-ui/src/modules/server/components/OverviewPanel.tsxpackages/shared-ui/src/modules/server/components/TaskRecordPanel.tsxpackages/shared-ui/src/modules/server/components/TasksPanel.tsxpackages/shared-ui/src/modules/server/components/index.tspackages/shared-ui/src/modules/server/util/formatters.tspackages/shared-ui/src/modules/sidebar/SidebarView.tsxpackages/shared-ui/src/modules/sidebar/taskFold.test.tspackages/shared-ui/src/modules/sidebar/taskFold.tspackages/shared-ui/src/themes/styles.tspackages/shared-ui/src/types/viewMenu.tspackages/shell-api/versions/v0.d.tspyproject.toml
💤 Files with no reviewable changes (7)
- apps/vscode/src/providers/WelcomeProvider.ts
- packages/ai/src/ai/account/deployment_store.py
- packages/ai/src/ai/node.py
- packages/ai/tests/ai/account/test_deployment_store.py
- apps/vscode/src/providers/SettingsProvider.ts
- apps/vscode/src/providers/views/Settings/ConnectionSettings.tsx
- apps/vscode/src/providers/shared/connection-message-handler.ts
kwit75
previously approved these changes
Jul 31, 2026
kwit75
left a comment
kwit75
left a comment
Collaborator
There was a problem hiding this comment.
Approving — all checks are green on e705092.
Scope, stated honestly: this is 159 files and I have not read all of it. Alexandru reviewed it and Ariel ran it locally, so rather than duplicate that, I spent my time on the thing that is actually going to decide this PR — the 33 open CodeRabbit threads — and triaged all of them so the decision to clear them can be made on evidence instead of fatigue.
The good news: the large majority are cosmetic. Unused styles, detached docblocks, a stale comment, extract-this-constant, barrel exports, "add a regression test". Clearing those in bulk costs nothing.
These are the ones I would not clear without a look. Not because they are necessarily right — several may already be handled — but because each has a failure mode that survives a green test run:
| Where | What it says | Why it is on this list |
|---|---|---|
ProjectProvider.ts |
Validate message.method against an allowlist before invoking it |
The only security-tagged thread of the 33. Dispatching a method name that arrives in a webview message is the pattern where a bad message reaches a method nobody meant to expose. Worth 60 seconds to confirm the set is already constrained. |
task_scheduler.py |
The overlap guard has a gap while a dispatch is in flight | Concurrency hole in new scheduling code — the failure is two runs of the same thing, which bills twice and repeats side effects. |
cmd_deploy.py |
A manual run bypasses the scheduler's overlap guard | Same hole reached by a second door. These two are one issue and should be judged together. |
ProjectView.tsx |
Gate the deploy panel on isReadonly, not just the tab entry |
A permission gate applied to navigation but not to the panel is reachable by anyone who gets to the panel another way. |
deployment_backend.py |
Make the meta CREATE path conflict-safe | Create races are invisible until two people deploy at once, which is exactly what teams-as-environments invites. |
EnvironmentProvider.tsx |
Read defaultTeam from authUser, not defaultTeamId |
Reads as a plain wrong-field bug: if the field name is wrong the default team silently never resolves. |
Six of thirty-three. If those six are either fixed or consciously dismissed, I do not think the rest carry risk worth another round.
One process note, since it has now bitten three PRs today: dismiss_stale_reviews is on here, so this approval dies the moment anything else is pushed. If another commit lands, ping me and I will re-approve immediately — no re-review needed for a change I have already looked at.
asclearuc
previously approved these changes
Jul 31, 2026
…ploy commands, and UI surfaces
Server (packages/ai):
- task_scheduler: close the overlap-guard gap while a dispatch is in
flight — the tick loop stamps a _DISPATCHING marker into _active_tokens
BEFORE creating the dispatch task, and _start_run clears it on every
non-dispatch exit (try/finally). Without this, a tick due during a slow
startup (subprocess launch + run-log open) passed the guard and fired a
second concurrent run of the same source.
- task_scheduler: extract the guard into _is_previous_run_active() so the
tick loop, the manual run path, and the tests all evaluate the SAME
predicate; add is_run_active()/register_manual_run() as the public face.
- cmd_deploy: the manual run-now path now honors that same guard — it
refuses while a run of the (team, project, source) is active and
registers its token with the scheduler so the next cron tick skips.
Both runs would otherwise share the team storage anchor.
- cmd_deploy: schedule_set and source_config validate sourceId against the
deployed artifact's components — a schedule for a phantom source
previously failed on every tick with nothing but a server log line.
- cmd_deploy: preview wraps croniter occurrence generation in the
validation guard — calendar-impossible 5-field expressions construct
fine but raise in get_next, which surfaced as a DAP error instead of
{valid: false}.
- task_engine: clamp/serialize the trace payload only inside the tracing
branch — synthetic tool-call traces arrive regardless of trace level,
so the json.dumps was pure waste when tracing is off.
- cmd_misc: hoist monitor-key construction out of the per-connection loop
(depends only on the control).
- deployment_backend: correct the false "first writer wins" comment on
the meta CREATE path — the store contract has no create-if-absent
precondition, so racing first creators last-write-win; serializing
creates stays a parked follow-up (single-process OSS backend; SaaS
overrides with DB CAS).
- base.py/store.py: Store gains a public raw_store() accessor so the
account base stops reaching into the private _store attribute, and
_deployments_backend is declared as documented class state.
- task_server_facade: un-interleave the ttl/trace-level comment.
- tests: pin the empty-store scheduler feed (iter_enabled on a fresh
store), assert the scheduler's own guard predicate instead of a local
re-implementation, cover the in-flight marker and manual registration,
reject-unknown-source cases, and give the cmd_misc dashboard stubs the
owner_id every real TaskControl exposes.
Shared UI (packages/shared-ui):
- deploy-panel/types: DEPLOY_STATE_COLOR + createDeployHistoryColumns()
become the single shared definitions; DeploymentView and
TeamDeploymentRecordPanel drop their hand-copied twins (same rationale
as DEPLOY_ACTION_VERBS — two copies drift).
- DeploymentRecordPanel: clear staged config / stop-confirm / error when
the focused record identity changes — one drawer instance persists
across selections, so a staged edit could re-arm Save against a
DIFFERENT record.
- DeployPanel: closeDialogs() drops the error text on every dialog
cancel/close path (it previously survived under the version strip);
publish card, version cards, badges, where-live rows, kill-switch dot,
schedule pills, and picker rows get keyboard parity via the new
buttonize() util (role/tabIndex/Enter/Space — the stock Card pattern).
- SchedulePanel: trigger options and run-for rows are proper radios
(role/aria-checked/radiogroup), day chips are checkboxes, all keyboard
operable; row keydown ignores events bubbling out of embedded inputs so
Space in the cron box cannot flip the trigger.
- DeploymentView: deployedAt renders with formatDayTime (time-only hid
the day) and five leftover schedule-editing styles are deleted.
- ProjectView: the deploy panel no longer mounts for readonly hosts —
TabPanel mounts every panel, so the hidden DeployPanel was fetching a
lifecycle the user can never open.
- barrel: export IOverviewPanelProps (renamed to the I-prefix convention)
alongside its sibling panel prop types; export buttonize.
Shell UI (apps/shell-ui):
- freeze-shell-api --regen: a pre-check failure after the destructive
reset now prints the git-restore recovery — re-running shell:regen
fails at the same point, so the stub note's advice was a dead end.
- OverlayManager: the modal carries role=dialog/aria-modal and the close
button an accessible name.
- EnvironmentProvider: read ConnectResult.defaultTeam (the field that
exists) instead of defaultTeamId, which was always undefined and made
the team card fall back to the first roster team.
VS Code (apps/vscode):
- ProjectProvider: logsession:call validates method against an explicit
allowlist — the webview cast gives no runtime protection at a process
boundary, so any callable on the stream object was reachable.
- ProjectProvider: per-source cron previews fan out with Promise.all
(serialized round-trips stalled the drawer by one RTT per source), and
the two orphaned docblocks move onto sendDeployData /
handleDeploymentMessage where hover tooling attributes them correctly.
- runClassification.ts: one host-side isDeployRunBody() replaces the two
inline runKind tests in ProjectProvider and SidebarProvider (the host
bundle cannot import shared-ui's isDevLiveEvent — no `shared` alias).
- deployMapping: rows without a teamId are filtered instead of cast —
`dep.teamId as string` handed the webview a row with undefined identity.
- ProjectWebview: drop the stale polling-cadence comment (push replaced
the poll).
Validated: full ai pytest suite (1794 passed), shared-ui:test,
vscode:build, shell-ui:build, ruff, prettier.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
@coderabbitai pause Six review passes on one PR is enough. Shut the fuck up. |
Contributor
|
🐇 ✅ Action performedReviews paused. |
kwit75
approved these changes
Jul 31, 2026
kwit75
left a comment
kwit75
left a comment
Collaborator
There was a problem hiding this comment.
Approved on 7711c13d.
Approving ahead of the three OS builds deliberately — they are still running, and CI OK is a required check, so this approval cannot let anything through that the builds would have caught. It just means there is nothing left waiting on a human once they land.
I checked whether the six I flagged were fixed or merely dismissed, since "0 unresolved" can mean either. They were fixed, with tests:
apps/vscode/src/providers/ProjectProvider.ts +55/-36 <- the security one
packages/ai/.../task_scheduler.py +109/-63 <- the overlap gap
packages/ai/.../commands/cmd_deploy.py +43/-2 <- manual-run bypass
packages/ai/src/ai/account/deployment_backend.py +5/-2 <- conflict-safe CREATE
apps/shell-ui/.../EnvironmentProvider.tsx +2/-1 <- defaultTeam field
tests: test_task_scheduler +24/-4, test_cmd_deploy +44/-2, test_deployment_backend +7/-0
Every one carries a code change, and the three with real failure modes carry test changes too. That is the difference between clearing a bot and answering it, and it is worth saying out loud after six passes.
Note for whoever presses merge: this approval dies if anything else is pushed — dismiss_stale_reviews is on, and that has now happened four times today across three PRs. If another commit lands, ping me and I will re-approve on sight; no re-review needed.
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
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Teams-as-environments deployment, owner-scoped task identity, and push-driven deploy surfaces
This branch delivers the deploy-2 epic end to end: teams become the deployment environments, a task's identity becomes owner-scoped so the same pipeline can finally run everywhere it should, monitoring routes by that same identity, and the deploy surfaces stop polling. Eleven commits, each building on the last — summarized by arc below, with the why first.
Why this branch exists
Deployments needed to be first-class: publish an immutable version, point a team at it, schedule its sources, watch and replay its runs — without a human credential riding any of it. Getting there exposed a foundational flaw: a task's identity was a deterministic
sha256({userId, project_id, source})— no team, no run kind — so the same pipeline could not run in two teams, nor in development while its deployment was live ("Pipeline is already running"), and the monitor keyspace aliased dev and deploy events onto one key.1. Deployment backend — teams as environments (
a5657fd8,89f691a0,abb9abc6)vN), per-team deployment pointers (deploy/rollback = pointer move), per-source schedules with TTL run-windows and per-source trace/debug execution settings, an immutable audit history, and a credential-less scheduled dispatch: the scheduler constructs a synthetic team identity server-side — no stored user token can leak into a cron path.2. Team-scoped run-log continua (
31b1a79a,d044d44f)Deploy runs record into the team's tree (
@/Team/=<id>/…) so teammates can watch and replay each other's production runs; dev runs stay in the owner's tree. The wire convention was set here and now governs everything: the scope IS the kind —teamIdpresent addresses the deploy continuum, absent addresses your own dev runs;runKindis never a wire argument.3. The development-team model (
8edd607c)Clients no longer pick a launch team; dev runs are pinned to the profile-assigned development team, and
set_default_teamrequirestask.control. This makes "whose run is this" answerable — the prerequisite for owner-scoped identity.4. Both SDKs on the new model (
fa6d9835)The deploy namespace (publish/deploy/list/versions/history/schedules/run/preview) and team-scoped log reads land in the TypeScript and Python SDKs in lockstep.
5. UI: the deploy surface + architecture campaign (
1dd33236,abb9abc6)The Provider→View→Panel→Pane architecture across hosts; the DEPLOY page (version strip, where-live table), the team/source deployment record drawers with DVR replay, per-source schedule and execution-settings editing.
6. Owner-scoped task identity (
82b195aa) — the capstone{owner}.{projectId}.{source}: dev runs are owned by their user (once per user, total), deploy runs by their team (once per team, actor-independent). Token digests carry only the applicable owner field — the field name disambiguates the id spaces — plus akinddiscriminator sotk_/pk_digests can never collide (the public key was previously identical for two users on the same pair, making its auth scan nondeterministic).p.{teamId|userId}.{projectId}.{source}): a project/source subscription withoutteamIdis bound server-side to the caller's own dev runs; with it, to the team's deploy run. Dev and deploy runs of one pipeline can no longer alias; watching a teammate's dev run requires its task token (deliberate under the owner model). Both SDKs change the monitor key-string encode and decode symmetrically — reconnect replays subscriptions through that round-trip, and an encoder-only change would silently drop the team scope.parseServerEventgains an explicit continuum scope,isDevLiveEvent/isTeamLiveEventare the membership tests, andfoldTaskEventis the single task-lifecycle fold both sidebars delegate to — the hand-copied per-app filters had diverged (the VS Code fork re-admitted deploy runs on every bulk snapshot).EVENT_TYPE.DEPLOYclass;apaevt_deployinvalidation events broadcast org-scoped from every deployment mutation and from the scheduler. Receivers re-fetch — the event body is identity only. Drawer/list polling (10s/15s intervals) is gone; run state folds from the task-event stream seeded by the catch-up snapshot; the deploy record's report cards get a minimal host-fed live feed. The full session-self-subscribe monitor redesign is deliberately deferred to feat/alb (reconciliation notes accompany the plan) to avoid building what it will replace.Behavioral changes to review
pk_values rotate on upgrade (they were deterministic per pair; only valid while a task runs — small blast radius).Testing
test_task_identity.py(digest/collision matrices — including same-user-two-teams-dev collides by design — owner-scoped lookup semantics), monitor-key round-trip + reconnect tests in both SDKs, store-guard regression.Pairing
Pairs with rocketride-ai/rocketride-saas#450 (rocket-ui half + migrations + submodule pointer). Merge this PR first, then #450. feat/alb lands after this branch and rebases onto it — its
_resolve_task_by_*seams take the owner-scoped widening directly.🤖 Generated with Claude Code
Summary by CodeRabbit