Skip to content

fix(persona-kit): warn and skip approvalPolicy on codex 0.1.77+#166

Closed
kjgbot wants to merge 12 commits into
mainfrom
fix/codex-approval-policy-flag
Closed

fix(persona-kit): warn and skip approvalPolicy on codex 0.1.77+#166
kjgbot wants to merge 12 commits into
mainfrom
fix/codex-approval-policy-flag

Conversation

@kjgbot
Copy link
Copy Markdown
Contributor

@kjgbot kjgbot commented May 29, 2026

Root cause

buildInteractiveSpec in persona-kit/src/interactive-spec.ts was generating:

```
codex exec -m --sandbox --ask-for-approval ...
```

--ask-for-approval was removed from codex in 0.1.77 (replaced by --sandbox <mode> for filesystem access and --dangerously-bypass-approvals-and-sandbox for full bypass). Codex exits immediately with a parse error:

```
error: unexpected argument '--ask-for-approval' found
tip: to pass '--ask-for-approval' as a value, use '-- --ask-for-approval'
```

Impact

Any persona with harnessSettings.approvalPolicy set (e.g. proactive-agent-builder.json with "approvalPolicy": "on-request") caused every codex agent step to fail. In ricky's auto-fix/repair loop this surfaced as a captured warning and the repair was retried up to 7 times, burning hours of wall-clock per spec in overnight runs.

Fix

  • Emit a deprecation warning and skip the flag entirely — no codex 0.1.77+ equivalent for the non-bypass policies; on-request (the most common value) is the default behavior anyway
  • The bypass case (approvalPolicy: 'never') is already covered by dangerouslyBypassApprovalsAndSandbox: true
  • Mark approvalPolicy as @deprecated in the type definition
  • Update the test: assert the flag is NOT emitted and a warning IS present

Verification

  • npm test in packages/persona-kit → 229/229 pass

🤖 Generated with Claude Code

Ricky Schema Cascade and others added 11 commits May 28, 2026 12:10
…st methods

Adds sandbox policy to PersonaSpec:
- sandbox: true | 'required' — always boot sandbox (default, preserves behavior)
- sandbox: false | 'optional' — skip sandbox boot, use provider list methods

When sandbox is 'optional':
- ctx.sandbox.exec() throws SandboxNotAvailableError
- ctx.sandbox.readFile/writeFile still work (VFS-backed)
- ctx.harness.run() still works
- Provider clients gain list methods (listProjects, listIssues, listChannels, etc.)

This enables fast-path handler agents that respond in milliseconds instead
of seconds by avoiding Daytona sandbox boot overhead.

Runtime changes:
- SandboxNotAvailableError exported from @agentworkforce/runtime
- cloud-defaults creates no-exec sandbox wrapper when persona.sandbox='optional'
- LinearClient gains: listProjects, listIssues, getTeams, getWorkflowStates
- SlackClient gains: listChannels, listUsers, getMessage, getThreadMessages

Schema auto-regenerated; emit-schema test needs manual update to locked fixture.
… clients

- Add sandbox?: boolean to PersonaSpec (true=boot sandbox, false=skip)
- parseSandbox() validates boolean only, rejects 'required'/'optional' strings
- SandboxNotAvailableError thrown from sandbox.exec() when sandbox is false
- readFile/writeFile still work via wrapper sandbox object
- Add LinearClient.listProjects(), listIssues(), getTeams(), getWorkflowStates()
- Add SlackClient.listChannels(), listUsers(), getMessage(), getThreadMessages()
- Import Linear types from @relayfile/adapter-linear/types
- Update emit-schema locked v1 test to remove stale sandbox assertion
- Add @relayfile/adapter-linear to runtime dependencies
No more adding a new client every time a new adapter is created. The
runtime now only exports generic VFS helpers (listJsonFiles / readJsonFile
/ writeJsonFile / etc.) — handlers use these directly against provider
path conventions (e.g. /linear/issues, /slack/channels).

Removed:
- Typed clients: GithubClient, LinearClient, SlackClient, NotionClient,
  JiraClient, and their factory functions
- Per-provider client files (github.ts, linear.ts, slack.ts, notion.ts,
  jira.ts) and their tests
- IntegrationClients interface from WorkforceCtx — ctx no longer has
  typed provider fields; VFS helpers are the interface
