Conversation
Contributor
christse
commented
Feb 5, 2026
christse
commented
- maintain repo root to realm root mapping
- don't exclude index.json (as that could be a custom home page) when pushing to the GH repo.
The share command was incorrectly using the workspace name from the URL as a subfolder in the target repo. For example, sharing from a workspace called "steady-loon" would create "target-repo/steady-loon/" instead of copying files to the repo root. Changes: - Remove auto-detection of subfolder from workspace URL - Share now copies to repo root by default - Use --subfolder flag explicitly if a subfolder is needed - Update CLAUDE.md to document the correct behavior Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where the share command was incorrectly creating workspace folder names as subfolders in the target GitHub repository instead of copying to the repo root. It also enables sharing of index.json and cards-grid.json files, which may contain custom home pages.
Changes:
- Modified
detectSubfolderfunction to returnundefinedby default, preventing automatic subfolder creation - Removed
index.jsonandcards-grid.jsonfrom preserve and skip lists in the share command - Updated documentation to note that files are copied to repo root by default
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/commands/share.ts | Disabled auto-detection of subfolders and enabled sharing of index.json and cards-grid.json files |
| .claude/CLAUDE.md | Added documentation note about default copy behavior to repo root |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| **Share** copies workspace state to a GitHub repo branch: | ||
| - Copies to repo root by default (use `--subfolder` to target a specific directory) | ||
| - Preserves repo-level files (package.json, LICENSE, README, etc.) | ||
| - Skips realm-specific files (.realm.json, index.json, cards-grid.json) |
There was a problem hiding this comment.
The documentation is outdated. According to the changes in share.ts, index.json and cards-grid.json are no longer skipped when sharing to a GitHub repo. Only .realm.json is preserved/skipped now. This line should be updated to reflect that only .realm.json is skipped.
Suggested change
| - Skips realm-specific files (.realm.json, index.json, cards-grid.json) | |
| - Skips realm-specific files (.realm.json only) |
| // - index.json: contains realm-specific URLs and metadata | ||
| // - cards-grid.json: realm index card | ||
| const skipCopy = new Set(['.realm.json', 'index.json', 'cards-grid.json']); | ||
| const skipCopy = new Set(['.realm.json']); |
There was a problem hiding this comment.
There's an asymmetry between the share and gather commands. The share command now copies index.json and cards-grid.json to the GitHub repo (only skipping .realm.json), but the gather command still skips these files when pulling from the repo (line 99 in gather.ts). This creates an inconsistency where:
- share will overwrite index.json and cards-grid.json in the repo
- gather will not pull those changes back to the workspace
Consider whether gather.ts should also be updated to match this behavior, or if there's a specific reason for this asymmetry that should be documented.
Comment on lines
+348
to
355
| function detectSubfolder(_workspaceDir: string): string | undefined { | ||
| // Don't auto-detect subfolder from workspace URL | ||
| // This was causing the workspace folder name (e.g., "steady-loon") to be | ||
| // created as a subfolder in the target repo instead of copying to root. | ||
| // | ||
| // If you need to copy to a specific subfolder, use --subfolder explicitly. | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
The detectSubfolder function in share.ts now returns undefined to fix the subfolder bug, but the gather.ts command still has the old implementation that tries to auto-detect subfolders from the workspace URL. This creates an inconsistency:
- share will copy to repo root by default (unless --subfolder is explicitly provided)
- gather will try to read from a subfolder by default (extracted from workspace URL)
While gather has a fallback to root if the detected path doesn't exist (line 47 in gather.ts), this asymmetry could cause confusion. Consider updating gather.ts to use the same detectSubfolder logic for consistency, so both commands default to repo root.
christse
pushed a commit
that referenced
this pull request
Apr 20, 2026
Six fixes in one commit, addressing every Copilot comment except #7 (dead code, deferred to a later cleanup pass). #4 — manifest shape mismatch (critical): push.ts was still reading/writing the old manifest format (files[path] = hashString) while pull.ts and sync.ts use the new {localHash, remoteMtime} shape. push now uses the new shape, migrates old manifests on read (mirrors sync.ts detector), and refreshes remoteMtime via getRemoteMtimes() after a successful upload so the next pull/sync sees a consistent picture. #3 — partial-success manifest (important): In --batch mode we were marking every instance as synced when result.uploaded > 0, even if some failed. Now we derive failed paths from result.errors and only add successes to the manifest. Failed uploads stay out and get retried on the next run. #6 — ATOMIC_SOURCE_EXTENSIONS gap (important): EXTENSION_MAP was missing .tsx/.jsx/.cjs/.scss/.less/.sass, so those extensions got application/octet-stream and were routed through individual POST instead of /_atomic despite being listed as atomic-source compatible. Now mapped to application/typescript/javascript and text/x-* respectively, so isTextFile() returns true and they take the atomic path. #2 — CLI --version out of sync: program.version() was hardcoded to '1.0.0' while package.json is at 1.0.1. Now reads from package.json via createRequire so boxel --version stays accurate on every release. #1 — --batch-size NaN guard: parseInt on bad input (e.g. "abc") returned NaN, which bypasses `?? 10` and flows into the uploader as NaN. Now uses a parsePositiveInt parser that throws InvalidArgumentError with a friendly message on non-positive-integer input. #5 — invalid-JSON test expectation: The test expected data.type='file' but the code (correctly) emits 'source' because /_atomic only accepts 'card' and 'source' resource types. Test updated to match the correct contract. All 164 tests pass (was 163/164 before this commit). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
christse
pushed a commit
that referenced
this pull request
Apr 20, 2026
Six fixes in one commit, addressing every Copilot comment except #7 (dead code, deferred to a later cleanup pass). #4 — manifest shape mismatch (critical): push.ts was still reading/writing the old manifest format (files[path] = hashString) while pull.ts and sync.ts use the new {localHash, remoteMtime} shape. push now uses the new shape, migrates old manifests on read (mirrors sync.ts detector), and refreshes remoteMtime via getRemoteMtimes() after a successful upload so the next pull/sync sees a consistent picture. #3 — partial-success manifest (important): In --batch mode we were marking every instance as synced when result.uploaded > 0, even if some failed. Now we derive failed paths from result.errors and only add successes to the manifest. Failed uploads stay out and get retried on the next run. #6 — ATOMIC_SOURCE_EXTENSIONS gap (important): EXTENSION_MAP was missing .tsx/.jsx/.cjs/.scss/.less/.sass, so those extensions got application/octet-stream and were routed through individual POST instead of /_atomic despite being listed as atomic-source compatible. Now mapped to application/typescript/javascript and text/x-* respectively, so isTextFile() returns true and they take the atomic path. #2 — CLI --version out of sync: program.version() was hardcoded to '1.0.0' while package.json is at 1.0.1. Now reads from package.json via createRequire so boxel --version stays accurate on every release. #1 — --batch-size NaN guard: parseInt on bad input (e.g. "abc") returned NaN, which bypasses `?? 10` and flows into the uploader as NaN. Now uses a parsePositiveInt parser that throws InvalidArgumentError with a friendly message on non-positive-integer input. #5 — invalid-JSON test expectation: The test expected data.type='file' but the code (correctly) emits 'source' because /_atomic only accepts 'card' and 'source' resource types. Test updated to match the correct contract. All 164 tests pass (was 163/164 before this commit). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
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
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.