Skip to content

fix: mount every relay-helpers provider — scope audit of all personas (#40 class) #42

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
khaliqgant merged 3 commits into from
Jun 4, 2026
Merged

fix: mount every relay-helpers provider — scope audit of all personas (#40 class) #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3ab51db
fix: mount every relay-helpers provider — scope audit of all personas…
khaliqgant Jun 4, 2026
f770ecb
fix(granola): widen linear scope to cover the teams-listing fallback
khaliqgant Jun 4, 2026
599ec63
test: derive the provider guard list from the writeback catalog
khaliqgant Jun 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion granola/persona.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,18 @@ export default definePersona({
// `granola-relay:fetch-notes` sync, which writes each note to the VFS at
// /granola/notes/<id>.json and fires a storage `file.created` event.
granola: {},
linear: {},
// linear has no trigger here, so `scope` is the only thing that mounts
// /linear — without it the issue creation and PR-link comment writes
// are silent no-ops. Issues draft to /linear/issues and comments to
// /linear/issues/{issueId}/comments; the teams subtree backs the
// listLinearTeams() fallback when LINEAR_TEAM_ID is unset. (Scope
// values must be strings — one entry per subtree, not an array.)
linear: {
scope: {
issues: '/linear/issues/**',
teams: '/linear/teams/**'
}
},
// The cloud materializes this repo into the sandbox (ctx.sandbox.cwd) via
// relayfile — the agent never clones it.
github: { scope: { repo: 'your-org/your-repo' } }
Expand Down
6 changes: 5 additions & 1 deletion hn-monitor/persona.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ export default definePersona({
cloud: true,

// `slack` gives the handler the ctx.slack client to post the digest.
integrations: { slack: {} },
integrations: {
// No slack trigger here (cron-only persona), so `scope` is the only
// thing that mounts /slack — without it every post is a silent no-op.
slack: { scope: { paths: '/slack/channels/**' } }
},

inputs: {
TOPICS: {
Expand Down
12 changes: 10 additions & 2 deletions repo-hygiene/persona.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,16 @@
"useSubscription": true,
"integrations": {
"github": {},
"notion": {},
"slack": {}
"notion": {
"scope": {
"paths": "/notion/databases/**"
}
},
"slack": {
"scope": {
"paths": "/slack/channels/**"
}
}
},
"inputs": {
"NOTION_DATABASE_ID": {
Expand Down
9 changes: 7 additions & 2 deletions repo-hygiene/persona.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,13 @@ export default definePersona({

integrations: {
github: {},
notion: {},
slack: {}
// github above is mounted via the agent's triggers; notion and slack
// have no triggers, so `scope` is the only thing that mounts them —
// without it the Notion journal write and the Slack summary post are
// silent no-ops. The database is picked at deploy (NOTION_DATABASE_ID),
// so scope the databases subtree.
notion: { scope: { paths: '/notion/databases/**' } },
slack: { scope: { paths: '/slack/channels/**' } }
},

inputs: {
Expand Down
7 changes: 6 additions & 1 deletion spotify-releases/persona.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ export default definePersona({
description: 'Checks daily for new releases from artists you follow on Spotify and DMs you about them.',
cloud: true,

integrations: { slack: {} },
integrations: {
// No slack trigger here (cron-only persona), so `scope` is the only
// thing that mounts /slack. DMs write to /slack/users/{userId}/messages,
// a different subtree than channel posts.
slack: { scope: { paths: '/slack/users/**' } }
},

inputs: {
SLACK_USER: {
Expand Down
101 changes: 101 additions & 0 deletions tests/persona-integration-scopes.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
import assert from 'node:assert/strict';
import { readFileSync, readdirSync, existsSync } from 'node:fs';
import { join, dirname } from 'node:path';
import { fileURLToPath } from 'node:url';
import test from 'node:test';

import { parseIntegrations } from '@agentworkforce/persona-kit';
import { WRITEBACK_PATH_CATALOG } from '@relayfile/adapter-core/writeback-paths';

/**
* Class guard for the agents#40 trap, generalized: cloud mounts an
* integration's relayfile subtree ONLY from the agent's triggers or the
* integration's `scope`. A provider that the handler touches through a
* relay-helpers client but that has neither is mounted nowhere — every
* read fails and every write is a silent no-op (the draft lands on
* unmounted local disk and the writeback worker never sees it). That is
* how the pr-reviewer's Slack pings (agents#40), hn-monitor's posts,
* spotify-releases' DMs, vendor-monitor's posts, repo-hygiene's
* slack/notion legs, and granola's Linear issue pipeline all shipped dead.
*
* Invariant: every provider referenced via `<provider>Client(` in a
* persona's agent.ts must appear in the agent's `triggers` OR carry a
* non-empty `scope` that survives persona-kit parsing (empty `scope: {}`
* objects are DISCARDED client-side, so they don't count).
*/

const repoRoot = join(dirname(fileURLToPath(import.meta.url)), '..');
// Provider names come from the writeback catalog itself, so a persona using
// a newly-catalogued provider is guarded automatically (no hand-kept list).
const PROVIDERS = Object.keys(WRITEBACK_PATH_CATALOG)
.map((name) => name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'))
.join('|');
// Three usage shapes, all of which require the provider's subtree mounted:
// slackClient().post(...) — named factory client
// relayClient('linear') / providerClient(...) — generic factory client
// writeJsonFile(c, 'notion', op, `/notion/…`) — raw VFS helper w/ path literal
//
// Deliberate static-analysis trade-offs:
// - Dynamic provider args (`relayClient(someVar)`) are invisible — the
// guard can false-negative there; reviewers still check those by hand.
// - Matching raw source means comments/strings can false-positive — the
// safe direction for a guard (it forces a look, never hides a gap).
const CLIENT_RE = new RegExp(`\\b(${PROVIDERS})Client\\s*\\(`, 'g');
const GENERIC_CLIENT_RE = new RegExp(`\\b(?:relayClient|providerClient)\\s*\\(\\s*['"\`](${PROVIDERS})['"\`]`, 'g');

@codeant-ai codeant-ai Bot Jun 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: The generic-client regex only matches string-literal provider arguments, so calls like relayClient(providerName) are invisible to this test. That creates a silent coverage hole where a persona can still touch an unmounted provider and bypass this guard. Add handling for identifier-based arguments (with simple constant resolution/AST analysis) to avoid false negatives. [incomplete implementation]

Severity Level: Major ⚠️ 
- ❌ relayClient variable providers bypass unmounted-provider integration guard.
- ⚠️ Dynamic provider usage may silently skip mount validation.
Steps of Reproduction ✅ 
1. Note that existing generic-client usage in this repo, such as `const linear =
relayClient('linear');` in `/workspace/agents/granola/agent.ts:35-37`, passes a string
literal and is therefore caught by `GENERIC_CLIENT_RE` defined at
`tests/persona-integration-scopes.test.mjs:33`.

2. Introduce a new persona whose `agent.ts` uses a dynamic provider argument, e.g. `const
provider = 'linear'; const client = relayClient(provider);`, following the same import
pattern as `granola/agent.ts:9-11` but without using a string literal as the first
argument to `relayClient`.

3. Run `npm test`; `tests/persona-integration-scopes.test.mjs` discovers the new persona
via `personaDirs()` at lines 36-45, reads its `agent.ts` at line 64, and passes the source
into `clientProviders()` at lines 48-55. Inside `clientProviders`, `GENERIC_CLIENT_RE`
(line 33) only matches `relayClient`/`providerClient` calls where the first argument is a
string or template literal containing one of the `PROVIDERS` names, so
`relayClient(provider)` produces no matches and the corresponding provider name (e.g.
`'linear'`) is never added to the `used` set.

4. Because the provider reached only via `relayClient(variable)` is absent from `used`,
the loop at `tests/persona-integration-scopes.test.mjs:73-79` never evaluates its triggers
or `parsed[provider]?.scope`. The final assertion at lines 82-89 can succeed even if the
persona's `persona.ts` mirrors `/workspace/agents/granola/persona.ts:16-29` with `linear:
{}` but no scope and the handler writes via `client.write('issues', …)`, allowing an
unmounted provider regression to bypass this guard and recreate the silent no-op behavior
described in the class comment at lines 9-23.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** tests/persona-integration-scopes.test.mjs
**Line:** 33:33
**Comment:**
	*Incomplete Implementation: The generic-client regex only matches string-literal provider arguments, so calls like `relayClient(providerName)` are invisible to this test. That creates a silent coverage hole where a persona can still touch an unmounted provider and bypass this guard. Add handling for identifier-based arguments (with simple constant resolution/AST analysis) to avoid false negatives.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

const PATH_LITERAL_RE = new RegExp(`['"\`]/(${PROVIDERS})/`, 'g');

function personaDirs() {
return readdirSync(repoRoot, { withFileTypes: true })
.filter((entry) => entry.isDirectory())
.map((entry) => entry.name)
.filter((name) =>
existsSync(join(repoRoot, name, 'persona.ts')) &&
existsSync(join(repoRoot, name, 'agent.ts')) &&
existsSync(join(repoRoot, '.test-build', name, 'persona.js')) &&
existsSync(join(repoRoot, '.test-build', name, 'agent.js')),
);
}

function clientProviders(agentSource) {
const providers = new Set();
for (const re of [CLIENT_RE, GENERIC_CLIENT_RE, PATH_LITERAL_RE]) {
for (const match of agentSource.matchAll(re)) {
providers.add(match[1]);
}
}
return providers;
}
Comment on lines +59 to +67

@codeant-ai codeant-ai Bot Jun 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: Provider detection runs regexes against raw agent.ts text, so comments and unrelated string literals are treated as real provider usage. That can produce false failures (for example, a commented example path like '/jira/...' would be counted as a live dependency) and make the test noisy/flaky. Restrict detection to executable code (AST-based parsing or at least comment stripping) before matching providers. [logic error]

Severity Level: Major ⚠️ 
- ⚠️ Commented example paths can fail integration scope test.
- ⚠️ Spurious violations can block CI when adding comments.
Steps of Reproduction ✅ 
1. Open an existing agent file such as `/workspace/agents/hn-monitor/agent.ts:1-43`, which
is included in `personaDirs()` because it has both `persona.ts` and `agent.ts` plus
compiled outputs (see `personaDirs` at `tests/persona-integration-scopes.test.mjs:36-45`).

2. Add a purely commented example path to that file, e.g. `// Example JIRA path
'/jira/projects/**'`, alongside the existing header comments at
`/workspace/agents/hn-monitor/agent.ts:1-9`, without changing any executable code to
actually reference a JIRA provider.

3. Run `npm test` so `tests/persona-integration-scopes.test.mjs` executes; the test reads
the raw `agent.ts` text via `readFileSync` at line 64 and passes it into
`clientProviders(agentSource)` at lines 48-55, where `PATH_LITERAL_RE` from line 34 is
applied over the entire source with `agentSource.matchAll(re)` (line 51) and any
`'/jira/…'` substring in comments is treated as a real path literal, causing `'jira'` to
be added to the `providers` set.

4. For `hn-monitor`, the persona definition at
`/workspace/agents/hn-monitor/persona.ts:14-19` only declares a `slack` integration with
scope and no `jira` entry, and the agent has no `jira` triggers. During the loop at
`tests/persona-integration-scopes.test.mjs:73-79`, `provider === 'jira'` lacks both a
trigger and a non-empty scope, so a violation is pushed and the final
`assert.deepEqual(violations, [], …)` at lines 82-89 fails even though no executable code
uses a JIRA provider—this is a pure false positive driven by comment text.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** tests/persona-integration-scopes.test.mjs
**Line:** 48:56
**Comment:**
	*Logic Error: Provider detection runs regexes against raw `agent.ts` text, so comments and unrelated string literals are treated as real provider usage. That can produce false failures (for example, a commented example path like `'/jira/...'` would be counted as a live dependency) and make the test noisy/flaky. Restrict detection to executable code (AST-based parsing or at least comment stripping) before matching providers.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎


test('every relay-helpers provider a persona uses is mounted via trigger or non-empty scope', async () => {
const dirs = personaDirs();
assert.ok(dirs.length >= 5, `expected to discover the persona set, found: ${dirs.join(', ')}`);

const violations = [];
for (const dir of dirs) {
const agentSource = readFileSync(join(repoRoot, dir, 'agent.ts'), 'utf8');
const used = clientProviders(agentSource);
if (used.size === 0) continue;

const { default: persona } = await import(`../.test-build/${dir}/persona.js`);
const { default: agent } = await import(`../.test-build/${dir}/agent.js`);
const parsed = parseIntegrations(persona.integrations ?? {}, `${dir}.integrations`) ?? {};
const triggerProviders = new Set(Object.keys(agent?.triggers ?? {}));

for (const provider of used) {
const scope = parsed[provider]?.scope;
const hasScope = Boolean(scope && Object.keys(scope).length > 0);
if (!triggerProviders.has(provider) && !hasScope) {
violations.push(`${dir}: touches "${provider}" via relay-helpers/VFS but it has no trigger and no scope — nothing mounts /${provider}, so its reads fail and its writes are silent no-ops`);
}
}
}

assert.deepEqual(
violations,
[],
`personas with unmounted relay-helpers providers:\n ${violations.join('\n ')}\n` +
'Fix: add a scope to the integration (e.g. slack posts → { scope: { paths: "/slack/channels/**" } }, ' +
'slack DMs → "/slack/users/**", linear issues/comments → "/linear/issues/**") or declare a trigger. ' +
'See the writing-agent-personas skill §1.',
);
});
6 changes: 5 additions & 1 deletion vendor-monitor/persona.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ export default definePersona({
description: 'Watches the vendors in your stack for new releases and posts the changes to your team Slack channel.',
cloud: true,

integrations: { slack: {} },
integrations: {
// No slack trigger here (cron-only persona), so `scope` is the only
// thing that mounts /slack — without it every post is a silent no-op.
slack: { scope: { paths: '/slack/channels/**' } }
},

inputs: {
VENDORS: {
Expand Down