- createDefaultIntegrations() — now returns undefined
- @relayfile/adapter-linear dependency (no longer needed)

Updated:
- types.ts: removed IntegrationClients, WorkforceCtx no longer extends it
- cloud-defaults.ts: removed client factory imports and construction
- index.ts: removed client exports, kept VFS helpers + errors
- runner.test.ts: removed 3 tests that depended on typed clients

Handlers who want typed provider shapes can import directly from
@relayfile/adapter-<provider>/types — no runtime changes needed for
new adapters.
Subagent review found two blocking issues:

1. mcp-workforce/src/tools/integrations.ts imported createGithubClient
   and GithubClient from runtime - compile error after typed client removal.
   Rewrote to use VFS helpers (writeJsonFile, readJsonFile) directly.

2. SandboxNotAvailableError messages referenced sandbox optional string
   but field is now boolean false. Updated error messages and removed
   stale type-assertion workaround in cloud-defaults.ts.
Resolved merge conflicts in deploy/src/connect.ts and connect.test.ts:
- connect.ts: take origin/main's allowWorkspaceFallback feature (two-arg
  workspaceFallbackSource, sessionId+configKey matching in fallback polling,
  readProviderConfigKey/statusMatchesConnectionId/integrationAllowsWorkspaceFallback)
- connect.test.ts: update isConnected call to pass allowWorkspaceFallback,
  add new test for explicit deployer_user source, update connect mock response
  and expectations to match origin/main's connectionId matching logic,
  add new test for ignoring fallback rows with different connectionId
- runtime: ctx.sandbox.exec() now rejects (async) instead of throwing
  sync, so .catch() chains see SandboxNotAvailableError correctly.
- runtime: drop dead createDefaultIntegrations() (always returned
  undefined) and the `integrations` field from CloudRuntimeDefaults.
  runner.ts no longer reads cloudDefaults.integrations.
- persona-kit: JSDoc for `sandbox: false` now references the VFS
  helpers (listJsonFiles / readJsonFile / writeJsonFile) instead of
  phantom typed clients (ctx.linear.listProjects, ctx.gmail.listThreads
  did not exist after the typed-client refactor). Schema regenerated.
- mcp-workforce: integration.github.getPr now reads `{ target }` from
  args, matching the prior MCP shape; bare-args was a regression.
- mcp-workforce: validate comment body with asNonEmptyString.
- mcp-workforce: replace asNumber with asPositiveInteger for PR/issue
  numbers and review-comment line numbers — negatives and fractions
  are rejected.
- pnpm-lock.yaml: drop @relayfile/adapter-linear (already removed from
  runtime/package.json), fixing the ERR_PNPM_OUTDATED_LOCKFILE CI fail.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The prior PR commit (9020e5d) dropped GithubClient / LinearClient /
SlackClient / NotionClient / JiraClient from the runtime, but the
example agents still reached for ctx.github / ctx.linear / ctx.slack
and createGithubClient, so the examples typecheck step fails on CI.
Migrate them to the runtime's VFS helpers (writeJsonFile / readJsonFile
/ draftFile) against the canonical Relayfile path conventions:

- examples/review-agent: getPr reads pulls/<n>.json; comment / postReview
  write drafts under issues/comments/, pulls/reviews/; slack reply writes
  under slack/channels/<channel>/messages/<ts>/reply/.
- examples/linear-shipper: getIssue reads linear/issues/<id>.json; comment
  drafts under linear/issues/<id>/comments/; github createIssue drafts
  under github/repos/<owner>/<repo>/issues/.
- examples/notion-essay-pr: createPullRequest drafts under
  github/repos/<owner>/<repo>/pulls/. URL is read from the writeback
  receipt (fallback to the draft path).
- examples/weekly-digest: upsertIssue drafts under
  github/repos/<owner>/<repo>/issues/, dropping the createGithubClient
  fallback that no longer exists.

Tests/docs:
- Smoke tests for notion-essay-pr (examples/ and packages/deploy/test/e2e)
  now create a temp RELAYFILE_MOUNT_ROOT and assert the draft JSON written
  to disk, instead of mocking a typed ctx.github.
- runtime: handler / ctx / request.ts JSDoc updated to reference VFS
  helpers in the usage examples (ctx.github.* references were stale).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cubic flagged that the migrated examples were treating the in-mount
