From b80ac6b44028ef67f63e30c061206f26f1751f93 Mon Sep 17 00:00:00 2001 From: Khaliq Date: Mon, 17 Aug 2026 15:33:22 +0200 Subject: [PATCH 1/2] fix(core): address the Slack gate to the workspace that owns the integration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slack human assistance was asking the run's own provisioned workspace to post the question. That workspace is a throwaway for agent file scope and has no Slack connection, so the gate could never deliver — it parked, then reported `Token has expired`, which is not what was wrong: relayfile integration list --workspace rw_84e3ff6b # the run's workspace -> error: workspace not found in ~/.relayfile/workspaces.json relayfile integration list --workspace rw_7ccfea89 # the registered one -> slack: ready, webhookHealthy: true Slack lives on a real OAuth connection owned by a registered workspace. The gate now resolves against that workspace instead. `chooseIntegrationWorkspace` is deliberately conservative, because guessing wrong here would break working setups: - An explicitly configured `integrations.relayfile.workspaceId` always wins. Naming a workspace is a decision, not a guess to override. - With no `~/.relayfile/workspaces.json` — headless and cloud runs — it keeps whatever was resolved. There the deploy's workspace already owns the integrations, so there is nothing to correct. - A workspace that IS registered is kept, including when it is the recorded default but missing from the id list (duplicate/absent entries are common; the real file here has 13 entries for 12 unique ids). Only an unregistered workspace alongside a known default is redirected, and only when a credential for the target can actually be found — a workspace id without a usable token is not an improvement, so otherwise it keeps the original and lets the credential preflight report the real problem instead of inventing a second one. Credentials are matched to the target by the JWT's `wks` claim rather than assumed. Every redirect is logged with its reason and how to override it. Verified: 8 new tests, plus a check against the real workspaces.json on this machine (correctly redirects rw_84e3ff6b -> rw_7ccfea89). Suite is 854 passed with the one pre-existing workflow-runner persona-runtime failure unchanged. No new typecheck errors (7 before, 7 after). Co-Authored-By: Claude Opus 5 --- .../integration-workspace-choice.test.ts | 92 +++++++++++++ packages/core/src/runner.ts | 123 +++++++++++++++++- 2 files changed, 214 insertions(+), 1 deletion(-) create mode 100644 packages/core/src/__tests__/integration-workspace-choice.test.ts diff --git a/packages/core/src/__tests__/integration-workspace-choice.test.ts b/packages/core/src/__tests__/integration-workspace-choice.test.ts new file mode 100644 index 0000000..4b406ab --- /dev/null +++ b/packages/core/src/__tests__/integration-workspace-choice.test.ts @@ -0,0 +1,92 @@ +/** + * Which workspace a Slack human-assistance gate should address. + * + * Slack lives on a real OAuth connection owned by one of the operator's + * registered workspaces. A run's provisioned workspace is a throwaway for agent + * file scope and has no integrations, so asking it to post a Slack question can + * never work — that was the actual cause of a gate that parked forever. + */ +import { describe, it, expect } from 'vitest'; +import { chooseIntegrationWorkspace } from '../runner.js'; + +const REGISTRY = { defaultId: 'rw_7ccfea89', ids: ['rw_7ccfea89', 'rw_31684d8c', 'rw_fc7b534b'] }; + +describe('chooseIntegrationWorkspace', () => { + it('redirects an unregistered (provisioned) workspace to the registered default', () => { + // rw_84e3ff6b is the per-run workspace; it is absent from workspaces.json. + const choice = chooseIntegrationWorkspace({ + resolvedWorkspaceId: 'rw_84e3ff6b', + registry: REGISTRY, + }); + expect(choice.workspaceId).toBe('rw_7ccfea89'); + expect(choice.reason).toContain('rw_84e3ff6b'); + expect(choice.reason).toContain('no Slack integration'); + // The operator needs to know how to override the decision. + expect(choice.reason).toContain('integrations.relayfile.workspaceId'); + }); + + it('keeps a workspace that is registered', () => { + const choice = chooseIntegrationWorkspace({ + resolvedWorkspaceId: 'rw_31684d8c', + registry: REGISTRY, + }); + expect(choice.workspaceId).toBe('rw_31684d8c'); + expect(choice.reason).toBeUndefined(); + }); + + it('an explicitly configured workspace always wins, even if unregistered', () => { + // Naming a workspace is a decision, not a guess to second-guess. + const choice = chooseIntegrationWorkspace({ + resolvedWorkspaceId: 'rw_84e3ff6b', + configuredWorkspaceId: 'rw_deliberate', + registry: REGISTRY, + }); + expect(choice.workspaceId).toBe('rw_deliberate'); + expect(choice.reason).toBeUndefined(); + }); + + it('is a no-op with no registry — headless and cloud runs keep their workspace', () => { + // There is no ~/.relayfile/workspaces.json in cloud; the deploy's workspace + // is the integration-owning one already. + const choice = chooseIntegrationWorkspace({ resolvedWorkspaceId: 'rw_cloud_deploy' }); + expect(choice.workspaceId).toBe('rw_cloud_deploy'); + expect(choice.reason).toBeUndefined(); + }); + + it('is a no-op when the registry records no default', () => { + const choice = chooseIntegrationWorkspace({ + resolvedWorkspaceId: 'rw_84e3ff6b', + registry: { ids: ['rw_a', 'rw_b'] }, + }); + expect(choice.workspaceId).toBe('rw_84e3ff6b'); + expect(choice.reason).toBeUndefined(); + }); + + it('does not redirect a workspace that already IS the default but is missing from ids', () => { + // Duplicate/absent id entries are common in workspaces.json; matching the + // default is enough. + const choice = chooseIntegrationWorkspace({ + resolvedWorkspaceId: 'rw_7ccfea89', + registry: { defaultId: 'rw_7ccfea89', ids: [] }, + }); + expect(choice.workspaceId).toBe('rw_7ccfea89'); + expect(choice.reason).toBeUndefined(); + }); + + it('ignores a blank configured workspace rather than treating it as a choice', () => { + const choice = chooseIntegrationWorkspace({ + resolvedWorkspaceId: 'rw_84e3ff6b', + configuredWorkspaceId: ' ', + registry: REGISTRY, + }); + expect(choice.workspaceId).toBe('rw_7ccfea89'); + }); + + it('trims a configured workspace id', () => { + const choice = chooseIntegrationWorkspace({ + resolvedWorkspaceId: 'rw_x', + configuredWorkspaceId: ' rw_padded ', + }); + expect(choice.workspaceId).toBe('rw_padded'); + }); +}); diff --git a/packages/core/src/runner.ts b/packages/core/src/runner.ts index cf858d3..86df14f 100644 --- a/packages/core/src/runner.ts +++ b/packages/core/src/runner.ts @@ -578,6 +578,66 @@ const DEFAULT_RELAYFILE_BASE_URL = 'https://file.agentrelay.com'; * documented way to point a workflow at a different Relayfile, and the * provisioning path used to ignore it entirely. */ +/** The operator's locally registered Relayfile workspaces, as recorded by the CLI. */ +export interface RelayfileWorkspaceRegistry { + /** Workspace the CLI treats as current. */ + defaultId?: string; + /** Every registered workspace id. */ + ids: string[]; +} + +export interface IntegrationWorkspaceChoice { + /** Workspace the Slack gate should address. */ + workspaceId: string; + /** Why, for the log line. Undefined when the resolved workspace was kept as-is. */ + reason?: string; +} + +/** + * Choose the workspace a Slack human-assistance gate should address. + * + * Slack lives on a real OAuth connection owned by one of the operator's + * REGISTERED workflows. A run's provisioned workspace is a throwaway for agent + * file scope and has no integrations, so asking it to post a Slack question can + * never work. Prefer the registered workspace that actually owns the connection. + * + * Deliberately conservative: + * - An explicitly configured workspace always wins. If someone names a workspace, + * that is a decision, not a guess to override. + * - With no registry (headless / cloud, where there is no + * `~/.relayfile/workspaces.json`), keep whatever was resolved. There the + * deploy's workspace is the integration-owning one already. + * - If the resolved workspace IS registered, keep it. + * Only an unregistered workspace alongside a known default gets redirected. + */ +export function chooseIntegrationWorkspace(input: { + resolvedWorkspaceId: string; + configuredWorkspaceId?: string; + registry?: RelayfileWorkspaceRegistry; +}): IntegrationWorkspaceChoice { + const { resolvedWorkspaceId, configuredWorkspaceId, registry } = input; + + if (configuredWorkspaceId?.trim()) { + return { workspaceId: configuredWorkspaceId.trim() }; + } + if (!registry || !registry.defaultId?.trim()) { + return { workspaceId: resolvedWorkspaceId }; + } + if (registry.ids.includes(resolvedWorkspaceId)) { + return { workspaceId: resolvedWorkspaceId }; + } + if (registry.defaultId === resolvedWorkspaceId) { + return { workspaceId: resolvedWorkspaceId }; + } + return { + workspaceId: registry.defaultId, + reason: + `workspace "${resolvedWorkspaceId}" is not one of the locally registered Relayfile ` + + `workspaces, so it carries no Slack integration; using the registered default ` + + `"${registry.defaultId}" instead. Set integrations.relayfile.workspaceId to override.`, + }; +} + /** Decode a Relayfile JWT payload. Returns undefined for anything unparseable. */ export function relayfileJwtPayloadOf(token: string): Record | undefined { const parts = token.split('.'); @@ -8790,9 +8850,11 @@ export class WorkflowRunner { } for (let attempt = 0; attempt < 2; attempt++) { - const runtime: RelayfileRuntimeConfig | undefined = this.relayfileRuntimeConfig; + let runtime: RelayfileRuntimeConfig | undefined = this.relayfileRuntimeConfig; if (!runtime) break; try { + // Slack lives on the integration-owning workspace, not the run's throwaway. + runtime = await this.redirectToIntegrationWorkspace(runtime); // Fail closed rather than parking on a question that cannot be delivered. this.assertRelayfileCredentialUsable(runtime, 'Slack human assistance'); this.log(`Slack human assistance using Relayfile ${this.describeRelayfileCredential(runtime)}`); @@ -9341,6 +9403,65 @@ export class WorkflowRunner { return /^[cdg][a-z0-9]{8,}$/i.test(channel); } + /** + * Point a Slack human-assistance call at the workspace that owns the Slack + * integration, when the resolved one demonstrably does not. + * + * Only redirects when it can also produce a credential for the target — a + * workspace id without a usable token is not an improvement, so in that case it + * keeps the original and lets `assertRelayfileCredentialUsable` report the real + * problem rather than inventing a second one. + */ + private async redirectToIntegrationWorkspace( + runtime: RelayfileRuntimeConfig + ): Promise { + const registry = await this.readRelayfileWorkspaceRegistry(); + const choice = chooseIntegrationWorkspace({ + resolvedWorkspaceId: runtime.workspaceId, + configuredWorkspaceId: this.currentConfig?.integrations?.relayfile?.workspaceId, + registry, + }); + if (choice.workspaceId === runtime.workspaceId) return runtime; + + // Find a credential minted for the target workspace. The local-credentials + // files carry their workspace in the JWT's `wks` claim, so this is a match + // rather than an assumption. + const candidate = await this.resolveRelayfileRuntimeConfigFromLocalCredentials(this.currentConfig); + if (!candidate || candidate.workspaceId !== choice.workspaceId) { + this.log( + `Slack human assistance: ${choice.reason} No local credential was found for ` + + `"${choice.workspaceId}", so continuing with "${runtime.workspaceId}".` + ); + return runtime; + } + + this.log(`Slack human assistance: ${choice.reason}`); + this.relayfileRuntimeConfig = candidate; + this.relayfileClient = undefined; + return candidate; + } + + /** + * Read `~/.relayfile/workspaces.json`. Absent or unreadable is a normal state + * (headless runs, fresh machines), so this returns undefined rather than + * throwing and callers treat undefined as "no opinion". + */ + private async readRelayfileWorkspaceRegistry(): Promise { + const registryPath = path.join(homedir(), '.relayfile', 'workspaces.json'); + const raw = await readFile(registryPath, 'utf8').catch(() => undefined); + if (!raw) return undefined; + const parsed = this.tryParseJson(raw); + if (!parsed || typeof parsed !== 'object') return undefined; + const record = parsed as Record; + const entries = Array.isArray(record.workspaces) ? record.workspaces : []; + const ids = entries + .map((entry) => (entry && typeof entry === 'object' ? (entry as Record).id : undefined)) + .filter((id): id is string => typeof id === 'string' && id.trim().length > 0); + const defaultId = typeof record.default === 'string' && record.default.trim() ? record.default.trim() : undefined; + if (!ids.length && !defaultId) return undefined; + return { defaultId, ids }; + } + private async resolveRelayfileRuntimeConfigFromLocalCredentials(config?: RelayYamlConfig): Promise { const localRoot = await this.resolveRelayfileLocalRoot(config); if (!localRoot) return undefined; From 2b77cecf6db045d0d35228fd9b9d65e3363c2757 Mon Sep 17 00:00:00 2001 From: Khaliq Date: Mon, 17 Aug 2026 15:54:07 +0200 Subject: [PATCH 2/2] =?UTF-8?q?fix(core):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20scope=20the=20Slack=20redirect,=20resolve=20creds=20per=20wo?= =?UTF-8?q?rkspace?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings from Codex on #33, both valid. **P1: the redirect overwrote workflow-wide state.** Assigning `this.relayfileRuntimeConfig` and clearing `this.relayfileClient` permanently repointed the runner's shared Relayfile runtime at the Slack workspace. Any later `waitForRelayfileEvent` gate would then poll that workspace while the run's subscriptions and `relayfileIntegrationMount` still belonged to the original one — so a workflow combining Slack assistance with a subsequent Relayfile gate could wait on the wrong workspace with mismatched client/mount state. Exactly the cross-contamination this change was supposed to avoid, introduced by the change itself. The redirect is now scoped to the request: `redirectToIntegrationWorkspace` returns `{ runtime, client }` and `askSlackViaRelayfileRuntime` takes an optional client, falling back to the shared one. Nothing workflow-wide is touched. **P2: credentials were resolved for the wrong workspace.** The generic resolver stops at the first root `resolveRelayfileLocalRoot` returns, which may belong to a different workspace. Asking it for workspace X and comparing afterwards reported "no credential found for X" even when X's credential existed in another mount, silently leaving the gate on the throwaway workspace. `findLocalRelayfileCredentialForWorkspace` now scans every known root — the configured one plus every Pear-managed workspace mirror — and matches on the JWT's `wks` claim, so the answer is about the workspace rather than about which mount sorted first. Also rebased onto the updated #32. Suite 863 passed with the one pre-existing workflow-runner persona-runtime failure unchanged. No new typecheck errors (7 before, 7 after). Co-Authored-By: Claude Opus 5 --- packages/core/src/runner.ts | 82 +++++++++++++++++++++++++++++++------ 1 file changed, 69 insertions(+), 13 deletions(-) diff --git a/packages/core/src/runner.ts b/packages/core/src/runner.ts index 86df14f..d261c44 100644 --- a/packages/core/src/runner.ts +++ b/packages/core/src/runner.ts @@ -8854,7 +8854,8 @@ export class WorkflowRunner { if (!runtime) break; try { // Slack lives on the integration-owning workspace, not the run's throwaway. - runtime = await this.redirectToIntegrationWorkspace(runtime); + const redirected = await this.redirectToIntegrationWorkspace(runtime); + runtime = redirected.runtime; // Fail closed rather than parking on a question that cannot be delivered. this.assertRelayfileCredentialUsable(runtime, 'Slack human assistance'); this.log(`Slack human assistance using Relayfile ${this.describeRelayfileCredential(runtime)}`); @@ -8862,6 +8863,7 @@ export class WorkflowRunner { ...input, channel, runtime, + client: redirected.client, }); } catch (err) { if (attempt > 0 || !this.isRelayfileAuthExpiredError(err)) throw err; @@ -8884,8 +8886,10 @@ export class WorkflowRunner { mentions?: string[]; timeoutMs?: number; runtime: RelayfileRuntimeConfig; + /** Scoped client for a redirected workspace; falls back to the shared one. */ + client?: RelayFileClient; }): Promise<{ answer: { text: string; path?: string; eventId?: string } }> { - const client = this.getRelayfileClient(); + const client = input.client ?? this.getRelayfileClient(); const runtime = input.runtime; const channel = await this.resolveRelayfileSlackChannelId({ channel: input.channel, @@ -9403,6 +9407,55 @@ export class WorkflowRunner { return /^[cdg][a-z0-9]{8,}$/i.test(channel); } + /** + * Find a local Relayfile credential minted for a specific workspace. + * + * The generic resolver stops at the first root {@link resolveRelayfileLocalRoot} + * happens to return, which may belong to a different workspace — so asking it + * for workspace X and comparing afterwards reports "no credential" even when X's + * credential sits in another mount. This scans every known root and matches on + * the JWT's `wks` claim, so the answer is about the workspace rather than about + * which mount sorted first. + */ + private async findLocalRelayfileCredentialForWorkspace( + workspaceId: string + ): Promise { + const roots = new Set(); + const configured = await this.resolveRelayfileLocalRoot(this.currentConfig); + if (configured) roots.add(configured); + + // Every Pear-managed workspace mirror, not just the active one. + const pearRoot = path.join(homedir(), '.agentworkforce', 'pear', 'relayfile', 'workspaces'); + const pearEntries = await readdir(pearRoot, { withFileTypes: true }).catch(() => []); + for (const entry of pearEntries) { + if (entry.isDirectory()) roots.add(path.join(pearRoot, entry.name)); + } + + for (const root of roots) { + for (const rel of [ + path.join('discovery', 'slack', '.relay', 'creds.json'), + path.join('slack', '.relay', 'creds.json'), + ]) { + const raw = await readFile(path.join(root, rel), 'utf8').catch(() => undefined); + if (!raw) continue; + const parsed = this.tryParseJson(raw); + if (!parsed || typeof parsed !== 'object') continue; + const token = (parsed as Record).token; + if (typeof token !== 'string' || !token.trim()) continue; + if (this.relayfileWorkspaceIdFromJwt(token) !== workspaceId) continue; + return { + baseUrl: resolveRelayfileBaseUrl({ + configBaseUrl: this.currentConfig?.integrations?.relayfile?.baseUrl, + }), + workspaceId, + token, + source: 'local-creds', + }; + } + } + return undefined; + } + /** * Point a Slack human-assistance call at the workspace that owns the Slack * integration, when the resolved one demonstrably does not. @@ -9414,31 +9467,34 @@ export class WorkflowRunner { */ private async redirectToIntegrationWorkspace( runtime: RelayfileRuntimeConfig - ): Promise { + ): Promise<{ runtime: RelayfileRuntimeConfig; client?: RelayFileClient }> { const registry = await this.readRelayfileWorkspaceRegistry(); const choice = chooseIntegrationWorkspace({ resolvedWorkspaceId: runtime.workspaceId, configuredWorkspaceId: this.currentConfig?.integrations?.relayfile?.workspaceId, registry, }); - if (choice.workspaceId === runtime.workspaceId) return runtime; + if (choice.workspaceId === runtime.workspaceId) return { runtime }; - // Find a credential minted for the target workspace. The local-credentials - // files carry their workspace in the JWT's `wks` claim, so this is a match - // rather than an assumption. - const candidate = await this.resolveRelayfileRuntimeConfigFromLocalCredentials(this.currentConfig); - if (!candidate || candidate.workspaceId !== choice.workspaceId) { + const candidate = await this.findLocalRelayfileCredentialForWorkspace(choice.workspaceId); + if (!candidate) { this.log( `Slack human assistance: ${choice.reason} No local credential was found for ` + `"${choice.workspaceId}", so continuing with "${runtime.workspaceId}".` ); - return runtime; + return { runtime }; } this.log(`Slack human assistance: ${choice.reason}`); - this.relayfileRuntimeConfig = candidate; - this.relayfileClient = undefined; - return candidate; + // Deliberately NOT assigned to this.relayfileRuntimeConfig / relayfileClient. + // Those are workflow-wide: overwriting them would leave later + // waitForRelayfileEvent gates polling this Slack workspace while the run's + // subscriptions and integration mount still belong to the original one. The + // redirect is scoped to this request via a dedicated client. + return { + runtime: candidate, + client: new RelayFileClient({ baseUrl: candidate.baseUrl, token: candidate.token }), + }; } /**