fix: improve sync error messages to distinguish bad URLs from incompatible repo structures#33
Conversation
WalkthroughAdds URL validation to the sync flow, surfaces URL-validation errors immediately, replaces the single "no relevant files" message with structured, type-aware structure errors, and updates notifications to show categorized failure counts with per-repo reasons in the details view. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant SyncManager as SyncManager
participant Validator as validateRepositoryUrl()
participant Scanner as RepoScanner
participant Results as ResultsStore
participant Notifier as Notifications
Client->>SyncManager: start sync of multiple repos
loop for each repo
SyncManager->>Validator: validate repo URL
alt URL invalid
Validator-->>SyncManager: return URL error
SyncManager->>Results: record URL error for repo
SyncManager-->>Client: skip repo
else URL valid
Validator-->>SyncManager: valid
SyncManager->>Scanner: scan repo for relevant files
alt No relevant files
Scanner-->>SyncManager: none found
SyncManager->>Results: compute enabled types -> create structureError -> record for repo
else Relevant files found
Scanner-->>SyncManager: files and types
SyncManager->>Results: record success
end
end
end
SyncManager->>Notifier: request partial-sync summary
Notifier->>Results: read per-repo errors
Notifier->>Notifier: categorize & count (invalid URL, incompatible structure, other)
Notifier-->>Client: show partial sync toast with categorized failure summary and "Show Details"/"Retry Failed"
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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.
Pull request overview
Improves sync error messages in the Promptitude extension to help users distinguish between malformed repository URLs and repos that lack supported folder structures.
Changes:
- Added
validateRepositoryUrl()insyncManager.tsto reject GitHub URLs with extra path segments before making network calls. - Replaced the generic "No relevant files found" error with context-specific messages listing enabled sync types and the scanned branch.
- Updated
showPartialSyncSuccess()innotifications.tsto categorize and summarize failures in the toast notification.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/syncManager.ts | Added URL validation and improved error messages for empty sync results |
| src/utils/notifications.ts | Categorized failure counts in partial sync toast message |
| CHANGELOG.md | Documented the improvement under [Unreleased] |
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/utils/notifications.ts (1)
54-61: Consider using constants for error category markers.The string matching (
'Invalid URL','Incompatible repository','No sync types enabled') creates a tight coupling betweensyncManager.tsandnotifications.ts. If error message wording changes, categorization silently breaks.Consider extracting shared constants or using structured error objects with an explicit
categoryfield for more robust categorization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/notifications.ts` around lines 54 - 61, The current categorization loop in notifications.ts relies on hard-coded substrings ('Invalid URL', 'Incompatible repository', 'No sync types enabled') which couples it to syncManager.ts; instead define shared error category constants (e.g., INVALID_URL, INCOMPATIBLE_REPO, NO_SYNC_TYPES) in a common module or change syncManager.ts to throw/return structured error objects with a category property, then update the loop in notifications.ts to compare against those constants or inspect err.category (refer to the errors iteration and variables badUrlCount/badStructureCount/otherCount to locate the logic) so message wording changes won't break categorization.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/utils/notifications.ts`:
- Around line 50-70: The pluralization for the badStructureCount category is
inconsistent: update the failParts push for badStructureCount in
src/utils/notifications.ts (the block that builds failParts using badUrlCount,
badStructureCount, otherCount) to conditionally append an "s" when
badStructureCount > 1 (e.g., use `${badStructureCount} incompatible
structure${badStructureCount > 1 ? 's' : ''}`) so it matches the pluralization
pattern used for badUrlCount and otherCount and keeps the failSummary/message
strings correct.
---
Nitpick comments:
In `@src/utils/notifications.ts`:
- Around line 54-61: The current categorization loop in notifications.ts relies
on hard-coded substrings ('Invalid URL', 'Incompatible repository', 'No sync
types enabled') which couples it to syncManager.ts; instead define shared error
category constants (e.g., INVALID_URL, INCOMPATIBLE_REPO, NO_SYNC_TYPES) in a
common module or change syncManager.ts to throw/return structured error objects
with a category property, then update the loop in notifications.ts to compare
against those constants or inspect err.category (refer to the errors iteration
and variables badUrlCount/badStructureCount/otherCount to locate the logic) so
message wording changes won't break categorization.
🪄 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
Run ID: b08b2502-1e29-4d27-a476-d5a10504d96b
📒 Files selected for processing (3)
CHANGELOG.mdsrc/syncManager.tssrc/utils/notifications.ts
6d4e872 to
94d9101
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/syncManager.ts (1)
417-423: Avoid drift between file filtering and structure-error messaging.This block duplicates sync-type path logic and omits legacy chatmode paths used by
filterRelevantFiles. Centralize enabled-path derivation (or reuse one helper) so diagnostics always match actual filtering behavior.Suggested patch
- const enabledTypes: string[] = []; - if (this.config.syncChatmode) { enabledTypes.push(REPO_SYNC_CHAT_MODE_PATH); } + const enabledTypes: string[] = []; + if (this.config.syncChatmode) { + enabledTypes.push( + REPO_SYNC_CHAT_MODE_PATH, + REPO_SYNC_CHAT_MODE_LEGACY_PATH, + REPO_SYNC_CHAT_MODE_LEGACY_SINGULAR_PATH + ); + } if (this.config.syncInstructions) { enabledTypes.push(REPO_SYNC_INSTRUCTIONS_PATH); } if (this.config.syncPrompt) { enabledTypes.push(REPO_SYNC_PROMPT_PATH); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/syncManager.ts` around lines 417 - 423, The structureError message builds enabledTypes inline and can drift from the actual file filtering logic (filterRelevantFiles) and misses legacy chatmode paths; extract the enabled-path derivation into a single helper (e.g., getEnabledSyncPaths or reuse the existing filter helper) and have both this.block (where enabledTypes/structureError is created) and filterRelevantFiles call that helper; ensure the helper reads this.config.syncChatmode/syncInstructions/syncPrompt and returns the exact set of path constants (including legacy REPO_SYNC_CHAT_MODE_PATH variants) used for filtering so diagnostics and filtering remain identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/syncManager.ts`:
- Around line 339-341: The current check inside the URL validation branch that
inspects parsed.hostname === 'github.com' and segments.length !== 2 returns a
message that always calls the problem a "sub-path", which is misleading for URLs
with too few segments (e.g., https://github.com/owner). Update the conditional
handling around the parsed.hostname/segments logic to distinguish
segments.length < 2 (return a message saying the owner/repo is missing and show
expected format) from segments.length > 2 (return the existing message about
extra sub-paths and suggest removing /tree/…); ensure the returned messages
reference parsed.pathname or similar context so users get precise guidance.
---
Nitpick comments:
In `@src/syncManager.ts`:
- Around line 417-423: The structureError message builds enabledTypes inline and
can drift from the actual file filtering logic (filterRelevantFiles) and misses
legacy chatmode paths; extract the enabled-path derivation into a single helper
(e.g., getEnabledSyncPaths or reuse the existing filter helper) and have both
this.block (where enabledTypes/structureError is created) and
filterRelevantFiles call that helper; ensure the helper reads
this.config.syncChatmode/syncInstructions/syncPrompt and returns the exact set
of path constants (including legacy REPO_SYNC_CHAT_MODE_PATH variants) used for
filtering so diagnostics and filtering remain identical.
🪄 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
Run ID: 4d63e8ae-03b4-473a-b60e-6c940575e813
📒 Files selected for processing (3)
CHANGELOG.mdsrc/syncManager.tssrc/utils/notifications.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- CHANGELOG.md
- src/utils/notifications.ts
f7d1145 to
10a05d9
Compare
d46a2da to
40de0b9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/utils/notifications.ts (1)
54-62: Consider using error codes or structured error objects for more robust categorization.The current substring matching (
err.includes('Invalid URL'), etc.) works but is fragile—if error message wording changes insyncManager.ts, this categorization will silently break. A future improvement could use error codes or a structured error type instead of string parsing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/notifications.ts` around lines 54 - 62, Replace fragile substring matching in the error aggregation loop with structured error checks: have syncManager.ts throw either a custom Error subclass (e.g., SyncError) or error objects containing a stable code property, then in notifications.ts (the loop that updates badUrlCount, badStructureCount, otherCount) switch on err.code or use instanceof SyncError to classify errors instead of err.includes(...); update handling to treat missing code as "other" and add/align the new codes (e.g., INVALID_URL, INCOMPATIBLE_REPO, NO_SYNC_TYPES) where those errors are created so the categorization becomes robust to message text changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/syncManager.ts`:
- Around line 332-370: The validateRepositoryUrl function is missing a protocol
check, so URLs using http or other schemes can incorrectly pass; update
validateRepositoryUrl to verify parsed.protocol === 'https:' (for the hosts
github.com, dev.azure.com, and *.visualstudio.com) and return a clear error like
"Invalid URL – expected https://..." when the protocol is not https, before
performing the existing pathname/segment validation in validateRepositoryUrl.
---
Nitpick comments:
In `@src/utils/notifications.ts`:
- Around line 54-62: Replace fragile substring matching in the error aggregation
loop with structured error checks: have syncManager.ts throw either a custom
Error subclass (e.g., SyncError) or error objects containing a stable code
property, then in notifications.ts (the loop that updates badUrlCount,
badStructureCount, otherCount) switch on err.code or use instanceof SyncError to
classify errors instead of err.includes(...); update handling to treat missing
code as "other" and add/align the new codes (e.g., INVALID_URL,
INCOMPATIBLE_REPO, NO_SYNC_TYPES) where those errors are created so the
categorization becomes robust to message text changes.
🪄 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
Run ID: b7636f10-558a-424c-aaaa-8b7f48781b95
📒 Files selected for processing (3)
CHANGELOG.mdsrc/syncManager.tssrc/utils/notifications.ts
…tible repo structures
29d6323 to
8ac7b8e
Compare
When repository sync fails, Promptitude shows a generic "No relevant files found" for every failure — whether the configured URL is malformed (e.g. https://github.com/org/repo/tree/main/.github) or the repository simply doesn't use a supported folder layout. Users have no way to tell what went wrong without digging into debug logs.
syncManager.ts:validateRepositoryUrl()runs before any network call and rejects GitHub URLs that contain extra path segments (like/tree/…), with a clear message explaining what format is expectedAddedvalidateRepositoryUrl— runs before any network call and rejects GitHub URLs that contain extra path segments (like/tree/…), with a clear message explaining what format is expected.notifications.tsshowPartialSyncSuccess()to categorize failures in the summary toast (e.g. "Failures: 1 bad URL, 2 incompatible structure") so users can see the breakdown without clicking "Show Details."Updated showPartialSyncSuccess() to categorize failures in the summary toast (e.g. "Failures: 1 bad URL, 2 incompatible structure") so users can see the breakdown without clicking "Show Details.".CHANGELOG.md.../tree/main/.githubInvalid URL – expected https://github.com/owner/repo but got a sub-path (…)Incompatible repository – no .md/.txt files found under prompts/, instructions/ on branch "main"No sync types enabled – enable at least one of syncChatmode, syncInstructions, or syncPrompt in settings.⚠️ Partial sync completed! 0 items from 1/4 repositories.⚠️ Partial sync: 0 items from 1/4 repos. Failures: 1 bad URL, 2 incompatible structure.Verified with npm run compile — no errors.
Tested against repos: n
ventive/dev-prompt-kit(passes),itschanges/awesome-chatgpt-prompts(incompatible structure),openai/openai-cookbook/tree/main/.github(bad URL),openai/openai-cookbook(incompatible structure).Summary by CodeRabbit