draft path as if it were a clickable GitHub/Linear URL when the
writeback worker hadn't produced a receipt yet.

- linear-shipper: skip the Linear back-link comment when
  createIssue's writeback receipt is missing; log a warn line
  carrying the draft path instead of posting a bogus issue URL.
- notion-essay-pr: skip the memory.save / pr-created log line when
  the github createPullRequest receipt is missing; emit a
  pr-pending warn carrying the draft path.

Smoke tests now spin up a fake writeback worker (50ms interval) that
rewrites draft JSON under github/repos/.../pulls/ with a receipt
envelope ({ created, id, url }) so the agent's poll loop resolves
with a real-looking URL, mirroring how the production writeback
worker turns drafts into receipts.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `--ask-for-approval` flag was removed from codex in 0.1.77 (replaced
by `--sandbox <mode>` + `--dangerously-bypass-approvals-and-sandbox`).
`buildInteractiveSpec` was still generating `--ask-for-approval <policy>`
for any persona that set `harnessSettings.approvalPolicy`, causing every
codex agent step to fail immediately with a parse error:

  error: unexpected argument '--ask-for-approval' found

This surfaced in ricky's auto-fix loop as a warning that prevented
workflow repair and caused every auto-fix iteration to burn its full
retry budget (~hours per spec in overnight runs).

Fix: emit a deprecation warning and skip the flag entirely. The bypass
case is already handled by `dangerouslyBypassApprovalsAndSandbox: true`.
Mark `approvalPolicy` as @deprecated in the type definition.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@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 29, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR refactors the workforce runtime and agents away from direct per-provider integration client usage toward a unified, filesystem-mounted VFS/Relayfile-based integration pattern. It adds sandbox execution control via a new PersonaSpec.sandbox boolean field, deprecates the approvalPolicy flag, removes provider-specific client implementations, and updates example agents and tests to use VFS helpers.

Changes

VFS-First Integration & Sandbox Runtime Foundation

