Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/runtime/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
},
"dependencies": {
"@agent-assistant/proactive": "^0.4.32",
"@agentworkforce/persona-kit": "workspace:*"
"@agentworkforce/persona-kit": "workspace:*",
"@relayfile/adapter-core": "^0.3.29"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Since @relayfile/adapter-core has a peer dependency on @relayfile/sdk (>=0.6.0 <1), consumers of @agentworkforce/runtime may encounter peer dependency resolution warnings during installation if @relayfile/sdk is not explicitly declared.

Consider adding @relayfile/sdk to the peerDependencies of @agentworkforce/runtime to ensure clean installations:

  "peerDependencies": {
    "@relayfile/sdk": ">=0.6.0 <1"
  }

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Declining this one: @relayfile/adapter-core/vfs-client (the only subpath the runtime imports) doesn't import @relayfile/sdk at all, @relayfile/sdk isn't resolvable in the workforce tree, and the sibling consumer @agentworkforce/persona-kit (which also depends on adapter-core) doesn't declare it either — yet everything builds and tests green. Adding it as a peer would impose an unused, uninstalled dependency on every runtime consumer and diverge from the repo's existing convention. The unmet peer is adapter-core's to resolve (ideally peerDependenciesMeta.optional there) rather than something each consumer re-declares.

}
}
Comment on lines +54 to 56

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: Adding @relayfile/adapter-core introduces a transitive peer requirement on @relayfile/sdk; declare that peer at this package boundary so consumers get an explicit install contract and avoid unmet-peer warnings.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/runtime/package.json, line 54:

<comment>Adding `@relayfile/adapter-core` introduces a transitive peer requirement on `@relayfile/sdk`; declare that peer at this package boundary so consumers get an explicit install contract and avoid unmet-peer warnings.</comment>

<file context>
@@ -50,6 +50,7 @@
     "@agent-assistant/proactive": "^0.4.32",
-    "@agentworkforce/persona-kit": "workspace:*"
+    "@agentworkforce/persona-kit": "workspace:*",
+    "@relayfile/adapter-core": "^0.3.29"
   }
 }
</file context>
Suggested change
"@relayfile/adapter-core": "^0.3.29"
}
}
"@relayfile/adapter-core": "^0.3.29"
},
"peerDependencies": {
"@relayfile/sdk": ">=0.6.0 <1"
}
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Same as the gemini thread above — declining. @relayfile/adapter-core/vfs-client (the only subpath the runtime imports) does not import @relayfile/sdk, the sibling consumer @agentworkforce/persona-kit (which also deps adapter-core) doesn't declare it, and @relayfile/sdk isn't even resolvable in the tree — yet build/tests are green. Declaring it here imposes an unused, uninstalled dependency on every runtime consumer. The unmet peer is adapter-core's to fix (ideally peerDependenciesMeta.optional).

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.

Thanks for the feedback! I've saved this as a new learning to improve future reviews.

21 changes: 16 additions & 5 deletions packages/runtime/src/clients/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
// Shared VFS-backed transport surface. All provider interactions go through
// these helpers — no per-provider client code lives in the runtime. Handlers
// and custom clients import these directly instead of recreating the
// path-validation + receipt-polling logic.
// these helpers — no per-provider client code lives in the runtime.
//
// The transport itself now lives in the relayfile layer
// (`@relayfile/adapter-core/vfs-client`) — it's the generic Relayfile
// draft-write protocol, not anything workforce-specific. The runtime re-exports
// it so existing handlers that `import … from '@agentworkforce/runtime/clients'`
// keep working unchanged. The ergonomic per-provider clients live in
// `@relayfile/relay-helpers`.
export {
draftFile,
encodeSegment,
Expand All @@ -11,9 +16,15 @@ export {
readTextFile,
resolveMountRoot,
writeJsonFile,
RelayfileWritebackError,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

To ensure the public API is complete and fully backward-compatible, consider exporting RelayfileWritebackErrorOptions from @relayfile/adapter-core/vfs-client as well. This allows consumers to properly type-annotate error options when working with RelayfileWritebackError.

Suggested change
RelayfileWritebackError,
RelayfileWritebackError,
type RelayfileWritebackErrorOptions,

type RelayfileWritebackErrorOptions,
type IntegrationClientOptions,
type WritebackReceipt,
type WritebackResult
} from './request.js';
} from '@relayfile/adapter-core/vfs-client';

