feat(runtime): adopt Relayfile-VFS as the canonical integration-client style#92
Open
khaliqgant wants to merge 3 commits into
Open
feat(runtime): adopt Relayfile-VFS as the canonical integration-client style#92khaliqgant wants to merge 3 commits into
khaliqgant wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughThis PR adds a filesystem-backed Relayfile VFS transport for integration clients, rewrites the GitHub client to use VFS drafts+writeback receipts, implements Linear/Slack/Notion/Jira clients on the new transport, and migrates persona/tier runtime schema to top-level harness/model/systemPrompt/harnessSettings with corresponding CLI, persona-kit, and router updates. Documentation and tests updated. ChangesRelayfile VFS transport & integration clients
Persona schema & CLI / persona-kit / router migration
Sequence DiagramsequenceDiagram
participant Handler as Handler (agent/handler)
participant VFS as Relayfile VFS (mount)
participant Poll as writeback poller (client)
participant Worker as Writeback Worker
participant Provider as External Provider API
Handler->>VFS: writeJsonFile(draft at canonical path)
Note over VFS: atomic temp write + rename
alt writebackTimeoutMs > 0
Poll->>VFS: poll canonical file for receipt
alt receipt present
Poll-->>Handler: return WritebackResult (created/id/path)
else repeat until timeout
Poll->>VFS: re-read file
end
else
Note over Handler: fire-and-forget (no wait)
end
Worker->>VFS: read draft
Worker->>Provider: perform API operation (create/update)
Worker->>VFS: write receipt (created/id/path)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~150 minutes Possibly related issues
Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
This was referenced May 12, 2026
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Comment on lines
+156
to
+162
| await writeJsonFile( | ||
| opts, | ||
| 'github', | ||
| 'upsertIssue.update', | ||
| `${issueDir}/${encodeSegment(existing.value.number)}.json`, | ||
| { title: args.title, body: args.body, labels: args.labels } | ||
| ); |
Contributor
There was a problem hiding this comment.
🔴 upsertIssue update overwrites the issue file, losing the number field needed for subsequent match lookups
When upsertIssue finds an existing issue and updates it (line 156–162), it writes { title, body, labels } to the canonical <number>.json file via writeJsonFile. This completely replaces the file's previous content, which included number and html_url metadata. On the next upsertIssue call, listJsonFiles reads back the file as a GithubIssueFile but number is now undefined. The match check at line 154 (issue.value.title === args.matchTitle && issue.value.number) fails because number is falsy, so the code falls through to this.createIssue(args) and creates a duplicate issue. This breaks the weekly-digest agent's core use case — every run after the first update creates a new issue instead of updating the existing one.
Trace of the data loss
Before update, 7.json contains:
{"number": 7, "title": "Weekly digest — 2026-W20", "html_url": "https://..."}After line 161 writes, 7.json becomes:
{"title": "Weekly digest — 2026-W20", "body": "refreshed", "labels": ["weekly-digest"]}number and html_url are gone. Next lookup at line 154 sees number: undefined → no match → creates duplicate.
Suggested change
| await writeJsonFile( | |
| opts, | |
| 'github', | |
| 'upsertIssue.update', | |
| `${issueDir}/${encodeSegment(existing.value.number)}.json`, | |
| { title: args.title, body: args.body, labels: args.labels } | |
| ); | |
| await writeJsonFile( | |
| opts, | |
| 'github', | |
| 'upsertIssue.update', | |
| `${issueDir}/${encodeSegment(existing.value.number)}.json`, | |
| { number: existing.value.number, html_url: existing.value.html_url, url: existing.value.url, title: args.title, body: args.body, labels: args.labels } | |
| ); |
Was this helpful? React with 👍 or 👎 to provide feedback.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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/weekly-digest/README.md`:
- Around line 60-62: The RELAYFILE_MOUNT_ROOT environment variable is currently
scoped only to the echo process and not passed to the downstream node runner;
move the RELAYFILE_MOUNT_ROOT assignment so it applies to the node process
(i.e., export or place the variable before invoking node
/tmp/wf-weekly-digest/runner.mjs) so that runner.mjs can read
RELAYFILE_MOUNT_ROOT when processing the piped JSON from echo.
In `@packages/runtime/src/clients/github.ts`:
- Around line 107-129: The comment() and createIssue() (and the create-branch
path of upsertIssue()) currently return Relayfile paths or zero when
writeJsonFile() yields no receipt; update these methods to require a valid
writeback receipt before returning GitHub metadata by checking result.receipt
(and specific fields like receipt.created/receipt.id/receipt.url) and throw a
clear error if absent instead of returning result.path or 0; ensure the same
receipt-mandatory validation and error handling is applied in upsertIssue()’s
create branch so callers only receive real GitHub IDs/URLs.
- Around line 154-170: The upsertIssue implementation currently finds issues by
comparing stored.title to args.matchTitle but immediately rewrites the title to
args.title, so subsequent calls become non-idempotent; update the matching logic
inside upsertIssue (the issues.find predicate) to match either
existing.value.title === args.matchTitle OR existing.value.title === args.title
(or, if available, prefer matching by a stable identifier like
existing.value.number or args.number), so the same issue is found even after the
title is changed and createIssue is not called again; ensure you still use
existing.value.number when calling writeJsonFile and returning the upsert
result.
- Around line 174-189: The code is re-encoding the directory name returned by
findNumberSegment (pullSegment) which can double-escape slashes; instead build
pullRoot using pullSegment verbatim (i.e., remove encodeSegment(pullSegment)) so
subsequent reads via readJsonFile/readTextFile use the actual discovered
directory name; keep encoding only where you intentionally encode the numeric
target (e.g., encodeSegment(target.number)) if needed for fallbacks.
In `@packages/runtime/src/clients/jira.ts`:
- Around line 54-62: The transition method accepts a transition id that may be
empty after trimming; before calling writeJsonFile (and when computing id in
transition), validate that id is a non-empty string (after trim) and throw or
return a clear error if empty to avoid writing invalid drafts; update the logic
in async transition (and any helper that computes id) to trim, check id.length >
0, and bail with a descriptive error instead of calling writeJsonFile or
creating the draft via draftFile('create transition').
In `@packages/runtime/src/clients/notion.ts`:
- Around line 48-50: The createPage return currently sets id to
result.receipt?.created ?? result.receipt?.id ?? '' which can produce an empty
string; change it to return undefined instead of '' so callers don't receive a
bogus id. Update the returned object in createPage to use
result.receipt?.created ?? result.receipt?.id ?? undefined for the id field (and
adjust any type if needed) so the absence of a receipt yields undefined rather
than an empty string.
- Around line 29-33: readDatabaseId currently throws a generic Error on invalid
parent database id; replace that with throwing a WorkforceIntegrationError so
failures follow the integration error contract and carry retry metadata. Update
the readDatabaseId function to import/use WorkforceIntegrationError (or the
existing integration error class) and throw a WorkforceIntegrationError with a
descriptive message like "Notion createPage file writeback requires
parent.database_id" and include any required retry/metadata fields per
WorkforceIntegrationError's constructor so callers (createPage path) get
consistent error type and retry behavior.
In `@packages/runtime/src/clients/request.ts`:
- Around line 225-239: The loop in waitForReceipt reads/parses the file before
checking fire-and-forget mode, allowing payloads with top-level id/path/created
to be misinterpreted as receipts; modify waitForReceipt so it checks whether
writeback is disabled (client.writebackTimeoutMs <= 0) or the deadline has
already passed before calling readCurrentJson, and only parse/validate the JSON
when writeback is enabled and there is remaining time (use timeoutMs/deadline or
client.writebackTimeoutMs and client.writebackPollMs in the do/while loop).
Ensure the early-return behavior for fire-and-forget remains (return undefined
immediately when timeoutMs <= 0) and preserve the poll delay logic
(client.writebackPollMs) and the final deadline comparison (Date.now() <
deadline) around the parse step.
In `@packages/runtime/src/clients/slack.ts`:
- Around line 49-80: The post and dm methods currently allow empty channel/user
or empty text and write malformed drafts; add local validation at the start of
SlackClient.post and SlackClient.dm (and optionally SlackClient.reply) to reject
empty identifiers and empty text similar to parseThreadRef: verify channel/user
(and for reply the parsed thread.channel and thread.ts) are non-empty and that
text is a non-blank string, and throw a clear Error (or return a rejected
Promise) before calling writeJsonFile; reuse encodeSegment/tsPathSegment only
after validation and keep the existing receipt extraction logic 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: 99464476-b9fe-4e7c-8050-902e4384afc0
📒 Files selected for processing (18)
examples/weekly-digest/README.mdexamples/weekly-digest/agent.tspackages/runtime/src/clients/errors.tspackages/runtime/src/clients/github.test.tspackages/runtime/src/clients/github.tspackages/runtime/src/clients/index.tspackages/runtime/src/clients/jira.test.tspackages/runtime/src/clients/jira.tspackages/runtime/src/clients/linear.test.tspackages/runtime/src/clients/linear.tspackages/runtime/src/clients/notion.test.tspackages/runtime/src/clients/notion.tspackages/runtime/src/clients/request.tspackages/runtime/src/clients/slack.test.tspackages/runtime/src/clients/slack.tspackages/runtime/src/errors.tspackages/runtime/src/index.tspackages/runtime/src/types.ts
💤 Files with no reviewable changes (1)
- packages/runtime/src/clients/errors.ts
Comment on lines
+60
to
62
| RELAYFILE_MOUNT_ROOT=/path/to/mount \ | ||
| echo '{"id":"manual-1","workspace":"ws_demo","type":"cron.tick","occurredAt":"2026-05-12T09:00:00Z","name":"weekly","cron":"0 9 * * 6"}' \ | ||
| | node /tmp/wf-weekly-digest/runner.mjs |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Case 1: env before first command in pipeline"
RELAYFILE_MOUNT_ROOT=/tmp/demo printf 'x\n' | /usr/bin/env | rg -n 'RELAYFILE_MOUNT_ROOT' || true
echo "Case 2: env before second command in pipeline"
printf 'x\n' | RELAYFILE_MOUNT_ROOT=/tmp/demo /usr/bin/env | rg -n 'RELAYFILE_MOUNT_ROOT'Repository: AgentWorkforce/workforce
Length of output: 192
Move RELAYFILE_MOUNT_ROOT to before the node process in the pipeline.
At lines 60-62, the environment variable is scoped only to echo, not to the downstream node command, so the runner may not see RELAYFILE_MOUNT_ROOT. Place the variable assignment before node instead.
Suggested fix
-RELAYFILE_MOUNT_ROOT=/path/to/mount \
-echo '{"id":"manual-1","workspace":"ws_demo","type":"cron.tick","occurredAt":"2026-05-12T09:00:00Z","name":"weekly","cron":"0 9 * * 6"}' \
- | node /tmp/wf-weekly-digest/runner.mjs
+echo '{"id":"manual-1","workspace":"ws_demo","type":"cron.tick","occurredAt":"2026-05-12T09:00:00Z","name":"weekly","cron":"0 9 * * 6"}' \
+ | RELAYFILE_MOUNT_ROOT=/path/to/mount node /tmp/wf-weekly-digest/runner.mjs📝 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
| RELAYFILE_MOUNT_ROOT=/path/to/mount \ | |
| echo '{"id":"manual-1","workspace":"ws_demo","type":"cron.tick","occurredAt":"2026-05-12T09:00:00Z","name":"weekly","cron":"0 9 * * 6"}' \ | |
| | node /tmp/wf-weekly-digest/runner.mjs | |
| echo '{"id":"manual-1","workspace":"ws_demo","type":"cron.tick","occurredAt":"2026-05-12T09:00:00Z","name":"weekly","cron":"0 9 * * 6"}' \ | |
| | RELAYFILE_MOUNT_ROOT=/path/to/mount node /tmp/wf-weekly-digest/runner.mjs |
🤖 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/README.md` around lines 60 - 62, The
RELAYFILE_MOUNT_ROOT environment variable is currently scoped only to the echo
process and not passed to the downstream node runner; move the
RELAYFILE_MOUNT_ROOT assignment so it applies to the node process (i.e., export
or place the variable before invoking node /tmp/wf-weekly-digest/runner.mjs) so
that runner.mjs can read RELAYFILE_MOUNT_ROOT when processing the piped JSON
from echo.
Comment on lines
+107
to
+129
| const result = await writeJsonFile( | ||
| opts, | ||
| 'github', | ||
| 'comment', | ||
| `${repoRoot(target.owner, target.repo)}/issues/${encodeSegment(target.number)}/comments/${draftFile('create comment')}`, | ||
| { body } | ||
| ); | ||
| return { | ||
| id: result.receipt?.created ?? result.receipt?.id ?? result.path, | ||
| url: result.receipt?.url ?? result.path | ||
| }; | ||
| }, | ||
|
|
||
| async createIssue(args) { | ||
| const out = await request<{ number: number; html_url: string }>('createIssue', { | ||
| method: 'POST', | ||
| pathname: `/repos/${args.owner}/${args.repo}/issues`, | ||
| body: { | ||
| title: args.title, | ||
| body: args.body, | ||
| ...(args.labels ? { labels: args.labels } : {}) | ||
| } | ||
| }); | ||
| return { number: out.number, url: out.html_url }; | ||
| const result = await writeJsonFile( | ||
| opts, | ||
| 'github', | ||
| 'createIssue', | ||
| `${repoRoot(args.owner, args.repo)}/issues/${draftFile('create issue')}`, | ||
| { title: args.title, body: args.body, labels: args.labels } | ||
| ); | ||
| const number = Number(result.receipt?.created ?? result.receipt?.id ?? 0); | ||
| return { number: Number.isFinite(number) ? number : 0, url: result.receipt?.url ?? result.path }; |
There was a problem hiding this comment.
Require a writeback receipt before returning GitHub metadata.
If writeJsonFile() completes without a receipt, comment() returns a Relayfile path as the id/url and createIssue() returns number: 0. That looks like a successful GitHub mutation but gives callers unusable identifiers and leaks mount internals. Please fail fast here, or make receipt polling mandatory for methods that promise GitHub IDs/URLs. This also affects the create branch of upsertIssue() at Line 169.
Also applies to: 169-170
🤖 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/clients/github.ts` around lines 107 - 129, The comment()
and createIssue() (and the create-branch path of upsertIssue()) currently return
Relayfile paths or zero when writeJsonFile() yields no receipt; update these
methods to require a valid writeback receipt before returning GitHub metadata by
checking result.receipt (and specific fields like
receipt.created/receipt.id/receipt.url) and throw a clear error if absent
instead of returning result.path or 0; ensure the same receipt-mandatory
validation and error handling is applied in upsertIssue()’s create branch so
callers only receive real GitHub IDs/URLs.
Comment on lines
+154
to
+170
| const existing = issues.find((issue) => issue.value.title === args.matchTitle && issue.value.number); | ||
| if (existing?.value.number) { | ||
| await writeJsonFile( | ||
| opts, | ||
| 'github', | ||
| 'upsertIssue.update', | ||
| `${issueDir}/${encodeSegment(existing.value.number)}.json`, | ||
| { title: args.title, body: args.body, labels: args.labels } | ||
| ); | ||
| return { | ||
| number: existing.value.number, | ||
| url: existing.value.html_url ?? existing.value.url ?? '', | ||
| created: false | ||
| }; | ||
| } | ||
| const created = await request<{ number: number; html_url: string }>('upsertIssue.create', { | ||
| method: 'POST', | ||
| pathname: `/repos/${args.owner}/${args.repo}/issues`, | ||
| body: { | ||
| title: args.matchTitle === args.title ? args.title : args.matchTitle, | ||
| body: args.body, | ||
| ...(args.labels ? { labels: args.labels } : {}) | ||
| } | ||
| }); | ||
| return { number: created.number, url: created.html_url, created: true }; | ||
| const created = await this.createIssue(args); | ||
| return { ...created, created: true }; |
There was a problem hiding this comment.
upsertIssue() stops being idempotent when title !== matchTitle.
Line 154 only matches the current stored title against args.matchTitle, but Lines 156-161 immediately rewrite that title to args.title. A second call with the same inputs will no longer find the previously updated issue and will fall through to createIssue(), producing duplicates. Match against both titles, or use a stable key that survives title changes.
💡 Minimal fix
- const existing = issues.find((issue) => issue.value.title === args.matchTitle && issue.value.number);
+ const candidateTitles = new Set([args.matchTitle, args.title]);
+ const existing = issues.find(
+ (issue) => issue.value.number && issue.value.title && candidateTitles.has(issue.value.title)
+ );🤖 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/clients/github.ts` around lines 154 - 170, The
upsertIssue implementation currently finds issues by comparing stored.title to
args.matchTitle but immediately rewrites the title to args.title, so subsequent
calls become non-idempotent; update the matching logic inside upsertIssue (the
issues.find predicate) to match either existing.value.title === args.matchTitle
OR existing.value.title === args.title (or, if available, prefer matching by a
stable identifier like existing.value.number or args.number), so the same issue
is found even after the title is changed and createIssue is not called again;
ensure you still use existing.value.number when calling writeJsonFile and
returning the upsert result.
Comment on lines
+174
to
+189
| const pullSegment = await findNumberSegment(opts, 'pulls', target.owner, target.repo, target.number); | ||
| const pullRoot = `${repoRoot(target.owner, target.repo)}/pulls/${encodeSegment(pullSegment)}`; | ||
| const pr = await readJsonFile<GithubPullRequestFile>( | ||
| opts, | ||
| 'github', | ||
| 'getPr', | ||
| `${pullRoot}/meta.json` | ||
| ).catch(() => | ||
| readJsonFile<GithubPullRequestFile>( | ||
| opts, | ||
| 'github', | ||
| 'getPr', | ||
| `${repoRoot(target.owner, target.repo)}/pulls/${encodeSegment(target.number)}/metadata.json` | ||
| ) | ||
| ); | ||
| const diff = await readTextFile(opts, 'github', 'getPr.diff', `${pullRoot}/diff.patch`).catch(() => ''); |
There was a problem hiding this comment.
Use the discovered PR directory segment verbatim.
findNumberSegment() already returns the actual entry name from listDirectoryEntries(). Re-encoding it here can turn a valid directory like 123__fix%2Fci into 123__fix%252Fci, so meta.json and diff.patch reads miss the PR when the slug contains escaped characters.
💡 Minimal fix
- const pullRoot = `${repoRoot(target.owner, target.repo)}/pulls/${encodeSegment(pullSegment)}`;
+ const pullRoot = `${repoRoot(target.owner, target.repo)}/pulls/${pullSegment}`;📝 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 pullSegment = await findNumberSegment(opts, 'pulls', target.owner, target.repo, target.number); | |
| const pullRoot = `${repoRoot(target.owner, target.repo)}/pulls/${encodeSegment(pullSegment)}`; | |
| const pr = await readJsonFile<GithubPullRequestFile>( | |
| opts, | |
| 'github', | |
| 'getPr', | |
| `${pullRoot}/meta.json` | |
| ).catch(() => | |
| readJsonFile<GithubPullRequestFile>( | |
| opts, | |
| 'github', | |
| 'getPr', | |
| `${repoRoot(target.owner, target.repo)}/pulls/${encodeSegment(target.number)}/metadata.json` | |
| ) | |
| ); | |
| const diff = await readTextFile(opts, 'github', 'getPr.diff', `${pullRoot}/diff.patch`).catch(() => ''); | |
| const pullSegment = await findNumberSegment(opts, 'pulls', target.owner, target.repo, target.number); | |
| const pullRoot = `${repoRoot(target.owner, target.repo)}/pulls/${pullSegment}`; | |
| const pr = await readJsonFile<GithubPullRequestFile>( | |
| opts, | |
| 'github', | |
| 'getPr', | |
| `${pullRoot}/meta.json` | |
| ).catch(() => | |
| readJsonFile<GithubPullRequestFile>( | |
| opts, | |
| 'github', | |
| 'getPr', | |
| `${repoRoot(target.owner, target.repo)}/pulls/${encodeSegment(target.number)}/metadata.json` | |
| ) | |
| ); | |
| const diff = await readTextFile(opts, 'github', 'getPr.diff', `${pullRoot}/diff.patch`).catch(() => ''); |
🤖 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/clients/github.ts` around lines 174 - 189, The code is
re-encoding the directory name returned by findNumberSegment (pullSegment) which
can double-escape slashes; instead build pullRoot using pullSegment verbatim
(i.e., remove encodeSegment(pullSegment)) so subsequent reads via
readJsonFile/readTextFile use the actual discovered directory name; keep
encoding only where you intentionally encode the numeric target (e.g.,
encodeSegment(target.number)) if needed for fallbacks.
Comment on lines
+54
to
+62
| async transition(target, transition) { | ||
| const id = typeof transition === 'string' ? transition.trim() : transition.id.trim(); | ||
| await writeJsonFile( | ||
| opts, | ||
| 'jira', | ||
| 'transition', | ||
| `/jira/issues/${encodeSegment(target.issueIdOrKey)}/transitions/${draftFile('create transition')}`, | ||
| { cloudId: target.cloudId, transition: { id } } | ||
| ); |
There was a problem hiding this comment.
Validate non-empty transition id before writing drafts.
After trimming, an empty id is still accepted and written, which creates invalid transition payloads.
Suggested fix
+import { WorkforceIntegrationError } from '../errors.js';
import {
draftFile,
encodeSegment,
type IntegrationClientOptions,
writeJsonFile
} from './request.js';
@@
async transition(target, transition) {
const id = typeof transition === 'string' ? transition.trim() : transition.id.trim();
+ if (id.length === 0) {
+ throw new WorkforceIntegrationError({
+ provider: 'jira',
+ operation: 'transition',
+ cause: new Error('transition id is required'),
+ retryable: false
+ });
+ }
await writeJsonFile(
opts,
'jira',📝 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
| async transition(target, transition) { | |
| const id = typeof transition === 'string' ? transition.trim() : transition.id.trim(); | |
| await writeJsonFile( | |
| opts, | |
| 'jira', | |
| 'transition', | |
| `/jira/issues/${encodeSegment(target.issueIdOrKey)}/transitions/${draftFile('create transition')}`, | |
| { cloudId: target.cloudId, transition: { id } } | |
| ); | |
| import { WorkforceIntegrationError } from '../errors.js'; | |
| import { | |
| draftFile, | |
| encodeSegment, | |
| type IntegrationClientOptions, | |
| writeJsonFile | |
| } from './request.js'; | |
| async transition(target, transition) { | |
| const id = typeof transition === 'string' ? transition.trim() : transition.id.trim(); | |
| if (id.length === 0) { | |
| throw new WorkforceIntegrationError({ | |
| provider: 'jira', | |
| operation: 'transition', | |
| cause: new Error('transition id is required'), | |
| retryable: false | |
| }); | |
| } | |
| await writeJsonFile( | |
| opts, | |
| 'jira', | |
| 'transition', | |
| `/jira/issues/${encodeSegment(target.issueIdOrKey)}/transitions/${draftFile('create transition')}`, | |
| { cloudId: target.cloudId, transition: { id } } | |
| ); |
🤖 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/clients/jira.ts` around lines 54 - 62, The transition
method accepts a transition id that may be empty after trimming; before calling
writeJsonFile (and when computing id in transition), validate that id is a
non-empty string (after trim) and throw or return a clear error if empty to
avoid writing invalid drafts; update the logic in async transition (and any
helper that computes id) to trim, check id.length > 0, and bail with a
descriptive error instead of calling writeJsonFile or creating the draft via
draftFile('create transition').
Comment on lines
+29
to
+33
| function readDatabaseId(parent: Record<string, unknown>): string { | ||
| const databaseId = parent.database_id ?? parent.databaseId; | ||
| if (typeof databaseId !== 'string' || databaseId.trim().length === 0) { | ||
| throw new Error('Notion createPage file writeback requires parent.database_id'); | ||
| } |
There was a problem hiding this comment.
Use WorkforceIntegrationError for createPage input validation failures.
This path currently throws a generic Error, which makes Notion client failures inconsistent with the integration error contract and retry metadata handling.
Suggested fix
+import { WorkforceIntegrationError } from '../errors.js';
import {
draftFile,
encodeSegment,
type IntegrationClientOptions,
readJsonFile,
writeJsonFile
} from './request.js';
function readDatabaseId(parent: Record<string, unknown>): string {
const databaseId = parent.database_id ?? parent.databaseId;
if (typeof databaseId !== 'string' || databaseId.trim().length === 0) {
- throw new Error('Notion createPage file writeback requires parent.database_id');
+ throw new WorkforceIntegrationError({
+ provider: 'notion',
+ operation: 'createPage',
+ cause: new Error('parent.database_id is required'),
+ retryable: false
+ });
}
return databaseId.trim();
}📝 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 readDatabaseId(parent: Record<string, unknown>): string { | |
| const databaseId = parent.database_id ?? parent.databaseId; | |
| if (typeof databaseId !== 'string' || databaseId.trim().length === 0) { | |
| throw new Error('Notion createPage file writeback requires parent.database_id'); | |
| } | |
| import { WorkforceIntegrationError } from '../errors.js'; | |
| import { | |
| draftFile, | |
| encodeSegment, | |
| type IntegrationClientOptions, | |
| readJsonFile, | |
| writeJsonFile | |
| } from './request.js'; | |
| function readDatabaseId(parent: Record<string, unknown>): string { | |
| const databaseId = parent.database_id ?? parent.databaseId; | |
| if (typeof databaseId !== 'string' || databaseId.trim().length === 0) { | |
| throw new WorkforceIntegrationError({ | |
| provider: 'notion', | |
| operation: 'createPage', | |
| cause: new Error('parent.database_id is required'), | |
| retryable: false | |
| }); | |
| } | |
| return databaseId.trim(); | |
| } |
🤖 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/clients/notion.ts` around lines 29 - 33, readDatabaseId
currently throws a generic Error on invalid parent database id; replace that
with throwing a WorkforceIntegrationError so failures follow the integration
error contract and carry retry metadata. Update the readDatabaseId function to
import/use WorkforceIntegrationError (or the existing integration error class)
and throw a WorkforceIntegrationError with a descriptive message like "Notion
createPage file writeback requires parent.database_id" and include any required
retry/metadata fields per WorkforceIntegrationError's constructor so callers
(createPage path) get consistent error type and retry behavior.
Comment on lines
+48
to
+50
| return { | ||
| id: result.receipt?.created ?? result.receipt?.id ?? '', | ||
| url: result.receipt?.url, |
There was a problem hiding this comment.
Avoid returning an empty id from createPage.
When no receipt is present (common in fire-and-forget mode), id becomes '', which breaks the method contract for callers expecting a usable identifier.
Suggested fix
return {
- id: result.receipt?.created ?? result.receipt?.id ?? '',
+ id: result.receipt?.created ?? result.receipt?.id ?? result.path,
url: result.receipt?.url,
properties
};🤖 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/clients/notion.ts` around lines 48 - 50, The createPage
return currently sets id to result.receipt?.created ?? result.receipt?.id ?? ''
which can produce an empty string; change it to return undefined instead of ''
so callers don't receive a bogus id. Update the returned object in createPage to
use result.receipt?.created ?? result.receipt?.id ?? undefined for the id field
(and adjust any type if needed) so the absence of a receipt yields undefined
rather than an empty string.
Comment on lines
+225
to
+239
| const timeoutMs = client.writebackTimeoutMs ?? 0; | ||
| const deadline = Date.now() + timeoutMs; | ||
| do { | ||
| const parsed = await readCurrentJson(absolutePath); | ||
| if ( | ||
| isRecord(parsed) && | ||
| (typeof parsed.created === 'string' || | ||
| typeof parsed.path === 'string' || | ||
| typeof parsed.id === 'string') | ||
| ) { | ||
| return parsed as WritebackReceipt; | ||
| } | ||
| if (timeoutMs <= 0) return undefined; | ||
| await new Promise((resolve) => setTimeout(resolve, client.writebackPollMs ?? 250)); | ||
| } while (Date.now() < deadline); |
There was a problem hiding this comment.
waitForReceipt can return a false receipt in fire-and-forget mode.
It reads/parses the just-written payload before the timeout check. If payload includes top-level id, path, or created, it is treated as a writeback receipt.
Suggested fix
async function waitForReceipt(
absolutePath: string,
client: IntegrationClientOptions
): Promise<WritebackReceipt | undefined> {
const timeoutMs = client.writebackTimeoutMs ?? 0;
+ if (timeoutMs <= 0) return undefined;
const deadline = Date.now() + timeoutMs;
do {
const parsed = await readCurrentJson(absolutePath);
if (
isRecord(parsed) &&
(typeof parsed.created === 'string' ||
typeof parsed.path === 'string' ||
typeof parsed.id === 'string')
) {
return parsed as WritebackReceipt;
}
- if (timeoutMs <= 0) return undefined;
await new Promise((resolve) => setTimeout(resolve, client.writebackPollMs ?? 250));
} while (Date.now() < deadline);
return undefined;
}📝 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 timeoutMs = client.writebackTimeoutMs ?? 0; | |
| const deadline = Date.now() + timeoutMs; | |
| do { | |
| const parsed = await readCurrentJson(absolutePath); | |
| if ( | |
| isRecord(parsed) && | |
| (typeof parsed.created === 'string' || | |
| typeof parsed.path === 'string' || | |
| typeof parsed.id === 'string') | |
| ) { | |
| return parsed as WritebackReceipt; | |
| } | |
| if (timeoutMs <= 0) return undefined; | |
| await new Promise((resolve) => setTimeout(resolve, client.writebackPollMs ?? 250)); | |
| } while (Date.now() < deadline); | |
| const timeoutMs = client.writebackTimeoutMs ?? 0; | |
| if (timeoutMs <= 0) return undefined; | |
| const deadline = Date.now() + timeoutMs; | |
| do { | |
| const parsed = await readCurrentJson(absolutePath); | |
| if ( | |
| isRecord(parsed) && | |
| (typeof parsed.created === 'string' || | |
| typeof parsed.path === 'string' || | |
| typeof parsed.id === 'string') | |
| ) { | |
| return parsed as WritebackReceipt; | |
| } | |
| await new Promise((resolve) => setTimeout(resolve, client.writebackPollMs ?? 250)); | |
| } while (Date.now() < deadline); | |
| return undefined; |
🤖 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/clients/request.ts` around lines 225 - 239, The loop in
waitForReceipt reads/parses the file before checking fire-and-forget mode,
allowing payloads with top-level id/path/created to be misinterpreted as
receipts; modify waitForReceipt so it checks whether writeback is disabled
(client.writebackTimeoutMs <= 0) or the deadline has already passed before
calling readCurrentJson, and only parse/validate the JSON when writeback is
enabled and there is remaining time (use timeoutMs/deadline or
client.writebackTimeoutMs and client.writebackPollMs in the do/while loop).
Ensure the early-return behavior for fire-and-forget remains (return undefined
immediately when timeoutMs <= 0) and preserve the poll delay logic
(client.writebackPollMs) and the final deadline comparison (Date.now() <
deadline) around the parse step.
Comment on lines
+49
to
+80
| async post(channel, text) { | ||
| const result = await writeJsonFile( | ||
| opts, | ||
| 'slack', | ||
| 'post', | ||
| `/slack/channels/${encodeSegment(channel)}/messages/${draftFile('create message')}`, | ||
| { text } | ||
| ); | ||
| return { channel, ts: result.receipt?.created ?? result.receipt?.id ?? '' }; | ||
| }, | ||
|
|
||
| async reply(threadTs, text) { | ||
| const thread = parseThreadRef(threadTs); | ||
| const result = await writeJsonFile( | ||
| opts, | ||
| 'slack', | ||
| 'reply', | ||
| `/slack/channels/${encodeSegment(thread.channel)}/messages/${tsPathSegment(thread.ts)}/replies/${draftFile('create reply')}`, | ||
| { text } | ||
| ); | ||
| return { channel: thread.channel, ts: result.receipt?.created ?? result.receipt?.id ?? '' }; | ||
| }, | ||
|
|
||
| async dm(user, text) { | ||
| const result = await writeJsonFile( | ||
| opts, | ||
| 'slack', | ||
| 'dm', | ||
| `/slack/users/${encodeSegment(user)}/messages/${draftFile('create dm')}`, | ||
| { text } | ||
| ); | ||
| return { channel: user, ts: result.receipt?.created ?? result.receipt?.id ?? '' }; |
There was a problem hiding this comment.
Add non-empty input guards for post/dm payload and identifiers.
post('', '...'), dm('', '...'), or blank text currently writes malformed drafts instead of failing fast. Add local validation mirroring parseThreadRef behavior.
Suggested fix
export function createSlackClient(opts: IntegrationClientOptions): SlackClient {
+ const requireNonEmpty = (operation: 'post' | 'reply' | 'dm', field: string, value: string): string => {
+ const trimmed = value.trim();
+ if (!trimmed) {
+ throw new WorkforceIntegrationError({
+ provider: 'slack',
+ operation,
+ cause: new Error(`Slack ${operation} ${field} must be non-empty`),
+ retryable: false
+ });
+ }
+ return trimmed;
+ };
+
return {
async post(channel, text) {
+ const safeChannel = requireNonEmpty('post', 'channel', channel);
+ const safeText = requireNonEmpty('post', 'text', text);
const result = await writeJsonFile(
opts,
'slack',
'post',
- `/slack/channels/${encodeSegment(channel)}/messages/${draftFile('create message')}`,
- { text }
+ `/slack/channels/${encodeSegment(safeChannel)}/messages/${draftFile('create message')}`,
+ { text: safeText }
);
- return { channel, ts: result.receipt?.created ?? result.receipt?.id ?? '' };
+ return { channel: safeChannel, ts: result.receipt?.created ?? result.receipt?.id ?? '' };
},
@@
async dm(user, text) {
+ const safeUser = requireNonEmpty('dm', 'user', user);
+ const safeText = requireNonEmpty('dm', 'text', text);
const result = await writeJsonFile(
opts,
'slack',
'dm',
- `/slack/users/${encodeSegment(user)}/messages/${draftFile('create dm')}`,
- { text }
+ `/slack/users/${encodeSegment(safeUser)}/messages/${draftFile('create dm')}`,
+ { text: safeText }
);
- return { channel: user, ts: result.receipt?.created ?? result.receipt?.id ?? '' };
+ return { channel: safeUser, ts: result.receipt?.created ?? result.receipt?.id ?? '' };
}
};
}🤖 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/clients/slack.ts` around lines 49 - 80, The post and dm
methods currently allow empty channel/user or empty text and write malformed
drafts; add local validation at the start of SlackClient.post and SlackClient.dm
(and optionally SlackClient.reply) to reject empty identifiers and empty text
similar to parseThreadRef: verify channel/user (and for reply the parsed
thread.channel and thread.ts) are non-empty and that text is a non-blank string,
and throw a clear Error (or return a rejected Promise) before calling
writeJsonFile; reuse encodeSegment/tsPathSegment only after validation and keep
the existing receipt extraction logic unchanged.
4 tasks
khaliqgant
added a commit
that referenced
this pull request
May 12, 2026
Aligns the MCP server with the runtime's new client shape from #92. Before: the integration tools called createGithubClient({ token }) using WORKFORCE_INTEGRATION_GITHUB_TOKEN. That signature no longer exists in the runtime now that github speaks Relayfile-VFS, so the MCP package would fail to compile once #92 lands. Config (config.ts) - WorkforceMcpConfig drops `providerTokens`. The MCP server is no longer the place to hold provider PATs — Relayfile holds the credentials and the writeback worker uses them. - New `relayfileMountRoot` field, populated from RELAYFILE_MOUNT_ROOT (with RELAYFILE_ROOT as a legacy alias). The workforce runtime sets these env vars automatically when it spawns the harness via ctx.harness.run. - New `writebackTimeoutMs` field (default 30s, overridable via WORKFORCE_WRITEBACK_TIMEOUT_MS). Passed straight through to integration clients so handlers that need a synchronous receipt pay the same wait the runtime would. Integration dispatcher (tools/integrations.ts) - resolveGithub() now constructs createGithubClient with { relayfileMountRoot, writebackTimeoutMs } from config instead of a token. Refuses to construct when the mount root is missing with a message pointing at the runtime contract. Tests - config.test exercises RELAYFILE_MOUNT_ROOT (+ RELAYFILE_ROOT alias) and WORKFORCE_WRITEBACK_TIMEOUT_MS overrides. - integrations.test now writes against a tempdir mount and reads the resulting draft JSON file back to assert the canonical shape Relayfile expects. Covers happy path, missing-mount-root failure, postReview enum validation, and field-pointed errors. - server.test loads config with RELAYFILE_MOUNT_ROOT instead of a github PAT. - workflow.test + memory.test drop the obsolete providerTokens fixture key. 23 mcp-workforce tests pass (up from 21). PR base flips from main to feat/integrations-vfs (PR #92) since the new client shape lives there. Auto-rebases to main when #92 merges. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collapses PersonaSpec from per-tier runtime maps to a single flat shape. A persona is now Harness + model + systemPrompt + harnessSettings + skills/mcps/sidecars + mount/integrations + listeners + schedules + memory, with no `tiers` map or `defaultTier` field. Removes: - `PersonaTier`, `PERSONA_TIERS`, `PersonaRuntime` types - `parseRuntime`, `isTier`, `resolvePersonaByTier` - `@<tier>` CLI selectors; --filter-rating; --all on list/show - tier param on HarnessRunArgs and LlmContext.complete - workload-router RoutingProfileRule.tier (rationale stays) - launch-metadata personaTier All 17 persona JSONs (personas-core, personas/, examples/) collapse to their best-value runtime. `agentworkforce show` / `list` and the persona- improver flow now operate on the flat shape; legacy @tier selectors error with a migration hint. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…t style
Switches workforce's integration clients from direct REST calls to the
Relayfile-VFS writeback pattern used by sage + the cloud workflows.
Handler-side surface (ctx.github.upsertIssue, ctx.linear.comment, etc.)
stays identical; the wire underneath flips from "speak HTTP to GitHub"
to "write a JSON draft inside the Relayfile mount and let the writeback
worker do the actual API call." Aligns workforce with the rest of the
org's integration story and inherits writeback durability + retry for
free.
Substrate
- packages/runtime/src/errors.ts (top-level): WorkforceIntegrationError
moves here with the { provider, operation, cause, retryable } shape
sage/cloud already use. Old clients/errors.ts is removed; the public
surface re-exports it from the same package import path so existing
consumers (mcp-workforce) keep compiling.
- packages/runtime/src/clients/request.ts: shared VFS helpers
(readJsonFile, readTextFile, listJsonFiles, listDirectoryEntries,
writeJsonFile + atomic write-then-rename) with mount-root path
validation and optional writeback-receipt polling.
Clients
- github.ts is rewritten as a VFS client. Same GithubClient interface
(comment, createIssue, upsertIssue, getPr, postReview); each method
now reads/writes files at canonical paths under
`/github/repos/<owner>/<repo>/...`.
- linear, slack, notion, jira ship as new typed clients with the same
pattern. IntegrationClients in types.ts now types all five concretely
instead of leaving four as unknown.
Tests
- github.test.ts is rewritten end-to-end against a tempdir mount.
- linear/slack/notion/jira tests run against tempdir mounts too.
- 29 runtime tests pass (up from 18), 386 across the repo.
Example
- weekly-digest/agent.ts drops the WORKFORCE_INTEGRATION_GITHUB_TOKEN
plumbing; the github client picks up RELAYFILE_MOUNT_ROOT instead.
- weekly-digest/README.md documents the writeback model + Relayfile
mount env requirement, and drops the GITHUB_TOKEN setup step.
Notes
- mcp-workforce (PR #91) imports createGithubClient with a different
construction shape today (`{ token }`); it'll need a follow-up
commit to switch to IntegrationClientOptions once this lands. The
MCP package depends on the new shape, not the old.
- The direct-REST github implementation that shipped in #90 is
replaced wholesale. No persona today depends on it; weekly-digest
is updated in this commit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-on to the persona-tier flatten refactor — the openclaw-routing example still read from selection.runtime.* and selection.tier, which no longer exist after PersonaSelection lost its per-tier wrapper. Required to keep the examples typecheck green on top of the flatten.
khaliqgant
force-pushed
the
feat/integrations-vfs
branch
from
efb115b to
1f8dcdb
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/cli/src/cli.ts (1)
106-123:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the Codex mount description in
--help.This block still says Codex sessions never mount and ignore
--install-in-repo, butdecideCleanMode()and the updated tests make Codex default to the sandbox mount and use--install-in-repoas the opt-out. The current help text points users to the opposite behavior.🤖 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/cli/src/cli.ts` around lines 106 - 123, The help text for the "agent [flags] <persona>" command is incorrect about Codex mounting; update the description to say Codex sessions now default to the sandbox mount and that --install-in-repo opts out (i.e., disengages the sandbox and installs skills into the repo) to match decideCleanMode() and tests. Edit the help string surrounding the --install-in-repo flag in the agent help block so it references Codex using the sandbox by default and that the flag disables that behavior.packages/workload-router/src/index.ts (1)
182-199:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject cross-harness overrides instead of patching only
harness.
usePersona(..., { harness: 'codex' })can now return a selection whosemodel,harnessSettings, and sidecar fields still belong to the original harness. With tiers gone there's no alternate runtime to swap in, so this produces selections that are internally inconsistent and not safely launchable.Suggested fix
export function useSelection( baseSelection: PersonaSelection, options: { harness?: Harness; installRoot?: string; repoRoot?: string } = {} ): PersonaContext { - const effectiveHarness = options.harness ?? baseSelection.harness; - const selection = - effectiveHarness === baseSelection.harness - ? baseSelection - : { ...baseSelection, harness: effectiveHarness }; + if (options.harness && options.harness !== baseSelection.harness) { + throw new Error( + `Cannot override harness from ${baseSelection.harness} to ${options.harness}; ` + + 'resolve or construct a selection for the target harness instead.' + ); + } + const effectiveHarness = baseSelection.harness; + const selection = baseSelection;🤖 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/workload-router/src/index.ts` around lines 182 - 199, The function useSelection currently allows options.harness to override only the harness field but leaves model, harnessSettings, and sidecar from the original PersonaSelection, producing inconsistent selections; instead, add an early guard in useSelection that if options.harness is provided and differs from baseSelection.harness you reject it (throw a clear error or return a failure) rather than patching the object, and only proceed when options.harness is undefined or identical to baseSelection.harness; keep the rest of the logic (selection creation and calls to materializeSkillsFor and materializeSkills) unchanged but ensure the new guard runs before constructing selection or calling materializeSkillsFor/materializeSkills so inconsistent model/harnessSettings/sidecar combos can never be produced.
🧹 Nitpick comments (2)
packages/workload-router/routing-profiles/default.json (1)
17-33: ⚡ Quick winRationale text still references removed tier concepts.
The profile schema is tierless now, but many rationale strings still say “best-value/top tier/deepest tier.” Consider normalizing this wording so policy docs and runtime schema don’t contradict each other.
🤖 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/workload-router/routing-profiles/default.json` around lines 17 - 33, Many rationale strings in routing-profiles/default.json still reference removed tier concepts like "best-value", "deepest tier", and "top tier"; update each affected entry (e.g., "opencode-workflow-correctness", "npm-provenance", "cloud-sandbox-infra", "sage-slack-egress-migration", "sage-proactive-rewire", "cloud-slack-proxy-guard", "sage-cloud-e2e-conduction", "capability-discovery", "npm-package-compat", "posthog", "persona-authoring", "persona-improvement", "slop-audit", "api-contract-review", "local-stack-orchestration", "e2e-validation", "write-integration-tests") to remove tier language and replace it with neutral priority guidance (for example: "prefer deeper reasoning and thorough verification", "prefer lower latency and succinct checks", or "prefer thorough verification for safety-critical changes") so the rationale strings match the new tierless schema and policy wording consistently across the file.examples/openclaw-routing.ts (1)
16-16: ⚡ Quick winUse an exhaustive harness-to-runtime mapping to avoid silent fallback behavior.
Line 16 currently sends every non-
codexharness tosubagent. If a new harness is introduced later, this can route incorrectly without a compile-time signal.Suggested refactor
- const runtime = selection.harness === 'codex' ? 'acp' : 'subagent'; + const runtime = (() => { + switch (selection.harness) { + case 'codex': + return 'acp'; + case 'claude': + case 'opencode': + return 'subagent'; + default: { + const _exhaustive: never = selection.harness; + throw new Error(`Unsupported harness: ${_exhaustive}`); + } + } + })();🤖 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/openclaw-routing.ts` at line 16, The current expression setting runtime from selection.harness (const runtime = selection.harness === 'codex' ? 'acp' : 'subagent') silently defaults every unknown harness to 'subagent'; replace it with an exhaustive harness-to-runtime mapping (e.g., a switch or a closed mapping object keyed by known harness values) and explicitly handle unknown values by throwing or logging an error so new harnesses fail fast. Update the code that reads selection.harness and assigns runtime to use that mapping (referencing selection.harness and the runtime variable) and add a clear error path for unrecognized harness names.
🤖 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/weekly-digest/README.md`:
- Around line 56-57: Add a short prerequisite sentence to the README near the
instructions about driving the bundle against a Relayfile mount: state that
manual firing will only produce actual GitHub writes if the Relayfile writeback
worker is running for that mount, e.g., add a note mentioning the "Relayfile
writeback worker" must be active for the mount to persist real writes from
manual triggers.
In `@packages/cli/src/local-personas.ts`:
- Around line 945-951: The assignment for systemPrompt should treat empty or
whitespace-only override values as unset like standaloneSpecFromOverride() does;
replace the nullish-coalescing line that sets systemPrompt (currently using
override.systemPrompt ?? base.systemPrompt) with logic that checks
override.systemPrompt?.trim() and uses override.systemPrompt only if that
trimmed value is non-empty, otherwise fall back to base.systemPrompt; keep the
existing behavior for harness, model, and harnessSettings (symbols: harness,
model, systemPrompt, override, base, harnessSettings,
standaloneSpecFromOverride).
In `@packages/cli/src/persona-install.ts`:
- Around line 267-271: The collision-suffix logic in the while loop using
++suffix causes the first duplicate to be named "-2" instead of "-1"; change the
increment to post-increment so the first collision yields "-1" (i.e., in the
loop that references assetKeys, assetKey, and baseKey replace the ++suffix usage
with suffix++ or otherwise adjust initialization so the first appended suffix is
1).
In `@packages/persona-kit/src/parse.test.ts`:
- Around line 107-119: The test "parsePersonaSpec throws when required runtime
fields are missing" currently supplies invalid values instead of omitting
fields; update the three assertions to pass specs that actually omit the
required keys (harness, model, systemPrompt) when calling parsePersonaSpec so
each case tests absence rather than empty/invalid values, keeping the same
expected error regexes and the same test name; locate usages of parsePersonaSpec
and validSpec in this test file to remove the harness key for the first case and
remove model and systemPrompt keys for the second and third cases respectively.
In `@packages/personas-core/personas/flake-hunter.json`:
- Line 11: Update the persona's systemPrompt string in the
personas/flake-hunter.json (the "systemPrompt" entry) to remove stale cross-tier
phrases like "top tier" and "efficient mode" and replace them with wording that
reflects the single top-level runtime config; keep the intent (brief repro
status, flake class, root cause, hardening fix, evidence) and constraints (avoid
sleeps/retries/weakened assertions/vague CI blame), but rephrase the opening to
something like a single authoritative runtime instruction (e.g., "You are a
senior flake hunter operating under the repository's top-level runtime
configuration.") so the prompt is internally consistent and concise.
In `@packages/personas-core/personas/test-strategist.json`:
- Line 10: The systemPrompt currently includes tier-relative phrasing ("senior
test strategist in efficient mode" and "top tier") which is obsolete; update the
string value assigned to systemPrompt to remove references to tiers and modes
and rephrase to a stable, non-tiered directive (e.g., "You are a senior test
strategist. Maintain high quality while reducing depth and verbosity..."),
keeping the rest of the required outputs and priorities intact; modify the
systemPrompt entry in the JSON (key: systemPrompt) to the revised one-line
prompt.
In `@packages/personas-core/scripts/validate-personas.mjs`:
- Around line 74-81: The validator currently allows legacy fields to pass; add
explicit rejection checks for persona.tiers and persona.defaultTier inside the
same validation block that checks persona fields (where persona, rel, errors and
isObject are used). If persona has a defined tiers or defaultTier property, push
a clear error message like `${rel}.tiers is deprecated` or `${rel}.defaultTier
is deprecated` (or similar) to errors so CI fails on legacy schema; keep these
checks adjacent to the existing harness/harnessSettings validations to ensure
deprecated fields are rejected early.
In `@personas/persona-improver.json`:
- Line 22: The persona JSON has inconsistent proposal count limits between the
systemPrompt phrases "Produce 0-6" and the other phrase "Identify 0–8"; make
them identical to remove nondeterminism by updating the latter to match the
former (or vice versa if you prefer a different cap), e.g., change "Identify
0–8" to "Identify 0-6" so both use the same numeric range and formatting; ensure
both occurrences in the persona-improver.json "systemPrompt" use the same range
and hyphen style and keep all other constraints unchanged.
- Line 27: The agentsMdContent still references outdated tier patch paths like
"tiers.best.systemPrompt" and instructs editing "systemPrompt per tier", which
leads to invalid proposal patch paths; update the agentsMdContent string to
remove or replace those instructions with the new patch grammar (use the literal
top-level tier keys "tiers.best", "tiers.best-value", "tiers.minimum" only as
described in the persona schema) and explicitly document the correct dot-paths
for patches (e.g., "tiers.best.systemPrompt" should be represented per the new
schema guidance or replaced with the exact approved patch path patterns),
ensuring the prose aligns with the allowed patch ops and path grammar so
generated proposals target valid JSON paths.
In `@personas/persona-maker.json`:
- Line 36: The embedded authoring spec in agentsMdContent of persona-maker.json
still requires tiered-only fields (`tiers`, `defaultTier`, tiered prompt rules)
which conflicts with the new top-level schema; update agentsMdContent so the
spec instructs authors to use top-level harness, model, systemPrompt, and
harnessSettings as the canonical shape (replace examples and language that
mandate `tiers`-only prompts), remove or reframe tier-only examples and rules,
and ensure references to prompt rules and defaults point to top-level fields
(search for the string "tiers" and symbols agentsMdContent and persona-maker in
the JSON to locate and edit the guidance).
---
Outside diff comments:
In `@packages/cli/src/cli.ts`:
- Around line 106-123: The help text for the "agent [flags] <persona>" command
is incorrect about Codex mounting; update the description to say Codex sessions
now default to the sandbox mount and that --install-in-repo opts out (i.e.,
disengages the sandbox and installs skills into the repo) to match
decideCleanMode() and tests. Edit the help string surrounding the
--install-in-repo flag in the agent help block so it references Codex using the
sandbox by default and that the flag disables that behavior.
In `@packages/workload-router/src/index.ts`:
- Around line 182-199: The function useSelection currently allows
options.harness to override only the harness field but leaves model,
harnessSettings, and sidecar from the original PersonaSelection, producing
inconsistent selections; instead, add an early guard in useSelection that if
options.harness is provided and differs from baseSelection.harness you reject it
(throw a clear error or return a failure) rather than patching the object, and
only proceed when options.harness is undefined or identical to
baseSelection.harness; keep the rest of the logic (selection creation and calls
to materializeSkillsFor and materializeSkills) unchanged but ensure the new
guard runs before constructing selection or calling
materializeSkillsFor/materializeSkills so inconsistent
model/harnessSettings/sidecar combos can never be produced.
---
Nitpick comments:
In `@examples/openclaw-routing.ts`:
- Line 16: The current expression setting runtime from selection.harness (const
runtime = selection.harness === 'codex' ? 'acp' : 'subagent') silently defaults
every unknown harness to 'subagent'; replace it with an exhaustive
harness-to-runtime mapping (e.g., a switch or a closed mapping object keyed by
known harness values) and explicitly handle unknown values by throwing or
logging an error so new harnesses fail fast. Update the code that reads
selection.harness and assigns runtime to use that mapping (referencing
selection.harness and the runtime variable) and add a clear error path for
unrecognized harness names.
In `@packages/workload-router/routing-profiles/default.json`:
- Around line 17-33: Many rationale strings in routing-profiles/default.json
still reference removed tier concepts like "best-value", "deepest tier", and
"top tier"; update each affected entry (e.g., "opencode-workflow-correctness",
"npm-provenance", "cloud-sandbox-infra", "sage-slack-egress-migration",
"sage-proactive-rewire", "cloud-slack-proxy-guard", "sage-cloud-e2e-conduction",
"capability-discovery", "npm-package-compat", "posthog", "persona-authoring",
"persona-improvement", "slop-audit", "api-contract-review",
"local-stack-orchestration", "e2e-validation", "write-integration-tests") to
remove tier language and replace it with neutral priority guidance (for example:
"prefer deeper reasoning and thorough verification", "prefer lower latency and
succinct checks", or "prefer thorough verification for safety-critical changes")
so the rationale strings match the new tierless schema and policy wording
consistently across the file.
🪄 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: ad12c08d-583d-4b88-aa30-49ba96024db2
⛔ Files ignored due to path filters (1)
packages/workload-router/src/generated/personas.tsis excluded by!**/generated/**
📒 Files selected for processing (67)
examples/openclaw-routing.tsexamples/weekly-digest/README.mdexamples/weekly-digest/agent.tsexamples/weekly-digest/persona.jsonpackages/cli/src/cli.test.tspackages/cli/src/cli.tspackages/cli/src/launch-metadata.test.tspackages/cli/src/launch-metadata.tspackages/cli/src/local-personas.test.tspackages/cli/src/local-personas.tspackages/cli/src/persona-install.test.tspackages/cli/src/persona-install.tspackages/deploy/src/bundle.test.tspackages/deploy/src/deploy.test.tspackages/deploy/src/modes/sandbox.test.tspackages/persona-kit/src/constants.tspackages/persona-kit/src/execute.test.tspackages/persona-kit/src/index.test.tspackages/persona-kit/src/index.tspackages/persona-kit/src/parse.test.tspackages/persona-kit/src/parse.tspackages/persona-kit/src/plan.test.tspackages/persona-kit/src/plan.tspackages/persona-kit/src/sidecars.test.tspackages/persona-kit/src/skills.tspackages/persona-kit/src/triggers.test.tspackages/persona-kit/src/types.tspackages/personas-core/personas/architecture-planner.jsonpackages/personas-core/personas/capability-discoverer.jsonpackages/personas-core/personas/code-reviewer.jsonpackages/personas-core/personas/debugger.jsonpackages/personas-core/personas/e2e-validator.jsonpackages/personas-core/personas/flake-hunter.jsonpackages/personas-core/personas/frontend-implementer.jsonpackages/personas-core/personas/integration-test-author.jsonpackages/personas-core/personas/requirements-analyst.jsonpackages/personas-core/personas/security-reviewer.jsonpackages/personas-core/personas/tdd-guard.jsonpackages/personas-core/personas/technical-writer.jsonpackages/personas-core/personas/test-strategist.jsonpackages/personas-core/personas/verifier.jsonpackages/personas-core/scripts/validate-personas.mjspackages/runtime/src/clients/errors.tspackages/runtime/src/clients/github.test.tspackages/runtime/src/clients/github.tspackages/runtime/src/clients/index.tspackages/runtime/src/clients/jira.test.tspackages/runtime/src/clients/jira.tspackages/runtime/src/clients/linear.test.tspackages/runtime/src/clients/linear.tspackages/runtime/src/clients/notion.test.tspackages/runtime/src/clients/notion.tspackages/runtime/src/clients/request.tspackages/runtime/src/clients/slack.test.tspackages/runtime/src/clients/slack.tspackages/runtime/src/errors.tspackages/runtime/src/index.tspackages/runtime/src/runner.test.tspackages/runtime/src/types.tspackages/workload-router/routing-profiles/default.jsonpackages/workload-router/routing-profiles/schema.jsonpackages/workload-router/scripts/generate-personas.mjspackages/workload-router/src/eval.tspackages/workload-router/src/index.test.tspackages/workload-router/src/index.tspersonas/persona-improver.jsonpersonas/persona-maker.json
💤 Files with no reviewable changes (4)
- packages/persona-kit/src/constants.ts
- packages/runtime/src/clients/errors.ts
- packages/persona-kit/src/index.ts
- packages/workload-router/scripts/generate-personas.mjs
✅ Files skipped from review due to trivial changes (2)
- packages/persona-kit/src/triggers.test.ts
- packages/runtime/src/clients/jira.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- packages/runtime/src/clients/notion.test.ts
- packages/runtime/src/index.ts
- packages/runtime/src/clients/notion.ts
- packages/runtime/src/clients/slack.test.ts
- examples/weekly-digest/agent.ts
- packages/runtime/src/clients/slack.ts
- packages/runtime/src/clients/linear.test.ts
- packages/runtime/src/clients/linear.ts
- packages/runtime/src/clients/index.ts
- packages/runtime/src/clients/github.test.ts
- packages/runtime/src/clients/request.ts
- packages/runtime/src/clients/github.ts
Comment on lines
+56
to
+57
| the command line against a Relayfile mount you've already set up, drive | ||
| the bundle directly: |
There was a problem hiding this comment.
Add explicit prerequisite: writeback worker must be running.
Please add a short note here that manual firing only produces real GitHub writes when the Relayfile writeback worker is active for that mount.
🤖 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/README.md` around lines 56 - 57, Add a short
prerequisite sentence to the README near the instructions about driving the
bundle against a Relayfile mount: state that manual firing will only produce
actual GitHub writes if the Relayfile writeback worker is running for that
mount, e.g., add a note mentioning the "Relayfile writeback worker" must be
active for the mount to persist real writes from manual triggers.
Comment on lines
+945
to
+951
| const harness = override.harness ?? base.harness; | ||
| const model = override.model ?? base.model; | ||
| const systemPrompt = override.systemPrompt ?? base.systemPrompt; | ||
| const harnessSettings: HarnessSettings = { | ||
| ...base.harnessSettings, | ||
| ...(override.harnessSettings ?? {}) | ||
| }; |
There was a problem hiding this comment.
Treat blank systemPrompt overrides as unset here.
This merge path uses ??, so an overlay with "systemPrompt": "" or whitespace replaces the inherited prompt with an empty string. That diverges from standaloneSpecFromOverride(), which trims and falls back, and it makes migrated overlays easy to break.
Suggested fix
- const systemPrompt = override.systemPrompt ?? base.systemPrompt;
+ const systemPrompt =
+ typeof override.systemPrompt === 'string' && override.systemPrompt.trim().length > 0
+ ? override.systemPrompt
+ : base.systemPrompt;📝 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 harness = override.harness ?? base.harness; | |
| const model = override.model ?? base.model; | |
| const systemPrompt = override.systemPrompt ?? base.systemPrompt; | |
| const harnessSettings: HarnessSettings = { | |
| ...base.harnessSettings, | |
| ...(override.harnessSettings ?? {}) | |
| }; | |
| const harness = override.harness ?? base.harness; | |
| const model = override.model ?? base.model; | |
| const systemPrompt = | |
| typeof override.systemPrompt === 'string' && override.systemPrompt.trim().length > 0 | |
| ? override.systemPrompt | |
| : base.systemPrompt; | |
| const harnessSettings: HarnessSettings = { | |
| ...base.harnessSettings, | |
| ...(override.harnessSettings ?? {}) | |
| }; |
🤖 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/cli/src/local-personas.ts` around lines 945 - 951, The assignment
for systemPrompt should treat empty or whitespace-only override values as unset
like standaloneSpecFromOverride() does; replace the nullish-coalescing line that
sets systemPrompt (currently using override.systemPrompt ?? base.systemPrompt)
with logic that checks override.systemPrompt?.trim() and uses
override.systemPrompt only if that trimmed value is non-empty, otherwise fall
back to base.systemPrompt; keep the existing behavior for harness, model, and
harnessSettings (symbols: harness, model, systemPrompt, override, base,
harnessSettings, standaloneSpecFromOverride).
Comment on lines
267
to
271
| let suffix = 1; | ||
| while (assetKeys.has(assetKey)) { | ||
| const dot = baseKey.lastIndexOf('.'); | ||
| assetKey = `${baseKey.slice(0, dot)}-${++suffix}${baseKey.slice(dot)}`; | ||
| } |
There was a problem hiding this comment.
Collision suffix starts at -2 due pre-increment.
The first duplicate asset currently becomes name-2.md instead of name-1.md. This is a small off-by-one in the collision naming loop.
🔧 Proposed fix
- let suffix = 1;
+ let suffix = 1;
while (assetKeys.has(assetKey)) {
const dot = baseKey.lastIndexOf('.');
- assetKey = `${baseKey.slice(0, dot)}-${++suffix}${baseKey.slice(dot)}`;
+ assetKey = `${baseKey.slice(0, dot)}-${suffix++}${baseKey.slice(dot)}`;
}📝 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
| let suffix = 1; | |
| while (assetKeys.has(assetKey)) { | |
| const dot = baseKey.lastIndexOf('.'); | |
| assetKey = `${baseKey.slice(0, dot)}-${++suffix}${baseKey.slice(dot)}`; | |
| } | |
| let suffix = 1; | |
| while (assetKeys.has(assetKey)) { | |
| const dot = baseKey.lastIndexOf('.'); | |
| assetKey = `${baseKey.slice(0, dot)}-${suffix++}${baseKey.slice(dot)}`; | |
| } |
🤖 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/cli/src/persona-install.ts` around lines 267 - 271, The
collision-suffix logic in the while loop using ++suffix causes the first
duplicate to be named "-2" instead of "-1"; change the increment to
post-increment so the first collision yields "-1" (i.e., in the loop that
references assetKeys, assetKey, and baseKey replace the ++suffix usage with
suffix++ or otherwise adjust initialization so the first appended suffix is 1).
Comment on lines
+107
to
119
| test('parsePersonaSpec throws when required runtime fields are missing', () => { | ||
| assert.throws( | ||
| () => parsePersonaSpec(validSpec({ harness: 'mystery' }), 'documentation'), | ||
| /persona\[documentation\]\.harness must be one of:/ | ||
| ); | ||
| assert.throws( | ||
| () => parsePersonaSpec(validSpec({ model: '' }), 'documentation'), | ||
| /persona\[documentation\]\.model must be a non-empty string/ | ||
| ); | ||
| assert.throws( | ||
| () => parsePersonaSpec(validSpec({ systemPrompt: ' ' }), 'documentation'), | ||
| /persona\[documentation\]\.systemPrompt must be a non-empty string/ | ||
| ); |
There was a problem hiding this comment.
“Missing required fields” test is currently checking invalid values, not missing fields.
Line 109/113/117 still provide the fields, so this test does not cover actual omission and can give false confidence.
Suggested test adjustment
test('parsePersonaSpec throws when required runtime fields are missing', () => {
+ const missingHarness = validSpec();
+ delete missingHarness.harness;
assert.throws(
- () => parsePersonaSpec(validSpec({ harness: 'mystery' }), 'documentation'),
- /persona\[documentation\]\.harness must be one of:/
+ () => parsePersonaSpec(missingHarness, 'documentation'),
+ /persona\[documentation\]\.harness/
);
+ const missingModel = validSpec();
+ delete missingModel.model;
assert.throws(
- () => parsePersonaSpec(validSpec({ model: '' }), 'documentation'),
- /persona\[documentation\]\.model must be a non-empty string/
+ () => parsePersonaSpec(missingModel, 'documentation'),
+ /persona\[documentation\]\.model/
);
+ const missingSystemPrompt = validSpec();
+ delete missingSystemPrompt.systemPrompt;
assert.throws(
- () => parsePersonaSpec(validSpec({ systemPrompt: ' ' }), 'documentation'),
- /persona\[documentation\]\.systemPrompt must be a non-empty string/
+ () => parsePersonaSpec(missingSystemPrompt, 'documentation'),
+ /persona\[documentation\]\.systemPrompt/
);
});🤖 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/persona-kit/src/parse.test.ts` around lines 107 - 119, The test
"parsePersonaSpec throws when required runtime fields are missing" currently
supplies invalid values instead of omitting fields; update the three assertions
to pass specs that actually omit the required keys (harness, model,
systemPrompt) when calling parsePersonaSpec so each case tests absence rather
than empty/invalid values, keeping the same expected error regexes and the same
test name; locate usages of parsePersonaSpec and validSpec in this test file to
remove the harness key for the first case and remove model and systemPrompt keys
for the second and third cases respectively.
| } | ||
| "harness": "opencode", | ||
| "model": "opencode/gpt-5-nano", | ||
| "systemPrompt": "You are a senior flake hunter in efficient mode. Keep the same quality bar as top tier; reduce only depth and verbosity. Reproduce the flake, isolate the unstable path, classify the failure mode, fix the root cause, and provide repeat-run evidence. Priorities remain reproducibility, root-cause correctness, and preserving test signal. Avoid arbitrary sleeps, blind retries, weakened assertions, and vague CI blame. Output contract: brief repro status, flake class, root cause, hardening fix, and evidence.", |
There was a problem hiding this comment.
Remove stale cross-tier wording from systemPrompt.
Line 11 still references “top tier” and “efficient mode,” but this persona now has a single top-level runtime config. That wording is internally inconsistent and weakens instruction clarity.
Suggested edit
- "systemPrompt": "You are a senior flake hunter in efficient mode. Keep the same quality bar as top tier; reduce only depth and verbosity. Reproduce the flake, isolate the unstable path, classify the failure mode, fix the root cause, and provide repeat-run evidence. Priorities remain reproducibility, root-cause correctness, and preserving test signal. Avoid arbitrary sleeps, blind retries, weakened assertions, and vague CI blame. Output contract: brief repro status, flake class, root cause, hardening fix, and evidence.",
+ "systemPrompt": "You are a senior flake hunter. Reproduce the flake, isolate the unstable path, classify the failure mode, fix the root cause, and provide repeat-run evidence. Priorities: reproducibility, root-cause correctness, and preserving test signal. Avoid arbitrary sleeps, blind retries, weakened assertions, and vague CI blame. Output contract: brief repro status, flake class, root cause, hardening fix, and evidence.",🤖 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/personas-core/personas/flake-hunter.json` at line 11, Update the
persona's systemPrompt string in the personas/flake-hunter.json (the
"systemPrompt" entry) to remove stale cross-tier phrases like "top tier" and
"efficient mode" and replace them with wording that reflects the single
top-level runtime config; keep the intent (brief repro status, flake class, root
cause, hardening fix, evidence) and constraints (avoid sleeps/retries/weakened
assertions/vague CI blame), but rephrase the opening to something like a single
authoritative runtime instruction (e.g., "You are a senior flake hunter
operating under the repository's top-level runtime configuration.") so the
prompt is internally consistent and concise.
| } | ||
| "harness": "opencode", | ||
| "model": "opencode/gpt-5-nano", | ||
| "systemPrompt": "You are a senior test strategist in efficient mode. Keep the same quality bar as top tier; reduce only depth and verbosity. Inspect the changed behavior, rank the biggest risks, recommend the smallest useful unit/integration/e2e coverage set, and label gaps as Critical, Important, or Nice-to-have. Priorities remain: regression prevention, contract safety, reliability, and fit with existing test patterns. Avoid noisy blanket coverage requests, implementation-detail coupling, and unnecessary end-to-end expansion. Output contract: brief test plan, risk-ranked gaps, recommended layer per behavior, and explicit deferrals.", |
There was a problem hiding this comment.
Drop tier-relative phrasing from systemPrompt.
Line 10 references “top tier” and “efficient mode,” but this persona no longer has tiered runtime settings. Keeping this phrasing makes the prompt self-contradictory.
Suggested edit
- "systemPrompt": "You are a senior test strategist in efficient mode. Keep the same quality bar as top tier; reduce only depth and verbosity. Inspect the changed behavior, rank the biggest risks, recommend the smallest useful unit/integration/e2e coverage set, and label gaps as Critical, Important, or Nice-to-have. Priorities remain: regression prevention, contract safety, reliability, and fit with existing test patterns. Avoid noisy blanket coverage requests, implementation-detail coupling, and unnecessary end-to-end expansion. Output contract: brief test plan, risk-ranked gaps, recommended layer per behavior, and explicit deferrals.",
+ "systemPrompt": "You are a senior test strategist. Inspect the changed behavior, rank the biggest risks, recommend the smallest useful unit/integration/e2e coverage set, and label gaps as Critical, Important, or Nice-to-have. Priorities: regression prevention, contract safety, reliability, and fit with existing test patterns. Avoid noisy blanket coverage requests, implementation-detail coupling, and unnecessary end-to-end expansion. Output contract: brief test plan, risk-ranked gaps, recommended layer per behavior, and explicit deferrals.",📝 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
| "systemPrompt": "You are a senior test strategist in efficient mode. Keep the same quality bar as top tier; reduce only depth and verbosity. Inspect the changed behavior, rank the biggest risks, recommend the smallest useful unit/integration/e2e coverage set, and label gaps as Critical, Important, or Nice-to-have. Priorities remain: regression prevention, contract safety, reliability, and fit with existing test patterns. Avoid noisy blanket coverage requests, implementation-detail coupling, and unnecessary end-to-end expansion. Output contract: brief test plan, risk-ranked gaps, recommended layer per behavior, and explicit deferrals.", | |
| "systemPrompt": "You are a senior test strategist. Inspect the changed behavior, rank the biggest risks, recommend the smallest useful unit/integration/e2e coverage set, and label gaps as Critical, Important, or Nice-to-have. Priorities: regression prevention, contract safety, reliability, and fit with existing test patterns. Avoid noisy blanket coverage requests, implementation-detail coupling, and unnecessary end-to-end expansion. Output contract: brief test plan, risk-ranked gaps, recommended layer per behavior, and explicit deferrals.", |
🤖 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/personas-core/personas/test-strategist.json` at line 10, The
systemPrompt currently includes tier-relative phrasing ("senior test strategist
in efficient mode" and "top tier") which is obsolete; update the string value
assigned to systemPrompt to remove references to tiers and modes and rephrase to
a stable, non-tiered directive (e.g., "You are a senior test strategist.
Maintain high quality while reducing depth and verbosity..."), keeping the rest
of the required outputs and priorities intact; modify the systemPrompt entry in
the JSON (key: systemPrompt) to the revised one-line prompt.
Comment on lines
+74
to
+81
| for (const field of ['harness', 'model', 'systemPrompt']) { | ||
| if (typeof persona[field] !== 'string' || persona[field].trim() === '') { | ||
| errors.push(`${rel}.${field} must be a non-empty string`); | ||
| } | ||
| } | ||
| if (!isObject(persona.harnessSettings)) { | ||
| errors.push(`${rel}.harnessSettings must be an object`); | ||
| } |
There was a problem hiding this comment.
Validator should explicitly reject deprecated tier fields.
The migration is to top-level runtime fields, but the validator still allows tiers / defaultTier to remain silently. Add explicit rejection so legacy schema cannot pass CI.
Suggested patch
for (const field of ['harness', 'model', 'systemPrompt']) {
if (typeof persona[field] !== 'string' || persona[field].trim() === '') {
errors.push(`${rel}.${field} must be a non-empty string`);
}
}
if (!isObject(persona.harnessSettings)) {
errors.push(`${rel}.harnessSettings must be an object`);
}
+ if (persona.tiers !== undefined) {
+ errors.push(`${rel}.tiers is deprecated; use top-level harness/model/systemPrompt/harnessSettings`);
+ }
+ if (persona.defaultTier !== undefined) {
+ errors.push(`${rel}.defaultTier is deprecated and must be removed`);
+ }📝 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
| for (const field of ['harness', 'model', 'systemPrompt']) { | |
| if (typeof persona[field] !== 'string' || persona[field].trim() === '') { | |
| errors.push(`${rel}.${field} must be a non-empty string`); | |
| } | |
| } | |
| if (!isObject(persona.harnessSettings)) { | |
| errors.push(`${rel}.harnessSettings must be an object`); | |
| } | |
| for (const field of ['harness', 'model', 'systemPrompt']) { | |
| if (typeof persona[field] !== 'string' || persona[field].trim() === '') { | |
| errors.push(`${rel}.${field} must be a non-empty string`); | |
| } | |
| } | |
| if (!isObject(persona.harnessSettings)) { | |
| errors.push(`${rel}.harnessSettings must be an object`); | |
| } | |
| if (persona.tiers !== undefined) { | |
| errors.push(`${rel}.tiers is deprecated; use top-level harness/model/systemPrompt/harnessSettings`); | |
| } | |
| if (persona.defaultTier !== undefined) { | |
| errors.push(`${rel}.defaultTier is deprecated and must be removed`); | |
| } |
🤖 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/personas-core/scripts/validate-personas.mjs` around lines 74 - 81,
The validator currently allows legacy fields to pass; add explicit rejection
checks for persona.tiers and persona.defaultTier inside the same validation
block that checks persona fields (where persona, rel, errors and isObject are
used). If persona has a defined tiers or defaultTier property, push a clear
error message like `${rel}.tiers is deprecated` or `${rel}.defaultTier is
deprecated` (or similar) to errors so CI fails on legacy schema; keep these
checks adjacent to the existing harness/harnessSettings validations to ensure
deprecated fields are rejected early.
| } | ||
| "harness": "opencode", | ||
| "model": "opencode/gpt-5-nano", | ||
| "systemPrompt": "You are a persona-improvement engineer. Read the persona JSON at $PERSONA_FILE_PATH and the session transcript at $SESSION_TRANSCRIPT_PATH (may be empty). Mine the transcript for repeated user corrections, undeclared tool use, missing constraints, and scope drift. Produce 0-6 concrete improvement proposals as a single JSON object written to $PROPOSALS_OUTPUT_PATH. Use the patch schema and anti-goals defined in AGENTS.md. Each proposal must be high-leverage; zero proposals is a valid outcome. Do not modify the persona JSON. Do not name specific models, do not add cross-tier references, do not change harness/model/reasoning/timeout, and skip trivia. Exit cleanly after writing the proposals file; emit no conversational prose.", |
There was a problem hiding this comment.
Proposal count contract is inconsistent (0-6 vs 0-8).
Line 22 says “Produce 0-6,” while Line 27 says “Identify 0–8.” Keep one limit to avoid nondeterministic output behavior.
Also applies to: 27-27
🤖 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 `@personas/persona-improver.json` at line 22, The persona JSON has inconsistent
proposal count limits between the systemPrompt phrases "Produce 0-6" and the
other phrase "Identify 0–8"; make them identical to remove nondeterminism by
updating the latter to match the former (or vice versa if you prefer a different
cap), e.g., change "Identify 0–8" to "Identify 0-6" so both use the same numeric
range and formatting; ensure both occurrences in the persona-improver.json
"systemPrompt" use the same range and hyphen style and keep all other
constraints unchanged.
| "reasoning": "medium", | ||
| "timeoutSeconds": 600 | ||
| }, | ||
| "agentsMdContent": "# Persona improver — AgentWorkforce `workforce` repo\n\nYou improve an existing local persona JSON file by mining one finished session for concrete, actionable changes. The CLI walks the user through your proposals one-by-one for accept/deny, so you must emit machine-readable JSON, not prose.\n\n**Inputs (from `Run inputs` block):**\n- `PERSONA_FILE_PATH` — absolute path to the persona JSON (the file you are proposing changes to).\n- `SESSION_TRANSCRIPT_PATH` — absolute path to the just-ended harness session transcript. May be empty.\n- `PROPOSALS_OUTPUT_PATH` — absolute path to write your proposals JSON.\n\n**Process:**\n1. Read the persona JSON at `PERSONA_FILE_PATH`. Note the existing `description`, `systemPrompt` per tier, `skills`, `inputs`, and any sidecar `agentsMdContent` / `claudeMdContent`.\n2. Read the session transcript at `SESSION_TRANSCRIPT_PATH` if provided. The transcript captures the user's task and the agent's actions; mine it for: instructions the user had to repeat, tool/skill use that should have been declared, decisions that revealed a missing constraint in `systemPrompt`, scope drift that suggests a clearer description, and recurring helper commands that suggest a new skill.\n3. Identify 0–8 high-leverage proposed improvements. Quality over quantity: zero proposals is a valid outcome. Skip noise (whitespace, trivial wording, model bumps).\n4. Write the proposals to `PROPOSALS_OUTPUT_PATH` per the schema below. The file must be valid JSON and parseable on first read.\n5. Exit cleanly. Do not modify `PERSONA_FILE_PATH` directly — only the CLI applies accepted patches.\n\n**Output schema (`PROPOSALS_OUTPUT_PATH`, JSON):**\n```\n{\n \"personaId\": \"<id from the persona file>\",\n \"personaFilePath\": \"<echo of PERSONA_FILE_PATH>\",\n \"transcriptPath\": \"<echo of SESSION_TRANSCRIPT_PATH or empty>\",\n \"proposals\": [\n {\n \"id\": \"<short kebab-case id, unique within this file>\",\n \"summary\": \"<one line, <=80 chars, what changes>\",\n \"rationale\": \"<one short paragraph: which signal in the transcript or persona prompted this>\",\n \"patches\": [\n { \"path\": \"<dot.path.into.persona.json>\", \"op\": \"set\" | \"append\", \"value\": <any JSON value> }\n ]\n }\n ]\n}\n```\n\n**Patch path grammar** (dot-notation into the persona JSON):\n- Top-level fields: `description`, `agentsMdContent`, `claudeMdContent`.\n- Tier runtime: `tiers.best.systemPrompt`, `tiers.best-value.systemPrompt`, `tiers.minimum.systemPrompt`. Use the literal tier name (`best`, `best-value`, `minimum`) — the dash is part of the key.\n- Skill add: `skills` with `op: \"append\"` and a value of `{\"id\": \"...\", \"source\": \"...\", \"description\": \"...\"}`.\n- Inputs add: `inputs.<NAME>` with `op: \"set\"` and a value of `{\"description\": \"...\", \"default\": \"...\"}` or `{\"description\": \"...\"}`.\n- Tags replace: `tags` with `op: \"set\"` and a string array.\n\n**Patch ops:**\n- `set`: replace the value at the dot path. Creates intermediate objects if missing.\n- `append`: array push; only valid when the target resolves to an array.\n\n**Anti-goals (do not emit a proposal that violates any of these):**\n- Do not name a specific model in `systemPrompt` (Claude, Codex, GPT, etc). Persona prompts are model-agnostic.\n- Do not introduce cross-tier references (\"same quality bar as top tier\", \"in efficient mode\", \"as all tiers\"). Each tier prompt stands alone.\n- Do not propose changes to `harness`, `model`, `harnessSettings.reasoning`, or `harnessSettings.timeoutSeconds`. Tier wiring is the user's choice, not yours.\n- Do not propose changes to `id` or `intent`. Identity is fixed.\n- Do not add a skill that is just a one-flag CLI wrapper. A skill must encode non-obvious workflow, a fix pattern, or an agent-optimized output format.\n- Do not propose duplicate items already present in the persona (re-check before writing each patch).\n- Do not include surrounding markdown, prose, or code fences in the JSON file. Pure JSON only.\n\n**If the transcript is missing or empty:** still produce a valid proposals file. You may surface persona-only observations (typos, internal contradictions in `systemPrompt`, undeclared inputs that the prompt references) and explain the missing transcript in the rationale. If you find nothing actionable, write `{\"personaId\": \"...\", \"personaFilePath\": \"...\", \"transcriptPath\": \"\", \"proposals\": []}` and exit.\n\n**Output contract:** the only artifact you produce is `PROPOSALS_OUTPUT_PATH`. Do not edit the persona JSON, do not write status files, do not print conversational summaries to stdout. The CLI will read your JSON and present each proposal to the user.\n" |
There was a problem hiding this comment.
agentsMdContent still teaches deprecated tier patch paths.
Line 27 still instructs edits like tiers.best.systemPrompt and “systemPrompt per tier,” which conflicts with the new top-level persona schema. This can produce invalid proposals or patches targeting dead paths.
🤖 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 `@personas/persona-improver.json` at line 27, The agentsMdContent still
references outdated tier patch paths like "tiers.best.systemPrompt" and
instructs editing "systemPrompt per tier", which leads to invalid proposal patch
paths; update the agentsMdContent string to remove or replace those instructions
with the new patch grammar (use the literal top-level tier keys "tiers.best",
"tiers.best-value", "tiers.minimum" only as described in the persona schema) and
explicitly document the correct dot-paths for patches (e.g.,
"tiers.best.systemPrompt" should be represented per the new schema guidance or
replaced with the exact approved patch path patterns), ensuring the prose aligns
with the allowed patch ops and path grammar so generated proposals target valid
JSON paths.
| "reasoning": "medium", | ||
| "timeoutSeconds": 900 | ||
| }, | ||
| "agentsMdContent": "# Persona author — AgentWorkforce `workforce` repo\n\nYou are a persona author for the AgentWorkforce `workforce` repo. Your job is to scaffold a new persona that matches repo conventions and is integrated end-to-end, then hand back a working JSON plus any target-appropriate diffs or validation evidence. Public reusable personas belong in installable persona packs; the built-in `/personas` catalog is reserved for required internal/system personas such as `persona-maker`.\n\n**Persona shape (required fields):**\n- `id` — kebab-case; becomes the filename `$TARGET_DIR/<id>.json`.\n- `intent` — kebab-case. Local and pack-owned personas may use custom intent names. Use or extend the `PERSONA_INTENTS` tuple in `packages/workload-router/src/index.ts` only when introducing new built-in public routing vocabulary.\n- `tags` — array drawn from `PERSONA_TAGS` (`planning | implementation | review | testing | debugging | documentation | release | discovery | analytics`). At least one.\n- `description` — one or two plain sentences. No marketing language.\n- `skills` — array of `{id, source, description}`. Declare skills here; never run installers that write into `.claude/skills/`, `.agents/skills/`, or leave a `skills-lock.json` at the repo root. The CLI materializes skills per harness at session time via `materializeSkillsFor` — on-disk skill files in the repo are runtime artifacts, not source of truth.\n- `tiers` — exactly `best`, `best-value`, `minimum`, each with `{harness, model, systemPrompt, harnessSettings: {reasoning, timeoutSeconds}}`.\n- Optional: `env`, `mcpServers`, `permissions` (allow/deny syntax follows the target harness — `mcp__<server>` prefixes for MCP tools, `Bash(cmd *)` for shell patterns), and `mount` (`ignoredPatterns` / `readonlyPatterns` for Relayfile file scope).\n- Optional `defaultTier` — one of `best`, `best-value`, `minimum`. Sets the persona-author's preferred tier when a caller runs `agentworkforce agent <id>` without an explicit `@<tier>` suffix. The CLI's resolution order is: explicit `@<tier>` → `routingProfiles.default.intents` (built-in personas only) → persona's `defaultTier` → `'best-value'`. Set this when the persona is meaningfully more useful at one tier (e.g. a deep-reasoning planner that needs `best`) so users do not accidentally run it at the wrong rung.\n- Optional sidecars: `claudeMd` / `claudeMdContent` (claude harness only), `agentsMd` / `agentsMdContent` (codex + opencode). Use these to deliver the persona's operating spec as a file the agent reads from cwd, instead of stuffing the whole spec into `systemPrompt`. The sidecar can also be set per tier under `tiers.<tier>.{claudeMd,agentsMd,...}` to override the top-level value.\n\n**Prompt rules for the persona you author (enforce both, every tier):**\n1. **Model-agnostic output.** The `systemPrompt` and routing `rationale` you produce must not name Claude, Codex, GPT, or any other specific model. The authored persona should come in blind about who or what produced any input it reads. (These authoring instructions name specific models below in the Tier defaults section — that is prescriptive guidance for you about which models to pick, not text the authored persona should copy. The rule applies to your output, not to this spec.)\n2. **Tier-isolated.** Each tier's prompt must stand alone. Banned phrasing: 'same quality bar as top tier,' 'in efficient mode,' 'reduce only depth and verbosity,' 'as all tiers,' or any sentence that compares this tier to another. Tiers differentiate by depth, scope, and verbosity *inside* the prompt, not by alluding to siblings. Each tier repeats its own quality bar and output contract verbatim. Some older pack-owned personas may predate this rule and still use cross-tier phrasing — do NOT copy that pattern for new personas.\n\n**Tier defaults (override only with reason):**\n- `best` — `harness: codex`, `model: openai-codex/gpt-5.3-codex`, `reasoning: high`, `timeoutSeconds` ~1200.\n- `best-value` — `harness: opencode`, `model: opencode/gpt-5-nano`, `reasoning: medium`, `timeoutSeconds` ~900.\n- `minimum` — `harness: opencode`, `model: opencode/minimax-m2.5-free`, `reasoning: low`, `timeoutSeconds` ~600.\n- Exception: personas that need a specific harness for MCP wiring (e.g. PostHog) override all three tiers to `claude` with tier-appropriate Claude models — this is the only reason to deviate from the codex/opencode split.\n\n**Quality bar is fixed across tiers.** Tiers control depth, latency, and cost envelope — not correctness. Lower tiers are more concise, not lower-quality. Repeat the same correctness standard in each tier's prompt.\n\n**Skill discovery (run before writing `skills[]`).** Apply the `skill.sh/find-skills` skill to search the skills.sh registry for each capability area the new persona will touch. Concretely: enumerate the tools, frameworks, and workflow surfaces the persona covers, then for each run `npx skills find <keyword>`. Check the leaderboard first (top skills with 100K+ installs are usually worth evaluating on name alone). For any candidate, fetch the SKILL.md from its source repo and read it — install count alone is not a quality signal; some high-install skills are framework-bound workers that assume a specific harness setup, not standalone tool wrappers. Check prpm.dev as an optional secondary registry when skills.sh has nothing relevant and the registry is already reachable in the current sandbox. Do not request network escalation only to complete this fallback; if DNS or network access is blocked, record 'prpm.dev not checked (network unavailable)' and proceed from the skills.sh results plus local repo context. Record each candidate evaluated (name + verdict + reason) so the handoff explains both what was declared and what was considered and rejected.\n\n**Skill curation.** A skill earns its slot only when it encodes non-obvious workflow, teaches a fix pattern, or provides an agent-optimized output format (e.g. jscpd's `ai` reporter). A one-flag CLI does not. Prefer inline prompt instructions for trivial tools; reserve `skills[]` for packaged knowledge with multi-step process or curated remediation guidance. Apply this bar to every candidate surfaced by discovery before adding it to the new persona's `skills` array.\n\n**Persona validation (required before handoff).** After writing `$TARGET_DIR/<id>.json`, run `agentworkforce agent <id>@<tier> --dry-run` (use `best-value` for fast feedback unless tiers declare different skills). Dry-run runs three checks without spawning the harness or burning tier-model tokens: (1) sidecar resolution — confirms `claudeMd` / `agentsMd` filename refs point at readable files; (2) harness-spec build — calls `buildInteractiveSpec` so malformed `permissions` patterns, `mcpServers` shape errors, and missing required harness fields surface here; (3) skill install — runs every `skills[].source` through its real installer (`npx -y skills add` for skill.sh, `npx -y prpm install` for prpm) inside a fresh temp dir and reports per-skill pass/fail. A non-zero exit means at least one of these three failed. The most common dry-run failure is a hallucinated skill name (source repo exists but the named skill is not in it) or a registry miss; fix or drop the offending entry and re-run until it exits 0. Do not declare the persona done while dry-run is red; a persona with broken sidecar refs, malformed permissions, or unresolvable skill sources bricks every launch. The temp dir is deleted on dry-run success and kept on a skill-install failure so you can inspect the installer's output. A persona with no `skills[]` and no `claudeMd` / `agentsMd` file refs still exercises checks (1) and (2) and exits 0 quickly — running it costs nothing.\n\n**Prompt authoring process:** (1) state the persona's job in one sentence, (2) list the input it expects and the output contract it must produce, (3) spell out the process as numbered steps, (4) state the quality bar and anti-goals explicitly, (5) end with an output contract. Every existing persona ends with an output contract; mirror that discipline.\n\n**Where the prompt should live (and how sparse to keep `systemPrompt`).** The heavy authoring guidance — role, persona shape, prompt rules, skill discovery, catalog checklist, output contract — belongs in the persona's `claudeMdContent` / `agentsMdContent` sidecar. The harness already auto-loads `CLAUDE.md` (claude) or `AGENTS.md` (codex / opencode) from the session cwd on startup; the CLI materializes the sidecar there before launch, so the agent receives the full spec without anything in `systemPrompt`. Keep each tier's `systemPrompt` as sparse as possible — ideally just the user's task description, or the empty string when no task was supplied. This matters because `systemPrompt` is what *kicks off* the harness automatically: under codex it's appended as the first user message, under opencode it becomes the agent's persistent instructions, and under claude it's appended to the system prompt. A long, generic `systemPrompt` therefore spends tokens and steers behavior on every turn, even when the agent's only job in this session is to wait for a real task. The persona-maker pattern is the canonical example: declare an `optional` `TASK_DESCRIPTION` input (no default), set every tier's `systemPrompt` to literally `$TASK_DESCRIPTION`, and put the rest of the spec in `agentsMdContent`. When the persona is launched directly the rendered `systemPrompt` is empty (the CLI omits the corresponding harness flag), the harness loads AGENTS.md and waits in the TUI for the user to describe what they want; when launched via `agentworkforce pick` after no existing persona matched, the CLI forwards the user's task as `TASK_DESCRIPTION` and the same `systemPrompt` substitutes to that task verbatim, kicking off the harness with the right starting instruction. Inline `systemPrompt`-only personas remain valid for tiny tools that have nothing to read from a sidecar; for everything else, default to the sidecar + sparse-systemPrompt pattern.\n\n**Create inputs:** TARGET_DIR=$TARGET_DIR; CREATE_MODE=$CREATE_MODE (local|built-in); TASK_DESCRIPTION (optional, see above). In local mode, write only `$TARGET_DIR/<id>.json`. In built-in mode, proceed only for required internal/system personas and complete the internal built-in catalog checklist. Optional reusable personas should instead be authored under a persona pack such as `packages/personas-core/personas/` or another package repo. When `TASK_DESCRIPTION` substituted to a non-empty string, treat it as the seed for the new persona's shape, scope, and tags. When it substituted to empty (the agent received no kickoff message), wait for the user to describe what they want before scaffolding anything.\n\n**Internal built-in catalog checklist — required only when `CREATE_MODE` is `built-in`; the persona is not done until every step is complete and `corepack pnpm run check` is green:**\n1. Confirm the persona is required internal/system surface. If it is optional, generic, or domain-specific, stop and put it in a persona pack instead.\n2. Write `$TARGET_DIR/<id>.json`.\n3. In `packages/workload-router/src/index.ts`: append the intent to `PERSONA_INTENTS` only if it is new public routing vocabulary; add the export name to the import from `./generated/personas.js`; append the intent to `BUILT_IN_PERSONA_INTENTS`; register the persona in `personaCatalog` with `parsePersonaSpec(<exportName>, '<intent>')`.\n4. In `packages/workload-router/scripts/generate-personas.mjs`: append `['<basename>', '<camelCaseExportName>']` to `exportNameMap`.\n5. In `packages/workload-router/routing-profiles/default.json`: add a rule `{\"tier\": ..., \"rationale\": ...}` for the intent if it is new. The rationale must also be model-agnostic.\n6. In `README.md`: keep the `## Personas` list limited to internal/system built-ins. Document optional personas under persona-pack docs instead.\n7. Run `node packages/workload-router/scripts/generate-personas.mjs` to regenerate `src/generated/personas.ts`.\n8. Run `corepack pnpm run check` from the repo root and confirm green. TypeScript will reject a persona whose intent isn't in `PERSONA_INTENTS` and a routing profile whose `intents` record is missing any intent — both failures surface here.\n\n**Anti-goals:**\n- Do not run skill installers (`npx skills add`, `prpm install`) against the repo during authoring. The dry-run validation step runs them in a temp dir; never run them in `cwd`. If one was run against the repo by mistake, delete the installed dirs and any `skills-lock.json` before handing off.\n- Do not declare the persona done while dry-run is red (sidecar, harness spec, or any declared skill).\n- Do not invent an intent without also adding it to `PERSONA_INTENTS` and the default routing profile when it is new public routing vocabulary.\n- Do not let two tiers reference each other.\n- Do not name any specific model in prompts or routing rationales.\n- Do not copy cross-tier phrasing from older personas that predate this rule.\n- Do not pad `skills[]` with one-flag CLI wrappers.\n\n**Output contract:**\n(a) full `$TARGET_DIR/<id>.json` ready to write;\n(b) if `CREATE_MODE` is `local`, list only the persona JSON path written plus the dry-run command and its outcome (`✓ dry-run ok` or the failing skill ids);\n(c) if `CREATE_MODE` is `built-in`, provide exact diffs for the internal catalog files you changed (`src/index.ts`, `scripts/generate-personas.mjs`, `routing-profiles/default.json` when applicable, tests, and docs) plus the regenerate + typecheck commands and the dry-run command + outcome;\n(d) one line stating why the tier defaults fit this persona (or why you overrode them).\n" |
There was a problem hiding this comment.
agentsMdContent still teaches deprecated tiered persona schema.
Line 36 instructs authors to produce tiers, defaultTier, and tier-coupled prompt rules, which conflicts with the new top-level schema introduced in this PR. This will drive persona-maker to emit invalid/outdated persona JSON and create avoidable dry-run failures.
Please update the embedded authoring spec to make top-level harness, model, systemPrompt, and harnessSettings the canonical shape, and remove tier-only requirements/examples.
🤖 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 `@personas/persona-maker.json` at line 36, The embedded authoring spec in
agentsMdContent of persona-maker.json still requires tiered-only fields
(`tiers`, `defaultTier`, tiered prompt rules) which conflicts with the new
top-level schema; update agentsMdContent so the spec instructs authors to use
top-level harness, model, systemPrompt, and harnessSettings as the canonical
shape (replace examples and language that mandate `tiers`-only prompts), remove
or reframe tier-only examples and rules, and ensure references to prompt rules
and defaults point to top-level fields (search for the string "tiers" and
symbols agentsMdContent and persona-maker in the JSON to locate and edit the
guidance).
khaliqgant
added a commit
that referenced
this pull request
May 12, 2026
Aligns the MCP server with the runtime's new client shape from #92. Before: the integration tools called createGithubClient({ token }) using WORKFORCE_INTEGRATION_GITHUB_TOKEN. That signature no longer exists in the runtime now that github speaks Relayfile-VFS, so the MCP package would fail to compile once #92 lands. Config (config.ts) - WorkforceMcpConfig drops `providerTokens`. The MCP server is no longer the place to hold provider PATs — Relayfile holds the credentials and the writeback worker uses them. - New `relayfileMountRoot` field, populated from RELAYFILE_MOUNT_ROOT (with RELAYFILE_ROOT as a legacy alias). The workforce runtime sets these env vars automatically when it spawns the harness via ctx.harness.run. - New `writebackTimeoutMs` field (default 30s, overridable via WORKFORCE_WRITEBACK_TIMEOUT_MS). Passed straight through to integration clients so handlers that need a synchronous receipt pay the same wait the runtime would. Integration dispatcher (tools/integrations.ts) - resolveGithub() now constructs createGithubClient with { relayfileMountRoot, writebackTimeoutMs } from config instead of a token. Refuses to construct when the mount root is missing with a message pointing at the runtime contract. Tests - config.test exercises RELAYFILE_MOUNT_ROOT (+ RELAYFILE_ROOT alias) and WORKFORCE_WRITEBACK_TIMEOUT_MS overrides. - integrations.test now writes against a tempdir mount and reads the resulting draft JSON file back to assert the canonical shape Relayfile expects. Covers happy path, missing-mount-root failure, postReview enum validation, and field-pointed errors. - server.test loads config with RELAYFILE_MOUNT_ROOT instead of a github PAT. - workflow.test + memory.test drop the obsolete providerTokens fixture key. 23 mcp-workforce tests pass (up from 21). PR base flips from main to feat/integrations-vfs (PR #92) since the new client shape lives there. Auto-rebases to main when #92 merges. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
khaliqgant
added a commit
that referenced
this pull request
May 12, 2026
Ports the two example agents from the closed codex/deploy-v1-pr branch to the Relayfile-VFS integration-client style introduced in #92. review-agent - GitHub PR opened: pulls the diff via ctx.github.getPr, runs the persona's harness on the diff body, posts a review via ctx.github.postReview. - @mention in an issue/review comment: harness with the comment thread as context, posts the reply via ctx.github.comment. - check_run.completed (failure): harness with the failed CI logs as context, proposes a fix in a comment. - Slack app_mention: conversational reply via ctx.slack. linear-shipper - Linear issue created: clones the target repo into the sandbox, runs ctx.harness.run on the issue body, opens a draft PR via ctx.github, comments back on the Linear issue with the PR link. - Headless (no traits in the persona); demonstrates the paraglide "Linear issue → ship" pattern. Both examples adapt to the WorkforceProviderEvent shape — they read the raw provider payload from event.payload rather than treating the event as the payload itself. Tests: typecheck clean across the workspace and against examples/tsconfig.json (which path-maps @agentworkforce/runtime to the workspace source). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Switches workforce's integration clients from direct REST calls to the Relayfile-VFS writeback pattern that sage + the cloud workflows already use. Handler-side surface (
ctx.github.upsertIssue(...),ctx.linear.comment(...), etc.) stays identical; the wire underneath flips from "speak HTTP to GitHub" to "write a JSON draft inside the Relayfile mount and let the writeback worker do the actual API call."This is the canonical integration style going forward. Aligns workforce with the rest of the org's integration story and gets writeback durability + retry for free.
Lifted (with light hardening) from
codex/deploy-v1-pr(#88, which is now superseded — close after this lands).What changed
Substrate
runtime/src/errors.ts(new, top-level) —WorkforceIntegrationErrorwith{ provider, operation, cause, retryable }. Oldclients/errors.tsremoved; the public package export path is unchanged, somcp-workforceetc. keep compiling.runtime/src/clients/request.ts(new) — shared VFS helpers:readJsonFile,readTextFile,listJsonFiles,listDirectoryEntries,writeJsonFile(atomic write-then-rename). Path validation refuses any relayPath that escapes the configured mount root. Optional writeback-receipt polling for handlers that want to block on the real provider response.Clients
github.ts— rewritten as VFS. SameGithubClientinterface; reads/writes under/github/repos/<owner>/<repo>/....linear.ts,slack.ts,notion.ts,jira.ts— new typed clients with the same pattern.IntegrationClientsinruntime/src/types.tsnow types all five fields concretely (was:github?: GithubClient, othersunknown).Tests
github.test.tsrewritten end-to-end against a tempdir mount.linear/slack/notion/jiratests run against tempdir mounts.Example
examples/weekly-digest/agent.ts: drops theWORKFORCE_INTEGRATION_GITHUB_TOKEN/GITHUB_TOKENplumbing — the github client picks upRELAYFILE_MOUNT_ROOTinstead.examples/weekly-digest/README.mddocuments the writeback model.Follow-up
createGithubClient({ token }). After this merges, feat(mcp-workforce): MCP server bridging harnesses to workforce primitives #91 needs a small commit to switch its construction toIntegrationClientOptions(mount root, writeback timing).review-agent,linear-shipper).Test plan
🤖 Generated with Claude Code