Layer / File(s) Summary
Persona Sandbox Support
packages/persona-kit/schemas/persona.schema.json, packages/persona-kit/src/types.ts, packages/persona-kit/src/parse.ts, packages/persona-kit/src/parse.test.ts, packages/persona-kit/src/emit-schema.test.ts, packages/persona-kit/src/interactive-spec.test.ts, packages/persona-kit/src/interactive-spec.ts
Schema, types, and parsing are updated to accept sandbox: boolean in PersonaSpec; approvalPolicy is marked deprecated with a warning when used in Codex harness settings; test coverage validates parsing of boolean values and rejection of invalid types.
Sandbox Availability Error & Cloud Defaults
packages/runtime/src/errors.ts, packages/runtime/src/cloud-defaults.ts
SandboxNotAvailableError is added and thrown when ctx.sandbox.exec() is called on personas with sandbox: false; cloud defaults conditionally wrap sandbox execution and remove integration setup, leaving only file and harness runner contexts.
Provider Client Implementation Removal
packages/runtime/src/clients/index.ts, packages/runtime/src/clients/request.ts
GitHub, Linear, Slack, Notion, and Jira client implementations and their test modules are deleted; module commentary is reorganized and error re-exports are updated.
Runtime Type & Export Surface Refactor
packages/runtime/src/types.ts, packages/runtime/src/index.ts, packages/runtime/src/runner.ts, packages/runtime/src/runner.test.ts, packages/runtime/src/ctx.ts, packages/runtime/src/ctx.test.ts, packages/runtime/src/handler.ts
IntegrationClients interface and per-provider client extensions to WorkforceCtx are removed; public runtime exports shift from provider-client factories to VFS helpers; integration defaults are no longer merged into context; documentation examples updated to use VFS patterns.
MCP-Workforce GitHub Tool Dispatcher Refactoring
packages/mcp-workforce/src/tools/integrations.ts, packages/mcp-workforce/src/tools/integrations.test.ts
GitHub integration tool methods (comment, createIssue, upsertIssue, postReview, getPr) are rewritten to use readJsonFile/writeJsonFile with Relayfile mount paths; PR metadata is discovered via directory listing; new test validates the VFS-backed PR metadata read flow including diff fallback.
Agent Handlers VFS Conversion
examples/linear-shipper/agent.ts, examples/notion-essay-pr/agent.ts, examples/review-agent/agent.ts, examples/weekly-digest/agent.ts
All four agents replace direct integration client calls with VFS-based reads/writes; vfsClient() helpers resolve mount roots; inputDefault resolution adapts to use ctx.persona.inputSpecs/ctx.persona.inputs; writeback results are checked for receipt URLs before proceeding.
Agent Test Infrastructure & Smoke Test Refactoring
examples/notion-essay-pr/notion-essay-pr.smoke.test.ts, packages/deploy/test/e2e/notion-essay-pr.smoke.test.ts
Smoke tests are refactored to use temp Relayfile mount roots, fake writeback workers that inject receipt envelopes into draft JSON, and disk-based PR writeback collection instead of in-memory GitHub PR mocks; mock runtime file I/O now mirrors /workspace/* writes to disk.
Minor Documentation & Test Cleanups
packages/runtime/src/handler.ts, packages/runtime/src/ctx.ts, packages/deploy/src/connect.test.ts, packages/runtime/src/runner.test.ts
Documentation examples updated to show VFS usage; test layout adjusted; environment setup for relay auth tests clarified; deprecated GitHub defaults test removed.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes


Possibly related issues

  • AgentWorkforce/cloud#1028: The main changes that remove per-provider client implementations and rewire handlers to use the runtime VFS helpers (readJsonFile/writeJsonFile) directly are related to requests to rewire integration clients (ctx.slack/ctx.workflow) and adjust writeback behavior.

Possibly related PRs

  • AgentWorkforce/workforce#92: Core move to Relayfile-VFS as the canonical integration-client style, directly aligned with agent and handler refactors in this PR.
  • AgentWorkforce/workforce#159: Implements the PersonaSpec.sandbox?: boolean behavior and runtime enforcement via SandboxNotAvailableError, touching the same persona-kit and cloud-defaults code.
  • AgentWorkforce/workforce#109: Adds the notion-essay-pr example that this PR later refactors to route PR writeback through VFS instead of direct client calls.

Suggested reviewers

  • willwashburn

Poem

🐰 Hops through Relayfile mounts with glee,
No more clients cluttering our tree,
Sandbox control when we say "false,"
Files written to the VFS waltz,
One clean pattern—hip hip hooray!
We ship integration the new way. 🚀

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.87% 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 accurately summarizes the main change: deprecating and removing the approvalPolicy flag from codex 0.1.77+ due to a breaking change in the CLI interface.
Description check ✅ Passed The description thoroughly explains the root cause (removed CLI flag), impact (agent failures), and fix (deprecation warning and flag removal), directly relating to the changeset.
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 fix/codex-approval-policy-flag
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch fix/codex-approval-policy-flag

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.

@agent-relay-bot
Copy link
Copy Markdown

Reviewed and fixed the PR locally.

Key fixes:

  • Restored GitHub PR metadata/diff reading for nested Relayfile VFS layouts in review-agent and MCP integration.github.getPr.
  • Added MCP regression coverage for nested PR meta.json + diff.patch.
  • Fixed runtime memory test isolation under ambient cloud/Relayfile tokens.
  • Cleaned indentation/file-end formatting in touched files.

Local verification:

  • corepack pnpm run check passed.
  • Follow-up corepack pnpm --filter @agentworkforce/runtime run typecheck passed after final formatting cleanup.

Copy link
Copy Markdown

@agent-relay-bot agent-relay-bot Bot left a comment

Choose a reason for hiding this comment

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

Reviewed and fixed the PR locally.

Key fixes:

  • Restored GitHub PR metadata/diff reading for nested Relayfile VFS layouts in review-agent and MCP integration.github.getPr.
  • Added MCP regression coverage for nested PR meta.json + diff.patch.
  • Fixed runtime memory test isolation under ambient cloud/Relayfile tokens.
  • Cleaned indentation/file-end formatting in touched files.

Local verification:

  • corepack pnpm run check passed.
  • Follow-up corepack pnpm --filter @agentworkforce/runtime run typecheck passed after final formatting cleanup.

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 (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="packages/persona-kit/schemas/persona.schema.json">

<violation number="1" location="packages/persona-kit/schemas/persona.schema.json:264">
P2: `deprecated` must be a boolean per JSON Schema 2020-12. Using a string value is non-standard and will not be recognized as a deprecation by schema-aware tooling (IDE warnings, documentation generators, validators). The informational message should live in `description` alongside `"deprecated": true`.</violation>
</file>

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

Re-trigger cubic

"approvalPolicy": {
"$ref": "#/definitions/CodexApprovalPolicy",
"description": "Codex CLI approval policy (`--ask-for-approval`)."
"deprecated": "`--ask-for-approval` was removed in codex 0.1.77+. Use `dangerouslyBypassApprovalsAndSandbox` or `sandboxMode` instead. Setting this field emits a warning and has no effect."
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: deprecated must be a boolean per JSON Schema 2020-12. Using a string value is non-standard and will not be recognized as a deprecation by schema-aware tooling (IDE warnings, documentation generators, validators). The informational message should live in description alongside "deprecated": true.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/persona-kit/schemas/persona.schema.json, line 264:

<comment>`deprecated` must be a boolean per JSON Schema 2020-12. Using a string value is non-standard and will not be recognized as a deprecation by schema-aware tooling (IDE warnings, documentation generators, validators). The informational message should live in `description` alongside `"deprecated": true`.</comment>

<file context>
@@ -261,7 +261,7 @@
         "approvalPolicy": {
           "$ref": "#/definitions/CodexApprovalPolicy",
-          "description": "Codex CLI approval policy (`--ask-for-approval`)."
+          "deprecated": "`--ask-for-approval` was removed in codex 0.1.77+. Use `dangerouslyBypassApprovalsAndSandbox` or `sandboxMode` instead. Setting this field emits a warning and has no effect."
         },
         "workspaceWriteNetworkAccess": {
</file context>
Suggested change
"deprecated": "`--ask-for-approval` was removed in codex 0.1.77+. Use `dangerouslyBypassApprovalsAndSandbox` or `sandboxMode` instead. Setting this field emits a warning and has no effect."
"deprecated": true,
"description": "**Deprecated**: `--ask-for-approval` was removed in codex 0.1.77+. Use `dangerouslyBypassApprovalsAndSandbox` or `sandboxMode` instead. Setting this field emits a warning and has no effect."

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/runtime/src/cloud-defaults.ts (1)

139-158: 💤 Low value

Consider making the exec signature explicit for clarity.

The exec() wrapper at line 148 could explicitly show its ignored parameters to match the base SandboxContext signature and clarify intent:

- async exec() {
+ async exec(_cmd, _opts) {
   throw new SandboxNotAvailableError();
 },

This makes it clear that the function intentionally ignores the parameters rather than having them implicitly unused.

🤖 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/runtime/src/cloud-defaults.ts` around lines 139 - 158, The exec
wrapper in createSandboxOptionalSandbox currently omits parameters; update its
signature to explicitly accept the same parameters as SandboxContext.exec (e.g.,
the command and options) so it's clear they are intentionally ignored—modify the
exec method in createSandboxOptionalSandbox to include the matching parameter
names/types from the SandboxContext interface and then simply throw
SandboxNotAvailableError, leaving readFile/writeFile delegation unchanged.
🤖 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 `@examples/notion-essay-pr/agent.ts`:
- Around line 28-30: The vfsClient() factory returns an IntegrationClientOptions
object without writebackTimeoutMs, causing the runtime to use the 3_000ms
default and drop long writebacks (making pr.receipt?.url undefined); update
vfsClient() to include an explicit writebackTimeoutMs (e.g. 30_000) in the
returned object alongside relayfileMountRoot (resolveMountRoot({})) so the
github/createPullRequest writeback has enough time to complete.

In `@examples/weekly-digest/agent.ts`:
- Around line 87-99: The code falls back to result.path and persists a draft
path as a "published" memory; change the logic so you still compute issueUrl for
logging if desired but only call ctx.memory.save (and include the "published"
message) when result.receipt?.url is present (i.e., a real receipt URL). Locate
the variables issueUrl and result.receipt and the call to ctx.memory.save in the
weekly-digest flow and guard that save behind a check like if
(result.receipt?.url) so drafts/timeouts are not recorded as published.

---

Nitpick comments:
In `@packages/runtime/src/cloud-defaults.ts`:
- Around line 139-158: The exec wrapper in createSandboxOptionalSandbox
currently omits parameters; update its signature to explicitly accept the same
parameters as SandboxContext.exec (e.g., the command and options) so it's clear
they are intentionally ignored—modify the exec method in
createSandboxOptionalSandbox to include the matching parameter names/types from
the SandboxContext interface and then simply throw SandboxNotAvailableError,
leaving readFile/writeFile delegation unchanged.
🪄 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: 82eb6083-7f2d-4eaa-9aef-e88940904c25

📥 Commits

Reviewing files that changed from the base of the PR and between 50c3e7c and eb04a0b.

📒 Files selected for processing (37)
  • examples/linear-shipper/agent.ts
  • examples/notion-essay-pr/agent.ts
  • examples/notion-essay-pr/notion-essay-pr.smoke.test.ts
  • examples/review-agent/agent.ts
  • examples/weekly-digest/agent.ts
  • packages/deploy/src/connect.test.ts
  • packages/deploy/test/e2e/notion-essay-pr.smoke.test.ts
  • packages/mcp-workforce/src/tools/integrations.test.ts
  • packages/mcp-workforce/src/tools/integrations.ts
  • packages/persona-kit/schemas/persona.schema.json
  • packages/persona-kit/src/emit-schema.test.ts
  • packages/persona-kit/src/interactive-spec.test.ts
  • packages/persona-kit/src/interactive-spec.ts
  • packages/persona-kit/src/parse.test.ts
  • packages/persona-kit/src/parse.ts
  • packages/persona-kit/src/types.ts
  • packages/runtime/src/clients/github.test.ts
  • packages/runtime/src/clients/github.ts
  • packages/runtime/src/clients/index.ts
  • packages/runtime/src/clients/jira.test.ts
  • packages/runtime/src/clients/jira.ts
  • packages/runtime/src/clients/linear.test.ts
  • packages/runtime/src/clients/linear.ts
  • packages/runtime/src/clients/notion.test.ts
  • packages/runtime/src/clients/notion.ts
  • packages/runtime/src/clients/request.ts
  • packages/runtime/src/clients/slack.test.ts
  • packages/runtime/src/clients/slack.ts
  • packages/runtime/src/cloud-defaults.ts
  • packages/runtime/src/ctx.test.ts
  • packages/runtime/src/ctx.ts
  • packages/runtime/src/errors.ts
  • packages/runtime/src/handler.ts
  • packages/runtime/src/index.ts
  • packages/runtime/src/runner.test.ts
  • packages/runtime/src/runner.ts
  • packages/runtime/src/types.ts
💤 Files with no reviewable changes (13)
  • packages/runtime/src/clients/notion.test.ts
  • packages/runtime/src/clients/jira.ts
  • packages/runtime/src/clients/linear.ts
  • packages/runtime/src/clients/linear.test.ts
  • packages/persona-kit/src/emit-schema.test.ts
  • packages/runtime/src/clients/jira.test.ts
  • packages/runtime/src/clients/github.ts
  • packages/runtime/src/clients/slack.test.ts
  • packages/runtime/src/clients/slack.ts
  • packages/runtime/src/clients/github.test.ts
  • packages/runtime/src/runner.test.ts
  • packages/deploy/src/connect.test.ts
  • packages/runtime/src/clients/notion.ts

Comment on lines +28 to +30
function vfsClient(): IntegrationClientOptions {
return { relayfileMountRoot: resolveMountRoot({}) };
}
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 | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Show the default writeback timeout constant and where it's applied
rg -nP 'DEFAULT_WRITEBACK_TIMEOUT_MS' -C2

Repository: AgentWorkforce/workforce

Length of output: 1729


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant regions in the example agent
FILE="examples/notion-essay-pr/agent.ts"
if [ -f "$FILE" ]; then
  echo "=== $FILE (lines 1-120) ==="
  nl -ba "$FILE" | sed -n '1,140p'
else
  echo "Missing $FILE"
  exit 1
fi

echo
echo "=== Search for key strings in $FILE ==="
rg -n "writebackTimeoutMs|weekly-digest|pr-pending|createPullRequest|receipt\.url|writeJsonFile|relayfileMountRoot" "$FILE" -n || true

Repository: AgentWorkforce/workforce

Length of output: 185


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="examples/notion-essay-pr/agent.ts"
[ -f "$FILE" ] || { echo "Missing $FILE"; exit 1; }

echo "=== $FILE (lines 1-140) ==="
cat -n "$FILE" | sed -n '1,140p'

echo
echo "=== Search for key strings in $FILE ==="
rg -n "writebackTimeoutMs|weekly-digest|pr-pending|createPullRequest|receipt\.url|writeJsonFile|relayfileMountRoot" "$FILE" || true

Repository: AgentWorkforce/workforce

Length of output: 5476


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find the weekly-digest example and any explicit writeback timeout config
rg -n "weekly-digest" -S . || true
rg -n "writebackTimeoutMs" -S . || true
rg -n "30_000|30000|3_000|DEFAULT_WRITEBACK_TIMEOUT_MS" -S examples packages | head -n 200 || true

Repository: AgentWorkforce/workforce

Length of output: 20168


examples/notion-essay-pr: raise writebackTimeoutMs to avoid dropping the PR URL path

vfsClient() omits writebackTimeoutMs, so the runtime falls back to DEFAULT_WRITEBACK_TIMEOUT_MS = 3_000ms. For the github/createPullRequest writeback that uploads branch+commit+files, this can exceed 3s; when it does, pr.receipt?.url is undefined and the code logs notion-essay-pr.pr-pending and returns before ctx.memory.save(...).

Mirror the approach used in examples/weekly-digest/agent.ts (it sets writebackTimeoutMs: 30_000) by setting an explicit timeout in vfsClient() (e.g. 30_000ms).

🛡️ Proposed fix: set an explicit writeback timeout
 function vfsClient(): IntegrationClientOptions {
-  return { relayfileMountRoot: resolveMountRoot({}) };
+  return { relayfileMountRoot: resolveMountRoot({}), writebackTimeoutMs: 30_000 };
 }
📝 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
function vfsClient(): IntegrationClientOptions {
return { relayfileMountRoot: resolveMountRoot({}) };
}
function vfsClient(): IntegrationClientOptions {
return { relayfileMountRoot: resolveMountRoot({}), writebackTimeoutMs: 30_000 };
}
🤖 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 `@examples/notion-essay-pr/agent.ts` around lines 28 - 30, The vfsClient()
factory returns an IntegrationClientOptions object without writebackTimeoutMs,
causing the runtime to use the 3_000ms default and drop long writebacks (making
pr.receipt?.url undefined); update vfsClient() to include an explicit
writebackTimeoutMs (e.g. 30_000) in the returned object alongside
relayfileMountRoot (resolveMountRoot({})) so the github/createPullRequest
writeback has enough time to complete.

Comment on lines +87 to 99
const issueUrl = result.receipt?.url ?? result.path;
ctx.log('info', 'weekly-digest.issue.upserted', {
week: isoWeek,
number: result.number,
url: result.url,
created: result.created,
url: issueUrl,
receipt: result.receipt,
clusterCount: clusters.length,
itemCount: deduped.length
});

await ctx.memory.save(`Weekly digest ${isoWeek} published: ${result.url}`, {
await ctx.memory.save(`Weekly digest ${isoWeek} published: ${issueUrl}`, {
tags: ['weekly-digest', `week:${isoWeek}`],
scope: 'workspace'
});
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 | 🟡 Minor | ⚡ Quick win

Falling back to result.path persists a misleading draft path into memory.

When the writeback times out, result.receipt?.url is undefined and issueUrl becomes the in-mount draft path. That value is then logged and saved to workspace memory as ... published: <draftPath>, which is misleading — notion-essay-pr and linear-shipper deliberately skip on a missing receipt for exactly this reason. Gate the "published" memory save on a real receipt URL.

🛡️ Proposed fix: only record a published URL when the receipt is present
-  const issueUrl = result.receipt?.url ?? result.path;
+  const issueUrl = result.receipt?.url;
+  if (!issueUrl) {
+    ctx.log('warn', 'weekly-digest.issue.pending', {
+      week: isoWeek,
+      draftPath: result.path,
+      clusterCount: clusters.length,
+      itemCount: deduped.length
+    });
+    return;
+  }
   ctx.log('info', 'weekly-digest.issue.upserted', {
     week: isoWeek,
     url: issueUrl,
     receipt: result.receipt,
     clusterCount: clusters.length,
     itemCount: deduped.length
   });
📝 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
const issueUrl = result.receipt?.url ?? result.path;
ctx.log('info', 'weekly-digest.issue.upserted', {
week: isoWeek,
number: result.number,
url: result.url,
created: result.created,
url: issueUrl,
receipt: result.receipt,
clusterCount: clusters.length,
itemCount: deduped.length
});
await ctx.memory.save(`Weekly digest ${isoWeek} published: ${result.url}`, {
await ctx.memory.save(`Weekly digest ${isoWeek} published: ${issueUrl}`, {
tags: ['weekly-digest', `week:${isoWeek}`],
scope: 'workspace'
});
const issueUrl = result.receipt?.url;
if (!issueUrl) {
ctx.log('warn', 'weekly-digest.issue.pending', {
week: isoWeek,
draftPath: result.path,
clusterCount: clusters.length,
itemCount: deduped.length
});
return;
}
ctx.log('info', 'weekly-digest.issue.upserted', {
week: isoWeek,
url: issueUrl,
receipt: result.receipt,
clusterCount: clusters.length,
itemCount: deduped.length
});
await ctx.memory.save(`Weekly digest ${isoWeek} published: ${issueUrl}`, {
tags: ['weekly-digest', `week:${isoWeek}`],
scope: 'workspace'
});
🤖 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 `@examples/weekly-digest/agent.ts` around lines 87 - 99, The code falls back to
result.path and persists a draft path as a "published" memory; change the logic
so you still compute issueUrl for logging if desired but only call
ctx.memory.save (and include the "published" message) when result.receipt?.url
is present (i.e., a real receipt URL). Locate the variables issueUrl and
result.receipt and the call to ctx.memory.save in the weekly-digest flow and
guard that save behind a check like if (result.receipt?.url) so drafts/timeouts
are not recorded as published.

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.

2 issues found

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="packages/mcp-workforce/src/tools/integrations.ts">

<violation number="1" location="packages/mcp-workforce/src/tools/integrations.ts:104">
P2: `createIssue`/`upsertIssue` now return a different payload shape (`id`/`identifier`) than the tool contract (`number`/`created`), which can break existing MCP callers.</violation>
</file>

<file name="examples/review-agent/agent.ts">

<violation number="1" location="examples/review-agent/agent.ts:18">
P2: Do not default integration mount root to `process.cwd()` here; it can silently drop outbound GitHub/Slack actions when Relayfile env vars are missing.</violation>
</file>

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

...(Array.isArray(fields.labels) ? { labels: fields.labels.filter(isString) } : {})
});
return {
id: result.receipt?.id ?? '',
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: createIssue/upsertIssue now return a different payload shape (id/identifier) than the tool contract (number/created), which can break existing MCP callers.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/mcp-workforce/src/tools/integrations.ts, line 104:

<comment>`createIssue`/`upsertIssue` now return a different payload shape (`id`/`identifier`) than the tool contract (`number`/`created`), which can break existing MCP callers.</comment>

<file context>
@@ -75,72 +79,111 @@ async function invokeGithub(
+          ...(Array.isArray(fields.labels) ? { labels: fields.labels.filter(isString) } : {})
+        });
+      return {
+        id: result.receipt?.id ?? '',
+        identifier: result.receipt?.identifier ?? '',
+        url: result.receipt?.url ?? result.path
</file context>

// Resolve the Relayfile mount the cloud-managed runtime exposes through
// env (RELAYFILE_MOUNT_ROOT / RELAYFILE_ROOT). Falls back to process.cwd()
// for local smoke runs.
return { relayfileMountRoot: resolveMountRoot({}) };
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: Do not default integration mount root to process.cwd() here; it can silently drop outbound GitHub/Slack actions when Relayfile env vars are missing.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At examples/review-agent/agent.ts, line 18:

<comment>Do not default integration mount root to `process.cwd()` here; it can silently drop outbound GitHub/Slack actions when Relayfile env vars are missing.</comment>

<file context>
@@ -1,7 +1,23 @@
+  // Resolve the Relayfile mount the cloud-managed runtime exposes through
+  // env (RELAYFILE_MOUNT_ROOT / RELAYFILE_ROOT). Falls back to process.cwd()
+  // for local smoke runs.
+  return { relayfileMountRoot: resolveMountRoot({}) };
+}
+
</file context>

@kjgbot
Copy link
Copy Markdown
Contributor Author

kjgbot commented May 29, 2026

Superseded by #167 (clean cherry-pick onto latest main after resolving rebase conflicts)

@kjgbot kjgbot closed this May 29, 2026
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