Skip to content

Require ready integration status before deploy#155

Merged
khaliqgant merged 2 commits into
mainfrom
codex/issue-154-deploy-reconnect
May 28, 2026
Merged

Require ready integration status before deploy#155
khaliqgant merged 2 commits into
mainfrom
codex/issue-154-deploy-reconnect

Conversation

@khaliqgant
Copy link
Copy Markdown
Member

Summary

  • Make deploy preflight read GET /api/v1/workspaces/<workspace>/integrations/<provider>/status instead of treating integration list rows as authoritative.
  • Count only runtime-ready status as connected; pending/syncing/degraded now drive reconnect instead of silently deploying a dead proactive agent.
  • Add repeatable --reconnect <provider> / --reconnect=a,b to force a fresh connect flow without an interactive confirmation.

Fixes #154. Companion PRs: AgentWorkforce/cloud#1295 and AgentWorkforce/relayfile#214.

Verification

  • corepack pnpm --filter @agentworkforce/persona-kit build && corepack pnpm --filter @agentworkforce/runtime build && corepack pnpm --filter @agentworkforce/workload-router build
  • corepack pnpm --filter @agentworkforce/deploy test
  • corepack pnpm --filter @agentworkforce/cli test

@gemini-code-assist
Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 27, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds a new --reconnect <provider> CLI flag (repeatable, comma-aware) to force fresh OAuth connection flows for specified integration providers even when already connected. The flag is threaded through DeployOptions and ConnectAllInput, integrated into the connectIntegrations control flow to bypass "already connected" early exits when forced, and backed by a refactored status detection system using provider-scoped endpoints with readiness-focused validation.

Changes

Forced Integration Reconnection Feature

Layer / File(s) Summary
CLI flag definition and type contracts
packages/cli/src/deploy-command.ts, packages/cli/src/deploy-command.test.ts, packages/deploy/src/types.ts, packages/deploy/src/connect.ts
New --reconnect <provider> flag is repeatable and comma-aware; DeployOptions.reconnectProviders and ConnectAllInput.reconnectProviders are added to carry the flag through the call stack.
Forced reconnection control flow
packages/deploy/src/connect.ts
connectIntegrations computes forceReconnect per provider and bypasses "already connected" early exits when true; user prompts and non-prompt/no-connect failures are likewise conditional on !forceReconnect.
Status endpoint and readiness detection refactoring
packages/deploy/src/connect.ts
New fetchIntegrationStatusForScope queries provider-scoped cloud status endpoints; response handling branches between list-shaped (legacy) and status-shaped responses; readiness checks are narrowed to status === 'ready', state === 'ready', or ready === true.
Integration connection test updates
packages/deploy/src/connect.test.ts
Tests for isConnected are updated to mock provider-scoped status endpoints (with scope=deployer_user or scope=workspace) and single-object responses; readiness coverage verifies rejection of non-ready statuses until ready; fallback behaviors are retested with workspace-scoped endpoints; a new test verifies forced reconnect bypasses "already connected" exit.
Deployment command wiring and test fixtures
packages/deploy/src/deploy.ts, packages/deploy/src/deploy.test.ts
deploy() conditionally passes reconnectProviders into connectAndCollectIntegrations; cloud integration catalog mock in auth-recovery test is updated to single-provider object shape.
Trajectory metadata
.trajectories/completed/2026-05/traj_61qz5tibb2lm.json, .trajectories/completed/2026-05/traj_61qz5tibb2lm.md, .trajectories/index.json
Completion record documents workforce#155 work, source-aware fallback behavior, added regression tests, and verification steps.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

  • AgentWorkforce/workforce#154: Main changes directly address objectives by introducing provider-scoped status endpoints with readiness-focused checks and a repeatable --reconnect option to force reconnection for specified providers.

Possibly related PRs

  • AgentWorkforce/workforce#115: Both PRs modify packages/deploy/src/connect.ts's connectIntegrations control flow around integration status/preflight checks; main PR adds forced reconnect handling via reconnectProviders, while the referenced PR changes behavior to fail fast on auth (401/403) failures in the same function's decision logic.

  • AgentWorkforce/workforce#133: Both PRs modify packages/deploy/src/connect.ts and connect.test.ts around how integration connection/status polling requests are scoped by source.kind (and related URL/body shapes), with overlap in connect-session and status-poll wiring.

  • AgentWorkforce/workforce#131: Both PRs modify packages/deploy/src/connect.ts and connect.test.ts around the isConnected preflight logic—especially scope/endpoint selection and readiness/status-matching semantics—while the main PR additionally layers on the reconnectProviders forced-reconnect flow.

Poem

