Skip to content

fix(factory): use shared cloud session for relayfile auth#343

Merged
khaliqgant merged 5 commits into
mainfrom
ar-262-factory-shared-cloud-session
Jun 14, 2026
Merged

fix(factory): use shared cloud session for relayfile auth#343
khaliqgant merged 5 commits into
mainfrom
ar-262-factory-shared-cloud-session

Conversation

@khaliqgant

@khaliqgant khaliqgant commented Jun 14, 2026

Copy link
Copy Markdown
Member

Summary

  • move factory relayfile auth off per-CLI credential files and onto the shared @agent-relay/cloud session
  • provide the relayfile SDK a refreshable shared-session token provider with in-flight coalescing
  • narrow default factory relayfile scopes and reject stale credsPath callers
  • update mount-client auth tests for shared-session refresh, coalescing, and legacy credential rejection

Tests

  • npx vitest run packages/factory-sdk/src/mount/relayfile-cloud-mount-client.test.ts
  • npx vitest run packages/factory-sdk/src/cli/fleet.test.ts packages/factory-sdk/src/mount/relayfile-cloud-mount-client.test.ts
  • npm run typecheck:node

Note: this PR covers the pear/factory-sdk slice of AR-262. The relayfile CLI and cloud delegated-token route contract changes remain in their respective repos.

Review in cubic

@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

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@khaliqgant, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 29 minutes and 9 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b22a9638-f8af-45bc-b0e0-ca3e95a40954

📥 Commits

Reviewing files that changed from the base of the PR and between cc2c42f and 9a26245.

📒 Files selected for processing (2)
  • packages/factory-sdk/src/mount/relayfile-cloud-mount-client.test.ts
  • packages/factory-sdk/src/mount/relayfile-cloud-mount-client.ts
📝 Walkthrough

Walkthrough

RelayfileCloudMountClient drops its file-based credential helpers (resolveCloudCredentials, persistCloudCredentials, CredsFileIo, ResolvedCloudCredentials) and replaces them with a shared CloudSessionProvider. fromConfig now rejects credsPath, resolves sessions via a coalescing createSharedCloudSessionResolver, and translates CloudAuthError into a user-facing login prompt. Tests are updated to match.

Changes

RelayfileCloudMountClient — cloud session provider refactor

Layer / File(s) Summary
New exported types, scopes, and config interface
packages/factory-sdk/src/mount/relayfile-cloud-mount-client.ts
Adds FACTORY_RELAYFILE_SCOPES, CloudSessionProvider, RelayfileWorkspaceHandleLike, and RelayfileSetupFactory exports. Updates RelayfileCloudMountClientConfig to replace credsPath/agentName with cloudApiUrl, cloudSessionProvider, cloudSessionRefreshTimeoutMs, cloudSessionEnv, relayfileSetupFactory, and scopes. Removes RelayfileCloudTokenSet import.
fromConfig and shared session resolver
packages/factory-sdk/src/mount/relayfile-cloud-mount-client.ts
Rewrites fromConfig to reject credsPath, obtain a token via the shared resolver, and delegate workspace construction to relayfileSetupFactory. Adds createDefaultRelayfileSetup and createSharedCloudSessionResolver with inFlight promise caching and CloudAuthError"run 'agent-relay login'" translation.
Updated fromConfig test suite
packages/factory-sdk/src/mount/relayfile-cloud-mount-client.test.ts
Replaces legacy credential fixture imports with storedAuth/cloudSession helpers. Adds five fromConfig test cases covering scope delegation, token refresh reuse, concurrent session coalescing, credsPath rejection, and CloudAuthError surfacing.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Possibly related PRs

  • AgentWorkforce/pear#266: Modifies the fleet CLI to call RelayfileCloudMountClient.fromConfig via an injected cloudMountFromConfig, meaning the config interface changes in this PR (removing credsPath, adding session provider fields) directly affect that integration point.

Poem

🐇 Hoppity-hop, no more creds on the shelf,
The session provider now handles itself!
inFlight coalesces, one promise shared,
CloudAuthError caught—browser login declared.
"Run agent-relay login," the rabbit declared,
No credential files, the burrow is cleared! 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main change: migrating relayfile authentication to use a shared cloud session instead of credential files.
Description check ✅ Passed The description clearly relates to the changeset, explaining the motivation and scope of the auth migration across multiple files and test updates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ar-262-factory-shared-cloud-session

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 `@packages/factory-sdk/src/mount/relayfile-cloud-mount-client.ts`:
- Around line 122-125: The deprecated credsPath validation check in the
fromConfig method comes after the early return for the client config path,
allowing callers to bypass the intended rejection. Move the credsPath guard
check (the if statement checking for 'credsPath' in config) to execute before
the client fast-path return (the if statement checking for config.client). This
ensures that any config containing the deprecated credsPath field is rejected
regardless of whether client is also present.
🪄 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: e48084a2-9738-40c2-b696-d4cb8cd7eae4