export { WorkforceIntegrationError, SandboxNotAvailableError } from '../errors.js';
export {
WorkforceIntegrationError,
type WorkforceIntegrationErrorOptions,
SandboxNotAvailableError
} from '../errors.js';
262 changes: 0 additions & 262 deletions packages/runtime/src/clients/request.ts

This file was deleted.

52 changes: 13 additions & 39 deletions packages/runtime/src/errors.ts
Original file line number Diff line number Diff line change
@@ -1,44 +1,18 @@
/**
* Error type every integration client throws when a remote write or read
* fails. Lives at the package root rather than inside `clients/` so the
* persona's handler can `import { WorkforceIntegrationError }` without
* reaching into a subpath that may evolve.
*
* The error carries enough metadata for the runtime's retry layer to
* make decisions without parsing message strings:
*
* - `provider` — `github` / `linear` / `slack` / …
* - `operation` — the client method that failed (`comment`,
* `upsertIssue.create`, …)
* - `retryable` — `true` for transient failures (the runtime resends
* the originating event); `false` for permanent shape/validation
* failures that won't change on retry
* - `cause` — the original thrown value, preserved for logs
* Error every integration read/write throws on failure. The class now lives
* with the transport in `@relayfile/adapter-core/vfs-client` as
* `RelayfileWritebackError`; `WorkforceIntegrationError` is kept as an **alias
* of the same class** (not a subclass) so existing handlers that
* `catch (e) { if (e instanceof WorkforceIntegrationError) … }` still match
* what the relocated transport throws. Same `{ provider, operation, cause,
* retryable }` shape.
*/
export interface WorkforceIntegrationErrorOptions {
provider: string;
operation: string;
cause?: unknown;
retryable?: boolean;
}

export class WorkforceIntegrationError extends Error {
readonly provider: string;
readonly operation: string;
override readonly cause?: unknown;
readonly retryable: boolean;

constructor(options: WorkforceIntegrationErrorOptions) {
super(`${options.provider}.${options.operation} failed${
options.cause instanceof Error ? `: ${options.cause.message}` : ''
}`);
this.name = 'WorkforceIntegrationError';
this.provider = options.provider;
this.operation = options.operation;
if (options.cause !== undefined) this.cause = options.cause;
this.retryable = options.retryable ?? false;
}
}
export {
RelayfileWritebackError,
RelayfileWritebackError as WorkforceIntegrationError,

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: Aliasing WorkforceIntegrationError to RelayfileWritebackError changes the emitted error name, which can break existing err.name-based handlers.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/runtime/src/errors.ts, line 12:

<comment>Aliasing `WorkforceIntegrationError` to `RelayfileWritebackError` changes the emitted error `name`, which can break existing `err.name`-based handlers.</comment>

<file context>
@@ -1,44 +1,18 @@
-}
+export {
+  RelayfileWritebackError,
+  RelayfileWritebackError as WorkforceIntegrationError,
+  type RelayfileWritebackErrorOptions,
+  type RelayfileWritebackErrorOptions as WorkforceIntegrationErrorOptions
</file context>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Intentional, declining. The alias keeps WorkforceIntegrationError as the same class as RelayfileWritebackError, which is what preserves the critical back-compat: a transport error stays instanceof WorkforceIntegrationError (a subclass would break that, since the relocated transport throws the base). The trade-off is err.name now reads RelayfileWritebackError — which is accurate, and nothing in workforce or the agents keys on that string (grepped; all use instanceof). The provider/operation/retryable/message fields are unchanged.

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.

Thanks for the feedback! I've saved this as a new learning to improve future reviews.

type RelayfileWritebackErrorOptions,
type RelayfileWritebackErrorOptions as WorkforceIntegrationErrorOptions
} from '@relayfile/adapter-core/vfs-client';

/**
* Thrown when handler code calls `ctx.sandbox.exec()` on a persona that
Expand Down
3 changes: 3 additions & 0 deletions packages/runtime/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,10 @@ export {
type IntegrationClientOptions,
type WritebackReceipt,
type WritebackResult,
RelayfileWritebackError,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

To expose the error option types in the main entry point of the package, consider re-exporting RelayfileWritebackErrorOptions and WorkforceIntegrationErrorOptions here as well.

Suggested change
RelayfileWritebackError,
RelayfileWritebackError,
type RelayfileWritebackErrorOptions,
type WorkforceIntegrationErrorOptions,

type RelayfileWritebackErrorOptions,
WorkforceIntegrationError,
type WorkforceIntegrationErrorOptions,
SandboxNotAvailableError
} from './clients/index.js';

Expand Down
Loading
Loading