feat(dropbox): export relayfile adapter surfaces, webhook normalizer, and digests#118
Merged
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 52 minutes and 52 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR extends the Dropbox adapter to support folders, shared-folders, and shared-links as discoverable resource types. It refactors path mapping to use uniform, resource-type-aware builders, updates discovery configuration and digest event handling, adds auxiliary file emission for indices and aliases, introduces layout documentation and webhook normalization, and exports the new modules via updated package exports. ChangesDropbox Resource Type Expansion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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/dropbox/src/emit-auxiliary-files.ts`:
- Around line 72-76: Replace the hard-coded literal "/dropbox/_index.json" with
the typed path helper by importing dropboxRootIndexPath from the adapter's
path-mapper and passing input.workspaceId (and any required args) to it; update
the safeWrite call (the one that currently passes "/dropbox/_index.json") to use
dropboxRootIndexPath(...) instead so path construction is centralized and
consistent with the path-mapper contract.
- Around line 120-155: When upserting aliases in the records loop, detect and
remove stale by-path aliases when an item's path_lower changes (and also handle
deletes that lack path_lower) by: before setting rows.set(id,...), look up the
existing entry for this id (rows.get(id)) to obtain the previous
path_lower/canonicalPath; if the previous path_lower exists and is different
from the new path_lower, call safeDelete(client, workspaceId,
dropboxFileByPathAliasPath(previousPathLower), aggregate) (use the same
dropboxFileByPathAliasPath helper and by-path alias logic used for writes);
apply the same change for the folder-handling block (lines ~173-205) so
moves/renames remove the old by-path alias and for delete records fallback to
the stored previous path_lower when record.path_lower is missing.
In `@packages/dropbox/src/path-mapper.ts`:
- Around line 55-69: dropboxFilePath and dropboxFolderPath (and the
computeDropboxPath logic handling file/folder types) currently emit path-derived
filenames; change them to emit canonical flat-record filenames in the form
<slug>__<id>.json under the respective resource directory (e.g.,
`${DROPBOX_PATH_ROOT}/files/` and `${DROPBOX_PATH_ROOT}/folders/`). Use the
existing slug utilities (slugifyAlias and aliasCollisionSuffix from
packages/core/src/alias-slug.ts) to produce the slug and append the collision
suffix when needed, then concatenate as `${slug}__${id}.json`; remove reliance
on encodedPathOrId for these flat records. Update computeDropboxPath branches
for file and folder to call the new dropboxFilePath/dropboxFolderPath behavior
so canonical paths are stable on rename/move.
- Around line 153-160: parseRelayfilePath is stripping ".json" from every path
segment and treating any path that starts with LIFECYCLE_RESOURCE_PATH as a
lifecycle record even when there is no trailing id; change the segmentation so
decodeURIComponent is applied to all segments but the ".json" suffix is only
removed from the terminal segment (use normalized.split('/').filter(Boolean) ->
map decode then for last segment strip /\.json$/u), and change the lifecycle
detection (where lifecycleSegments and the every(...) comparison is used) to
require an extra segment (e.g., segments.length > lifecycleSegments.length)
before returning a lifecycle resource with id = last segment; update references
in parseRelayfilePath, segments, LIFECYCLE_RESOURCE_PATH, lifecycleSegments
accordingly.
In `@packages/dropbox/src/resources.ts`:
- Around line 15-24: The pathPattern regexes for the 'files' and 'folders'
resources are too broad and match auxiliary paths like _index.json and by-id/*;
update the pathPattern entries (for the resources defined alongside name:
'files' and name: 'folders') to only match a single filename segment ending in
.json (or explicitly exclude auxiliary prefixes) so they don't capture nested
paths or reserved filenames, and keep findResourceByPath behavior correct;
adjust the patterns accordingly (e.g., require no additional slashes after
/dropbox/files/ or use a negative lookahead to exclude _index.json, by-id/,
by-path/) and verify the idPattern use 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: c3508453-e150-4253-bf14-acc3f6c9946f
📒 Files selected for processing (15)
packages/dropbox/discovery/dropbox/folders/.create.example.jsonpackages/dropbox/discovery/dropbox/folders/.schema.jsonpackages/dropbox/discovery/dropbox/shared-folders/.create.example.jsonpackages/dropbox/discovery/dropbox/shared-folders/.schema.jsonpackages/dropbox/discovery/dropbox/shared-links/.create.example.jsonpackages/dropbox/discovery/dropbox/shared-links/.schema.jsonpackages/dropbox/package.jsonpackages/dropbox/src/digest.test.tspackages/dropbox/src/digest.tspackages/dropbox/src/emit-auxiliary-files.tspackages/dropbox/src/index.tspackages/dropbox/src/layout-prompt.tspackages/dropbox/src/path-mapper.tspackages/dropbox/src/resources.tspackages/dropbox/src/webhook-normalizer.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
13 issues found across 15 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/dropbox/src/path-mapper.ts">
<violation number="1" location="packages/dropbox/src/path-mapper.ts:4">
P1: This changes object path shape in a breaking way (removes account scope) without a compatibility path.</violation>
</file>
<file name="packages/dropbox/package.json">
<violation number="1" location="packages/dropbox/package.json:3">
P2: Do not bump the package version in this feature PR; versioning is handled by the publish workflow.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Member
Author
|
Addressed review feedback in commit
Validation run in this branch:
All passing locally. |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
packages/dropbox/src/path-mapper.ts (1)
92-98:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep shared-folder and shared-link records on the canonical flat-record format.
These helpers still emit
<id>.json, while the rest of the Dropbox flat records now use<slug>__<id>.json. That leavesshared-foldersandshared-linksoutside the repository path contract. Threadnamethrough here and reusestableLeafName(...)for both resource types.As per coding guidelines,
Flat records (entities with no sub-artifacts) must use <slug>__<id>.json at a canonical resource directory.🤖 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/dropbox/src/path-mapper.ts` around lines 92 - 98, The dropboxSharedFolderPath and dropboxSharedLinkPath helpers must produce canonical flat-record filenames using stableLeafName, so update both functions to accept the resource name (thread/name) in addition to the id (e.g., dropboxSharedFolderPath(name: string, sharedFolderId: string) and dropboxSharedLinkPath(name: string, sharedLinkId: string)), call stableLeafName(name, id) to build the filename (instead of `<id>.json`), and return `${DROPBOX_PATH_ROOT}/shared-folders/${stableLeafName(...)}.json` and `${DROPBOX_PATH_ROOT}/shared-links/${stableLeafName(...)}.json`; remove or stop relying on encodePathSegment for the final leaf since stableLeafName handles canonicalization.packages/dropbox/src/resources.ts (1)
31-39:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExclude shared
_index.jsonfiles from canonical resource matching.The direct-segment arm still matches
_index.json, sofindResourceByPath('/dropbox/shared-folders/_index.json')and the shared-links equivalent resolve as writeback resources instead of auxiliary indices.Suggested regex tightening
- pathPattern: /^\/dropbox\/shared-folders\/(?:[^/]+|by-id\/[^/]+)\.json$/, + pathPattern: /^\/dropbox\/shared-folders\/(?:(?!_index\.json$)[^/]+|by-id\/[^/]+)\.json$/, @@ - pathPattern: /^\/dropbox\/shared-links\/(?:[^/]+|by-id\/[^/]+)\.json$/, + pathPattern: /^\/dropbox\/shared-links\/(?:(?!_index\.json$)[^/]+|by-id\/[^/]+)\.json$/,Also applies to: 46-47
🤖 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/dropbox/src/resources.ts` around lines 31 - 39, The pathPattern for shared-folders and shared-links currently matches `_index.json`; update those regexes (the pathPattern entries for the resources named 'shared-folders' and 'shared-links') to exclude `_index.json` by adding a negative lookahead for the direct segment (and the by-id segment) — e.g. replace the `[^/]+` segments with `(?!_index)[^/]+` (and the `by-id\/[^/]+` with `by-id\/(?!_index)[^/]+`) so `findResourceByPath('/dropbox/.../_index.json')` no longer resolves as a writeback resource.
🤖 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/dropbox/src/__tests__/emit-auxiliary-files.test.ts`:
- Around line 59-178: Add deterministic collision tests for both alias subtrees:
one test that verifies emitDropboxAuxiliaryFiles handles a by-id collision
(e.g., when a new write would overwrite an existing by-id alias path) and one
for a by-path collision (e.g., conflicting pathLower alias); use createClient to
seed the index and existing alias writes/deletes, call emitDropboxAuxiliaryFiles
with the conflicting input, and assert errors include the specific alias path
and that no unsafe overwrite occurred. Reference emitDropboxAuxiliaryFiles and
the alias helpers dropboxFileByIdAliasPath, dropboxFolderByIdAliasPath,
dropboxFileByPathAliasPath, and dropboxFolderByPathAliasPath to locate where to
assert collision behavior. Ensure each alias subtree (by-id and by-path) has at
least one deterministic test in this file following the existing test patterns.
In `@packages/dropbox/src/__tests__/path-mapper.test.ts`:
- Around line 13-67: The tests currently assert individual outputs but must also
verify full compose->parse->equality round-trips for each mapper helper; add
tests that call each composer (dropboxFilePath, dropboxFolderPath,
computeDropboxPath, toObjectRelayfilePath) to produce a path, then feed that
path into parseRelayfilePath and assert the parsed result reconstructs the
original identifying pieces (resource, id, objectType/name/path segments as
applicable) so the composed path round-trips back to the same logical identity;
ensure you add cases for files, folders, shared-folders, lifecycle entries, and
the computeDropboxPath stability case so each helper has a corresponding
compose→parse→equality assertion.
In `@packages/dropbox/src/path-mapper.ts`:
- Around line 171-188: The branch logic uses objectType checks that only match
hyphenated forms, causing values like "DropboxSharedFolder", "sharedfolder",
"sharedlink" to fall through; normalize objectType before routing (reuse the
same normalization used by toObjectRelayfilePath or add a small helper) so that
variants become the hyphenated canonical forms ("shared-folder", "shared-link")
or pass the already-normalized value from toObjectRelayfilePath into this
switch; update the checks around the objectType variable and calls to
computeDropboxPath to use the normalized value (refer to objectType,
computeDropboxPath and toObjectRelayfilePath in the diff).
---
Duplicate comments:
In `@packages/dropbox/src/path-mapper.ts`:
- Around line 92-98: The dropboxSharedFolderPath and dropboxSharedLinkPath
helpers must produce canonical flat-record filenames using stableLeafName, so
update both functions to accept the resource name (thread/name) in addition to
the id (e.g., dropboxSharedFolderPath(name: string, sharedFolderId: string) and
dropboxSharedLinkPath(name: string, sharedLinkId: string)), call
stableLeafName(name, id) to build the filename (instead of `<id>.json`), and
return `${DROPBOX_PATH_ROOT}/shared-folders/${stableLeafName(...)}.json` and
`${DROPBOX_PATH_ROOT}/shared-links/${stableLeafName(...)}.json`; remove or stop
relying on encodePathSegment for the final leaf since stableLeafName handles
canonicalization.
In `@packages/dropbox/src/resources.ts`:
- Around line 31-39: The pathPattern for shared-folders and shared-links
currently matches `_index.json`; update those regexes (the pathPattern entries
for the resources named 'shared-folders' and 'shared-links') to exclude
`_index.json` by adding a negative lookahead for the direct segment (and the
by-id segment) — e.g. replace the `[^/]+` segments with `(?!_index)[^/]+` (and
the `by-id\/[^/]+` with `by-id\/(?!_index)[^/]+`) so
`findResourceByPath('/dropbox/.../_index.json')` no longer resolves as a
writeback resource.
🪄 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: a90f2a09-5df9-425f-b131-3548695eb390
📒 Files selected for processing (9)
packages/dropbox/package.jsonpackages/dropbox/src/__tests__/emit-auxiliary-files.test.tspackages/dropbox/src/__tests__/layout-prompt.test.tspackages/dropbox/src/__tests__/path-mapper.test.tspackages/dropbox/src/__tests__/resources.test.tspackages/dropbox/src/emit-auxiliary-files.tspackages/dropbox/src/layout-prompt.tspackages/dropbox/src/path-mapper.tspackages/dropbox/src/resources.ts
✅ Files skipped from review due to trivial changes (1)
- packages/dropbox/src/layout-prompt.ts
There was a problem hiding this comment.
2 issues found across 9 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Member
Author
|
Addressed the newly raised feedback in commit be673ad:\n\n- Normalized Dropbox object type routing in / so non-hyphen forms like and map correctly.\n- Added explicit compose→parse round-trip assertions for mapper helpers (, , , ) including shared-folder/shared-link and lifecycle cases.\n- Added deterministic alias collision tests in for both and write-failure paths, asserting error surfacing and no unsafe overwrite.\n- Added backward-compat coverage for legacy path-like helper inputs.\n\nValidation rerun:\n- \n- \n- \n\nAll passed locally. |
Member
Author
|
Addressed the newly raised feedback in commit
Validation rerun:
All passed locally. |
Member
Author
|
Follow-up pass for remaining unresolved feedback landed in commit
Validation rerun:
All passed locally. |
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/dropbox/src/__tests__/resources.test.ts (1)
16-27:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate shared canonical-path assertions to the slug+id format.
This test still treats
/dropbox/shared-folders/<id>.jsonas canonical. Given the canonical filename contract is<slug>__<id>.json, this can preserve/validate the wrong behavior and miss regressions on current path mapping.Suggested test adjustment
test('dropbox shared resources match canonical and by-id alias paths', () => { - assert.equal(findResourceByPath('/dropbox/shared-folders/845281924.json')?.name, 'shared-folders'); + assert.equal( + findResourceByPath('/dropbox/shared-folders/finance-team__845281924.json')?.name, + 'shared-folders', + ); + assert.equal( + findResourceByPath('/dropbox/shared-links/project-plan__url_1234567890abcdef.json')?.name, + 'shared-links', + ); assert.equal( findResourceByPath('/dropbox/shared-folders/by-id/845281924.json')?.name, 'shared-folders', );As per coding guidelines, “Flat records (entities with no sub-artifacts) must use
<slug>__<id>.jsonat a canonical resource directory.”🤖 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/dropbox/src/__tests__/resources.test.ts` around lines 16 - 27, The test "dropbox shared resources match canonical and by-id alias paths" wrongly assumes canonical filenames like '/dropbox/shared-folders/845281924.json'; update assertions to use the slug__id canonical format (e.g. '/dropbox/shared-folders/shared-folders__845281924.json') and similarly for shared-links (e.g. 'shared-links__url_1234567890abcdef.json'), while still checking the by-id alias paths with '/by-id/<id>.json' via findResourceByPath; modify the expectations in the test block that calls findResourceByPath (and keep the checks for _index.json returning undefined) so they validate canonical slug__id filenames instead of bare id filenames, referencing the existing findResourceByPath helper and the test name to locate the changes.
🧹 Nitpick comments (3)
packages/dropbox/src/path-mapper.ts (2)
198-204: 💤 Low valueRedundant branching—all paths call the same function with identical arguments.
Lines 198-204 have three conditional branches that all call
computeDropboxPath(objectType, objectId, { path, name }). SincecomputeDropboxPathalready handles type routing internally, the branching here adds no value and can be collapsed.Suggested simplification
- if (objectType === 'folder') { - return computeDropboxPath(objectType, objectId, { path, name }); - } - if (objectType === 'shared-folder' || objectType === 'shared-link') { - return computeDropboxPath(objectType, objectId, { path, name }); - } - return computeDropboxPath(objectType, objectId, { path, name }); + return computeDropboxPath(objectType, objectId, { path, name });🤖 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/dropbox/src/path-mapper.ts` around lines 198 - 204, The three conditional branches that call computeDropboxPath(objectType, objectId, { path, name }) are redundant; remove the if/else blocks and replace them with a single direct call to computeDropboxPath using the existing variables (objectType, objectId, path, name), since computeDropboxPath already handles routing by type and the extra branching adds no value.
254-269: ⚡ Quick winOverly permissive fallback matching in type normalization.
The conditions
normalized.includes('shared') && normalized.includes('folder')andnormalized.includes('shared') && normalized.includes('link')are very loose. Strings like"unshared-folder-info","shared-not-a-link", or"folder-shared-with"would incorrectly match. Consider restricting to known variations or removing the catch-all fallbacks.Suggested tightening
if ( normalized === 'shared-folder' || normalized === 'sharedfolder' || - normalized === 'dropboxsharedfolder' || - (normalized.includes('shared') && normalized.includes('folder')) + normalized === 'dropboxsharedfolder' ) { return 'shared-folder'; } if ( normalized === 'shared-link' || normalized === 'sharedlink' || - normalized === 'dropboxsharedlink' || - (normalized.includes('shared') && normalized.includes('link')) + normalized === 'dropboxsharedlink' ) { return 'shared-link'; }🤖 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/dropbox/src/path-mapper.ts` around lines 254 - 269, The loose fallback checks using normalized.includes('shared') && normalized.includes('folder') (and similarly for 'link') will mis-classify many strings; replace those broad includes with a whitelist or precise word-boundary regexes that only match intended variants. Update the conditional blocks that inspect the normalized variable (the two if blocks returning 'shared-folder' and 'shared-link') to either check against an explicit array of allowed alternatives (e.g., ['shared-folder','sharedfolder','dropboxsharedfolder','dropbox-shared-folder']) or use /\b(shared|dropboxshared|sharedfolder)([-_]?(folder))\b/ and a corresponding regex for link so only legitimate forms match. Ensure you apply the same tightening to both the folder and link branches and remove the generic includes-based fallback.packages/dropbox/src/resources.ts (1)
31-31: ⚡ Quick winInconsistent
by-idalias handling between resource types.The
filesandfolderspatterns (Lines 15, 23) use[^/]+which correctly excludesby-id/andby-path/alias paths from matching as writeback resources. However,shared-foldersandshared-linksexplicitly includeby-id\/...as an alternative, meaning/dropbox/shared-folders/by-id/foo.jsonwill match as a valid writeback resource.If auxiliary alias paths should not be writeback targets (consistent with the fix applied to files/folders), align the shared resource patterns:
Suggested fix to exclude by-id aliases
{ name: 'shared-folders', path: '/dropbox/shared-folders', - pathPattern: /^\/dropbox\/shared-folders\/(?:(?!_index\.json$)[^/]+|by-id\/(?!_index\.json$)[^/]+)\.json$/, + pathPattern: /^\/dropbox\/shared-folders\/(?!_index\.json$)[^/]+\.json$/, idPattern: /^[A-Za-z0-9_.:`@-`]+$/, schema: 'discovery/dropbox/shared-folders/.schema.json', createExample: 'discovery/dropbox/shared-folders/.create.example.json', }, { name: 'shared-links', path: '/dropbox/shared-links', - pathPattern: /^\/dropbox\/shared-links\/(?:(?!_index\.json$)[^/]+|by-id\/(?!_index\.json$)[^/]+)\.json$/, + pathPattern: /^\/dropbox\/shared-links\/(?!_index\.json$)[^/]+\.json$/, idPattern: /^[A-Za-z0-9_.:`@-`]+$/, schema: 'discovery/dropbox/shared-links/.schema.json', createExample: 'discovery/dropbox/shared-links/.create.example.json', },Also applies to: 39-39
🤖 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/dropbox/src/resources.ts` at line 31, The shared resource pathPattern for shared-folders/shared-links currently allows the explicit "by-id/..." alternative so paths like /dropbox/shared-folders/by-id/foo.json match; change the regex to match the same style as files/folders by removing the "by-id\/(?!_index\.json$)[^/]+" alternative (or otherwise ensure the segment rejects "by-id" and "by-path" prefixes) so that shared-folders and shared-links use the same exclusion logic as files and folders; update the pathPattern for shared-folders and shared-links to use the single-segment form (e.g., the same (?:(?!_index\.json$)[^/]+) approach) so alias paths are not treated as writeback targets.
🤖 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.
Outside diff comments:
In `@packages/dropbox/src/__tests__/resources.test.ts`:
- Around line 16-27: The test "dropbox shared resources match canonical and
by-id alias paths" wrongly assumes canonical filenames like
'/dropbox/shared-folders/845281924.json'; update assertions to use the slug__id
canonical format (e.g. '/dropbox/shared-folders/shared-folders__845281924.json')
and similarly for shared-links (e.g. 'shared-links__url_1234567890abcdef.json'),
while still checking the by-id alias paths with '/by-id/<id>.json' via
findResourceByPath; modify the expectations in the test block that calls
findResourceByPath (and keep the checks for _index.json returning undefined) so
they validate canonical slug__id filenames instead of bare id filenames,
referencing the existing findResourceByPath helper and the test name to locate
the changes.
---
Nitpick comments:
In `@packages/dropbox/src/path-mapper.ts`:
- Around line 198-204: The three conditional branches that call
computeDropboxPath(objectType, objectId, { path, name }) are redundant; remove
the if/else blocks and replace them with a single direct call to
computeDropboxPath using the existing variables (objectType, objectId, path,
name), since computeDropboxPath already handles routing by type and the extra
branching adds no value.
- Around line 254-269: The loose fallback checks using
normalized.includes('shared') && normalized.includes('folder') (and similarly
for 'link') will mis-classify many strings; replace those broad includes with a
whitelist or precise word-boundary regexes that only match intended variants.
Update the conditional blocks that inspect the normalized variable (the two if
blocks returning 'shared-folder' and 'shared-link') to either check against an
explicit array of allowed alternatives (e.g.,
['shared-folder','sharedfolder','dropboxsharedfolder','dropbox-shared-folder'])
or use /\b(shared|dropboxshared|sharedfolder)([-_]?(folder))\b/ and a
corresponding regex for link so only legitimate forms match. Ensure you apply
the same tightening to both the folder and link branches and remove the generic
includes-based fallback.
In `@packages/dropbox/src/resources.ts`:
- Line 31: The shared resource pathPattern for shared-folders/shared-links
currently allows the explicit "by-id/..." alternative so paths like
/dropbox/shared-folders/by-id/foo.json match; change the regex to match the same
style as files/folders by removing the "by-id\/(?!_index\.json$)[^/]+"
alternative (or otherwise ensure the segment rejects "by-id" and "by-path"
prefixes) so that shared-folders and shared-links use the same exclusion logic
as files and folders; update the pathPattern for shared-folders and shared-links
to use the single-segment form (e.g., the same (?:(?!_index\.json$)[^/]+)
approach) so alias paths are not treated as writeback targets.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 95f1dc45-98ab-416a-a071-2ce460b71625
📒 Files selected for processing (7)
packages/dropbox/src/__tests__/emit-auxiliary-files.test.tspackages/dropbox/src/__tests__/path-mapper.test.tspackages/dropbox/src/__tests__/resources.test.tspackages/dropbox/src/emit-auxiliary-files.tspackages/dropbox/src/layout-prompt.tspackages/dropbox/src/path-mapper.tspackages/dropbox/src/resources.ts
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
Validation