📥 Commits

Reviewing files that changed from the base of the PR and between e7571b7 and cc2c42f.

📒 Files selected for processing (2)
  • packages/factory-sdk/src/mount/relayfile-cloud-mount-client.test.ts
  • packages/factory-sdk/src/mount/relayfile-cloud-mount-client.ts

Comment thread packages/factory-sdk/src/mount/relayfile-cloud-mount-client.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cc2c42fe6a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +2 to +5
CloudAuthError,
ensureCloudSession,
type CloudSession,
type CloudSessionOptions,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Bump @agent-relay/cloud before using session APIs

With the current lockfile, clean installs still get @agent-relay/cloud 8.3.7 (package-lock.json lines 216-218), and that version's public exports do not include CloudAuthError, ensureCloudSession, CloudSession, or CloudSessionOptions. In any npm ci/CI/release install these new named imports fail typechecking or module loading before the factory CLI can start, so the dependency and lockfile need to be updated to a version that exports these APIs or this code needs to use the 8.3.7 auth API.

Useful? React with 👍 / 👎.

@khaliqgant

Copy link
Copy Markdown
Member Author

Review blocker fixed in 1e2aabe.

  • Added relayfile:fs:read:/slack/users/** to FACTORY_RELAYFILE_SCOPES so Slack DM event paths watched by SLACK_DM_EVENT_GLOBS remain covered.
  • Added a regression assertion that default factory scopes include Slack users read access while still excluding root /** scopes.

Validation:

  • npx vitest run packages/factory-sdk/src/mount/relayfile-cloud-mount-client.test.ts
  • npm run typecheck:node

I attempted Agent Relay DM to ar-262-review, but this MCP session is still not registered and registration is blocked by missing workspace key.

@agent-relay-code

Copy link
Copy Markdown
Contributor

Findings

  1. Blocking: the PR imports cloud-session symbols that do not exist in the locked @agent-relay/cloud@8.3.7 package.

    npm run typecheck:node fails because CloudAuthError, ensureCloudSession, CloudSession, and CloudSessionOptions are not exported. The installed package exports readStoredAuth, refreshStoredAuth, ensureAuthenticated, StoredAuth, etc. The focused test also fails at runtime with Right-hand side of 'instanceof' is not an object.

    I did not auto-edit this because choosing the replacement auth/session flow is semantic and safety-relevant, not a mechanical import cleanup.

Addressed Comments

  • gemini-code-assist[bot]: quota warning only; no actionable review finding to validate.
  • coderabbitai[bot]: review-in-progress/tool-failure comment only; no actionable inline finding to validate. Current CodeRabbit status is still pending.

Verification

  • npm ci: passed.
  • npm run verify:mcp-resources-drift: passed.
  • npm run lint: passed with existing warnings only.
  • npm run typecheck:web: passed.
  • npm test: passed.
  • npm run typecheck:node: failed on the PR’s changed cloud imports.
  • npx vitest run packages/factory-sdk/src/mount/relayfile-cloud-mount-client.test.ts: failed on the PR’s changed cloud auth test.
  • npx vitest run: failed on the same PR test plus one factory heartbeat test; the heartbeat test passed when rerun in isolation.
  • GitHub reports the PR is open, mergeable: false, and CodeRabbit is pending.

No files were changed.

@khaliqgant

Copy link
Copy Markdown
Member Author

Final cleanup pushed after approval: ea42b99 fix(factory): reject creds path before client injection.

  • Moves the deprecated credsPath guard before the injected-client fast path.
  • Adds regression coverage for client + credsPath still rejecting.

Validation after this commit:

  • npx vitest run packages/factory-sdk/src/mount/relayfile-cloud-mount-client.test.ts (27/27)
  • npm run typecheck:node

I attempted Agent Relay DM to ar-262-review, but this MCP session still reports Not registered.

@agent-relay-code

Copy link
Copy Markdown
Contributor

Findings

  • Blocking: packages/factory-sdk/src/mount/relayfile-cloud-mount-client.ts imports CloudAuthError, ensureCloudSession, CloudSession, and CloudSessionOptions from @agent-relay/cloud, but locked @agent-relay/cloud@8.3.7 does not export them. npm run typecheck:node fails with TS2305 for these symbols, and the new login-error test fails at runtime because CloudAuthError is undefined. This needs a human-authored patch: either use the currently exported cloud auth APIs (readStoredAuth / refreshStoredAuth / ensureAuthenticated as appropriate) or update the dependency that actually provides the new session API.

I did not auto-edit this because choosing the replacement auth/session behavior is semantic and safety-relevant.

Addressed Comments

  • No bot or reviewer comment files were present in .workforce; there were no external comments to validate or address.

Verification

  • npm ci: passed.
  • npm run verify:mcp-resources-drift: passed.
  • npm run lint: passed with existing warnings only.
  • npm run typecheck:web: passed.
  • npm run typecheck:node: failed on missing @agent-relay/cloud exports above.
  • npm test: passed.
  • npx vitest run packages/factory-sdk/src/mount/relayfile-cloud-mount-client.test.ts: failed on the undefined CloudAuthError path.
  • npx vitest run: failed on the same PR-local test, plus one factory heartbeat test that passed when rerun in isolation.
  • npm run build: passed.

No files were changed; the worktree is clean.

@agent-relay-code

Copy link
Copy Markdown
Contributor

Findings

  1. Blocking: the PR imports cloud-session APIs that the locked dependency does not export.
    packages/factory-sdk/src/mount/relayfile-cloud-mount-client.ts, packages/factory-sdk/src/mount/relayfile-cloud-mount-client.test.ts, package-lock.json

    Clean npm ci installs @agent-relay/cloud@8.3.7; its dist/index.d.ts exports readStoredAuth, refreshStoredAuth, ensureAuthenticated, StoredAuth, etc., but not CloudAuthError, ensureCloudSession, CloudSession, or CloudSessionOptions. npm run typecheck:node fails with TS2305 on these imports, and the new auth error test fails at runtime with Right-hand side of 'instanceof' is not an object.

    I did not auto-edit this because choosing between a dependency upgrade and rewriting the shared auth/session flow is semantic and safety-relevant.

Addressed comments

  • gemini-code-assist[bot]: quota warning only; no actionable finding.
  • coderabbitai[bot]: initial rate-limit/walkthrough comment only; no actionable finding.
  • coderabbitai[bot]: raised that credsPath could bypass rejection when client was injected. Stale in current checkout: guard now runs before the client fast path at relayfile-cloud-mount-client.ts:123, with regression coverage at relayfile-cloud-mount-client.test.ts:290.
  • chatgpt-codex-connector[bot]: review wrapper comment; actionable inline was the missing @agent-relay/cloud exports. Valid, left unchanged for human patch as noted above.
  • khaliqgant: noted Slack users read scope fix. Confirmed present at relayfile-cloud-mount-client.ts:39.
  • agent-relay-code[bot]: missing @agent-relay/cloud exports. Valid in current checkout; left unchanged because fix is semantic.
  • khaliqgant: noted final credsPath cleanup. Confirmed present at relayfile-cloud-mount-client.ts:123 and relayfile-cloud-mount-client.test.ts:290.
  • agent-relay-code[bot]: repeated missing @agent-relay/cloud exports. Valid in current checkout; left unchanged because fix is semantic.

Verification

  • npm ci: passed.
  • npm run verify:mcp-resources-drift: passed before and after build.
  • npm run lint: passed with existing warnings.
  • npm run typecheck:web: passed.
  • npm run typecheck:node: failed on the missing cloud exports.
  • npm test: passed.
  • npx vitest run packages/factory-sdk/src/mount/relayfile-cloud-mount-client.test.ts: failed on the undefined CloudAuthError path.
  • npx vitest run: failed on the PR-local auth test plus one factory heartbeat test; the heartbeat test passed when rerun in isolation.
  • npm run build: passed.
  • npm run build:web: killed by the sandbox after transform/rendering started.
  • GitHub API reports PR mergeable: false, mergeable_state: dirty; check-runs showed only cubic · AI code reviewer neutral, and combined status showed CodeRabbit success.

No source edits were applied.

…-cloud-session

# Conflicts:
#	packages/factory-sdk/src/mount/relayfile-cloud-mount-client.test.ts
#	packages/factory-sdk/src/mount/relayfile-cloud-mount-client.ts
@khaliqgant khaliqgant merged commit 361bdc9 into main Jun 14, 2026
5 checks passed
@khaliqgant khaliqgant deleted the ar-262-factory-shared-cloud-session branch June 14, 2026 19:02
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.

1 participant