From e52ea21e42d0b2867d24f1f25940b0d9681e88c1 Mon Sep 17 00:00:00 2001 From: Ricky Schema Cascade Date: Sat, 15 Aug 2026 21:51:30 +0200 Subject: [PATCH 1/3] feat(persona-kit): lint integration mount scopes at deploy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A persona's `scope` decides two things at once: which Relayfile paths the sandbox mirrors, and what the runtime token may touch. Both failure modes are silent. A scope the mount rejects mirrors nothing, so reads come back empty and writebacks land on unmounted disk as no-ops — the agent reports success and does nothing. `lintScopes` walks `integrations[].scope` and warns, non-fatally, on the same channel `lintTriggers` already uses: surfaced by `deploy` before any side effect, and returned on `DeployResult.warnings`. Six of the seven rules mirror cloud's authoritative `relayfile/mount-intent.ts`, so a path this lint flags is one the mount would also reject: non-absolute, trailing slash, mid-path wildcard, traversal segment, an explicitly empty `scope: {}`, and a provider-root `/slack/**`. The seventh is a judgment call, and is the reason this was written. The mount is *traversed* at sandbox boot, so its size is paid on every run — but size is invisible in the glob. `/slack/channels/**` is syntactically identical to `/linear/issues/**`; in one real workspace it was ~5,950 entries (2,008 files, 3,940 directories), could not converge inside the mount budget, and left runs either degraded ("scoped initial sync failed; continuing without preloaded reads") or failing outright with exit 124 once cloud began cancelling non-converging mounts at the hard deadline. Narrowing to the single channel the agent posted to took that bootstrap to a clean 127s. So `HIGH_CARDINALITY_ROOTS` names the collections that grow with workspace history rather than with configuration — Slack channels, Gmail messages and threads — and warns only on those. It is deliberately a short list rather than a rule about terminal `/**`: flagging every `/linear/issues/**` would be noise that trains authors to ignore the warning. Being on the list is not an error either; a read-heavy agent may genuinely need the collection. The warning asks the author to confirm that, and points write-only agents at the cheaper single-entry scope — posting never needed the mirror, only the grant. Co-Authored-By: Claude Opus 5 --- packages/deploy/src/preflight.ts | 13 +- packages/persona-kit/src/index.ts | 6 + packages/persona-kit/src/scopes.test.ts | 97 +++++++++++ packages/persona-kit/src/scopes.ts | 217 ++++++++++++++++++++++++ 4 files changed, 330 insertions(+), 3 deletions(-) create mode 100644 packages/persona-kit/src/scopes.test.ts create mode 100644 packages/persona-kit/src/scopes.ts diff --git a/packages/deploy/src/preflight.ts b/packages/deploy/src/preflight.ts index 6b9699f1..a1c05b80 100644 --- a/packages/deploy/src/preflight.ts +++ b/packages/deploy/src/preflight.ts @@ -2,6 +2,7 @@ import { stat } from 'node:fs/promises'; import path from 'node:path'; import { KNOWN_TRIGGER_PROVIDER_ALIASES, + lintScopes, lintTriggers, type AgentSpec } from '@agentworkforce/persona-kit'; @@ -97,9 +98,15 @@ export async function preflightPersona(personaPath: string): Promise `${issue.path}: ${issue.message}` - ); + // Scope lints ride the same warning channel as trigger lints: surfaced at + // deploy time, non-fatal. A bad scope mirrors nothing and the agent reads an + // empty tree without erroring, so a warning here is the only place it gets + // said before the author is debugging a silently inert agent. + const scopeLint = lintScopes(persona); + const warnings = [ + ...triggerLint.map((issue) => `${issue.path}: ${issue.message}`), + ...scopeLint.map((issue) => issue.message) + ]; return { persona, diff --git a/packages/persona-kit/src/index.ts b/packages/persona-kit/src/index.ts index 1c7819fd..5719af61 100644 --- a/packages/persona-kit/src/index.ts +++ b/packages/persona-kit/src/index.ts @@ -160,6 +160,12 @@ export { type TriggerLintIssue, type TriggerLintLevel } from './triggers.js'; +export { + lintScopes, + type ScopeLintCode, + type ScopeLintIssue, + type ScopeLintLevel +} from './scopes.js'; // Skill materialization export { diff --git a/packages/persona-kit/src/scopes.test.ts b/packages/persona-kit/src/scopes.test.ts new file mode 100644 index 00000000..89a6e983 --- /dev/null +++ b/packages/persona-kit/src/scopes.test.ts @@ -0,0 +1,97 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { lintScopes } from './scopes.js'; +import type { PersonaSpec } from './types.js'; + +function persona(integrations: Record): PersonaSpec { + return { id: 'demo', integrations } as unknown as PersonaSpec; +} + +test('a concrete subpath scope is clean', () => { + const issues = lintScopes( + persona({ + linear: { scope: { projects: '/linear/projects/**', issues: '/linear/issues/**' } }, + slack: { scope: { channel: '/slack/channels/C0B9Z4CLG1J/**' } } + }) + ); + assert.deepEqual(issues, []); +}); + +test('an integration with no scope at all is clean', () => { + // Credential-only providers (an MCP server) have no Relayfile side, so + // omitting scope is correct and must not warn. + assert.deepEqual(lintScopes(persona({ 'supabase-mcp': {} })), []); +}); + +test('an explicitly empty scope object is flagged', () => { + // Distinct from omitting scope: `scope: {}` reads as "I scoped this" while + // mirroring nothing, so reads come back empty and writes no-op silently. + const issues = lintScopes(persona({ slack: { scope: {} } })); + assert.equal(issues.length, 1); + assert.equal(issues[0].code, 'scope_empty'); + assert.equal(issues[0].provider, 'slack'); +}); + +test('a mid-path wildcard is flagged — the mount rejects it silently', () => { + // cloud's mount-intent allows ONLY a terminal /**; anything else mirrors + // nothing and the agent reads an empty tree without an error. + const issues = lintScopes(persona({ slack: { scope: { msgs: '/slack/*/messages' } } })); + assert.equal(issues.length, 1); + assert.equal(issues[0].code, 'scope_mid_path_wildcard'); + assert.equal(issues[0].path, 'integrations.slack.scope.msgs'); +}); + +test('a provider-root mirror is flagged as a cost, not an error', () => { + const issues = lintScopes(persona({ slack: { scope: { all: '/slack/**' } } })); + assert.equal(issues.length, 1); + assert.equal(issues[0].code, 'scope_provider_root'); + assert.equal(issues[0].level, 'warning'); + assert.match(issues[0].message, /mirrors the whole slack tree/u); +}); + +test('non-absolute, trailing-slash and traversal scopes are flagged', () => { + const cases: [string, string][] = [ + ['slack/channels/**', 'scope_not_absolute'], + ['/slack/channels/', 'scope_trailing_slash'], + ['/slack/../linear/issues/**', 'scope_traversal_segment'] + ]; + for (const [value, code] of cases) { + const issues = lintScopes(persona({ slack: { scope: { x: value } } })); + assert.equal(issues.length, 1, `expected one issue for ${value}`); + assert.equal(issues[0].code, code, `wrong code for ${value}`); + } +}); + +test('never throws on malformed personas', () => { + assert.deepEqual(lintScopes({ id: 'x' } as unknown as PersonaSpec), []); + assert.deepEqual(lintScopes(persona({ slack: 'nope' as unknown as object })), []); + assert.deepEqual(lintScopes(persona({ slack: { scope: { n: 42 as unknown as string } } })), []); +}); + +test('a bounded collection wildcard is NOT flagged', () => { + // The lint does not flag terminal /** in general — whether a collection is + // affordable depends on the workspace, not the glob, and flagging every + // `/linear/issues/**` would be noise that trains authors to ignore it. + assert.deepEqual(lintScopes(persona({ linear: { scope: { i: '/linear/issues/**' } } })), []); +}); + +test('a history-sized collection wildcard IS flagged', () => { + // The exception to the rule above, and the reason this lint was written: + // `/slack/channels/**` is syntactically identical to `/linear/issues/**` but + // mirrored ~5,950 entries in a real workspace and could not converge inside + // the mount budget. Only collections that grow with history, not with + // configuration, are on the list. + const issues = lintScopes(persona({ slack: { scope: { channels: '/slack/channels/**' } } })); + assert.equal(issues.length, 1); + assert.equal(issues[0].code, 'scope_high_cardinality_root'); + assert.match(issues[0].message, /only WRITES here/u); +}); + +test('a single entry under a history-sized collection is clean', () => { + // The fix the warning above points at: scoping the one channel the agent + // posts to must not itself warn, or the advice is unfollowable. + assert.deepEqual( + lintScopes(persona({ slack: { scope: { channel: '/slack/channels/C0B9Z4CLG1J/**' } } })), + [] + ); +}); diff --git a/packages/persona-kit/src/scopes.ts b/packages/persona-kit/src/scopes.ts new file mode 100644 index 00000000..341dcfac --- /dev/null +++ b/packages/persona-kit/src/scopes.ts @@ -0,0 +1,217 @@ +import type { PersonaSpec } from './types.js'; + +/** + * Lint a persona's integration mount `scope` globs. + * + * ## Why this exists + * + * `scope` decides two things at once: which Relayfile paths the sandbox + * mirrors, and what the runtime token is allowed to touch. Both failure modes + * are SILENT at runtime — a scope the mount rejects produces an empty mirror, + * so reads come back empty and writebacks land on unmounted disk as no-ops. + * Nothing throws. The agent simply does nothing and reports success. + * + * Every rule below mirrors the authoritative validation in cloud's + * `relayfile/mount-intent.ts`, so a path this lint rejects is one the mount + * would also reject. The lint runs at deploy time, where the author can still + * fix it, rather than leaving them to infer it from an agent that quietly + * read nothing. + */ +export type ScopeLintLevel = 'warning'; + +/** + * Machine-readable issue category, so callers can branch on `issue.code` + * without parsing the human-readable `message`. + */ +export type ScopeLintCode = + | 'scope_empty' + | 'scope_not_absolute' + | 'scope_trailing_slash' + | 'scope_mid_path_wildcard' + | 'scope_traversal_segment' + | 'scope_provider_root' + | 'scope_high_cardinality_root'; + +export interface ScopeLintIssue { + level: ScopeLintLevel; + code: ScopeLintCode; + /** Provider slug the issue was raised under (`slack`, `linear`, …). */ + provider: string; + /** The scope key that was flagged (`channels`, `issues`, …). */ + resource: string; + /** Field-pointed location, e.g. `integrations.slack.scope.channels`. */ + path: string; + message: string; +} + +/** + * Collections that grow with workspace HISTORY rather than with configuration, + * so their size is unbounded from the persona author's point of view. + * + * This is a heuristic and deliberately short. It exists because the failure it + * predicts is not otherwise visible in the glob: `/slack/channels/**` is + * syntactically identical to `/linear/projects/**`, but one is a handful of + * entries and the other was ~5,950 (2,008 files / 3,940 directories) in a real + * workspace — enough that the boot mirror could not traverse it inside its + * budget, leaving the agent running degraded and later failing outright. + * + * Being on this list is not an error. A read-heavy agent may genuinely need + * the whole collection; the warning asks the author to confirm that, and points + * write-only agents at the much cheaper single-entry scope. + */ +const HIGH_CARDINALITY_ROOTS = new Set([ + 'slack/channels', + 'google-mail/messages', + 'google-mail/threads', + 'gmail/messages', + 'gmail/threads' +]); + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +/** + * Walk a persona's `integrations[].scope` globs and flag ones the runtime + * mount would reject or silently over-mirror. Always returns; never throws. + * + * The deploy CLI surfaces these as yellow warnings before deploy and continues + * regardless — same contract as {@link lintTriggers}. None of these are fatal: + * a persona whose scope is wrong still deploys, it just will not do what its + * author expects, and the warning is the only place that gets said out loud. + */ +export function lintScopes(persona: PersonaSpec): ScopeLintIssue[] { + const issues: ScopeLintIssue[] = []; + const integrations = persona.integrations; + if (!isRecord(integrations)) return issues; + + for (const [provider, config] of Object.entries(integrations)) { + if (!isRecord(config)) continue; + + // An integration declared with no scope at all is legitimate — a + // credential-only provider (an MCP server, say) has no VFS side. But an + // explicitly EMPTY scope object reads as "I meant to scope this" while + // mirroring nothing, which is the trap worth naming. + const scope = config.scope; + if (isRecord(scope) && Object.keys(scope).length === 0) { + issues.push({ + level: 'warning', + code: 'scope_empty', + provider, + resource: '', + path: `integrations.${provider}.scope`, + message: + `integrations.${provider}.scope is an empty object, which mirrors nothing. ` + + `Reads return empty and writebacks are silent no-ops. Either list the ` + + `concrete paths this agent touches, or omit \`scope\` entirely if the ` + + `provider has no Relayfile side.` + }); + continue; + } + if (!isRecord(scope)) continue; + + for (const [resource, raw] of Object.entries(scope)) { + if (typeof raw !== 'string') continue; + const path = `integrations.${provider}.scope.${resource}`; + const value = raw.trim(); + if (!value) continue; + + if (!value.startsWith('/')) { + issues.push({ + level: 'warning', + code: 'scope_not_absolute', + provider, + resource, + path, + message: `${path}: "${value}" is not an absolute Relayfile path; it must start with "/".` + }); + continue; + } + + if (value !== '/' && value.endsWith('/')) { + issues.push({ + level: 'warning', + code: 'scope_trailing_slash', + provider, + resource, + path, + message: `${path}: "${value}" has a trailing slash, which the mount rejects.` + }); + continue; + } + + // Only a TERMINAL `/**` is allowed. A wildcard anywhere else — the + // classic `/slack/*/messages` — is rejected by the mount and the agent + // silently sees an empty tree. + const withoutGlob = value.endsWith('/**') ? value.slice(0, -3) : value; + if (/[*?{}[\]]/u.test(withoutGlob)) { + issues.push({ + level: 'warning', + code: 'scope_mid_path_wildcard', + provider, + resource, + path, + message: + `${path}: "${value}" uses a wildcard outside a terminal "/**". The mount ` + + `accepts only a terminal "/**", so this scope mirrors nothing and the ` + + `agent will read an empty tree without error.` + }); + continue; + } + + const segments = withoutGlob.split('/').slice(1); + if (segments.some((s) => s === '' || s === '.' || s === '..')) { + issues.push({ + level: 'warning', + code: 'scope_traversal_segment', + provider, + resource, + path, + message: `${path}: "${value}" contains an empty or traversal ("."/"..") segment, which the mount rejects.` + }); + continue; + } + + // Valid, but it mirrors the provider's entire tree. This is a cost + // warning rather than a correctness one: the mirror is traversed at + // sandbox boot, so a collection that grows with workspace history (Slack + // channels being the usual one) can outgrow the mount budget and leave + // the agent running degraded — or, once a deployment cancels a + // non-converging mount, failing outright. + if (segments.length === 2 && value.endsWith('/**') && HIGH_CARDINALITY_ROOTS.has(segments.join('/'))) { + issues.push({ + level: 'warning', + code: 'scope_high_cardinality_root', + provider, + resource, + path, + message: + `${path}: "${value}" mirrors every entry under ${segments.join('/')}, which grows ` + + `with workspace history rather than with configuration. The mirror is traversed at ` + + `sandbox boot, so this can outrun the mount budget and leave the agent reading an ` + + `empty tree or failing to start. If the agent only WRITES here (posting a message, ` + + `say), scope the single entry it targets instead — the mirror is not needed for the ` + + `write, only the grant is.` + }); + continue; + } + + if (segments.length === 1 && value.endsWith('/**')) { + issues.push({ + level: 'warning', + code: 'scope_provider_root', + provider, + resource, + path, + message: + `${path}: "${value}" mirrors the whole ${provider} tree. The mirror is ` + + `traversed at sandbox boot, so this grows with workspace history and can ` + + `outrun the mount budget. Scope the concrete subpaths the handler reads ` + + `or writes back to instead.` + }); + } + } + } + + return issues; +} From b0c536824a33d8f441310de66cc50b82183f202e Mon Sep 17 00:00:00 2001 From: Ricky Schema Cascade Date: Sat, 15 Aug 2026 21:59:04 +0200 Subject: [PATCH 2/3] fix(persona-kit): stop linting scope values that are not paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review caught that `scope` is overloaded, and the first cut of this lint only knew about half of it. `PersonaIntegrationConfig` documents `scope` as "provider-specific filter metadata" — `{ repo: 'org/repo' }` for github, `{ database: '' }` for notion — while the cloud-persona docs use it for Relayfile mount globs. Both are real and both ship today. `scope_not_absolute` therefore fired on correct personas. `'org/repo'` is not a path missing its anchor, and nothing in the value tells the two apart, so the rule is removed and every remaining path rule now applies only to `/`-leading values. A warning channel works only while every warning is worth reading; this one stays silent where it cannot be sure. `scope_empty` is removed for the opposite reason — it could never fire. `parseIntegrationConfig` assigns `out.scope` only when the parsed map is non-empty, so an authored `scope: {}` is already `undefined` by the time deploy lints it, and indistinguishable from an omitted scope (which is legitimate: a credential-only provider like an MCP server has no Relayfile side). Shipping it advertised protection that did not exist. Two real gaps it missed are now covered, both from `parseStringMap` storing values verbatim: - `scope_empty_value` — `{ channels: '' }` survives parsing and reaches the mount, where it matches nothing under either interpretation. The old code skipped it via `if (!value) continue`. - `scope_untrimmed` — the lint checked `raw.trim()` while deploy forwards the original, so a padded path linted clean and then failed to match at the mount. It now lints the exact string deploy sends and flags the padding. Co-Authored-By: Claude Opus 5 --- packages/persona-kit/src/scopes.test.ts | 51 +++++++++++++--- packages/persona-kit/src/scopes.ts | 80 ++++++++++++++++--------- 2 files changed, 96 insertions(+), 35 deletions(-) diff --git a/packages/persona-kit/src/scopes.test.ts b/packages/persona-kit/src/scopes.test.ts index 89a6e983..377fb920 100644 --- a/packages/persona-kit/src/scopes.test.ts +++ b/packages/persona-kit/src/scopes.test.ts @@ -23,13 +23,42 @@ test('an integration with no scope at all is clean', () => { assert.deepEqual(lintScopes(persona({ 'supabase-mcp': {} })), []); }); -test('an explicitly empty scope object is flagged', () => { - // Distinct from omitting scope: `scope: {}` reads as "I scoped this" while - // mirroring nothing, so reads come back empty and writes no-op silently. - const issues = lintScopes(persona({ slack: { scope: {} } })); +test('provider filter metadata is left alone', () => { + // `scope` is overloaded: PersonaIntegrationConfig documents it as + // provider-specific filter metadata, and these are the documented examples. + // Treating them as malformed paths would warn on every github/notion/linear + // persona in the fleet, which is exactly how a warning channel dies. + const issues = lintScopes( + persona({ + github: { scope: { repo: 'org/repo' } }, + notion: { scope: { database: 'abc123' } }, + linear: { scope: { team: 'ENG' } } + }) + ); + assert.deepEqual(issues, []); +}); + +test('an empty scope object is not flagged — the parser drops it first', () => { + // parseIntegrationConfig only assigns out.scope when the parsed map is + // non-empty, so by deploy time `scope: {}` is indistinguishable from an + // omitted scope. Warning here would advertise protection that does not exist. + assert.deepEqual(lintScopes(persona({ slack: { scope: {} } })), []); +}); + +test('an empty scope VALUE is flagged', () => { + // Unlike `scope: {}`, an empty string survives parseStringMap verbatim and + // reaches the mount, where it matches nothing under either interpretation. + const issues = lintScopes(persona({ slack: { scope: { channels: ' ' } } })); assert.equal(issues.length, 1); - assert.equal(issues[0].code, 'scope_empty'); - assert.equal(issues[0].provider, 'slack'); + assert.equal(issues[0].code, 'scope_empty_value'); +}); + +test('a padded path is flagged, not silently accepted', () => { + // parseStringMap does not trim, so deploy forwards the padded string. Linting + // a trimmed copy would pass a value the mount then fails to match. + const issues = lintScopes(persona({ slack: { scope: { c: ' /slack/channels/C1/** ' } } })); + assert.equal(issues.length, 1); + assert.equal(issues[0].code, 'scope_untrimmed'); }); test('a mid-path wildcard is flagged — the mount rejects it silently', () => { @@ -49,9 +78,8 @@ test('a provider-root mirror is flagged as a cost, not an error', () => { assert.match(issues[0].message, /mirrors the whole slack tree/u); }); -test('non-absolute, trailing-slash and traversal scopes are flagged', () => { +test('trailing-slash and traversal paths are flagged', () => { const cases: [string, string][] = [ - ['slack/channels/**', 'scope_not_absolute'], ['/slack/channels/', 'scope_trailing_slash'], ['/slack/../linear/issues/**', 'scope_traversal_segment'] ]; @@ -62,6 +90,13 @@ test('non-absolute, trailing-slash and traversal scopes are flagged', () => { } }); +test('a path missing its leading slash is NOT flagged', () => { + // Ambiguous by construction: `slack/channels/**` may be a forgotten anchor or + // deliberate filter metadata, and nothing in the value distinguishes them. + // The lint stays silent rather than guess — see `provider filter metadata`. + assert.deepEqual(lintScopes(persona({ slack: { scope: { x: 'slack/channels/**' } } })), []); +}); + test('never throws on malformed personas', () => { assert.deepEqual(lintScopes({ id: 'x' } as unknown as PersonaSpec), []); assert.deepEqual(lintScopes(persona({ slack: 'nope' as unknown as object })), []); diff --git a/packages/persona-kit/src/scopes.ts b/packages/persona-kit/src/scopes.ts index 341dcfac..360106e1 100644 --- a/packages/persona-kit/src/scopes.ts +++ b/packages/persona-kit/src/scopes.ts @@ -11,11 +11,25 @@ import type { PersonaSpec } from './types.js'; * so reads come back empty and writebacks land on unmounted disk as no-ops. * Nothing throws. The agent simply does nothing and reports success. * - * Every rule below mirrors the authoritative validation in cloud's + * Every path rule below mirrors the authoritative validation in cloud's * `relayfile/mount-intent.ts`, so a path this lint rejects is one the mount * would also reject. The lint runs at deploy time, where the author can still * fix it, rather than leaving them to infer it from an agent that quietly * read nothing. + * + * ## Why only `/`-leading values are checked + * + * `scope` is overloaded. {@link PersonaIntegrationConfig} documents it as + * "provider-specific filter metadata" — `{ repo: 'org/repo' }` for github, + * `{ database: '' }` for notion, `{ team: 'ENG' }` for linear — while the + * cloud-persona docs use it for Relayfile mount globs. Both are real and in use. + * + * A value that does not start with `/` is therefore filter metadata, not a + * malformed path, and this lint says nothing about it. There is no way to tell + * `'org/repo'` (correct) from a path someone forgot to anchor, and guessing + * wrong would fire a warning on every github and notion persona in the fleet. + * A warning channel only works while every warning is worth reading, so this + * one stays silent where it cannot be sure. */ export type ScopeLintLevel = 'warning'; @@ -24,8 +38,8 @@ export type ScopeLintLevel = 'warning'; * without parsing the human-readable `message`. */ export type ScopeLintCode = - | 'scope_empty' - | 'scope_not_absolute' + | 'scope_empty_value' + | 'scope_untrimmed' | 'scope_trailing_slash' | 'scope_mid_path_wildcard' | 'scope_traversal_segment' @@ -88,46 +102,58 @@ export function lintScopes(persona: PersonaSpec): ScopeLintIssue[] { for (const [provider, config] of Object.entries(integrations)) { if (!isRecord(config)) continue; - // An integration declared with no scope at all is legitimate — a - // credential-only provider (an MCP server, say) has no VFS side. But an - // explicitly EMPTY scope object reads as "I meant to scope this" while - // mirroring nothing, which is the trap worth naming. + // An empty `scope: {}` is deliberately NOT flagged: `parseIntegrationConfig` + // drops it before the spec ever reaches this lint (it only assigns + // `out.scope` when the parsed map is non-empty), so by deploy time it is + // indistinguishable from an omitted scope — which is itself legitimate, as a + // credential-only provider like an MCP server has no Relayfile side at all. + // There is no signal here to warn on. const scope = config.scope; - if (isRecord(scope) && Object.keys(scope).length === 0) { - issues.push({ - level: 'warning', - code: 'scope_empty', - provider, - resource: '', - path: `integrations.${provider}.scope`, - message: - `integrations.${provider}.scope is an empty object, which mirrors nothing. ` + - `Reads return empty and writebacks are silent no-ops. Either list the ` + - `concrete paths this agent touches, or omit \`scope\` entirely if the ` + - `provider has no Relayfile side.` - }); - continue; - } if (!isRecord(scope)) continue; for (const [resource, raw] of Object.entries(scope)) { if (typeof raw !== 'string') continue; const path = `integrations.${provider}.scope.${resource}`; - const value = raw.trim(); - if (!value) continue; - if (!value.startsWith('/')) { + // Lint the value DEPLOY sees, not a cleaned-up copy of it. `parseStringMap` + // stores scope values verbatim — no trimming — so checking `raw.trim()` + // here would clear a padded path that then fails at the mount. + const value = raw; + + if (!value.trim()) { issues.push({ level: 'warning', - code: 'scope_not_absolute', + code: 'scope_empty_value', provider, resource, path, - message: `${path}: "${value}" is not an absolute Relayfile path; it must start with "/".` + message: + `${path} is empty, which mirrors nothing and filters nothing. Reads come ` + + `back empty and writebacks are silent no-ops. Give it the concrete path or ` + + `filter this agent needs, or drop the key.` }); continue; } + if (value !== value.trim()) { + issues.push({ + level: 'warning', + code: 'scope_untrimmed', + provider, + resource, + path, + message: + `${path}: "${value}" has leading or trailing whitespace, which is stored and ` + + `forwarded verbatim — the mount sees the padded string and will not match.` + }); + continue; + } + + // Everything below is a PATH rule. A value that does not start with "/" is + // provider filter metadata (`{ repo: 'org/repo' }`), not a malformed path, + // and is left alone — see the note on this module. + if (!value.startsWith('/')) continue; + if (value !== '/' && value.endsWith('/')) { issues.push({ level: 'warning', From 159cacf48065c9a402c9cdc4e7a0dd26629436a6 Mon Sep 17 00:00:00 2001 From: Ricky Schema Cascade Date: Sat, 15 Aug 2026 22:22:32 +0200 Subject: [PATCH 3/3] fix(persona-kit): don't flag a collection cloud already narrows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Applying this lint to the fleet it was written for showed it warning on the CORRECT configuration. Cloud rewrites a picker-gated collection scope down to the single record the deploy input resolves to — `/slack/channels/**` becomes `/slack/channels//**` for reads and writebacks alike (`persona-deploy.ts`, `pickerTargetPath`). It fires when the integration carries `enabledByInput` and the named input carries a `picker` whose provider and resource match the scope being narrowed. That arrangement is the right answer, and strictly better than the alternative the warning was nudging people toward: hard-coding a channel id in the scope pins one channel and takes the choice away from the operator, because scope does not interpolate inputs. Flagging it would have pushed authors from the good fix to the worse one. `isPickerNarrowed` mirrors cloud's own matching (lowercased provider, resource stripped of surrounding slashes) so the lint stays silent exactly where cloud acts. A gate WITHOUT a matching picker still warns: `enabledByInput` alone narrows nothing, so the broad mount is still paid. Verified against the seven watchdog personas: Slack is now clean on all of them, and the lint still surfaces the two costs nobody has addressed — `/google-mail/{messages,threads}/**` on six agents, and a `/github/**` provider root on meeting-actions. Co-Authored-By: Claude Opus 5 --- packages/persona-kit/src/scopes.test.ts | 34 ++++++++++++++++++ packages/persona-kit/src/scopes.ts | 47 ++++++++++++++++++++++++- 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/packages/persona-kit/src/scopes.test.ts b/packages/persona-kit/src/scopes.test.ts index 377fb920..49ef7e38 100644 --- a/packages/persona-kit/src/scopes.test.ts +++ b/packages/persona-kit/src/scopes.test.ts @@ -122,6 +122,40 @@ test('a history-sized collection wildcard IS flagged', () => { assert.match(issues[0].message, /only WRITES here/u); }); +test('a picker-gated collection is not flagged — cloud narrows it at deploy', () => { + // The supported fix, and the one that keeps the channel choice with the + // operator: cloud rewrites this to /slack/channels//** when the + // integration is gated by an input carrying a matching picker. Warning here + // would flag the correct configuration. + const spec = { + id: 'x', + integrations: { + slack: { + optional: true, + enabledByInput: 'SLACK_CHANNEL', + scope: { channels: '/slack/channels/**' } + } + }, + inputs: { SLACK_CHANNEL: { picker: { provider: 'slack', resource: 'channels' } } } + } as unknown as PersonaSpec; + assert.deepEqual(lintScopes(spec), []); +}); + +test('a gate without a matching picker is still flagged', () => { + // The rewrite is picker-driven; `enabledByInput` alone does not narrow + // anything, so the broad mount is still paid and still worth warning about. + const spec = { + id: 'x', + integrations: { + slack: { optional: true, enabledByInput: 'SLACK_CHANNEL', scope: { channels: '/slack/channels/**' } } + }, + inputs: { SLACK_CHANNEL: { description: 'no picker' } } + } as unknown as PersonaSpec; + const issues = lintScopes(spec); + assert.equal(issues.length, 1); + assert.equal(issues[0].code, 'scope_high_cardinality_root'); +}); + test('a single entry under a history-sized collection is clean', () => { // The fix the warning above points at: scoping the one channel the agent // posts to must not itself warn, or the advice is unfollowable. diff --git a/packages/persona-kit/src/scopes.ts b/packages/persona-kit/src/scopes.ts index 360106e1..2722df12 100644 --- a/packages/persona-kit/src/scopes.ts +++ b/packages/persona-kit/src/scopes.ts @@ -85,6 +85,46 @@ function isRecord(value: unknown): value is Record { return typeof value === 'object' && value !== null && !Array.isArray(value); } +/** + * Is this collection scope already narrowed for us at deploy time? + * + * Cloud rewrites a picker-gated collection scope down to the single record the + * deploy input resolves to — `/slack/channels/**` becomes + * `/slack/channels//**` for both reads and writebacks (cloud's + * `persona-deploy.ts`, `pickerTargetPath`). It fires only when the integration + * carries `enabledByInput` AND the named input carries a `picker` whose + * `provider` matches this integration and whose `resource` matches the + * collection segment being scoped. + * + * Warning on that arrangement would flag the CORRECT configuration — the one + * that preserves the operator's choice of channel instead of hard-coding an id — + * which is worse than staying quiet, so it is excluded here. + */ +function isPickerNarrowed( + persona: PersonaSpec, + provider: string, + config: Record, + collection: string +): boolean { + const gate = config.enabledByInput; + if (typeof gate !== 'string' || !gate.trim()) return false; + + const inputs = (persona as { inputs?: unknown }).inputs; + if (!isRecord(inputs)) return false; + const spec = inputs[gate.trim()]; + if (!isRecord(spec)) return false; + const picker = spec.picker; + if (!isRecord(picker)) return false; + + // Compared the same way cloud does: lowercased provider, and the resource + // stripped of surrounding slashes. + const pickerProvider = + typeof picker.provider === 'string' ? picker.provider.trim().toLowerCase() : ''; + const pickerResource = + typeof picker.resource === 'string' ? picker.resource.trim().replace(/^\/+|\/+$/gu, '') : ''; + return pickerProvider === provider.trim().toLowerCase() && pickerResource === collection; +} + /** * Walk a persona's `integrations[].scope` globs and flag ones the runtime * mount would reject or silently over-mirror. Always returns; never throws. @@ -204,7 +244,12 @@ export function lintScopes(persona: PersonaSpec): ScopeLintIssue[] { // channels being the usual one) can outgrow the mount budget and leave // the agent running degraded — or, once a deployment cancels a // non-converging mount, failing outright. - if (segments.length === 2 && value.endsWith('/**') && HIGH_CARDINALITY_ROOTS.has(segments.join('/'))) { + if ( + segments.length === 2 && + value.endsWith('/**') && + HIGH_CARDINALITY_ROOTS.has(segments.join('/')) && + !isPickerNarrowed(persona, provider, config, segments[1]) + ) { issues.push({ level: 'warning', code: 'scope_high_cardinality_root',