🐰 A reconnect flag hops into the fold,
Forcing fresh flows when connections grow old,
Status endpoints scoped by source and by cheer,
Readiness checks made crystal-clear,
Tests verify the bouncing won't stall—
The reconnect journey completes it all! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.77% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Require ready integration status before deploy' directly and accurately summarizes the main change—enforcing runtime-ready status checks before deployment.
Description check ✅ Passed The description is directly related to the changeset, detailing the integration status preflight switch, readiness enforcement, reconnect flag addition, and providing verification steps.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/issue-154-deploy-reconnect

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 7 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/deploy/src/connect.ts Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
packages/deploy/src/connect.ts (1)

344-347: ⚡ Quick win

Validate reconnectProviders against declared persona integrations to avoid silent no-ops.

A typo like --reconnect slakc is currently ignored silently, which can mislead operators into thinking a forced reconnect ran. Consider warning or failing fast for providers not present in persona.integrations.

Proposed guard
 export async function connectIntegrations(input: ConnectAllInput): Promise<ConnectAllResult> {
   const integrations = input.persona.integrations ?? {};
+  const declared = new Set(Object.keys(integrations));
+  const unknownReconnect = [...new Set(input.reconnectProviders ?? [])]
+    .filter((provider) => !declared.has(provider));
+  if (unknownReconnect.length > 0) {
+    input.io.warn(
+      `reconnect requested for undeclared integration(s): ${unknownReconnect.join(', ')}`
+    );
+  }
   const outcomes: IntegrationConnectOutcome[] = [];
🤖 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/deploy/src/connect.ts` around lines 344 - 347, The loop over
Object.keys(integrations) should validate entries from input.reconnectProviders
to avoid silent no-ops; before using forceReconnect (and inside the surrounding
function in connect.ts), iterate input.reconnectProviders (if present) and check
each requested provider exists in integrations (or persona.integrations if that
canonical map is used), and if any do not exist either log a warning or throw a
user-facing error that names the unknown provider(s) so typos like "slakc" are
surfaced; update references to reconnectProviders, integrations, and the
forceReconnect computation to rely on the validated list (or bail early) so
intended forced-reconnects cannot be accidentally ignored.
🤖 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 @.trajectories/completed/2026-05/traj_61qz5tibb2lm.json:
- Line 51: The trajectory metadata currently embeds a user-specific absolute
path in the "projectId" JSON field which leaks PII and is non-portable; replace
the value of "projectId" in the JSON object (the "projectId" key inside the
trajectory file) with a repo-relative identifier or a neutral project slug
(e.g., "workforce-issue-154" or omit the field entirely), ensuring no local
filesystem paths or usernames remain; update any code that consumes this
trajectory metadata to expect the new identifier format if necessary.

In @.trajectories/index.json:
- Line 45: The index currently stores a host-local absolute path under the
"path" key (e.g. the entry in .trajectories/index.json with value
"/Users/khaliqgant/.../traj_61qz5tibb2lm.json"); update the
generation/serialization logic to record repository-relative paths instead of
absolute machine paths (for example "completed/2026-05/traj_61qz5tibb2lm.json").
Locate the code that writes entries into .trajectories/index.json (the logic
that sets the "path" field for trajectory files) and replace absolute path usage
with a repo-root-relative computation (use the repo root as base when computing
the stored path) so the index is portable and no local user/machine information
is leaked.

---

Nitpick comments:
In `@packages/deploy/src/connect.ts`:
- Around line 344-347: The loop over Object.keys(integrations) should validate
entries from input.reconnectProviders to avoid silent no-ops; before using
forceReconnect (and inside the surrounding function in connect.ts), iterate
input.reconnectProviders (if present) and check each requested provider exists
in integrations (or persona.integrations if that canonical map is used), and if
any do not exist either log a warning or throw a user-facing error that names
the unknown provider(s) so typos like "slakc" are surfaced; update references to
reconnectProviders, integrations, and the forceReconnect computation to rely on
the validated list (or bail early) so intended forced-reconnects cannot be
accidentally ignored.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5e4a72fb-20b7-4fdc-9697-0550a1483374

📥 Commits

Reviewing files that changed from the base of the PR and between ec5dbeb and 547765c.

📒 Files selected for processing (10)
  • .trajectories/completed/2026-05/traj_61qz5tibb2lm.json
  • .trajectories/completed/2026-05/traj_61qz5tibb2lm.md
  • .trajectories/index.json
  • packages/cli/src/deploy-command.test.ts
  • packages/cli/src/deploy-command.ts
  • packages/deploy/src/connect.test.ts
  • packages/deploy/src/connect.ts
  • packages/deploy/src/deploy.test.ts
  • packages/deploy/src/deploy.ts
  • packages/deploy/src/types.ts

},
"commits": [],
"filesChanged": [],
"projectId": "/Users/khaliqgant/Projects/AgentWorkforce/workforce-issue-154",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid committing user-specific absolute filesystem paths in trajectory metadata.

projectId currently embeds a local path with a username, which leaks PII and makes the artifact non-portable across machines/CI. Prefer a repo-relative identifier (or omit host-local paths entirely).

Suggested change
-  "projectId": "/Users/khaliqgant/Projects/AgentWorkforce/workforce-issue-154",
+  "projectId": "workforce-issue-154",
📝 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.

Suggested change
"projectId": "/Users/khaliqgant/Projects/AgentWorkforce/workforce-issue-154",
"projectId": "workforce-issue-154",
🤖 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 @.trajectories/completed/2026-05/traj_61qz5tibb2lm.json at line 51, The
trajectory metadata currently embeds a user-specific absolute path in the
"projectId" JSON field which leaks PII and is non-portable; replace the value of
"projectId" in the JSON object (the "projectId" key inside the trajectory file)
with a repo-relative identifier or a neutral project slug (e.g.,
"workforce-issue-154" or omit the field entirely), ensuring no local filesystem
paths or usernames remain; update any code that consumes this trajectory
metadata to expect the new identifier format if necessary.

Comment thread .trajectories/index.json
"status": "completed",
"startedAt": "2026-05-28T07:46:09.716Z",
"completedAt": "2026-05-28T07:47:24.475Z",
"path": "/Users/khaliqgant/Projects/AgentWorkforce/workforce-issue-154/.trajectories/completed/2026-05/traj_61qz5tibb2lm.json"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Store trajectory index paths as repo-relative, not host-local absolute paths.

This path leaks local user/machine information and is brittle in shared environments. Use a repository-relative path so the index is portable.

Suggested change
-      "path": "/Users/khaliqgant/Projects/AgentWorkforce/workforce-issue-154/.trajectories/completed/2026-05/traj_61qz5tibb2lm.json"
+      "path": ".trajectories/completed/2026-05/traj_61qz5tibb2lm.json"
📝 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.

Suggested change
"path": "/Users/khaliqgant/Projects/AgentWorkforce/workforce-issue-154/.trajectories/completed/2026-05/traj_61qz5tibb2lm.json"
"path": ".trajectories/completed/2026-05/traj_61qz5tibb2lm.json"
🤖 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 @.trajectories/index.json at line 45, The index currently stores a host-local
absolute path under the "path" key (e.g. the entry in .trajectories/index.json
with value "/Users/khaliqgant/.../traj_61qz5tibb2lm.json"); update the
generation/serialization logic to record repository-relative paths instead of
absolute machine paths (for example "completed/2026-05/traj_61qz5tibb2lm.json").
Locate the code that writes entries into .trajectories/index.json (the logic
that sets the "path" field for trajectory files) and replace absolute path usage
with a repo-root-relative computation (use the repo root as base when computing
the stored path) so the index is portable and no local user/machine information
is leaked.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 5 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name=".trajectories/index.json">

<violation number="1" location=".trajectories/index.json:45">
P2: Store trajectory index paths as repo-relative, not host-local absolute paths. This path leaks local user/machine information and is brittle in shared environments.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread .trajectories/index.json
"status": "completed",
"startedAt": "2026-05-28T07:46:09.716Z",
"completedAt": "2026-05-28T07:47:24.475Z",
"path": "/Users/khaliqgant/Projects/AgentWorkforce/workforce-issue-154/.trajectories/completed/2026-05/traj_61qz5tibb2lm.json"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Store trajectory index paths as repo-relative, not host-local absolute paths. This path leaks local user/machine information and is brittle in shared environments.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .trajectories/index.json, line 45:

<comment>Store trajectory index paths as repo-relative, not host-local absolute paths. This path leaks local user/machine information and is brittle in shared environments.</comment>

<file context>
@@ -36,6 +36,13 @@
+      "status": "completed",
+      "startedAt": "2026-05-28T07:46:09.716Z",
+      "completedAt": "2026-05-28T07:47:24.475Z",
+      "path": "/Users/khaliqgant/Projects/AgentWorkforce/workforce-issue-154/.trajectories/completed/2026-05/traj_61qz5tibb2lm.json"
     }
   }
</file context>
Suggested change
"path": "/Users/khaliqgant/Projects/AgentWorkforce/workforce-issue-154/.trajectories/completed/2026-05/traj_61qz5tibb2lm.json"
"path": ".trajectories/completed/2026-05/traj_61qz5tibb2lm.json"

@khaliqgant khaliqgant merged commit f8491fe into main May 28, 2026
3 checks passed
@khaliqgant khaliqgant deleted the codex/issue-154-deploy-reconnect branch May 28, 2026 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

deploy reports 'already connected' while the runtime connection is pending (no force-reconnect)

1 participant