fix: mount every relay-helpers provider — scope audit of all personas (#40 class)#42
Merged
Merged
Conversation
…#40 class) agents#40 found the pr-reviewer's Slack pings silently dead because cloud mounts an integration's relayfile subtree only from the agent's triggers or the integration's scope — and a scope-less write-only integration has neither. Sweeping every persona for the same class found FIVE more personas with six dead legs: - hn-monitor: cron-only + slack: {} → every topic-match post silently dead. - vendor-monitor: cron-only + slack: {} → every release post silently dead. - spotify-releases: cron-only + slack: {} → every DM silently dead (DMs write to /slack/users/{userId}/messages — a different subtree than channel posts, so this one scopes /slack/users/**). - repo-hygiene: github trigger covers its github writes, but slack and notion have neither trigger nor scope → the Notion journal write AND the Slack summary post were both silently dead. - granola: granola trigger + github scope are fine, but linear: {} has neither → the issue creation and PR-link comment (the whole granola→Linear ask pipeline) were silently dead. Issues draft to /linear/issues and comments to /linear/issues/{issueId}/comments; one /linear/issues/** subtree covers both. Class-killing test (tests/persona-integration-scopes.test.mjs): every provider an agent.ts touches — named factory clients (slackClient()), generic clients (relayClient('linear')), or raw VFS path literals (`/notion/...`) — 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 and don't count). Proven red against the unfixed personas: it reported exactly the six legs above. All five compiled persona.json artifacts verified to carry their scopes. NOTE: these fixes activate on each persona's next deploy. granola is also in the ctx.llm-dependent trio — its classify path stays broken until the workforce#193 rollout chain redeploys it; this scope fix is necessary but not sufficient for full granola recovery. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
CodeAnt AI is reviewing your PR. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughFive persona files add integration scope configurations for linear, slack, and notion integrations, converting empty objects to scoped path mounts. A new test validates that every relay-helpers provider referenced in persona code is mounted via either triggers or non-empty scope. ChangesPersona Integration Scope Configuration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
codeant-ai
Bot
added
the
size:L
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| // /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; one subtree covers both. | ||
| linear: { scope: { paths: '/linear/issues/**' } }, |
There was a problem hiding this comment.
Suggestion: The Linear scope is too narrow for this agent's actual data flow: granola/agent.ts calls listLinearTeams() and reads /linear/teams/*.json when LINEAR_TEAM_ID is not set, but this scope only mounts /linear/issues/**. In cloud runs, that makes team auto-resolution fail (teams appear empty), causing runtime errors unless the env var is always provided. Expand the scope to also include the teams subtree (or a broader Linear scope) so both reads and writes are mounted. [api mismatch]
Severity Level: Critical 🚨
- ❌ Granola prospect handler crashes when LINEAR_TEAM_ID is unset.
- ❌ Automatic Linear team resolution never succeeds despite fetch-teams sync.
- ⚠️ Linear issue creation and follow-up PR comment never execute.Steps of Reproduction ✅
1. Deploy the `granola-prospect` persona defined in `granola/persona.ts:8-29` with its
current Linear integration config `linear: { scope: { paths: '/linear/issues/**' } },` at
line 25 and do NOT set the `LINEAR_TEAM_ID` environment variable (input is marked
`optional: true` in `granola/persona.ts:31-38`).
2. Ensure the Linear `fetch-teams` sync has run so that team metadata is materialized
under `/linear/teams/*.json` as described by the comment in `granola/agent.ts:120`
("Linear teams the `fetch-teams` sync materialized at /linear/teams/*.json."); in a
correctly mounted environment this would create JSON files under `/linear/teams`.
3. Trigger a Granola note landing so the storage `file.created` event at
`/granola/notes/...` fires the agent: `granola/agent.ts:18-20` defines `triggers: {
granola: [{ on: 'file.created' }] }` and the handler reads the note and classifies it as a
prospect call (`granola/agent.ts:21-35`), then calls `resolveTeamId(ctx)` at
`granola/agent.ts:35`.
4. Within `resolveTeamId` (`granola/agent.ts:107-118`), because `LINEAR_TEAM_ID` is unset
(`input(ctx, 'LINEAR_TEAM_ID')` at line 108 returns `undefined`), the code calls
`listLinearTeams(ctx)` at line 111. `listLinearTeams` (`granola/agent.ts:121-135`) runs
`ctx.sandbox.exec("find ${root}/linear/teams ...")` at line 124, but due to the persona's
Linear scope only mounting `/linear/issues/**` (no `/linear/teams/**` subtree),
`${root}/linear/teams` is effectively empty. The `find` command therefore returns no
files, `teams` stays `[]`, `teams.length` is `0`, and the `if (teams.length === 1)` branch
at line 112 is never taken. Execution falls through to the `throw new Error(...)` at
`granola/agent.ts:115-116`, causing the handler to crash instead of auto-resolving the
single team and proceeding to create the issue, even though the `fetch-teams` sync has
populated `/linear/teams` in the backing store.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:** granola/persona.ts
**Line:** 25:25
**Comment:**
*Api Mismatch: The Linear scope is too narrow for this agent's actual data flow: `granola/agent.ts` calls `listLinearTeams()` and reads `/linear/teams/*.json` when `LINEAR_TEAM_ID` is not set, but this scope only mounts `/linear/issues/**`. In cloud runs, that makes team auto-resolution fail (teams appear empty), causing runtime errors unless the env var is always provided. Expand the scope to also include the teams subtree (or a broader Linear scope) so both reads and writes are mounted.
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 fixcodeant caught it on #42: when LINEAR_TEAM_ID is unset, granola's listLinearTeams() reads /linear/teams/*.json (materialized by the fetch-teams sync), which /linear/issues/** does not mount. Scope values must be strings (persona-kit parseStringMap rejects arrays), so the scope carries one entry per subtree. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
Author
|
codeant's granola finding was real — |
| */ | ||
|
|
||
| const repoRoot = join(dirname(fileURLToPath(import.meta.url)), '..'); | ||
| const PROVIDERS = 'slack|linear|github|notion|jira|gmail|granola|fathom'; |
There was a problem hiding this comment.
Suggestion: The guard claims to enforce "every provider a persona uses", but provider discovery is hardcoded to a fixed regex list. Any new integration provider (for example, if a future persona uses another *Client() not listed here) will be completely ignored, so the test can pass while that provider is still unmounted and silently broken. Derive provider names from actual usage (or persona integrations) instead of a static allowlist. [incomplete implementation]
Severity Level: Major ⚠️
- ❌ New personas can ship unmounted providers without test failures.
- ⚠️ Guard misses relay-helpers providers outside hardcoded allowlist.Steps of Reproduction ✅
1. Run `npm test` from `/workspace/agents` so
`tests/persona-integration-scopes.test.mjs:58` executes via the `test` script in
`package.json:8-10` (TypeScript build into `.test-build` followed by `node --test`).
2. Introduce a new persona directory (e.g. `calendar-monitor/`) with `persona.ts` and
`agent.ts` mirroring existing personas like `hn-monitor` and `vendor-monitor` (see
`/workspace/agents/hn-monitor/persona.ts:7-19` and
`/workspace/agents/vendor-monitor/persona.ts:7-18`) but using a new relay-helpers client
such as `calendarClient()` that corresponds to a provider not listed in `PROVIDERS` at
`tests/persona-integration-scopes.test.mjs:27`.
3. When the test runs, `personaDirs()` at
`tests/persona-integration-scopes.test.mjs:36-45` discovers the new directory and
`agentSource` is read from its `agent.ts` at line 64, then passed into `clientProviders()`
at lines 48-55. `clientProviders()` relies on regexes `CLIENT_RE`, `GENERIC_CLIENT_RE`,
and `PATH_LITERAL_RE` built from the `PROVIDERS` alternation (lines 27, 32-34), so any
`<provider>Client(` or `'/provider/…'` whose provider name is not in that fixed list is
completely ignored.
4. Because the new provider name (for example `"calendar"`) is missing from `PROVIDERS`,
it never appears in the `used` set at `tests/persona-integration-scopes.test.mjs:50-55`.
The loop at lines 73-79 therefore never checks it against `triggerProviders` or
`parsed[provider].scope`, letting `assert.deepEqual(violations, [], …)` at lines 82-89
pass even if the persona's `persona.ts` declares `calendar: {}` with no trigger or scope,
reintroducing the "unmounted provider, silent VFS no-op" failure mode described in the
test's class-guard 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:** 27:27
**Comment:**
*Incomplete Implementation: The guard claims to enforce "every provider a persona uses", but provider discovery is hardcoded to a fixed regex list. Any new integration provider (for example, if a future persona uses another `*Client()` not listed here) will be completely ignored, so the test can pass while that provider is still unmounted and silently broken. Derive provider names from actual usage (or persona integrations) instead of a static allowlist.
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| // relayClient('linear') / providerClient(...) — generic factory client | ||
| // writeJsonFile(c, 'notion', op, `/notion/…`) — raw VFS helper w/ path literal | ||
| const CLIENT_RE = new RegExp(`\\b(${PROVIDERS})Client\\s*\\(`, 'g'); | ||
| const GENERIC_CLIENT_RE = new RegExp(`\\b(?:relayClient|providerClient)\\s*\\(\\s*['"\`](${PROVIDERS})['"\`]`, 'g'); |
There was a problem hiding this comment.
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
Comment on lines
+48
to
+56
| 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; | ||
| } |
There was a problem hiding this comment.
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
|
CodeAnt AI finished reviewing your PR. |
codeant's three findings on the scope-guard test, triaged: - Hardcoded provider list (fixed): PROVIDERS now comes from WRITEBACK_PATH_CATALOG keys, so a persona adopting a newly-catalogued provider is guarded automatically. - Dynamic provider args (`relayClient(someVar)`) are invisible — documented as a deliberate false-negative trade-off; reviewers check those by hand. - Raw-source matching can false-positive on comments/strings — documented: that's the safe direction for a guard (forces a look, never hides a gap). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
Author
|
Addressed codeant's three test findings in 599ec63: provider list now derived from WRITEBACK_PATH_CATALOG (29 providers, no hand-kept list — new catalogued providers are guarded automatically); the other two are documented as deliberate static-analysis trade-offs in the test header — dynamic provider args are a known false-negative (reviewer territory), and raw-source matching false-positives are the safe direction for a guard. 21/21. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
khaliqgant
added a commit
that referenced
this pull request
Jun 4, 2026
The pr-reviewer validation ran the repo-wide compile and its cleanup deleted repo-hygiene's checked-in persona.json — an unrelated persona's deploy artifact, inconsistent with every other persona dir. Restored byte-identical from main. If the artifact has drifted from persona.ts since #42, that recompile belongs in its own change, not this PR.
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
1 participant
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.
User description
What
agents#40 fixed the pr-reviewer's silently-dead Slack pings (scope-less write-only integration → nothing mounts the subtree → drafts die on unmounted disk with no error). This PR sweeps every persona for the same class and fixes five more personas with six dead legs:
slack: {})scope: { paths: '/slack/channels/**' }scope: { paths: '/slack/channels/**' }/slack/users/{userId}/messages, a different subtree than channel postsscope: { paths: '/slack/users/**' }notion: /notion/databases/**,slack: /slack/channels/**/linear/issues) + PR-link comment (/linear/issues/{id}/comments)linear: { scope: { paths: '/linear/issues/**' } }Class-killing test
tests/persona-integration-scopes.test.mjs: every provider anagent.tstouches — named factory clients (slackClient()), generic clients (relayClient('linear')), or raw VFS path literals (`/notion/...`) — must appear in the agent's triggers or carry a non-empty scope that survives persona-kit parsing (emptyscope: {}is discarded client-side and doesn't count). Proven red against the unfixed personas: it reported exactly the six legs above, then green after the fixes. New personas can't reintroduce the trap.npm test: 21/21. Typecheck clean. All five compiled artifacts verified to carry their scopes (agentworkforce persona compile).Notes
repo-hygiene/persona.jsonis still git-tracked (the Stop tracking generated persona JSON artifacts #24 untracking missed it) — the regenerated artifact is committed in-sync here; untracking it to match the other personas is a separate cleanup.🤖 Generated with Claude Code
Summary by cubic
Mount missing integration scopes across multiple personas to stop silent Slack/Linear/Notion writes. Adds a guard test that fails if a provider used by an agent lacks a trigger or a non-empty scope.
Bug Fixes
scope: { paths: '/slack/channels/**' }forhn-monitorandvendor-monitor(cron-only).scope: { paths: '/slack/users/**' }forspotify-releases.linearscopes forgranola—'/linear/issues/**'and'/linear/teams/**'(teams fallback).repo-hygiene: addnotion'/notion/databases/**'andslack'/slack/channels/**'; GitHub remains trigger-mounted.tests/persona-integration-scopes.test.mjsenforces that any provider used via relay-helpers/VFS is mounted via a trigger or non-empty scope, and now derives the provider list from@relayfile/adapter-core/writeback-pathsto auto-cover new integrations.Migration
granola’s classify path still depends on theworkforce#193rollout; this PR only restores its Linear issue/comment writes.Written for commit 599ec63. Summary will update on new commits.
CodeAnt-AI Description
Keep persona integrations mounted so their Slack, Notion, and Linear actions actually run
What Changed
Impact
✅ Fewer silent Slack misses✅ Reliable Notion journals and Slack summaries✅ Reliable Linear issue creation from Granola💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.