Classify adapter lifecycle states in digests#97
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughI can’t generate the required hidden review artifact because it must list every provided rangeId exactly once and the PR includes too many ranges for me to reliably enumerate here. If you want, I can:
Which would you prefer? ✨ Finishing Touches🧪 Generate unit tests (beta)
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Comment on lines
+110
to
+111
| if (hasActionVerb(action, 'close|closed|complete|completed|resolve|resolved|done')) { | ||
| return 'was completed'; |
There was a problem hiding this comment.
🔴 Jira digest pastTense omits canceled terminal state, violating the digest contract rule
The AGENTS.md rule (lines 325-328) and .claude/rules/relayfile-integration-digests.md (lines 20-22) both require that terminal-state actions such as closed, merged, archived, completed, canceled, and resolved be explicitly classified and must not fall through to the generic "updated" line. The Jira pastTense function at packages/jira/src/digest.ts:110 classifies close|closed|complete|completed|resolve|resolved|done but omits cancel|canceled. Since Jira is a project management tool where issues can have a "Cancelled" status/resolution, canceled is a provider-relevant terminal state. When a canceled action arrives, it will fall through to return 'was updated' at line 113, producing a misleading digest bullet like issue ENG-42 was updated instead of issue ENG-42 was completed (or a more specific was canceled).
Suggested change
| if (hasActionVerb(action, 'close|closed|complete|completed|resolve|resolved|done')) { | |
| return 'was completed'; | |
| if (hasActionVerb(action, 'close|closed|complete|completed|resolve|resolved|done|cancel|canceled|cancelled')) { | |
| return 'was completed'; |
Was this helpful? React with 👍 or 👎 to provide feedback.
…ters Add digest.ts and digest.test.ts for airtable, asana, clickup, hubspot, intercom, pipedrive, salesforce, zendesk. Each digest handler classifies provider-specific lifecycle states (completed, merged, won/lost, solved, converted, archived, reopened) and exports deterministic bullets sorted by event time and id. Tests cover ordering, terminal states, and empty windows. Package index.ts files updated to re-export digest module. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add state-diff inference in 5 adapters so terminal lifecycle transitions are reflected in _webhook.action instead of being flattened to "updated": - asana: detect task completion via change.field === "completed" - clickup: detect task completion/archival via history_items + status type - pipedrive: detect deal won/lost via current vs previous status comparison - salesforce: detect case closure/lead conversion via ChangeEventHeader - zendesk: detect ticket solved via current vs previous status comparison This makes the digest handlers' terminal state classifications reachable from real webhook data, closing the gaps identified by lifecycle audit. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/asana/src/webhook-normalizer.ts (1)
390-417:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid inferring
completedfrom steady-state flags alone.On Line 414, any event for an already completed resource is coerced to
completed, which can override real actions (e.g., non-lifecycle edits) and misclassify downstream digest bullets.Proposed fix
function extractAsanaAction(event: AsanaRecord, payload: AsanaRecord): string { const change = getRecord(event.change); - const action = - inferAsanaLifecycleAction(event, payload) ?? - readOptionalString(event.action) ?? - readOptionalString(change?.action) ?? - readOptionalString(payload.action) ?? - 'changed'; + const explicitAction = + readOptionalString(event.action) ?? + readOptionalString(change?.action) ?? + readOptionalString(payload.action); + const action = + inferAsanaLifecycleAction(event, payload, explicitAction) ?? + explicitAction ?? + 'changed'; return normalizeAction(action); } -function inferAsanaLifecycleAction(event: AsanaRecord, payload: AsanaRecord): string | undefined { +function inferAsanaLifecycleAction( + event: AsanaRecord, + payload: AsanaRecord, + explicitAction?: string, +): string | undefined { const change = getRecord(event.change); const field = readOptionalString(change?.field)?.toLowerCase(); const newValue = change?.new_value ?? change?.newValue; const resource = getRecord(event.resource); const data = getRecord(payload.data); @@ ) { return 'completed'; } - if (resource?.completed === true || data?.completed === true || payload.completed === true) { + const explicit = explicitAction?.trim().toLowerCase(); + const isGeneric = !explicit || explicit === 'changed' || explicit === 'change' || explicit === 'updated' || explicit === 'update'; + if (!change && isGeneric && (resource?.completed === true || data?.completed === true || payload.completed === true)) { return 'completed'; } 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/asana/src/webhook-normalizer.ts` around lines 390 - 417, The current inferAsanaLifecycleAction function can return 'completed' based solely on steady-state flags (resource.completed, data.completed, payload.completed), which misclassifies non-lifecycle edits; update inferAsanaLifecycleAction so it only returns 'completed' when the change explicitly indicates completion (i.e., when change.field equals 'completed' and new_value/newValue is truthy), and remove the unconditional checks of resource?.completed, data?.completed, and payload.completed; adjust callers (extractAsanaAction) to rely on inferAsanaLifecycleAction for lifecycle moves only and keep the existing fallbacks otherwise.packages/github/src/digest.test.ts (1)
76-113: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd an explicit delete classification test to complete required digest coverage.
The suite now validates create/update/terminal/sorting/empty-window paths, but it still does not assert delete wording.
As per coding guidelines: `packages/**/*.test.ts`: Add digest tests beside adapter tests covering at least create/update, terminal state, delete, deterministic sorting, and empty-window behavior.➕ Proposed test addition
+test('digest classifies deleted events', async () => { + const ctx: DigestContext = { + provider: 'github', + window: { from: '2026-05-12T00:00:00.000Z', to: '2026-05-13T00:00:00.000Z' }, + async changeEvents() { + return [ + { + id: 'evt-del-1', + timestamp: '2026-05-12T10:00:00.000Z', + action: 'deleted', + canonicalPath: 'github/repos/acme/api/issues/45__cleanup/meta.json', + }, + ]; + }, + }; + + assert.deepEqual(await digest(ctx), { + provider: 'github', + bullets: [ + { + text: '#45 was deleted', + canonicalPath: 'github/repos/acme/api/issues/45__cleanup/meta.json', + }, + ], + }); +});🤖 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/github/src/digest.test.ts` around lines 76 - 113, Add a new test in packages/github/src/digest.test.ts that supplies a DigestContext (use the existing DigestContext shape and changeEvents async function) which returns a single change event with action 'deleted' (e.g., a canonicalPath for a pull or issue like 'github/repos/acme/api/pulls/44__ship-it/meta.json'), then call digest(ctx) and assert the result contains a bullet that uses the explicit delete wording your digest implementation expects (e.g., '#44 was deleted' or equivalent), mirroring the style of the existing merged/closed tests to cover the delete classification path.packages/gitlab/test/digest.test.ts (1)
81-134:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a GitLab delete lifecycle digest test.
This suite now covers create/update, terminal state, deterministic sort, and empty-window, but it still lacks an explicit delete case.
Proposed test addition
+test('digest classifies deleted GitLab records', async () => { + const ctx: DigestContext = { + provider: 'gitlab', + window: { from: '2026-05-12T00:00:00.000Z', to: '2026-05-13T00:00:00.000Z' }, + async changeEvents() { + return [ + { + id: 'evt-1', + timestamp: '2026-05-12T08:00:00.000Z', + action: 'deleted', + canonicalPath: 'gitlab/projects/acme/api/issues/99__cleanup/meta.json', + }, + ]; + }, + }; + + assert.deepEqual(await digest(ctx), { + provider: 'gitlab', + bullets: [ + { + text: 'issue `#99` was deleted', + canonicalPath: 'gitlab/projects/acme/api/issues/99__cleanup/meta.json', + }, + ], + }); +});As per coding guidelines
packages/**/*.test.ts: Add digest tests beside adapter tests covering at least create/update, terminal state, delete, deterministic sorting, and empty-window 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/gitlab/test/digest.test.ts` around lines 81 - 134, Add a new unit test in the same file that exercises the delete lifecycle case: create a DigestContext (use the existing DigestContext shape and changeEvents async function) that returns one change event with action 'deleted' (or 'delete' if consistent with your adapter), an appropriate timestamp and a canonicalPath (e.g., a resource under gitlab/projects/.../meta.json), then assert that digest(ctx) returns a bullet with the expected text indicating the resource was deleted and the same canonicalPath; place this test alongside the existing tests that reference digest and changeEvents so the suite covers create/update, terminal state, delete, deterministic sorting, and empty-window behavior.
🟡 Minor comments (11)
packages/azure-blob/src/digest.test.ts-6-85 (1)
6-85:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd an explicit update-classification digest test.
Current cases validate create/delete/terminal/determinism/empty-window, but there’s no assertion for update wording (e.g.,
was modified).As per coding guidelines:
packages/**/*.test.ts: Add digest tests beside adapter tests covering at least create/update, terminal state, delete, deterministic sorting, and empty-window 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/azure-blob/src/digest.test.ts` around lines 6 - 85, Add a new test in packages/azure-blob/src/digest.test.ts that exercises the "update" classification: create a DigestContext (same pattern as existing tests) with provider 'azure-blob', a window, and changeEvents returning a single event whose action represents an update (e.g., 'BlobUpdated' or the adapter's update action) and a canonicalPath; call await digest(ctx) and assert it returns the expected object with provider 'azure-blob' and a bullets array containing one entry whose text uses the update wording (e.g., "was modified") and matching canonicalPath; name the test clearly (e.g., "digest classifies Azure Blob updates") to satisfy coverage for create/update/terminal/delete/determinism/empty-window.packages/box/src/digest.test.ts-6-95 (1)
6-95:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd an explicit update-classification digest test.
These tests validate create/terminal/delete/determinism/empty-window, but they don’t assert update wording (
was modified) for non-terminal changes.As per coding guidelines:
packages/**/*.test.ts: Add digest tests beside adapter tests covering at least create/update, terminal state, delete, deterministic sorting, and empty-window 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/box/src/digest.test.ts` around lines 6 - 95, Add a test to cover update-classification by extending the existing digest tests: create a DigestContext with provider 'box' and changeEvents returning at least one event with action 'ITEM_UPDATE' (and a canonicalPath like 'box/acct/files/name__f123.json'), call await digest(ctx) and assert the result contains a bullet with text 'file name was modified' and the correct canonicalPath; place this alongside the other tests that use the digest function so it verifies update wording and integrates with existing determinism/empty-window tests.packages/calendly/src/digest.test.ts-6-83 (1)
6-83:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd an explicit
updateddigest assertion.The suite currently validates create/terminal/delete/deterministic/empty-window, but it does not assert an
updatedbullet classification.Suggested minimal test extension
return [ { id: 'evt-2', timestamp: '2026-05-12T09:00:00.000Z', action: 'canceled', canonicalPath: '/calendly/scheduled-events/abc-123.json', }, + { + id: 'evt-3', + timestamp: '2026-05-12T10:00:00.000Z', + action: 'invitee.updated', + canonicalPath: 'calendly/invitees/inv-789.json', + }, { id: 'evt-1', timestamp: '2026-05-12T08:00:00.000Z', action: 'invitee.created', canonicalPath: 'calendly/invitees/inv-456.json', }, ];bullets: [ { text: 'invitee inv-456 was created', canonicalPath: 'calendly/invitees/inv-456.json', }, { text: 'event abc-123 was canceled', canonicalPath: 'calendly/scheduled-events/abc-123.json', }, + { + text: 'invitee inv-789 was updated', + canonicalPath: 'calendly/invitees/inv-789.json', + }, ],As per coding guidelines
packages/**/*.test.ts: “Add digest tests beside adapter tests covering at least create/update, terminal state, delete, deterministic sorting, and empty-window 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/calendly/src/digest.test.ts` around lines 6 - 83, Add a new test case in packages/calendly/src/digest.test.ts that verifies the digest classifies "updated" events: construct a DigestContext (like the existing tests) whose async changeEvents returns an event with action set to an update variant (e.g., 'invitee.updated' or 'scheduled_event.updated'), include an appropriate timestamp and canonicalPath, call digest(ctx) and assert the returned object (provider and bullets) contains a bullet with text like "invitee <id> was updated" or "event <id> was updated" and the matching canonicalPath; mirror the style and deterministic checks used in the other tests (use DigestContext, changeEvents, digest, and assert.deepEqual).packages/intercom/src/digest.ts-102-119 (1)
102-119:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClassify more terminal actions before the generic fallback.
pastTensecurrently letscompleted,canceled/cancelled,resolved, andmergedfall through towas updated.Suggested patch
if (hasActionVerb(action, 'archive|archived')) { return 'was archived'; } + if (hasActionVerb(action, 'complete|completed|done')) { + return 'was completed'; + } + if (hasActionVerb(action, 'cancel|canceled|cancelled')) { + return 'was canceled'; + } + if (hasActionVerb(action, 'resolve|resolved')) { + return 'was resolved'; + } + if (hasActionVerb(action, 'merge|merged')) { + return 'was merged'; + } return 'was updated'; }As per coding guidelines
packages/*/src/digest.ts: “Classify lifecycle actions explicitly in digest handlers; terminal states likeclosed,merged,archived,completed,canceled,resolvedmust not fall through to generic 'updated'”.🤖 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/intercom/src/digest.ts` around lines 102 - 119, The pastTense function in packages/intercom/src/digest.ts currently falls back to "was updated" for several terminal actions; update pastTense (which takes a DigestChangeEvent and uses hasActionVerb) to explicitly check for additional terminal verbs before the final fallback—add checks for completed (e.g., 'complete|completed') returning 'was completed', canceled variants ('cancel|canceled|cancelled') returning 'was canceled' (or 'was cancelled' per project spelling), resolved ('resolve|resolved') returning 'was resolved', and merged ('merge|merged') returning 'was merged', placing these if-blocks above the generic return so those events no longer fall through to "was updated".packages/intercom/src/digest.test.ts-6-97 (1)
6-97:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd an explicit
updateddigest assertion.Current tests cover create/terminal/delete/deterministic/empty-window, but they don’t assert an
updatedaction path/text mapping.Suggested test extension
return [ { id: 'evt-2', timestamp: '2026-05-12T09:00:00.000Z', action: 'closed', canonicalPath: '/intercom/conversations/conv-456.json', }, + { + id: 'evt-3', + timestamp: '2026-05-12T10:00:00.000Z', + action: 'updated', + canonicalPath: '/intercom/companies/co-999.json', + }, { id: 'evt-1', timestamp: '2026-05-12T08:00:00.000Z', action: 'created', canonicalPath: 'intercom/contacts/contact-123.json', @@ { text: 'conversation conv-456 was closed', canonicalPath: 'intercom/conversations/conv-456.json', }, + { + text: 'company co-999 was updated', + canonicalPath: 'intercom/companies/co-999.json', + }, ], });As per coding guidelines
packages/**/*.test.ts: “Add digest tests beside adapter tests covering at least create/update, terminal state, delete, deterministic sorting, and empty-window 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/intercom/src/digest.test.ts` around lines 6 - 97, The tests for digest are missing an explicit assertion for handling the "updated" action; add a new test (beside the existing ones) that constructs a DigestContext with provider: 'intercom' and changeEvents returning at least one event whose action is 'updated' (include canonicalPath like 'intercom/contacts/contact-456.json' and a timestamp/id), call await digest(ctx) and assert the returned bullets contain the expected text mapping (e.g., "contact contact-456 was updated") and canonicalPath; reference the digest function, DigestContext type, and changeEvents method when adding this case to satisfy the create/update requirement in the test suite.packages/jira/src/digest.test.ts-6-58 (1)
6-58:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd explicit
updatedanddeletedJira digest coverage.This file now checks deterministic ordering and empty windows, but it doesn’t explicitly assert mapping for
updatedanddeletedactions.Suggested test extension
return [ { id: 'evt-2', timestamp: '2026-05-12T09:00:00.000Z', action: 'done', canonicalPath: '/jira/issues/ENG-42__release-plan.json', }, + { + id: 'evt-3', + timestamp: '2026-05-12T10:00:00.000Z', + action: 'jira:issue_updated', + canonicalPath: '/jira/issues/ENG-43__follow-up.json', + }, + { + id: 'evt-4', + timestamp: '2026-05-12T11:00:00.000Z', + action: 'deleted', + canonicalPath: '/jira/issues/ENG-44__obsolete.json', + }, { id: 'evt-1', timestamp: '2026-05-12T08:00:00.000Z', action: 'jira:issue_created', canonicalPath: 'jira/issues/ENG-41__triage-login.json', @@ { text: 'issue ENG-42 was completed', canonicalPath: 'jira/issues/ENG-42__release-plan.json', }, + { + text: 'issue ENG-43 was updated', + canonicalPath: 'jira/issues/ENG-43__follow-up.json', + }, + { + text: 'issue ENG-44 was deleted', + canonicalPath: 'jira/issues/ENG-44__obsolete.json', + }, ], });As per coding guidelines
packages/**/*.test.ts: “Add digest tests beside adapter tests covering at least create/update, terminal state, delete, deterministic sorting, and empty-window 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/jira/src/digest.test.ts` around lines 6 - 58, Add tests that assert the digest mapping for Jira actions "updated" and "deleted": create new test(s) in this file using DigestContext and its changeEvents mock (like the existing tests) that return events with action: 'updated' and action: 'deleted' (include distinct ids/timestamps and canonicalPath values), call digest(ctx) and assert the returned bullets contain entries with the expected text mappings (e.g., "issue <KEY> was updated" and "issue <KEY> was deleted") and correct canonicalPath, and also verify deterministic ordering when multiple events are present; keep test shape consistent with tests referencing digest, DigestContext, and changeEvents.packages/mixpanel/src/digest.ts-55-59 (1)
55-59:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAccept
/mixpanelroot canonical paths in the filter.Events with
canonicalPath === '/mixpanel'are dropped, even though path normalization already supports leading slashes. This can silently skip valid root-level events.💡 Suggested fix
function hasCanonicalPath(event: DigestChangeEvent): event is DigestChangeEvent & { canonicalPath: string } { return ( typeof event.canonicalPath === 'string' - && (event.canonicalPath === 'mixpanel' || event.canonicalPath.startsWith('mixpanel/') || event.canonicalPath.startsWith('/mixpanel/')) + && ( + event.canonicalPath === 'mixpanel' + || event.canonicalPath === '/mixpanel' + || event.canonicalPath.startsWith('mixpanel/') + || event.canonicalPath.startsWith('/mixpanel/') + ) ); }🤖 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/mixpanel/src/digest.ts` around lines 55 - 59, The filter in hasCanonicalPath currently rejects the root path '/mixpanel' because it only checks for 'mixpanel' and startsWith('/mixpanel/'); update hasCanonicalPath to also accept event.canonicalPath === '/mixpanel' so root-level canonicalPath values with a leading slash are included; locate the function hasCanonicalPath and add the equality check for '/mixpanel' alongside the existing equality and startsWith checks for canonicalPath.packages/onedrive/src/digest.ts-55-59 (1)
55-59:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInclude
/onedrivein canonical-path acceptance.
canonicalPath === '/onedrive'is currently filtered out, which can drop legitimate root-level events.💡 Suggested fix
function hasCanonicalPath(event: DigestChangeEvent): event is DigestChangeEvent & { canonicalPath: string } { return ( typeof event.canonicalPath === 'string' - && (event.canonicalPath === 'onedrive' || event.canonicalPath.startsWith('onedrive/') || event.canonicalPath.startsWith('/onedrive/')) + && ( + event.canonicalPath === 'onedrive' + || event.canonicalPath === '/onedrive' + || event.canonicalPath.startsWith('onedrive/') + || event.canonicalPath.startsWith('/onedrive/') + ) ); }🤖 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/onedrive/src/digest.ts` around lines 55 - 59, The hasCanonicalPath type-guard in function hasCanonicalPath currently rejects the literal '/onedrive' because the checks only compare to 'onedrive' or use startsWith('onedrive/') or startsWith('/onedrive/'); update the condition to also accept event.canonicalPath === '/onedrive' (or alternately adjust the startsWith checks to allow both '/onedrive' and 'onedrive' prefixes) so root-level '/onedrive' events are treated as canonical; modify the predicate around event.canonicalPath to include this equality check while keeping the existing type-guard shape.packages/pipedrive/src/digest.test.ts-6-90 (1)
6-90:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a dedicated update-case digest assertion.
This suite validates create/terminal/delete/order/empty, but it does not pin update wording behavior, which leaves a contract gap.
💡 Suggested additional test
+test('digest maps updated actions to updated wording', async () => { + const ctx: DigestContext = { + provider: 'pipedrive', + window: { from: '2026-05-12T00:00:00.000Z', to: '2026-05-13T00:00:00.000Z' }, + async changeEvents() { + return [ + { + id: 'evt-3', + timestamp: '2026-05-12T10:00:00.000Z', + action: 'updated', + canonicalPath: 'pipedrive/deals/renewal__11.json', + }, + ]; + }, + }; + + assert.deepEqual(await digest(ctx), { + provider: 'pipedrive', + bullets: [ + { text: 'deal renewal was updated', canonicalPath: 'pipedrive/deals/renewal__11.json' }, + ], + }); +});As per coding guidelines
packages/**/*.test.ts: “Add digest tests beside adapter tests covering at least create/update, terminal state, delete, deterministic sorting, and empty-window 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/pipedrive/src/digest.test.ts` around lines 6 - 90, Add a new test in packages/pipedrive/src/digest.test.ts that exercises the update case: create a DigestContext with provider 'pipedrive', a window, and changeEvents returning an event with action 'updated' (unique id like 'evt-1'), a timestamp, and a canonicalPath (e.g., 'pipedrive/deals/renewal__42.json'); call digest(ctx) and assert the result is not null and includes a bullet whose text reads "deal renewal was updated" (and correct canonicalPath). Use the same patterns/methods as the existing tests (DigestContext type, async changeEvents, and digest function) to keep style and deterministic ordering consistent.packages/onedrive/src/digest.test.ts-48-85 (1)
48-85:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd an explicit update/fallback digest test case.
Current cases validate create/move/delete/empty, but there is no assertion for update wording (e.g., non-terminal action mapping to
was modified), so a key contract can regress unnoticed.💡 Suggested additional test
+test('digest maps non-terminal actions to modified wording', async () => { + const ctx: DigestContext = { + provider: 'onedrive', + window: { from: '2026-05-12T00:00:00.000Z', to: '2026-05-13T00:00:00.000Z' }, + async changeEvents() { + return [ + { + id: 'evt-3', + timestamp: '2026-05-12T10:00:00.000Z', + action: 'updated', + canonicalPath: 'onedrive/me/Documents/notes.txt', + }, + ]; + }, + }; + + assert.deepEqual(await digest(ctx), { + provider: 'onedrive', + bullets: [ + { + text: 'item Documents/notes.txt was modified', + canonicalPath: 'onedrive/me/Documents/notes.txt', + }, + ], + }); +});As per coding guidelines
packages/**/*.test.ts: “Add digest tests beside adapter tests covering at least create/update, terminal state, delete, deterministic sorting, and empty-window 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/onedrive/src/digest.test.ts` around lines 48 - 85, Add a new unit test in packages/onedrive/src/digest.test.ts that covers the "update/fallback" mapping: create a DigestContext (reusing DigestContext, provider, window, and changeEvents) that returns an event with action not mapped to a terminal verb (e.g., action: 'updated' or any non-terminal string) and a canonicalPath, then assert that await digest(ctx) returns a bullets entry whose text uses the fallback wording "was modified" (e.g., "item <path> was modified") and includes the canonicalPath; place this alongside the existing create/move/delete/empty tests to satisfy the create/update/terminal/delete/empty test requirements.packages/zendesk/src/digest.test.ts-6-46 (1)
6-46:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd explicit
updatedcoverage in deterministic digest test.This test currently validates
created+ terminal wording, but notupdated. Add oneupdatedassertion so the suite satisfies the required create/update coverage.Proposed patch
- action: 'solved', + action: 'updated', canonicalPath: '/zendesk/tickets/4567.json', @@ - text: 'ticket 4567 was solved', + text: 'ticket 4567 was updated', canonicalPath: 'zendesk/tickets/4567.json',As per coding guidelines:
packages/**/*.test.ts: Add digest tests beside adapter tests covering at least create/update, terminal state, delete, deterministic sorting, and empty-window 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/zendesk/src/digest.test.ts` around lines 6 - 46, The test for deterministic Zendesk digest lacks coverage for an "updated" event; update the mocked changeEvents in the DigestContext used by digest to include a third event with action 'updated' (e.g., id 'evt-3', timestamp between the other two or appropriately ordered) and then update the expected bullets in the deepEqual assertion to include the corresponding "ticket <id> was updated" bullet in the correct sorted position; ensure you keep the provider 'zendesk', the canonicalPath strings (e.g., 'zendesk/tickets/<id>.json'), and that both first and second digest() results are still asserted equal to preserve determinism.
🧹 Nitpick comments (6)
packages/airtable/src/digest.test.ts (1)
6-58: ⚡ Quick winAdd an explicit “updated” digest assertion.
Current cases cover create/delete/sorting/empty-window, but miss update wording coverage, which makes regressions in the fallback action text easy to miss.
Suggested test extension
return [ { id: 'evt-2', timestamp: '2026-05-12T09:00:00.000Z', action: 'deleted', canonicalPath: '/airtable/bases/appXYZ/tables/tblABC/records/rec456__invoice.json', }, + { + id: 'evt-3', + timestamp: '2026-05-12T10:00:00.000Z', + action: 'updated', + canonicalPath: '/airtable/bases/appXYZ/tables/tblABC/records/rec789__status.json', + }, { id: 'evt-1', timestamp: '2026-05-12T08:00:00.000Z', action: 'created', canonicalPath: 'airtable/bases/appXYZ/tables/tblABC/records/rec123__contact.json', }, ]; @@ { text: 'record rec456 was deleted', canonicalPath: 'airtable/bases/appXYZ/tables/tblABC/records/rec456__invoice.json', }, + { + text: 'record rec789 was updated', + canonicalPath: 'airtable/bases/appXYZ/tables/tblABC/records/rec789__status.json', + }, ], });As per coding guidelines:
packages/**/*.test.ts: Add digest tests beside adapter tests covering at least create/update, terminal state, delete, deterministic sorting, and empty-window behavior.packages/asana/src/digest.test.ts (1)
6-90: ⚡ Quick winAdd an explicit update-action case to lock update wording.
This suite validates create/terminal/delete/sorting/empty-window, but not a plain update path.
Suggested test addition
test('digest classifies terminal states distinctly', async () => { @@ }); +test('digest renders non-terminal changes as updated', async () => { + const ctx: DigestContext = { + provider: 'asana', + window: { from: '2026-05-12T00:00:00.000Z', to: '2026-05-13T00:00:00.000Z' }, + async changeEvents() { + return [ + { + id: 'evt-3', + timestamp: '2026-05-12T10:00:00.000Z', + action: 'changed', + canonicalPath: '/asana/tasks/rename-title__333.json', + }, + ]; + }, + }; + + assert.deepEqual(await digest(ctx), { + provider: 'asana', + bullets: [ + { text: 'task rename-title was updated', canonicalPath: 'asana/tasks/rename-title__333.json' }, + ], + }); +});As per coding guidelines:
packages/**/*.test.ts: Add digest tests beside adapter tests covering at least create/update, terminal state, delete, deterministic sorting, and empty-window behavior.packages/jira/src/digest.ts (1)
116-118: ⚡ Quick winRegExp construction is safe but could be more defensive.
The static analyzer correctly flags dynamic RegExp construction as a potential ReDoS vector. In this case, all call sites at lines 104, 107, and 110 pass static string literals, making the pattern safe. The regex itself uses simple alternation and negated character classes without nested quantifiers, so it's not vulnerable to catastrophic backtracking.
However, if this helper is ever called with untrusted input or moved to a shared utility, the risk resurfaces.
🛡️ Harden against future misuse
Consider either:
- Adding a comment documenting the safety assumption, or
- Refactoring to use static RegExp instances per verb set
Option 1 (lightweight):
function hasActionVerb(action: string, verbs: string): boolean { + // Safe: all call sites pass static string literals; pattern uses simple alternation return new RegExp(`(^|[^a-z0-9])(${verbs})([^a-z0-9]|$)`, 'u').test(action); }Option 2 (explicit):
+const CREATE_VERBS = /(^|[^a-z0-9])(create|created|open|opened|add|added|write|written)([^a-z0-9]|$)/u; +const DELETE_VERBS = /(^|[^a-z0-9])(delete|deleted|remove|removed)([^a-z0-9]|$)/u; +const COMPLETE_VERBS = /(^|[^a-z0-9])(close|closed|complete|completed|resolve|resolved|done)([^a-z0-9]|$)/u; + function pastTense(event: DigestChangeEvent): string { const action = (event.action ?? event.eventType ?? event.type ?? '').toLowerCase(); - if (hasActionVerb(action, 'create|created|open|opened|add|added|write|written')) { + if (CREATE_VERBS.test(action)) { return 'was created'; } - if (hasActionVerb(action, 'delete|deleted|remove|removed')) { + if (DELETE_VERBS.test(action)) { return 'was deleted'; } - if (hasActionVerb(action, 'close|closed|complete|completed|resolve|resolved|done')) { + if (COMPLETE_VERBS.test(action)) { return 'was completed'; } return 'was updated'; } - -function hasActionVerb(action: string, verbs: string): boolean { - return new RegExp(`(^|[^a-z0-9])(${verbs})([^a-z0-9]|$)`, 'u').test(action); -}🤖 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/jira/src/digest.ts` around lines 116 - 118, The helper hasActionVerb builds a dynamic RegExp from the verbs string which is currently safe because callers pass static literals, but to harden it add either a brief comment documenting that callers must pass only trusted/static verb lists, or (preferred) refactor hasActionVerb to precompile and cache RegExp objects per verb-set: validate/escape the verbs input, compute a stable cache key from the verbs parameter, store/reuse a RegExp instead of calling new RegExp each time, and keep the same function signature (hasActionVerb(action, verbs)) so callers are unchanged.packages/mailgun/src/digest.ts (1)
117-119: ⚡ Quick winRegExp construction is safe but could be more defensive.
Same pattern as
packages/jira/src/digest.ts. All call sites (lines 102, 105, 108, 111) pass static literals, and the regex is safe from catastrophic backtracking. The test at lines 97-115 indigest.test.tsverifies the word-boundary handling correctly distinguishes "undelivered" from "delivered".Consider the same hardening options as suggested for Jira's implementation: either add a safety comment or refactor to static RegExp constants.
🤖 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/mailgun/src/digest.ts` around lines 117 - 119, The dynamic RegExp creation in hasActionVerb(action: string, verbs: string) is safe for current usage but should be hardened: either add a concise comment above hasActionVerb explaining that all call sites supply static, validated literal verb lists (so the regex cannot be externally influenced or cause catastrophic backtracking), or refactor to precompiled/static RegExp constants (e.g., build a RegExp per verb-set once and reuse it) and update callers to use those constants; reference the hasActionVerb function and the callers that pass static literals when making the change.packages/segment/src/digest.ts (1)
117-119: ⚡ Quick winConsider precompiling regex patterns to address ReDoS warning.
The static analysis tool flagged regex construction from variable input. While current call sites use hardcoded literals, precompiling common patterns would eliminate the risk entirely and improve performance.
♻️ Proposed refactor
-function hasActionVerb(action: string, verbs: string): boolean { - return new RegExp(`(^|[^a-z0-9])(${verbs})([^a-z0-9]|$)`, 'u').test(action); -} +const CREATE_PATTERN = /(^|[^a-z0-9])(create|created|add|added|write|written)([^a-z0-9]|$)/u; +const UPSERT_PATTERN = /(^|[^a-z0-9])(upsert|upserted)([^a-z0-9]|$)/u; + +function hasActionVerb(action: string, pattern: RegExp): boolean { + return pattern.test(action); +}Then update call sites:
- if (hasActionVerb(action, 'create|created|add|added|write|written')) { + if (hasActionVerb(action, CREATE_PATTERN)) { return 'was created'; } - if (hasActionVerb(action, 'upsert|upserted')) { + if (hasActionVerb(action, UPSERT_PATTERN)) { return 'was upserted'; }🤖 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/segment/src/digest.ts` around lines 117 - 119, The hasActionVerb function currently constructs a RegExp from the verbs string at runtime which triggers a ReDoS warning; instead precompile regexes for known/common verb lists and look them up from a map (e.g., create a precompiledVerbsRegex Map keyed by the verbs literal and containing RegExp instances) and update hasActionVerb to use the precompiled RegExp (fall back to a safe, validated RegExp only if truly dynamic). This keeps the function name hasActionVerb, uses the precompiled map (e.g., precompiledVerbsRegex) to find the RegExp, and avoids building new regex objects from untrusted input at each call.packages/sendgrid/src/digest.ts (1)
117-119: ⚡ Quick winConsider precompiling regex patterns to address ReDoS warning.
The static analysis tool flagged regex construction from variable input. While current call sites use hardcoded literals, precompiling patterns eliminates the risk and improves performance.
♻️ Proposed refactor
-function hasActionVerb(action: string, verbs: string): boolean { - return new RegExp(`(^|[^a-z0-9])(${verbs})([^a-z0-9]|$)`, 'u').test(action); -} +const CREATE_PATTERN = /(^|[^a-z0-9])(create|created|add|added|write|written|process|processed)([^a-z0-9]|$)/u; +const DELIVER_PATTERN = /(^|[^a-z0-9])(deliver|delivered)([^a-z0-9]|$)/u; +const BOUNCE_PATTERN = /(^|[^a-z0-9])(bounce|bounced|drop|dropped)([^a-z0-9]|$)/u; +const DELETE_PATTERN = /(^|[^a-z0-9])(delete|deleted|remove|removed)([^a-z0-9]|$)/u; + +function hasActionVerb(action: string, pattern: RegExp): boolean { + return pattern.test(action); +}Then update call sites:
- if (hasActionVerb(action, 'create|created|add|added|write|written|process|processed')) { + if (hasActionVerb(action, CREATE_PATTERN)) { return 'was created'; } - if (hasActionVerb(action, 'deliver|delivered')) { + if (hasActionVerb(action, DELIVER_PATTERN)) { return 'was delivered'; } - if (hasActionVerb(action, 'bounce|bounced|drop|dropped')) { + if (hasActionVerb(action, BOUNCE_PATTERN)) { return 'was bounced'; } - if (hasActionVerb(action, 'delete|deleted|remove|removed')) { + if (hasActionVerb(action, DELETE_PATTERN)) { return 'was deleted'; }🤖 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/sendgrid/src/digest.ts` around lines 117 - 119, The hasActionVerb function currently constructs a RegExp from the verbs string on every call which risks ReDoS and wastes work; change this by precompiling and caching regex instances (e.g. a Map<string, RegExp> named regexCache) keyed by the verbs string and have hasActionVerb retrieve or build the RegExp once, then call .test(action) on the cached RegExp; also update any call sites that assume dynamic regex construction to continue passing the same verbs string so the cache is effective.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0dc892e3-19a9-4c31-80e5-8f06c10d8677
📒 Files selected for processing (128)
.claude/rules/relayfile-integration-digests.mdAGENTS.mdpackages/airtable/src/digest.test.tspackages/airtable/src/digest.tspackages/airtable/src/index.tspackages/asana/src/__tests__/webhook-normalizer.test.tspackages/asana/src/asana-adapter.tspackages/asana/src/digest.test.tspackages/asana/src/digest.tspackages/asana/src/index.tspackages/asana/src/webhook-normalizer.tspackages/azure-blob/src/digest.test.tspackages/azure-blob/src/digest.tspackages/azure-blob/src/index.tspackages/box/src/digest.test.tspackages/box/src/digest.tspackages/box/src/index.tspackages/calendly/src/__tests__/calendly-adapter.test.tspackages/calendly/src/calendly-adapter.tspackages/calendly/src/digest.test.tspackages/calendly/src/digest.tspackages/calendly/src/index.tspackages/clickup/src/clickup-adapter.tspackages/clickup/src/digest.test.tspackages/clickup/src/digest.tspackages/clickup/src/index.tspackages/confluence/src/__tests__/webhook-normalizer.test.tspackages/confluence/src/digest.test.tspackages/confluence/src/digest.tspackages/confluence/src/webhook-normalizer.tspackages/dropbox/src/digest.test.tspackages/dropbox/src/digest.tspackages/dropbox/src/index.tspackages/gcs/src/digest.test.tspackages/gcs/src/digest.tspackages/gcs/src/index.tspackages/github/src/__tests__/scaffold.test.tspackages/github/src/digest.test.tspackages/github/src/digest.tspackages/github/src/index.tspackages/gitlab/src/digest.tspackages/gitlab/test/digest.test.tspackages/gmail/src/digest.test.tspackages/gmail/src/digest.tspackages/gmail/src/index.tspackages/google-calendar/src/__tests__/digest.test.tspackages/google-calendar/src/__tests__/google-calendar-adapter.test.tspackages/google-calendar/src/digest.tspackages/google-calendar/src/index.tspackages/google-calendar/src/sync.tspackages/google-drive/src/digest.test.tspackages/google-drive/src/digest.tspackages/google-drive/src/index.tspackages/hubspot/src/digest.test.tspackages/hubspot/src/digest.tspackages/hubspot/src/index.tspackages/intercom/src/__tests__/intercom-adapter.test.tspackages/intercom/src/digest.test.tspackages/intercom/src/digest.tspackages/intercom/src/index.tspackages/intercom/src/intercom-adapter.tspackages/intercom/src/types.tspackages/jira/src/__tests__/webhook-normalizer.test.tspackages/jira/src/digest.test.tspackages/jira/src/digest.tspackages/jira/src/webhook-normalizer.tspackages/linear/src/digest.test.tspackages/linear/src/digest.tspackages/mailgun/src/digest.test.tspackages/mailgun/src/digest.tspackages/mailgun/src/index.tspackages/mixpanel/src/digest.test.tspackages/mixpanel/src/digest.tspackages/mixpanel/src/index.tspackages/notion/src/digest.test.tspackages/notion/src/digest.tspackages/onedrive/src/digest.test.tspackages/onedrive/src/digest.tspackages/onedrive/src/index.tspackages/pipedrive/src/digest.test.tspackages/pipedrive/src/digest.tspackages/pipedrive/src/index.tspackages/pipedrive/src/pipedrive-adapter.tspackages/postgres/src/digest.test.tspackages/postgres/src/digest.tspackages/postgres/src/index.tspackages/redis/src/digest.test.tspackages/redis/src/digest.tspackages/redis/src/index.tspackages/s3/src/digest.test.tspackages/s3/src/digest.tspackages/s3/src/index.tspackages/salesforce/src/__tests__/webhook-normalizer.test.tspackages/salesforce/src/digest.test.tspackages/salesforce/src/digest.tspackages/salesforce/src/index.tspackages/salesforce/src/salesforce-adapter.tspackages/salesforce/src/webhook-normalizer.tspackages/segment/src/digest.test.tspackages/segment/src/digest.tspackages/segment/src/index.tspackages/sendgrid/src/digest.test.tspackages/sendgrid/src/digest.tspackages/sendgrid/src/index.tspackages/sharepoint/src/digest.test.tspackages/sharepoint/src/digest.tspackages/sharepoint/src/index.tspackages/shopify/src/__tests__/shopify-adapter.test.tspackages/shopify/src/__tests__/webhook-normalizer.test.tspackages/shopify/src/digest.test.tspackages/shopify/src/digest.tspackages/shopify/src/index.tspackages/shopify/src/shopify-adapter.tspackages/shopify/src/webhook-normalizer.tspackages/slack/src/__tests__/emit-auxiliary-files.test.tspackages/slack/src/digest.test.tspackages/slack/src/digest.tspackages/slack/src/emit-auxiliary-files.tspackages/stripe/src/digest.test.tspackages/stripe/src/digest.tspackages/stripe/src/index.tspackages/teams/src/digest.tspackages/teams/src/index.tspackages/teams/test/digest.test.tspackages/zendesk/src/digest.test.tspackages/zendesk/src/digest.tspackages/zendesk/src/index.tspackages/zendesk/src/zendesk-adapter.ts
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
scripts/digest-layout-contracts.mjs (1)
103-106: ⚡ Quick winProvider filter check is too formatting-sensitive.
Using exact
.includes('ctx.changeEvents({ providers: [ctx.provider] })')will fail on equivalent formatting/style changes.Suggested fix
- if (!digestSource.includes('ctx.changeEvents({ providers: [ctx.provider] })')) { + if (!/ctx\.changeEvents\(\s*\{\s*providers:\s*\[\s*ctx\.provider\s*]\s*}\s*\)/u.test(digestSource)) { failures.push(`${provider}: digest must scope ctx.changeEvents to ctx.provider`); }🤖 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 `@scripts/digest-layout-contracts.mjs` around lines 103 - 106, The provider filter check is too sensitive to whitespace/formatting: instead of using digestSource.includes('ctx.changeEvents({ providers: [ctx.provider] })'), change the check to run a regex (or normalize whitespace) against digestSource to match ctx.changeEvents({...providers: [ctx.provider]...}) permissively; update the condition that pushes into failures (the code referencing digestSource, digestPath and failures) to use a regex test like one that allows arbitrary spacing around tokens (e.g., matching ctx.changeEvents\s*\(\s*{\s*providers\s*:\s*\[\s*ctx\.provider\s*\]\s*}\s*\)) so equivalent formatting/styling won’t cause false failures.
🤖 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 `@packages/mailgun/src/digest.ts`:
- Around line 55-60: The predicate hasCanonicalPath currently misses the root
'/mailgun' because it only checks for '/mailgun/' with a trailing slash; update
the condition in hasCanonicalPath (function name: hasCanonicalPath, type:
DigestChangeEvent) to accept the exact '/mailgun' string as well (or use
startsWith('/mailgun') instead of startsWith('/mailgun/')) so that canonicalPath
values of 'mailgun', '/mailgun', '/mailgun/...' and 'mailgun/...' are all
treated as valid.
In `@packages/s3/src/digest.ts`:
- Around line 55-59: The predicate hasCanonicalPath is excluding the exact '/s3'
value so such events are dropped; update the condition in hasCanonicalPath
(which checks event.canonicalPath) to also accept event.canonicalPath === '/s3'
(in addition to 's3', 's3/' prefix and '/s3/' prefix) so root-form '/s3' passes
the guard and proceeds to normalization.
In `@packages/segment/src/digest.ts`:
- Around line 60-64: hasCanonicalPath currently excludes the exact "/segment"
path because it only checks event.canonicalPath === 'segment' or
startsWith('segment/') or startsWith('/segment/'); update the predicate in
hasCanonicalPath to also accept event.canonicalPath === '/segment' (or simply
use startsWith('/segment') together with the existing 'segment' checks) so that
the root canonical path "/segment" passes the filter; reference the
hasCanonicalPath function and the DigestChangeEvent.canonicalPath property when
making this change.
In `@packages/sendgrid/src/digest.ts`:
- Around line 55-59: The type-guard hasCanonicalPath currently misses
canonicalPath === '/sendgrid' so root provider events are omitted; update the
predicate in hasCanonicalPath (DigestChangeEvent & { canonicalPath: string }) to
also allow exact '/sendgrid' in addition to 'sendgrid' and the two startsWith
checks (e.g., include canonicalPath === '/sendgrid' in the boolean expression)
so events with the root-form path are properly recognized.
In `@packages/slack/src/digest.ts`:
- Around line 55-59: The predicate hasCanonicalPath incorrectly excludes the
exact "/slack" path; update it to accept that case by either normalizing leading
slashes (e.g., strip a single leading '/') or explicitly checking
event.canonicalPath === '/slack' in addition to 'slack' and the startsWith
checks so that DigestChangeEvent values with canonicalPath '/slack' pass the
test; modify the hasCanonicalPath function and its conditional to include that
equality or normalization while keeping the type guard signature intact.
In `@scripts/digest-layout-contracts.mjs`:
- Around line 193-200: The function aliasesForResource can pick up an
aliasSegments from a later resource block because it searches the whole file
after finding the target path; change the search to restrict alias lookup to the
same resource block by finding the next "path:" occurrence after resourceIndex
(or end-of-file) and then searching for 'aliasSegments:' only between
resourceIndex and that nextPath index; update aliasesForResource to compute
nextResourceIndex = layoutSource.indexOf("path:", resourceIndex + 1) (or use
layoutSource.length if not found), then find aliasIndex =
layoutSource.indexOf('aliasSegments:', resourceIndex) and verify aliasIndex >= 0
&& aliasIndex < nextResourceIndex before extracting the aliasSegments with the
existing regex and matchAll logic.
---
Nitpick comments:
In `@scripts/digest-layout-contracts.mjs`:
- Around line 103-106: The provider filter check is too sensitive to
whitespace/formatting: instead of using
digestSource.includes('ctx.changeEvents({ providers: [ctx.provider] })'), change
the check to run a regex (or normalize whitespace) against digestSource to match
ctx.changeEvents({...providers: [ctx.provider]...}) permissively; update the
condition that pushes into failures (the code referencing digestSource,
digestPath and failures) to use a regex test like one that allows arbitrary
spacing around tokens (e.g., matching
ctx.changeEvents\s*\(\s*{\s*providers\s*:\s*\[\s*ctx\.provider\s*\]\s*}\s*\)) so
equivalent formatting/styling won’t cause false failures.
🪄 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: 4b38681f-a218-4232-ab42-f92ccfa5a112
📒 Files selected for processing (65)
.claude/rules/alias-subtrees.md.claude/rules/relayfile-integration-digests.md.trajectories/index.jsonAGENTS.mddocs/digest-layout-contract.mdpackage.jsonpackages/airtable/src/digest.test.tspackages/airtable/src/digest.tspackages/asana/src/__tests__/asana-adapter.test.tspackages/asana/src/__tests__/webhook-normalizer.test.tspackages/asana/src/asana-adapter.tspackages/asana/src/digest.test.tspackages/asana/src/webhook-normalizer.tspackages/azure-blob/src/digest.test.tspackages/box/src/digest.test.tspackages/calendly/src/digest.test.tspackages/confluence/src/digest.test.tspackages/confluence/src/digest.tspackages/dropbox/src/digest.test.tspackages/gcs/src/digest.test.tspackages/github/src/__tests__/emit-auxiliary-files.test.tspackages/github/src/__tests__/path-mapper.test.tspackages/github/src/digest.test.tspackages/github/src/emit-auxiliary-files.tspackages/github/src/layout-prompt.tspackages/github/src/layout.test.tspackages/github/src/layout.tspackages/github/src/path-mapper.tspackages/gitlab/src/emit-auxiliary-files.tspackages/gitlab/src/index.tspackages/gitlab/src/layout-prompt.tspackages/gitlab/src/layout.tspackages/gitlab/src/path-mapper.tspackages/gitlab/test/digest.test.tspackages/gitlab/test/emit-auxiliary-files.test.tspackages/gitlab/test/layout.test.tspackages/gitlab/test/path-mapper.test.tspackages/hubspot/src/digest.test.tspackages/hubspot/src/digest.tspackages/intercom/src/digest.test.tspackages/intercom/src/digest.tspackages/jira/src/digest.test.tspackages/jira/src/digest.tspackages/mailgun/src/digest.tspackages/mixpanel/src/digest.test.tspackages/mixpanel/src/digest.tspackages/onedrive/src/digest.test.tspackages/onedrive/src/digest.tspackages/pipedrive/src/__tests__/pipedrive-adapter.test.tspackages/pipedrive/src/digest.test.tspackages/pipedrive/src/pipedrive-adapter.tspackages/postgres/src/digest.test.tspackages/redis/src/digest.test.tspackages/s3/src/digest.test.tspackages/s3/src/digest.tspackages/salesforce/src/digest.test.tspackages/segment/src/digest.tspackages/sendgrid/src/digest.tspackages/slack/src/digest.test.tspackages/slack/src/digest.tspackages/stripe/src/digest.test.tspackages/zendesk/src/__tests__/zendesk-adapter.test.tspackages/zendesk/src/digest.test.tspackages/zendesk/src/zendesk-adapter.tsscripts/digest-layout-contracts.mjs
✅ Files skipped from review due to trivial changes (11)
- .claude/rules/alias-subtrees.md
- packages/gitlab/src/layout-prompt.ts
- packages/github/src/layout-prompt.ts
- packages/github/src/layout.test.ts
- packages/pipedrive/src/tests/pipedrive-adapter.test.ts
- .trajectories/index.json
- packages/gitlab/src/layout.ts
- packages/asana/src/tests/asana-adapter.test.ts
- docs/digest-layout-contract.md
- packages/gitlab/src/index.ts
- AGENTS.md
🚧 Files skipped from review as they are similar to previous changes (16)
- packages/zendesk/src/digest.test.ts
- packages/zendesk/src/zendesk-adapter.ts
- .claude/rules/relayfile-integration-digests.md
- packages/pipedrive/src/pipedrive-adapter.ts
- packages/redis/src/digest.test.ts
- packages/mixpanel/src/digest.test.ts
- packages/stripe/src/digest.test.ts
- packages/slack/src/digest.test.ts
- packages/postgres/src/digest.test.ts
- packages/hubspot/src/digest.test.ts
- packages/jira/src/digest.test.ts
- packages/asana/src/digest.test.ts
- packages/confluence/src/digest.test.ts
- packages/calendly/src/digest.test.ts
- packages/s3/src/digest.test.ts
- packages/gitlab/test/digest.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/jira/src/__tests__/path-mapper.test.ts (1)
82-91: 💤 Low valueMisplaced test: by-creator and by-priority assertions nested under
jiraIssueByAssigneeAliasPathdescribe block.This test is logically independent of the assignee alias and should be in its own describe block or at the top level alongside the other alias tests for consistency.
Additionally, the coding guidelines require round-trip tests for every path-mapper helper. Consider adding round-trip tests for
jiraIssueByCreatorAliasPathandjiraIssueByPriorityPathsimilar to the existingjiraIssueByAssigneeAliasPathround-trip test at lines 32-42.🤖 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/jira/src/__tests__/path-mapper.test.ts` around lines 82 - 91, The two assertions for jiraIssueByCreatorAliasPath and jiraIssueByPriorityPath are misplaced inside the jiraIssueByAssigneeAliasPath describe block; move them into their own describe block (or top-level sibling) so they are logically independent, and add round-trip tests for jiraIssueByCreatorAliasPath and jiraIssueByPriorityPath analogous to the existing jiraIssueByAssigneeAliasPath round-trip test (call the path function with an alias and id and assert the inverse mapper/expectation produces the same components).packages/gitlab/src/path-mapper.ts (1)
147-189: 💤 Low valueConsider adding non-empty validation for alias parameters.
The Jira and Linear path-mapper helpers validate that input segments are non-empty before constructing paths (e.g.,
assertNonEmptySegment). The new GitLab alias helpers don't have this validation, which could silently produce malformed paths if called with empty strings.For consistency and defensive programming, consider adding validation:
export function gitLabByStateAliasPath( projectPath: string, objectType: GitLabStatefulResourceType, state: string, objectId: number | string, ): string { + if (!state.trim()) { + throw new Error('state must be a non-empty string'); + } return `${gitLabProjectPrefix(projectPath)}/${objectType}/by-state/${encodeGitLabPathSegment( slugifyAlias(state), )}/${encodeGitLabPathSegment(String(objectId))}.json`; }Similar validation could be added to the other three helpers.
🤖 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/gitlab/src/path-mapper.ts` around lines 147 - 189, The new GitLab alias path helpers (gitLabByStateAliasPath, gitLabByAssigneeAliasPath, gitLabByCreatorAliasPath, gitLabByPriorityAliasPath) lack non-empty validation for their alias parameters; update each function to call the shared assertNonEmptySegment (or equivalent existing validator) on the alias argument (state, assignee, creator, priority) before passing it to slugifyAlias/encodeGitLabPathSegment, and add any missing import for assertNonEmptySegment so empty strings are rejected and malformed paths cannot be produced.packages/jira/src/emit-auxiliary-files.ts (1)
301-319: 💤 Low valueMinor code duplication in safeFields handling.
The safeFields extraction block is duplicated between lines 301-307 and 308-319. Consider consolidating:
♻️ Suggested refactor
- if (assigneeAccountId) { - const safeFields = isRecord(safe.fields) - ? safe.fields - : ({} as Record<string, unknown>); - safeFields.assigneeAccountId = assigneeAccountId; - safe.fields = safeFields; - } - if (creatorAccountId || priorityName) { - const safeFields = isRecord(safe.fields) - ? safe.fields - : ({} as Record<string, unknown>); - if (creatorAccountId) { - safeFields.creatorAccountId = creatorAccountId; - } - if (priorityName) { - safeFields.priorityName = priorityName; - } - safe.fields = safeFields; - } + if (assigneeAccountId || creatorAccountId || priorityName) { + const safeFields = isRecord(safe.fields) + ? safe.fields + : ({} as Record<string, unknown>); + if (assigneeAccountId) { + safeFields.assigneeAccountId = assigneeAccountId; + } + if (creatorAccountId) { + safeFields.creatorAccountId = creatorAccountId; + } + if (priorityName) { + safeFields.priorityName = priorityName; + } + safe.fields = safeFields; + }🤖 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/jira/src/emit-auxiliary-files.ts` around lines 301 - 319, The safe.fields extraction/assignment is duplicated; consolidate by extracting safeFields once (e.g., const safeFields = isRecord(safe.fields) ? safe.fields : ({} as Record<string, unknown>)) before the conditional blocks that check assigneeAccountId, creatorAccountId, and priorityName, mutate safeFields inside those ifs (set assigneeAccountId, creatorAccountId, priorityName as needed), and then assign safe.fields = safeFields once at the end; ensure you reuse the same isRecord check and keep the same types so behavior of assigneeAccountId, creatorAccountId, and priorityName handling remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/github/src/layout-prompt.ts`:
- Around line 37-39: The usage examples use "octocat__hello-world" with double
underscores but the correct owner/repo layout is
"/github/repos/<owner>/<repo>/..."; update the three example lines that
currently read "ls /github/repos/octocat__hello-world/..." to use a slash
between owner and repo (e.g. "ls
/github/repos/octocat/hello-world/issues/by-state/open", "ls
/github/repos/octocat/hello-world/issues/by-assignee/octocat", and "ls
/github/repos/octocat/hello-world/issues/by-priority/high") so the paths match
the intended layout.
---
Nitpick comments:
In `@packages/gitlab/src/path-mapper.ts`:
- Around line 147-189: The new GitLab alias path helpers
(gitLabByStateAliasPath, gitLabByAssigneeAliasPath, gitLabByCreatorAliasPath,
gitLabByPriorityAliasPath) lack non-empty validation for their alias parameters;
update each function to call the shared assertNonEmptySegment (or equivalent
existing validator) on the alias argument (state, assignee, creator, priority)
before passing it to slugifyAlias/encodeGitLabPathSegment, and add any missing
import for assertNonEmptySegment so empty strings are rejected and malformed
paths cannot be produced.
In `@packages/jira/src/__tests__/path-mapper.test.ts`:
- Around line 82-91: The two assertions for jiraIssueByCreatorAliasPath and
jiraIssueByPriorityPath are misplaced inside the jiraIssueByAssigneeAliasPath
describe block; move them into their own describe block (or top-level sibling)
so they are logically independent, and add round-trip tests for
jiraIssueByCreatorAliasPath and jiraIssueByPriorityPath analogous to the
existing jiraIssueByAssigneeAliasPath round-trip test (call the path function
with an alias and id and assert the inverse mapper/expectation produces the same
components).
In `@packages/jira/src/emit-auxiliary-files.ts`:
- Around line 301-319: The safe.fields extraction/assignment is duplicated;
consolidate by extracting safeFields once (e.g., const safeFields =
isRecord(safe.fields) ? safe.fields : ({} as Record<string, unknown>)) before
the conditional blocks that check assigneeAccountId, creatorAccountId, and
priorityName, mutate safeFields inside those ifs (set assigneeAccountId,
creatorAccountId, priorityName as needed), and then assign safe.fields =
safeFields once at the end; ensure you reuse the same isRecord check and keep
the same types so behavior of assigneeAccountId, creatorAccountId, and
priorityName handling remains 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: 5cebe4ad-a48f-4b01-bc1d-e01394ec7214
📒 Files selected for processing (34)
.claude/rules/alias-subtrees.md.claude/rules/relayfile-integration-digests.md.trajectories/index.jsonAGENTS.mddocs/digest-layout-contract.mdpackages/github/src/__tests__/emit-auxiliary-files.test.tspackages/github/src/__tests__/path-mapper.test.tspackages/github/src/emit-auxiliary-files.tspackages/github/src/layout-prompt.tspackages/github/src/layout.test.tspackages/github/src/layout.tspackages/github/src/path-mapper.tspackages/gitlab/src/emit-auxiliary-files.tspackages/gitlab/src/layout-prompt.tspackages/gitlab/src/layout.tspackages/gitlab/src/path-mapper.tspackages/gitlab/test/emit-auxiliary-files.test.tspackages/gitlab/test/layout.test.tspackages/gitlab/test/path-mapper.test.tspackages/jira/src/__tests__/emit-auxiliary-files.test.tspackages/jira/src/__tests__/path-mapper.test.tspackages/jira/src/emit-auxiliary-files.tspackages/jira/src/layout-prompt.tspackages/jira/src/layout.test.tspackages/jira/src/layout.tspackages/jira/src/path-mapper.tspackages/linear/src/__tests__/by-state.test.tspackages/linear/src/__tests__/emit-auxiliary-files.test.tspackages/linear/src/emit-auxiliary-files.tspackages/linear/src/layout-prompt.tspackages/linear/src/layout.test.tspackages/linear/src/layout.tspackages/linear/src/path-mapper.tsscripts/digest-layout-contracts.mjs
✅ Files skipped from review due to trivial changes (10)
- packages/linear/src/layout-prompt.ts
- packages/linear/src/tests/by-state.test.ts
- packages/jira/src/layout-prompt.ts
- .trajectories/index.json
- packages/github/src/layout.test.ts
- .claude/rules/alias-subtrees.md
- AGENTS.md
- packages/gitlab/src/layout-prompt.ts
- docs/digest-layout-contract.md
- .claude/rules/relayfile-integration-digests.md
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/gitlab/test/path-mapper.test.ts
- packages/gitlab/test/layout.test.ts
- packages/github/src/tests/emit-auxiliary-files.test.ts
- packages/github/src/tests/path-mapper.test.ts
- packages/gitlab/src/layout.ts
- packages/gitlab/test/emit-auxiliary-files.test.ts
- scripts/digest-layout-contracts.mjs
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
packages/asana/src/digest.ts (1)
102-114: ⚡ Quick winConsider adding archive/unarchive lifecycle handling.
Asana projects and sections support archiving, but
pastTensecurrently lets archive actions fall through to "was updated".🔧 Optional enhancement
if (hasActionVerb(action, 'close|closed|complete|completed|resolve|resolved|done')) { return 'was completed'; } + if (hasActionVerb(action, 'archive|archived')) { + return 'was archived'; + } + if (hasActionVerb(action, 'unarchive|unarchived|restore|restored')) { + return 'was unarchived'; + } return 'was updated';As per coding guidelines:
packages/*/src/digest.ts: Classify lifecycle actions explicitly; terminal states likearchivedshould not fall through to generic 'updated'.🤖 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/asana/src/digest.ts` around lines 102 - 114, The pastTense function currently maps lifecycle events to generic verbs and misses archive/unarchive; update pastTense (and use hasActionVerb) to explicitly detect archive-related verbs (e.g., archive|archived|archiving) and unarchive-related verbs (e.g., unarchive|unarchived|unarchiving) and return distinct strings like "was archived" and "was unarchived" instead of letting them fall through to "was updated"; ensure you add those checks before the final fallback so archive/unarchive are classified as terminal lifecycle states.packages/google-drive/src/digest.ts (1)
55-113: 💤 Low valueConsider extracting common digest utilities to reduce duplication.
All digest handlers share identical implementations for
hasCanonicalPath(parametrized by provider),compareEvents,eventTime,eventTimeMs,normalizeDigestPath, andhasActionVerb. While the current duplication keeps each adapter self-contained and the provider-specific identifier logic differs, extracting the common helpers to a shared module would improve maintainability if the pattern evolves.🤖 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/google-drive/src/digest.ts` around lines 55 - 113, Multiple digest adapters duplicate utility functions; extract shared helpers into a common module and import them from packages/google-drive/src/digest.ts. Move implementations of hasCanonicalPath (make it accept a provider string or pattern), compareEvents, eventTime, eventTimeMs, normalizeDigestPath, and hasActionVerb into a new shared file (e.g., digest-utils) and replace the local definitions in this file with imports; keep googleDriveIdentifier and pastTense here (pastTense should call the shared hasActionVerb), update hasCanonicalPath usage to pass the "google-drive" provider, and update any references to these functions (compareEvents, eventTimeMs, etc.) to use the imported symbols. Ensure types/exports are preserved so signatures (DigestChangeEvent, hasCanonicalPath type guard) remain compatible.
🤖 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 `@packages/clickup/src/digest.ts`:
- Around line 103-118: The pastTense function currently classifies cancel
actions as generic updates; update pastTense (in packages/clickup/src/digest.ts)
to explicitly detect cancel variants (e.g., "cancel|canceled|cancelled") using
the same hasActionVerb utility and return a terminal phrase like "was canceled"
(or "was cancelled") before the generic fallback; ensure the check is placed
above the final return so cancel lifecycle events
(DigestChangeEvent.action/eventType/type) do not fall through to "was updated".
In `@packages/shopify/src/digest.ts`:
- Around line 91-92: The code in packages/shopify/src/digest.ts uses
basename.lastIndexOf('--') to split slug and id, which fails for the canonical
Shopify joiner "__"; change the splitter to lastIndexOf('__') and compute label
using that separator (e.g., const separatorIndex = basename.lastIndexOf('__');
const label = separatorIndex > 0 ? basename.slice(0, separatorIndex) : basename)
so basename like "<slug>__<id>.json" yields the correct slug; update any related
logic that references separatorIndex/label in the same function (e.g.,
shopifyIdentifier handling) to use the new '__' delimiter.
In `@packages/teams/src/digest.ts`:
- Around line 90-93: The code that derives the Teams identifier only checks for
'metadata.json' and misses directory records ending with 'meta.json', causing
entity names to be wrong; update the logic around terminal/segment in digest.ts
(the segment, terminal, basename variables) to treat both 'metadata.json' and
'meta.json' as directory-record markers (e.g., terminal === 'metadata.json' ||
terminal === 'meta.json' or terminal.endsWith('meta.json')) so you pick
segments.at(-2) for those cases and then compute basename from that segment;
ensure basename computation (basename = segment.replace(/\.[^.]+$/u, ''))
remains unchanged.
In `@packages/zendesk/src/digest.ts`:
- Around line 110-114: The lifecycle-action branch in
packages/zendesk/src/digest.ts currently checks hasActionVerb(action,
'close|closed|solve|solved') and returns 'was solved', but it omits
resolve|resolved so those events fall through to the generic 'was updated';
update the condition(s) in the same function (the hasActionVerb check used to
derive the action text) to explicitly include resolve|resolved (or add a
separate hasActionVerb(action, 'resolve|resolved') branch) and return an
appropriate terminal label such as 'was resolved' so resolved events do not hit
the default 'was updated' branch.
---
Nitpick comments:
In `@packages/asana/src/digest.ts`:
- Around line 102-114: The pastTense function currently maps lifecycle events to
generic verbs and misses archive/unarchive; update pastTense (and use
hasActionVerb) to explicitly detect archive-related verbs (e.g.,
archive|archived|archiving) and unarchive-related verbs (e.g.,
unarchive|unarchived|unarchiving) and return distinct strings like "was
archived" and "was unarchived" instead of letting them fall through to "was
updated"; ensure you add those checks before the final fallback so
archive/unarchive are classified as terminal lifecycle states.
In `@packages/google-drive/src/digest.ts`:
- Around line 55-113: Multiple digest adapters duplicate utility functions;
extract shared helpers into a common module and import them from
packages/google-drive/src/digest.ts. Move implementations of hasCanonicalPath
(make it accept a provider string or pattern), compareEvents, eventTime,
eventTimeMs, normalizeDigestPath, and hasActionVerb into a new shared file
(e.g., digest-utils) and replace the local definitions in this file with
imports; keep googleDriveIdentifier and pastTense here (pastTense should call
the shared hasActionVerb), update hasCanonicalPath usage to pass the
"google-drive" provider, and update any references to these functions
(compareEvents, eventTimeMs, etc.) to use the imported symbols. Ensure
types/exports are preserved so signatures (DigestChangeEvent, hasCanonicalPath
type guard) remain compatible.
🪄 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: 039fd755-bac8-4970-9161-f93082220d7d
📒 Files selected for processing (30)
.trajectories/index.jsonpackages/airtable/src/digest.tspackages/asana/src/digest.tspackages/azure-blob/src/digest.tspackages/box/src/digest.tspackages/calendly/src/digest.tspackages/clickup/src/digest.tspackages/confluence/src/digest.tspackages/dropbox/src/digest.tspackages/gcs/src/digest.tspackages/github/src/digest.tspackages/gitlab/src/digest.tspackages/gmail/src/digest.tspackages/google-calendar/src/digest.tspackages/google-drive/src/digest.tspackages/hubspot/src/digest.tspackages/intercom/src/digest.tspackages/jira/src/digest.tspackages/linear/src/digest.test.tspackages/linear/src/digest.tspackages/pipedrive/src/digest.tspackages/postgres/src/digest.tspackages/redis/src/digest.tspackages/salesforce/src/digest.tspackages/sharepoint/src/digest.tspackages/shopify/src/digest.tspackages/stripe/src/digest.tspackages/teams/src/digest.tspackages/zendesk/src/digest.tsscripts/digest-layout-contracts.mjs
✅ Files skipped from review due to trivial changes (1)
- .trajectories/index.json
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/gitlab/src/digest.ts
- packages/azure-blob/src/digest.ts
- scripts/digest-layout-contracts.mjs
Comment on lines
+103
to
+118
| function pastTense(event: DigestChangeEvent): string { | ||
| const action = (event.action ?? event.eventType ?? event.type ?? '').toLowerCase(); | ||
| if (hasActionVerb(action, 'create|created|add|added|write|written')) { | ||
| return 'was created'; | ||
| } | ||
| if (hasActionVerb(action, 'delete|deleted|remove|removed')) { | ||
| return 'was deleted'; | ||
| } | ||
| if (hasActionVerb(action, 'close|closed|complete|completed|resolve|resolved|done')) { | ||
| return 'was completed'; | ||
| } | ||
| if (hasActionVerb(action, 'archive|archived')) { | ||
| return 'was archived'; | ||
| } | ||
| return 'was updated'; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add explicit handling for cancel lifecycle action.
ClickUp tasks support cancellation, but pastTense currently lets cancel actions fall through to "was updated".
🔧 Proposed fix
if (hasActionVerb(action, 'archive|archived')) {
return 'was archived';
}
+ if (hasActionVerb(action, 'cancel|canceled|cancelled')) {
+ return 'was canceled';
+ }
return 'was updated';As per coding guidelines: packages/*/src/digest.ts: Classify lifecycle actions explicitly; terminal states like canceled must not fall through to generic 'updated'.
📝 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 pastTense(event: DigestChangeEvent): string { | |
| const action = (event.action ?? event.eventType ?? event.type ?? '').toLowerCase(); | |
| if (hasActionVerb(action, 'create|created|add|added|write|written')) { | |
| return 'was created'; | |
| } | |
| if (hasActionVerb(action, 'delete|deleted|remove|removed')) { | |
| return 'was deleted'; | |
| } | |
| if (hasActionVerb(action, 'close|closed|complete|completed|resolve|resolved|done')) { | |
| return 'was completed'; | |
| } | |
| if (hasActionVerb(action, 'archive|archived')) { | |
| return 'was archived'; | |
| } | |
| return 'was updated'; | |
| } | |
| function pastTense(event: DigestChangeEvent): string { | |
| const action = (event.action ?? event.eventType ?? event.type ?? '').toLowerCase(); | |
| if (hasActionVerb(action, 'create|created|add|added|write|written')) { | |
| return 'was created'; | |
| } | |
| if (hasActionVerb(action, 'delete|deleted|remove|removed')) { | |
| return 'was deleted'; | |
| } | |
| if (hasActionVerb(action, 'close|closed|complete|completed|resolve|resolved|done')) { | |
| return 'was completed'; | |
| } | |
| if (hasActionVerb(action, 'archive|archived')) { | |
| return 'was archived'; | |
| } | |
| if (hasActionVerb(action, 'cancel|canceled|cancelled')) { | |
| return 'was canceled'; | |
| } | |
| return 'was updated'; | |
| } |
🤖 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/clickup/src/digest.ts` around lines 103 - 118, The pastTense
function currently classifies cancel actions as generic updates; update
pastTense (in packages/clickup/src/digest.ts) to explicitly detect cancel
variants (e.g., "cancel|canceled|cancelled") using the same hasActionVerb
utility and return a terminal phrase like "was canceled" (or "was cancelled")
before the generic fallback; ensure the check is placed above the final return
so cancel lifecycle events (DigestChangeEvent.action/eventType/type) do not fall
through to "was updated".
Comment on lines
+91
to
+92
| const separatorIndex = basename.lastIndexOf('--'); | ||
| const label = separatorIndex > 0 ? basename.slice(0, separatorIndex) : basename; |
There was a problem hiding this comment.
Use the canonical __ slug/id delimiter in Shopify identifier parsing.
shopifyIdentifier currently splits on --, so <slug>__<id>.json names won’t be parsed as intended and bullet labels can include the raw combined token.
Suggested fix
- const separatorIndex = basename.lastIndexOf('--');
+ const separatorIndex = basename.lastIndexOf('__');🤖 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/shopify/src/digest.ts` around lines 91 - 92, The code in
packages/shopify/src/digest.ts uses basename.lastIndexOf('--') to split slug and
id, which fails for the canonical Shopify joiner "__"; change the splitter to
lastIndexOf('__') and compute label using that separator (e.g., const
separatorIndex = basename.lastIndexOf('__'); const label = separatorIndex > 0 ?
basename.slice(0, separatorIndex) : basename) so basename like
"<slug>__<id>.json" yields the correct slug; update any related logic that
references separatorIndex/label in the same function (e.g., shopifyIdentifier
handling) to use the new '__' delimiter.
Comment on lines
+90
to
+93
| const segment = terminal === 'metadata.json' | ||
| ? segments.at(-2) ?? path | ||
| : terminal ?? path; | ||
| const basename = segment.replace(/\.[^.]+$/u, ''); |
There was a problem hiding this comment.
Handle meta.json directory records when deriving Teams identifiers.
Line 90 only recognizes metadata.json. For canonical directory records ending in meta.json, bullets can become channel meta/message meta instead of the actual entity identifier.
Suggested fix
function teamsIdentifier(path: string): string {
const segments = path.split('/').filter(Boolean);
const terminal = segments.at(-1);
- const segment = terminal === 'metadata.json'
+ const segment = terminal === 'meta.json' || terminal === 'metadata.json'
? segments.at(-2) ?? path
: terminal ?? path;Based on learnings: Entities with child artifacts MUST use directory records ending in meta.json; entities without child artifacts MUST use flat .json records.
📝 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 segment = terminal === 'metadata.json' | |
| ? segments.at(-2) ?? path | |
| : terminal ?? path; | |
| const basename = segment.replace(/\.[^.]+$/u, ''); | |
| function teamsIdentifier(path: string): string { | |
| const segments = path.split('/').filter(Boolean); | |
| const terminal = segments.at(-1); | |
| const segment = terminal === 'meta.json' || terminal === 'metadata.json' | |
| ? segments.at(-2) ?? path | |
| : terminal ?? path; | |
| const basename = segment.replace(/\.[^.]+$/u, ''); |
🤖 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/teams/src/digest.ts` around lines 90 - 93, The code that derives the
Teams identifier only checks for 'metadata.json' and misses directory records
ending with 'meta.json', causing entity names to be wrong; update the logic
around terminal/segment in digest.ts (the segment, terminal, basename variables)
to treat both 'metadata.json' and 'meta.json' as directory-record markers (e.g.,
terminal === 'metadata.json' || terminal === 'meta.json' or
terminal.endsWith('meta.json')) so you pick segments.at(-2) for those cases and
then compute basename from that segment; ensure basename computation (basename =
segment.replace(/\.[^.]+$/u, '')) remains unchanged.
Comment on lines
+110
to
+114
| if (hasActionVerb(action, 'close|closed|solve|solved')) { | ||
| return 'was solved'; | ||
| } | ||
| return 'was updated'; | ||
| } |
There was a problem hiding this comment.
Classify resolved actions explicitly instead of falling back to “updated”.
Line 110 misses resolve|resolved, so those terminal lifecycle events can hit the default branch at Line 113.
Suggested fix
- if (hasActionVerb(action, 'close|closed|solve|solved')) {
+ if (hasActionVerb(action, 'close|closed|solve|solved|resolve|resolved')) {
return 'was solved';
}As per coding guidelines: Classify lifecycle actions explicitly; terminal states (closed, merged, archived, completed, canceled, resolved) must not fall through to generic 'updated' unless provider has no terminal concept.
📝 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
| if (hasActionVerb(action, 'close|closed|solve|solved')) { | |
| return 'was solved'; | |
| } | |
| return 'was updated'; | |
| } | |
| if (hasActionVerb(action, 'close|closed|solve|solved|resolve|resolved')) { | |
| return 'was solved'; | |
| } | |
| return 'was updated'; | |
| } |
🤖 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/zendesk/src/digest.ts` around lines 110 - 114, The lifecycle-action
branch in packages/zendesk/src/digest.ts currently checks hasActionVerb(action,
'close|closed|solve|solved') and returns 'was solved', but it omits
resolve|resolved so those events fall through to the generic 'was updated';
update the condition(s) in the same function (the hasActionVerb check used to
derive the action text) to explicitly include resolve|resolved (or add a
separate hasActionVerb(action, 'resolve|resolved') branch) and return an
appropriate terminal label such as 'was resolved' so resolved events do not hit
the default 'was updated' branch.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 166 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/hubspot/src/digest.ts">
<violation number="1" location="packages/hubspot/src/digest.ts:114">
P2: Missing `unarchive|unarchived` lifecycle classification. The Slack adapter handles this case (returning `'was unarchived'`), and the PR description states it classifies unarchived lifecycle changes, but this handler omits it — causing unarchive events to fall through to `'was updated'`.</violation>
</file>
<file name="docs/digest-layout-contract.md">
<violation number="1" location="docs/digest-layout-contract.md:36">
P2: Alias path format in the Category Matrix contradicts the established convention in AGENTS.md. The canonical pattern is `by-<key>/<value>__<id>.json` (flat, double-underscore joiner), but this table shows `by-<key>/<value>/<id>.json` (nested subdirectory per value). Implementers following this doc will produce a different tree shape than what the rest of the system expects.</violation>
</file>
Partial review: This PR has more than 50 files, so cubic reviewed the highest-priority files first. During the trial, paid plans get a higher file limit.
You can try an ultrareview to bypass the file limit, comment @cubic-dev-ai ultrareview. Learn more.
Fix all with cubic
| if (hasActionVerb(action, 'merge|merged')) { | ||
| return 'was merged'; | ||
| } | ||
| if (hasActionVerb(action, 'archive|archived')) { |
There was a problem hiding this comment.
P2: Missing unarchive|unarchived lifecycle classification. The Slack adapter handles this case (returning 'was unarchived'), and the PR description states it classifies unarchived lifecycle changes, but this handler omits it — causing unarchive events to fall through to 'was updated'.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/hubspot/src/digest.ts, line 114:
<comment>Missing `unarchive|unarchived` lifecycle classification. The Slack adapter handles this case (returning `'was unarchived'`), and the PR description states it classifies unarchived lifecycle changes, but this handler omits it — causing unarchive events to fall through to `'was updated'`.</comment>
<file context>
@@ -0,0 +1,134 @@
+ if (hasActionVerb(action, 'merge|merged')) {
+ return 'was merged';
+ }
+ if (hasActionVerb(action, 'archive|archived')) {
+ return 'was archived';
+ }
</file context>
|
|
||
| | Category | Providers/resources | Required lookup | Rationale | | ||
| | --- | --- | --- | --- | | ||
| | issue-tracking | GitHub issues and pull requests, GitLab issues and merge requests, Jira issues, Linear issues | `by-state/<state>/<id>.json`, `by-assignee/<assignee>/<id>.json`, `by-creator/<creator>/<id>.json`, `by-priority/<priority>/<id>.json` | Agents often ask for open, closed, merged, completed, canceled, assigned, created-by, or priority-scoped work without already knowing an id. | |
There was a problem hiding this comment.
P2: Alias path format in the Category Matrix contradicts the established convention in AGENTS.md. The canonical pattern is by-<key>/<value>__<id>.json (flat, double-underscore joiner), but this table shows by-<key>/<value>/<id>.json (nested subdirectory per value). Implementers following this doc will produce a different tree shape than what the rest of the system expects.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/digest-layout-contract.md, line 36:
<comment>Alias path format in the Category Matrix contradicts the established convention in AGENTS.md. The canonical pattern is `by-<key>/<value>__<id>.json` (flat, double-underscore joiner), but this table shows `by-<key>/<value>/<id>.json` (nested subdirectory per value). Implementers following this doc will produce a different tree shape than what the rest of the system expects.</comment>
<file context>
@@ -0,0 +1,55 @@
+
+| Category | Providers/resources | Required lookup | Rationale |
+| --- | --- | --- | --- |
+| issue-tracking | GitHub issues and pull requests, GitLab issues and merge requests, Jira issues, Linear issues | `by-state/<state>/<id>.json`, `by-assignee/<assignee>/<id>.json`, `by-creator/<creator>/<id>.json`, `by-priority/<priority>/<id>.json` | Agents often ask for open, closed, merged, completed, canceled, assigned, created-by, or priority-scoped work without already knowing an id. |
+| ci-deploy | GitLab pipelines and deployments | `by-status/<status>/<id>.json` | Status is the primary lifecycle bucket for build and deploy resources. |
+| knowledge | Confluence pages | `by-state/<state>/<id>.json` | Pages can be current, archived, trashed, restored, or deleted and must remain discoverable by lifecycle bucket. |
</file context>
Suggested change
| | issue-tracking | GitHub issues and pull requests, GitLab issues and merge requests, Jira issues, Linear issues | `by-state/<state>/<id>.json`, `by-assignee/<assignee>/<id>.json`, `by-creator/<creator>/<id>.json`, `by-priority/<priority>/<id>.json` | Agents often ask for open, closed, merged, completed, canceled, assigned, created-by, or priority-scoped work without already knowing an id. | | |
| | issue-tracking | GitHub issues and pull requests, GitLab issues and merge requests, Jira issues, Linear issues | `by-state/<state>__<id>.json`, `by-assignee/<assignee>__<id>.json`, `by-creator/<creator>__<id>.json`, `by-priority/<priority>__<id>.json` | Agents often ask for open, closed, merged, completed, canceled, assigned, created-by, or priority-scoped work without already knowing an id. | |
There was a problem hiding this comment.
2 issues found across 7 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/digest-layout-contracts.mjs">
<violation number="1" location="scripts/digest-layout-contracts.mjs:214">
P2: `relayfileRoots` misses multi-segment roots (e.g. `/github/repos`), so digest canonical-path checks can incorrectly pass without validating real provider roots.</violation>
</file>
<file name="packages/gitlab/src/emit-auxiliary-files.ts">
<violation number="1" location="packages/gitlab/src/emit-auxiliary-files.ts:226">
P2: Using `prior.canonicalPath` here conflicts with title-based stale-path diffing and can delete then re-write the same canonical file on title changes, increasing the chance of ending with missing canonical content if the write fails.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
Re-trigger cubic
|
|
||
| function relayfileRoots(pathMapperSource) { | ||
| const roots = new Set(); | ||
| for (const match of pathMapperSource.matchAll(/(?:export\s+)?const\s+[A-Z0-9_]*(?:ROOT|PATH_ROOT|RELAYFILE_ROOT|DEFAULT_ROOT)\s*=\s*['"](\/[a-z0-9-]+)['"]/gu)) { |
There was a problem hiding this comment.
P2: relayfileRoots misses multi-segment roots (e.g. /github/repos), so digest canonical-path checks can incorrectly pass without validating real provider roots.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/digest-layout-contracts.mjs, line 214:
<comment>`relayfileRoots` misses multi-segment roots (e.g. `/github/repos`), so digest canonical-path checks can incorrectly pass without validating real provider roots.</comment>
<file context>
@@ -212,8 +209,34 @@ function assertTestMentions(provider, source, pattern, label) {
- return pathMapperSource.match(/RELAYFILE_ROOT\s*=\s*['"]([^'"]+)['"]/u)?.[1] ?? null;
+function relayfileRoots(pathMapperSource) {
+ const roots = new Set();
+ for (const match of pathMapperSource.matchAll(/(?:export\s+)?const\s+[A-Z0-9_]*(?:ROOT|PATH_ROOT|RELAYFILE_ROOT|DEFAULT_ROOT)\s*=\s*['"](\/[a-z0-9-]+)['"]/gu)) {
+ roots.add(match[1]);
+ }
</file context>
Suggested change
| for (const match of pathMapperSource.matchAll(/(?:export\s+)?const\s+[A-Z0-9_]*(?:ROOT|PATH_ROOT|RELAYFILE_ROOT|DEFAULT_ROOT)\s*=\s*['"](\/[a-z0-9-]+)['"]/gu)) { | |
| for (const match of pathMapperSource.matchAll(/(?:export\s+)?const\s+[A-Z0-9_]*(?:ROOT|PATH_ROOT|RELAYFILE_ROOT|DEFAULT_ROOT)\s*=\s*['"](\/[a-z0-9-]+(?:\/[a-z0-9-]+)*)['"]/gu)) { |
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/gitlab/src/emit-auxiliary-files.ts">
<violation number="1" location="packages/gitlab/src/emit-auxiliary-files.ts:376">
P2: Delete cleanup for pipelines can miss `record.ref` paths because delete mode forces `ref` to `prior.ref` before generating the "current" delete paths.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic
Re-trigger cubic
Comment on lines
+376
to
+378
| const ref = isDelete | ||
| ? prior?.ref ?? recordRef | ||
| : recordRef ?? prior?.ref; |
There was a problem hiding this comment.
P2: Delete cleanup for pipelines can miss record.ref paths because delete mode forces ref to prior.ref before generating the "current" delete paths.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/gitlab/src/emit-auxiliary-files.ts, line 376:
<comment>Delete cleanup for pipelines can miss `record.ref` paths because delete mode forces `ref` to `prior.ref` before generating the "current" delete paths.</comment>
<file context>
@@ -372,7 +372,10 @@ async function planPipelineRecord(
const isDelete = record._deleted === true;
- const ref = readNonEmptyString(record.ref);
+ const recordRef = readNonEmptyString(record.ref);
+ const ref = isDelete
+ ? prior?.ref ?? recordRef
+ : recordRef ?? prior?.ref;
</file context>
Suggested change
| const ref = isDelete | |
| ? prior?.ref ?? recordRef | |
| : recordRef ?? prior?.ref; | |
| const ref = recordRef ?? prior?.ref; |
Tip: Review your code locally with the cubic CLI to iterate faster.
There was a problem hiding this comment.
1 issue found across 45 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/digest-layout-contracts.mjs">
<violation number="1" location="scripts/digest-layout-contracts.mjs:177">
P2: The exact-root `digestEventPath(event)` check only matches single quotes, so valid double-quoted comparisons are incorrectly rejected.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic
Re-trigger cubic
| digestSource.includes(`startsWith('/${provider}/')`) | ||
| && !digestSource.includes(`event.canonicalPath === '/${provider}'`) | ||
| && !digestSource.includes(`canonicalPath === '/${provider}'`) | ||
| && !digestSource.includes(`digestEventPath(event) === '/${provider}'`) |
There was a problem hiding this comment.
P2: The exact-root digestEventPath(event) check only matches single quotes, so valid double-quoted comparisons are incorrectly rejected.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/digest-layout-contracts.mjs, line 177:
<comment>The exact-root `digestEventPath(event)` check only matches single quotes, so valid double-quoted comparisons are incorrectly rejected.</comment>
<file context>
@@ -143,10 +167,14 @@ for (const provider of providerPackages()) {
digestSource.includes(`startsWith('/${provider}/')`)
&& !digestSource.includes(`event.canonicalPath === '/${provider}'`)
&& !digestSource.includes(`canonicalPath === '/${provider}'`)
+ && !digestSource.includes(`digestEventPath(event) === '/${provider}'`)
) {
failures.push(`${provider}: digest canonical-path filter must accept exact /${provider}`);
</file context>
Suggested change
| && !digestSource.includes(`digestEventPath(event) === '/${provider}'`) | |
| && !digestSource.includes(`digestEventPath(event) === '/${provider}'`) | |
| && !digestSource.includes(`digestEventPath(event) === "/${provider}"`) |
Tip: Review your code locally with the cubic CLI to iterate faster.
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
added 26 commits
May 15, 2026 22:30
khaliqgant
pushed a commit
that referenced
this pull request
May 18, 2026
Brings codex/adapter-digest-lifecycle-coverage into #98 so the X social search adapter ships with its security and correctness hardening. Conflicts on packages/x/** resolved in favor of the hardened version. Includes beyond the original PR #98 contents: - X security: validated baseUrl, AbortController timeout, truncated error bodies, trustedPriorCanonicalPath rejecting cross-provider paths. - X path-mapper __ joiner round-trip + safe-decode helpers. - GitLab emit-aux trustedPriorCanonicalPath via parseGitLabPath. - GitHub emit-aux NumberedDeleteRecoveryCache (N+1 fix). - Asana + ClickUp digest tests rewired to real path-mapper helpers. - relay CLI hardening from the parallel review-loop run. - Digest lifecycle coverage (#97 follow-up). Each fix is mutation-checked. 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
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.
Summary
Verification