fix(core): emit portable paths in generated smrt-register manifests - #2348
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 272347249a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| return { | ||
| ...objectDef, | ||
| filePath: relative(root, filePath).split(sep).join('/'), |
There was a problem hiding this comment.
Omit out-of-root paths instead of relativizing them
When an external object's absolute filePath is outside projectRoot—a shape already exercised by the package-registration fixtures—relative(root, filePath) produces values such as ../../Users/alice/cache/pkg.ts. This passes the new “not absolute” assertion but still embeds host-specific directory names and varies with the checkout's nesting depth, so the tracked smrt-register.ts can continue to churn or leak machine details. Detect paths outside the project and omit filePath or replace it with a package/module-relative identifier rather than serializing that traversal.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR fixes a determinism/portability issue in the SvelteKit generator where generated smrt-register.ts embedded machine-specific absolute filePath values inside the inlined registration manifest JSON, making tracked generated output uncommittable and leaving consumer worktrees dirty (issue #2341).
Changes:
- Introduces a normalization step when embedding object definitions into the generated registration manifests so absolute
filePathvalues become project-relative with forward slashes. - Adds a regression assertion ensuring the generated registration file contains no absolute paths and that embedded
filePathvalues are relative/portable.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/core/src/vite-plugin/sveltekit-generator.ts | Normalizes embedded manifest object filePath values to avoid absolute, machine-specific paths in generated smrt-register.ts. |
| packages/core/src/vite-plugin/sveltekit-generator.test.ts | Adds regression checks to ensure the generated registration file does not contain absolute paths and emits a relative filePath. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const filePath = objectDef.filePath; | ||
| if (typeof filePath !== 'string' || !isAbsolute(filePath)) { | ||
| return objectDef; | ||
| } | ||
| return { | ||
| ...objectDef, | ||
| filePath: relative(root, filePath).split(sep).join('/'), | ||
| }; |
`smrt-register.ts` is a tracked file in consumer repositories, but the registration manifest embedded in it carried the scanner's absolute `filePath` — the generating machine's home directory ended up inside a committed source file. Consumers could not commit the generated output without breaking every other checkout and CI, and every `pnpm build` left a dirty working tree. Nothing reads `filePath` back through `_manifest` at runtime (the registry's `filePath` handling resolves manifest *files*, not object definitions), so the embedded value is now project-relative with forward slashes. Verified against the reproducing consumer (anytown template-small-town): the regenerated file contains zero absolute paths and `filePath` reads `src/lib/smrt/Redirect.ts`. The remaining diff there is the intended 0.40.68 `_manifest`/`_manifestKey` registration (#2308), which is now deterministic and committable. Validated: smrt-core 3,255 tests passed, generator suite 71 passed. Refs #2341
CI typecheck caught what the build did not: an interface without an index signature is not assignable to Record<string, unknown>.
3f27655 to
7883545
Compare
Closes #2341
Problem
smrt-register.tsis a tracked file in consumer repositories, but the registration manifest the SvelteKit generator embeds in it carried the scanner's absolutefilePath:So the generating machine's home directory ended up inside committed source. Consumers could not commit the generated output without breaking every other checkout and CI, and every
pnpm buildleft a dirty working tree — which trips cleanliness gates.Fix
The embedded
filePathis now project-relative with forward slashes. Nothing reads it back through_manifestat runtime — the registry'sfilePathhandling resolves manifest files, not object definitions — so relativizing keeps the manifest shape without the machine specifics. Non-absolute values pass through untouched.Verified against the reproducing consumer
anytown
@anytown/template-small-town, rebuilt with this branch linked:/Users/...occurrences in the regeneratedsmrt-register.ts: 0 (was 1)filePathnow readssrc/lib/smrt/Redirect.ts_manifest/_manifestKeyregistration (fix(core): preserve object identity and schema in production consumer bundles #2308) plus the.smrt/register.jsimport — deterministic, machine-independent, and now committableScope correction
I filed #2341 with a second symptom — the generator stripping lines from consumers' template
.gitignorefiles. That one is not a bug and is withdrawn (detail in the issue): the removed lines areLEGACY_GITIGNORE_HEADERand its pattern run, andstripLegacyGeneratedRouteIgnores()removes them deliberately to migrate projects onto the bounded# BEGIN/# ENDblock from #2185. The downstream fix is for the consumer to commit the migrated.gitignore. This PR changes nothing about that path.Validation
@happyvertical/smrt-core— 3,255 tests passed, 6 files skippedfilePathis relativeNo changeset is committed — AGENTS.md:39 says release automation generates them on merge.
{"schema":"hv-agent-run:v1","policy_revision":"1.0.0","runtime":"claude","session":"cc711e03-43fb-435d-90ab-92e8c8d225ea","issue":"https://github.com/happyvertical/smrt/issues/2341"}