Skip to content

Conversation

@akash1520
Copy link

@akash1520 akash1520 commented Sep 20, 2025

Fixes issue where GitHub automatically converts spaces to dashes in repo names but Dyad continued referencing the original names with spaces, causing connection and push failures.


Summary by cubic

Sanitizes GitHub repo and local app folder names (spaces → dashes) to match GitHub behavior. Fixes failures when creating, connecting, or pushing to repos with spaces.

  • Bug Fixes
    • Added sanitizeAppFolderName and ensureAppFolderSanitized to rename folders, update DB (path/githubRepo), and guard against conflicts.
    • Create repo: always uses sanitized folder name; stores sanitized repo; repo param is now optional.
    • Connect existing: sanitizes repo before lookup and save; clearer error includes attempted sanitized name.
    • Push: sanitizes folder first; normalizes and persists githubRepo before building the remote URL.
    • Updated IPC handler typing for github:create-repo.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file

Prompt for AI agents (all 1 issues)

Understand the root cause of the following 1 issues and fix them.


<file name="src/ipc/handlers/github_handlers.ts">

<violation number="1" location="src/ipc/handlers/github_handlers.ts:495">
Remove unused repo from args type; it’s not used by handleCreateRepo and can confuse callers.</violation>
</file>


Since this is your first cubic review, here's how it works:

  • cubic automatically reviews your code and comments on bugs and improvements
  • Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
  • Ask questions if you need clarification on any suggestion

React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.

@akash1520 akash1520 force-pushed the imp-github-reponame-sanitization branch from fbcc2fb to 7d621f8 Compare September 20, 2025 18:17
@wwwillchen
Copy link
Contributor

thanks for the PR. this fix feels a bit heavy-handed as it's actually renaming the folder as part of the git sync (which seems quite surprising from a user perspective). I don't think we need to rename the app folder (IIUC) to fix this issue.

Also, let's add an e2e test https://github.com/dyad-sh/dyad/blob/main/CONTRIBUTING.md#e2e-tests - probably add a test case here: https://github.com/dyad-sh/dyad/blob/main/e2e-tests/github.spec.ts

@akash1520 akash1520 force-pushed the imp-github-reponame-sanitization branch 5 times, most recently from 3b4b210 to 51b4023 Compare September 23, 2025 09:40
@akash1520 akash1520 force-pushed the imp-github-reponame-sanitization branch from 51b4023 to 56ce6fe Compare September 23, 2025 09:46
@akash1520
Copy link
Author

thanks for the PR. this fix feels a bit heavy-handed as it's actually renaming the folder as part of the git sync (which seems quite surprising from a user perspective). I don't think we need to rename the app folder (IIUC) to fix this issue.

Also, let's add an e2e test https://github.com/dyad-sh/dyad/blob/main/CONTRIBUTING.md#e2e-tests - probably add a test case here: https://github.com/dyad-sh/dyad/blob/main/e2e-tests/github.spec.ts

@wwwillchen Got rid of folder renaming, plus added an e2e test.

Fixes issue where GitHub automatically converts spaces to dashes in repo names
but Dyad continued referencing the original names with spaces, causing
connection and push failures by introducing a sanitization step for repo names.
@akash1520 akash1520 force-pushed the imp-github-reponame-sanitization branch from 56ce6fe to c041ba5 Compare September 25, 2025 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants