fix(slack): emit thread replies as directory records to end file/dir collision - #162
Conversation
…collision A channel thread reply was the only Slack record emitted as a flat leaf file, `threads/<threadTs>/replies/<replyTs>.json`, while its children (reactions) nest under a directory at the same stem, `threads/<threadTs>/replies/<replyTs>/...`. One name as both a file and a directory cannot be materialized on a POSIX mount: the relayfile mirror fails every sync cycle with `mkdir .../replies/<replyTs>.json: not a directory`, never completes bootstrap, and the teardown writeback flush hangs — which marked the daily-ship cron run FAILED even though its handler succeeded. Every other Slack record type already uses the `<ts>/meta.json` directory-record convention (messagePath, directMessagePath, threadPath). Bring thread replies in line: `threadReplyPath` now returns `replies/<ts>/meta.json`, so the reply and its `reactions/` are siblings under one `replies/<ts>/` directory — collision impossible by construction. - Add `threadReplyLegacyPath` + `slackThreadReplyReadCandidatePaths` for back-compat reads of replies mirrored by a pre-0.8.x adapter (mirrors the existing messagePath / messageLegacyPath / slackMessageReadCandidatePaths pattern). - `thread.ts` reply-listing regex accepts both the new `<ts>/meta.json` and the legacy `<ts>.json` form so a mid-migration mirror still maps a reply to its parent listing. - Update the LAYOUT.md prompt line and the two adapter tests that pinned the old flat path; add a regression test asserting the reply record and its reaction child cannot collide (and that legacy reads still resolve). Scope: channel thread replies only (the confirmed collision site). DM thread replies (`directMessageThreadReplyPath`) carry the same latent flat-file shape and should get the same treatment — left out here because that tree is under active change on the slack-dm-user-message-materialization branch; fold the same fix in there. The relayfile mount daemon is being hardened separately to quarantine any such collision regardless of adapter. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughSlack adapter migrates thread reply and direct-message record paths from flat-leaf formats to directory-based canonical forms ( ChangesSlack Adapter: Thread Reply Path Migration to Directory Structure
Other Adapter Resource Configuration Updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request updates the canonical thread-reply record path from a flat leaf file (replies/<ts>.json) to a directory record (replies/<ts>/meta.json) to prevent collisions with reaction children on POSIX mounts. It also introduces backward compatibility support for reading legacy flat paths, updates documentation, and adds corresponding tests. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
No issues found across 5 files
You’re at about 99% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
|
Reviewed PR #162 and made two narrow fixes:
Verification run:
Addressed comments
Source checked for PR conversation/comments: #162 |
|
Reviewed PR #162 and made two narrow fixes:
Verification run:
Addressed comments
Source checked for PR conversation/comments: #162 |
Bring 1:1 direct-message thread replies in line with channel thread replies and every other Slack record type: emit `users/<u>/messages/<ts>/replies/<ts>/meta.json` instead of the flat `replies/<ts>.json` leaf, so a reply's stem is a directory and cannot collide with a same-named child directory on a POSIX mount. - `directMessageThreadReplyPath` → `<ts>/meta.json`; add `directMessageThreadReplyLegacyPath` + `slackDirectMessageThreadReplyReadCandidatePaths`. - `parseSlackDirectMessageThreadReplyPath` accepts both the new `<ts>/meta.json` and legacy `<ts>.json` forms so routing works mid-migration. - Export the new helpers (and the channel-reply back-compat helpers from the prior commit) from the package index. - Update LAYOUT.md prompt + discovery `.adapter.md`; extend the DM path test with legacy-parse and read-candidate assertions. Typecheck clean; full slack suite green (104). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
I reviewed PR #162 and validated the current findings against the checkout before the workspace was unexpectedly replaced by a different repo ( Before that workspace change, I made and verified two narrow fixes:
Verification completed before the workspace replacement:
Addressed comments
Source checked for PR comments: |
|
I reviewed PR #162 and validated the current findings against the checkout before the workspace was unexpectedly replaced by a different repo ( Before that workspace change, I made and verified two narrow fixes:
Verification completed before the workspace replacement:
Addressed comments
Source checked for PR comments: |
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
Reviewed PR #162 and fixed a real drift in the generated Slack discovery contract. The PR manually updated Addressed comments
Verification run locally:
I did not print |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/slack/discovery/slack/.adapter.md`:
- Line 8: Update the legacy flat reply path wording in the Slack adapter docs:
replace the incorrect `/messages/<messageTs>/replies/<replyTs>.json` reference
with the thread-path family `/threads/<threadTs>/replies/<replyTs>.json` in the
line describing
`/slack/channels/<channelId>/threads/<threadTs>/replies/<replyTs>/meta.json` so
the legacy path matches the thread-reply migration contract.
🪄 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: a0a14cf8-55a6-4499-b8d1-fa48e2214851
📒 Files selected for processing (13)
packages/dropbox/src/resources.tspackages/hubspot/src/resources.tspackages/linear/src/resources.tspackages/slack/discovery/slack/.adapter.mdpackages/slack/discovery/slack/users/{userId}/messages/.schema.jsonpackages/slack/src/__tests__/path-mapper-v2.test.tspackages/slack/src/__tests__/slack-adapter.test.tspackages/slack/src/emit-auxiliary-files.tspackages/slack/src/index.tspackages/slack/src/layout-prompt.tspackages/slack/src/path-mapper.tspackages/slack/src/thread.tsscripts/writeback-discovery-data.mjs
💤 Files with no reviewable changes (1)
- packages/linear/src/resources.ts
| - `/slack/channels/<channelId>.json` - Channel records. | ||
| - `/slack/channels/<channelId>/messages/<messageTs>/meta.json` - Message records. | ||
| - `/slack/channels/<channelId>/messages/<messageTs>/replies/<replyTs>.json` - Thread reply records. | ||
| - `/slack/channels/<channelId>/threads/<threadTs>/replies/<replyTs>/meta.json` - Thread reply records (directory records; legacy flat `/messages/<messageTs>/replies/<replyTs>.json` writeback paths still route for edits/deletes). |
There was a problem hiding this comment.
Correct the legacy thread-reply path wording to avoid contract confusion.
The legacy flat reply path here should reference the thread path family (/threads/<threadTs>/replies/<replyTs>.json), not /messages/..., to match the thread-reply migration contract.
Suggested doc fix
-- `/slack/channels/<channelId>/threads/<threadTs>/replies/<replyTs>/meta.json` - Thread reply records (directory records; legacy flat `/messages/<messageTs>/replies/<replyTs>.json` writeback paths still route for edits/deletes).
+- `/slack/channels/<channelId>/threads/<threadTs>/replies/<replyTs>/meta.json` - Thread reply records (directory records; legacy flat `/threads/<threadTs>/replies/<replyTs>.json` paths still route for back-compat edits/deletes).📝 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.
| - `/slack/channels/<channelId>/threads/<threadTs>/replies/<replyTs>/meta.json` - Thread reply records (directory records; legacy flat `/messages/<messageTs>/replies/<replyTs>.json` writeback paths still route for edits/deletes). | |
| - `/slack/channels/<channelId>/threads/<threadTs>/replies/<replyTs>/meta.json` - Thread reply records (directory records; legacy flat `/threads/<threadTs>/replies/<replyTs>.json` paths still route for back-compat edits/deletes). |
🤖 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/slack/discovery/slack/.adapter.md` at line 8, Update the legacy flat
reply path wording in the Slack adapter docs: replace the incorrect
`/messages/<messageTs>/replies/<replyTs>.json` reference with the thread-path
family `/threads/<threadTs>/replies/<replyTs>.json` in the line describing
`/slack/channels/<channelId>/threads/<threadTs>/replies/<replyTs>/meta.json` so
the legacy path matches the thread-reply migration contract.
|
Reviewed the checked-out PR metadata as I validated the PR diff, fetched current bot comments, and verified the current checkout has the needed fixes for cubic’s findings:
Verification run:
Addressed comments
Not printing |
|
Reviewed the checked-out PR. Note: local Changes made:
Validation run:
Addressed comments
I did not print |
…rces CI's "generated writeback-path catalog is in sync with adapter resources" check (adapter-core) was failing: a prior commit changed dropbox/linear/hubspot `resources.ts` (e.g. dropbox `shared-folders`/`shared-links`/`folders` → `cursors`/`files`; linear dropped `agent-activities`) without regenerating the checked-in catalog. The generator reads each adapter's BUILT `dist/resources.js`, so the drift only surfaces on a fresh CI build — locally a stale `dist` masked it. Rebuilt all adapters and re-ran `adapter-core writeback-paths generate`. Slack is unaffected — this PR's reply-path change is read-side only and does not touch slack's writeback `resources.ts`, so its catalog entry is unchanged. `--check` passes; adapter-core (105) and slack (104) suites green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
…log to main The pr-reviewer bot's earlier commit on this branch changed dropbox/hubspot/linear `resources.ts` — unrelated to this slack reply-path PR — and in particular removed linear's `agent-activities` writeback resource (added in #146). That broke CI two ways: the writeback-path catalog went out of sync (adapter-core test), and once regenerated, `relay-helpers/src/linear.ts` failed to typecheck because `ProviderClient<"linear">` no longer had `agent-activities` (`agentActivity`/`respond`/`acknowledge`). Revert those three resources.ts files to origin/main and regenerate the writeback-path catalog (restoring agent-activities etc.), so this PR contains ONLY the slack thread/DM reply directory-record change it's about. The bot's slack-related edits (discovery readPaths, DM schema wording, emit-auxiliary-files) are kept — they align with this fix. Verified: adapter-core (105) and slack (104) suites green, relay-helpers typecheck clean, writeback-discovery + digest-contracts checks pass, full `turbo build` succeeds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
…e/dir collisions (#165) * fix(github,linear): emit comment records as directory records to end file/dir collision A GitHub issue comment and a Linear comment were emitted as flat leaf files (`issues/<n>__<slug>/comments/<id>.json`, `/linear/comments/<name>__<id>.json`) — the same latent shape that wedged Slack mounts (dea03fc, f5ca1ce, PR #162): once any child resource nests under the comment id (GitHub exposes per-comment reactions at /repos/{o}/{r}/issues/comments/{id}/reactions; Linear has per-comment emoji reactions — the webhook normalizer already recognizes 'reaction' payloads — and threaded replies), one name must be both a file and a directory, which cannot be materialized on a POSIX mount (`mkdir ... : not a directory`) and wedges the whole mirror. Apply the Slack directory-record pattern to both adapters: GitHub: - `githubIssueCommentPath` → `comments/<id>/meta.json`; add `githubIssueCommentLegacyPath` + `githubIssueCommentReadCandidatePaths` (mirrors slack threadReplyPath / threadReplyLegacyPath / slackThreadReplyReadCandidatePaths). - `mapIssueComment` and `computeScopedPath('issue_comment')` emit the directory record. - `ISSUE_COMMENT_WRITEBACK_PATH` accepts both `<id>/meta.json` and the legacy `<id>.json`, so patches against either mirror generation route. - Regenerated `resources.ts` / discovery `.adapter.md`: issue-comments pathPattern matches `/meta.json`, idPattern accepts the `meta` stem (handler re-derives the numeric id from the full path, like slack messages). - LAYOUT.md prompt documents the comment directory record. Linear: - `linearCommentPath` → `/linear/comments/<name>__<id>/meta.json`; add `linearCommentLegacyPath` + `linearCommentReadCandidatePaths`. - Comment delete tombstones target both the directory record and the legacy flat leaf so pre-migration mirrors are cleaned. - Linear writeback needs no route change: comment create drafts live in the separate `/linear/issues/{issueId}/comments/` tree. - LAYOUT.md prompt + discovery read-path docs updated. Both: regression tests pinning the directory-record path, the child-nesting invariant, and the read-candidate fallback order, mirroring the slack collision tests. New docs/architecture/writeback-resource-patterns.md explains the collision and the migration recipe so future adapters don't reintroduce the flat-leaf shape. Writeback-paths catalog: no regeneration needed — resource path templates are unchanged (`writeback-paths check` clean). Full repo suite green (85 turbo tasks); writeback discovery (82 endpoints), digest contracts, and internal-dependency checks clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore: apply pr-reviewer fixes for #165 * chore: apply pr-reviewer fixes for #165 * chore(core): regenerate writeback-path catalog after main merge Incorporates dropbox folders/shared-folders/shared-links resources added upstream plus updated github/linear comment dir-record path patterns. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Hubspot Adapter Bot <agent@agent-relay.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: agent-relay-code[bot] <agent-relay-code[bot]@users.noreply.github.com>
* fix(slack): emit thread replies as directory records to end file/dir collision A channel thread reply was the only Slack record emitted as a flat leaf file, `threads/<threadTs>/replies/<replyTs>.json`, while its children (reactions) nest under a directory at the same stem, `threads/<threadTs>/replies/<replyTs>/...`. One name as both a file and a directory cannot be materialized on a POSIX mount: the relayfile mirror fails every sync cycle with `mkdir .../replies/<replyTs>.json: not a directory`, never completes bootstrap, and the teardown writeback flush hangs — which marked the daily-ship cron run FAILED even though its handler succeeded. Every other Slack record type already uses the `<ts>/meta.json` directory-record convention (messagePath, directMessagePath, threadPath). Bring thread replies in line: `threadReplyPath` now returns `replies/<ts>/meta.json`, so the reply and its `reactions/` are siblings under one `replies/<ts>/` directory — collision impossible by construction. - Add `threadReplyLegacyPath` + `slackThreadReplyReadCandidatePaths` for back-compat reads of replies mirrored by a pre-0.8.x adapter (mirrors the existing messagePath / messageLegacyPath / slackMessageReadCandidatePaths pattern). - `thread.ts` reply-listing regex accepts both the new `<ts>/meta.json` and the legacy `<ts>.json` form so a mid-migration mirror still maps a reply to its parent listing. - Update the LAYOUT.md prompt line and the two adapter tests that pinned the old flat path; add a regression test asserting the reply record and its reaction child cannot collide (and that legacy reads still resolve). Scope: channel thread replies only (the confirmed collision site). DM thread replies (`directMessageThreadReplyPath`) carry the same latent flat-file shape and should get the same treatment — left out here because that tree is under active change on the slack-dm-user-message-materialization branch; fold the same fix in there. The relayfile mount daemon is being hardened separately to quarantine any such collision regardless of adapter. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(slack): apply directory-record fix to DM thread replies too Bring 1:1 direct-message thread replies in line with channel thread replies and every other Slack record type: emit `users/<u>/messages/<ts>/replies/<ts>/meta.json` instead of the flat `replies/<ts>.json` leaf, so a reply's stem is a directory and cannot collide with a same-named child directory on a POSIX mount. - `directMessageThreadReplyPath` → `<ts>/meta.json`; add `directMessageThreadReplyLegacyPath` + `slackDirectMessageThreadReplyReadCandidatePaths`. - `parseSlackDirectMessageThreadReplyPath` accepts both the new `<ts>/meta.json` and legacy `<ts>.json` forms so routing works mid-migration. - Export the new helpers (and the channel-reply back-compat helpers from the prior commit) from the package index. - Update LAYOUT.md prompt + discovery `.adapter.md`; extend the DM path test with legacy-parse and read-candidate assertions. Typecheck clean; full slack suite green (104). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore: apply pr-reviewer fixes for #162 * chore(core): regenerate writeback-path catalog to match adapter resources CI's "generated writeback-path catalog is in sync with adapter resources" check (adapter-core) was failing: a prior commit changed dropbox/linear/hubspot `resources.ts` (e.g. dropbox `shared-folders`/`shared-links`/`folders` → `cursors`/`files`; linear dropped `agent-activities`) without regenerating the checked-in catalog. The generator reads each adapter's BUILT `dist/resources.js`, so the drift only surfaces on a fresh CI build — locally a stale `dist` masked it. Rebuilt all adapters and re-ran `adapter-core writeback-paths generate`. Slack is unaffected — this PR's reply-path change is read-side only and does not touch slack's writeback `resources.ts`, so its catalog entry is unchanged. `--check` passes; adapter-core (105) and slack (104) suites green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * revert(out-of-scope): restore dropbox/hubspot/linear resources + catalog to main The pr-reviewer bot's earlier commit on this branch changed dropbox/hubspot/linear `resources.ts` — unrelated to this slack reply-path PR — and in particular removed linear's `agent-activities` writeback resource (added in #146). That broke CI two ways: the writeback-path catalog went out of sync (adapter-core test), and once regenerated, `relay-helpers/src/linear.ts` failed to typecheck because `ProviderClient<"linear">` no longer had `agent-activities` (`agentActivity`/`respond`/`acknowledge`). Revert those three resources.ts files to origin/main and regenerate the writeback-path catalog (restoring agent-activities etc.), so this PR contains ONLY the slack thread/DM reply directory-record change it's about. The bot's slack-related edits (discovery readPaths, DM schema wording, emit-auxiliary-files) are kept — they align with this fix. Verified: adapter-core (105) and slack (104) suites green, relay-helpers typecheck clean, writeback-discovery + digest-contracts checks pass, full `turbo build` succeeds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(granola): add file.created storage-bridge trigger The NB-Whisper pipeline writes transcripts directly to the granola VFS mount (/granola/notes/<id>.json), which fires a storage-dispatcher file.created event — not the Granola-native recording.created webhook. Only recording.created was listed, so the deploy CLI warned that file.created was unknown and the cloud rejected the trigger at deploy time. Add file.created to granola.mapping.yaml and regenerate catalog.generated.ts so the deploy lint accepts it and the cloud can register the subscription. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(core): regenerate trigger catalog to include granola file.created Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Hubspot Adapter Bot <agent@agent-relay.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: agent-relay-code[bot] <agent-relay-code[bot]@users.noreply.github.com>
Problem
A channel thread reply was the only Slack record type emitted as a flat leaf file:
One name used as both a file and a directory can't be materialized on a POSIX mount. The relayfile mirror fails every sync cycle with:
…so the mount never completes bootstrap, the teardown writeback flush hangs, and the daily-ship cron run was marked FAILED even though its handler succeeded. (Full chain: handler logged
runner.handler.ok, but the unreconciled mirror's flush timed out →flushExitCode: 124→ run failed. It also left the slack mount partially synced, so@-mentions and the channel didn't resolve and the post returnedts:''.)Fix
Every other Slack record already uses the
<ts>/meta.jsondirectory-record convention —messagePath,directMessagePath,threadPath. Thread replies were the lone exception. Bring them in line:Now the reply (
meta.json) and itsreactions/are siblings under onereplies/<ts>/directory — collision impossible by construction.threadReplyLegacyPath+slackThreadReplyReadCandidatePathsfor back-compat reads of replies mirrored by a pre-0.8.x adapter (mirrors the existingmessagePath/messageLegacyPath/slackMessageReadCandidatePathspattern).thread.tsreply-listing regex accepts both the new<ts>/meta.jsonand legacy<ts>.jsonform, so a mid-migration mirror still maps a reply to its parent listing.LAYOUT.mdprompt line and the two tests that pinned the old flat path.Tests
pnpm typecheckclean; full slack package suite green (104 tests, incl. the new collision-proof test).Scope / coordination notes
directMessageThreadReplyPath) carry the same latent flat-file shape and should get the same<ts>/meta.jsontreatment. Left out here because that tree is under active change on the localfeat/slack-dm-user-message-materializationbranch — please fold the same fix in there.replies/<ts>.jsonleaf files; new emits go toreplies/<ts>/meta.json. They don't collide (different names) and readers handle both, but a re-sync is wanted to retire the stale leaves.🤖 Generated with Claude Code