Skip to content

Add manifest generation to include_assets build step#7016

Open
alfonso-noriega wants to merge 5 commits intomainfrom
add_manifest_to_include_files_step
Open

Add manifest generation to include_assets build step#7016
alfonso-noriega wants to merge 5 commits intomainfrom
add_manifest_to_include_files_step

Conversation

@alfonso-noriega
Copy link
Copy Markdown
Contributor

@alfonso-noriega alfonso-noriega commented Mar 16, 2026

WHY are these changes introduced?

Fixes #0000

WHAT is this pull request doing?

How to test your changes?

Post-release steps

Measuring impact

How do we know this change was effective? Please choose one:

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
  • Existing analytics will cater for this addition
  • PR includes analytics changes to measure impact

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

@alfonso-noriega alfonso-noriega requested a review from a team as a code owner March 16, 2026 14:33
@github-actions
Copy link
Copy Markdown
Contributor

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run pnpm changeset add to track your changes and include them in the next release CHANGELOG.

Caution

DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release.

Copy link
Copy Markdown
Contributor Author

alfonso-noriega commented Mar 16, 2026

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 16, 2026

Coverage report

St.
Category Percentage Covered / Total
🟢 Statements 82.41% 15217/18466
🟡 Branches 74.88% 7521/10044
🟢 Functions 81.48% 3819/4687
🟢 Lines 82.8% 14374/17359

Test suite run success

3998 tests passing in 1526 suites.

Report generated by 🧪jest coverage report action from cd40db3

@alfonso-noriega alfonso-noriega force-pushed the add_manifest_to_include_files_step branch from ffcfa99 to 0fd3c26 Compare March 17, 2026 10:08
@alfonso-noriega alfonso-noriega force-pushed the 02-wire-build-config-into-extension-specs branch from 1d5c873 to 912e7f4 Compare March 17, 2026 10:08
@alfonso-noriega alfonso-noriega force-pushed the 02-wire-build-config-into-extension-specs branch from 912e7f4 to 0da8073 Compare March 17, 2026 11:02
@alfonso-noriega alfonso-noriega force-pushed the add_manifest_to_include_files_step branch from 0fd3c26 to 141233b Compare March 17, 2026 11:03
@alfonso-noriega alfonso-noriega force-pushed the 02-wire-build-config-into-extension-specs branch from 0da8073 to 8cc8f2f Compare March 17, 2026 11:07
@alfonso-noriega alfonso-noriega force-pushed the add_manifest_to_include_files_step branch from 141233b to 06bccee Compare March 17, 2026 11:07
@alfonso-noriega alfonso-noriega force-pushed the 02-wire-build-config-into-extension-specs branch from 8cc8f2f to 5068d26 Compare March 17, 2026 11:19
@alfonso-noriega alfonso-noriega force-pushed the add_manifest_to_include_files_step branch 2 times, most recently from d13b2b2 to ef2ca4e Compare March 17, 2026 11:20
@alfonso-noriega alfonso-noriega force-pushed the 02-wire-build-config-into-extension-specs branch from 5068d26 to d58c2f4 Compare March 17, 2026 11:20
@alfonso-noriega alfonso-noriega force-pushed the add_manifest_to_include_files_step branch from ef2ca4e to 9c06d65 Compare March 17, 2026 15:51
@alfonso-noriega alfonso-noriega force-pushed the 02-wire-build-config-into-extension-specs branch from d58c2f4 to a10faf5 Compare March 17, 2026 15:51
@alfonso-noriega alfonso-noriega changed the base branch from 02-wire-build-config-into-extension-specs to graphite-base/7016 March 18, 2026 10:43
@alfonso-noriega alfonso-noriega force-pushed the add_manifest_to_include_files_step branch from 9c06d65 to 6c8a52b Compare March 18, 2026 10:45
@alfonso-noriega alfonso-noriega changed the base branch from graphite-base/7016 to 02-wire-build-config-into-extension-specs March 18, 2026 10:45
@alfonso-noriega alfonso-noriega force-pushed the 02-wire-build-config-into-extension-specs branch from 3da8d68 to 924deaf Compare March 18, 2026 17:50
@alfonso-noriega alfonso-noriega force-pushed the add_manifest_to_include_files_step branch from 6c8a52b to 854f0c7 Compare March 18, 2026 17:50
@alfonso-noriega alfonso-noriega force-pushed the 02-wire-build-config-into-extension-specs branch from 924deaf to 870ce9f Compare March 19, 2026 10:38
@alfonso-noriega alfonso-noriega force-pushed the add_manifest_to_include_files_step branch from 854f0c7 to f4baca8 Compare March 19, 2026 10:38
@alfonso-noriega alfonso-noriega changed the base branch from 02-wire-build-config-into-extension-specs to graphite-base/7016 March 19, 2026 11:11
@alfonso-noriega alfonso-noriega force-pushed the add_manifest_to_include_files_step branch from a9f578a to a9f96c4 Compare March 24, 2026 17:36
@alfonso-noriega alfonso-noriega force-pushed the 02-wire-build-config-into-extension-specs branch from 0f7e86e to 2ca4c09 Compare March 24, 2026 17:36
@alfonso-noriega alfonso-noriega force-pushed the add_manifest_to_include_files_step branch from a9f96c4 to aece5c0 Compare March 24, 2026 20:27
@alfonso-noriega alfonso-noriega force-pushed the 02-wire-build-config-into-extension-specs branch from 2ca4c09 to 6188dbf Compare March 24, 2026 20:27
@alfonso-noriega alfonso-noriega force-pushed the add_manifest_to_include_files_step branch from aece5c0 to 4ccfac9 Compare March 24, 2026 20:35
@alfonso-noriega alfonso-noriega force-pushed the 02-wire-build-config-into-extension-specs branch from 6188dbf to 370d43e Compare March 24, 2026 20:35
@alfonso-noriega alfonso-noriega changed the base branch from 02-wire-build-config-into-extension-specs to graphite-base/7016 March 24, 2026 21:46
@alfonso-noriega alfonso-noriega force-pushed the add_manifest_to_include_files_step branch from db87cdd to 2a58e58 Compare March 24, 2026 23:48
@alfonso-noriega alfonso-noriega changed the base branch from graphite-base/7016 to 02-wire-build-config-into-extension-specs March 24, 2026 23:48
@alfonso-noriega alfonso-noriega changed the base branch from 02-wire-build-config-into-extension-specs to graphite-base/7016 March 25, 2026 15:06
@alfonso-noriega alfonso-noriega force-pushed the add_manifest_to_include_files_step branch from 2a58e58 to 4b9cf0a Compare March 25, 2026 19:57
@alfonso-noriega alfonso-noriega changed the base branch from graphite-base/7016 to 02-wire-build-config-into-extension-specs March 25, 2026 19:57
@github-actions
Copy link
Copy Markdown
Contributor

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

  • Some seemingly private modules might be re-exported through public modules.
  • If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/public/node/path.d.ts
 import type { URL } from 'url';
 /**
  * Joins a list of paths together.
  *
  * @param paths - Paths to join.
  * @returns Joined path.
  */
 export declare function joinPath(...paths: string[]): string;
 /**
  * Normalizes a path.
  *
  * @param path - Path to normalize.
  * @returns Normalized path.
  */
 export declare function normalizePath(path: string): string;
 /**
  * Resolves a list of paths together.
  *
  * @param paths - Paths to resolve.
  * @returns Resolved path.
  */
 export declare function resolvePath(...paths: string[]): string;
 /**
  * Returns the relative path from one path to another.
  *
  * @param from - Path to resolve from.
  * @param to - Path to resolve to.
  * @returns Relative path.
  */
 export declare function relativePath(from: string, to: string): string;
 /**
  * Returns whether the path is absolute.
  *
  * @param path - Path to check.
  * @returns Whether the path is absolute.
  */
 export declare function isAbsolutePath(path: string): boolean;
 /**
  * Returns the directory name of a path.
  *
  * @param path - Path to get the directory name of.
  * @returns Directory name.
  */
 export declare function dirname(path: string): string;
 /**
  * Returns the base name of a path.
  *
  * @param path - Path to get the base name of.
  * @param ext - Optional extension to remove from the result.
  * @returns Base name.
  */
 export declare function basename(path: string, ext?: string): string;
 /**
  * Returns the extension of the path.
  *
  * @param path - Path to get the extension of.
  * @returns Extension.
  */
 export declare function extname(path: string): string;
 /**
  * Parses a path into its components (root, dir, base, ext, name).
  *
  * @param path - Path to parse.
  * @returns Parsed path object.
  */
 export declare function parsePath(path: string): {
     root: string;
     dir: string;
     base: string;
     ext: string;
     name: string;
 };
 /**
  * Given an absolute filesystem path, it makes it relative to
  * the current working directory. This is useful when logging paths
  * to allow the users to click on the file and let the OS open it
  * in the editor of choice.
  *
  * @param path - Path to relativize.
  * @param dir - Current working directory.
  * @returns Relativized path.
  */
 export declare function relativizePath(path: string, dir?: string): string;
 /**
  * Given 2 paths, it returns whether the second path is a subpath of the first path.
  *
  * @param mainPath - The main path.
  * @param subpath - The subpath.
  * @returns Whether the subpath is a subpath of the main path.
  */
 export declare function isSubpath(mainPath: string, subpath: string): boolean;
 /**
  * Given a module's import.meta.url it returns the directory containing the module.
  *
  * @param moduleURL - The value of import.meta.url in the context of the caller module.
  * @returns The path to the directory containing the caller module.
  */
 export declare function moduleDirectory(moduleURL: string | URL): string;
 /**
  * When running a script using `npm run`, something interesting happens. If the current
  * folder does not have a `package.json` or a `node_modules` folder, npm will traverse
  * the directory tree upwards until it finds one. Then it will run the script and set
  * `process.cwd()` to that folder, while the actual path is stored in the INIT_CWD
  * environment variable (see here: https://docs.npmjs.com/cli/v9/commands/npm-run-script#description).
  *
  * @returns The path to the current working directory.
  */
 export declare function cwd(): string;
 /**
  * Tries to get the value of the `--path` argument, if provided.
  *
  * @param argv - The arguments to search for the `--path` argument.
  * @returns The value of the `--path` argument, if provided.
  */
 export declare function sniffForPath(argv?: string[]): string | undefined;
 /**
  * Returns whether the `--json` or `-j` flags are present in the arguments.
  *
  * @param argv - The arguments to search for the `--json` and `-j` flags.
  * @returns Whether the `--json` or `-j` flag is present in the arguments.
  */
 export declare function sniffForJson(argv?: string[]): boolean;
+/**
+ * Removes any `..` traversal segments from a relative path and calls `warn`
+ * if any were stripped. Normal `..` that cancel out within the path (e.g.
+ * `foo/../bar` → `bar`) are collapsed but never allowed to escape the root.
+ * Both `/` and `\` are treated as separators for cross-platform safety.
+ *
+ * @param input - The relative path to sanitize.
+ * @param warn - Called with a human-readable warning when traversal segments are removed.
+ * @returns The sanitized path (may be an empty string if all segments were traversal).
+ */
+export declare function sanitizeRelativePath(input: string, warn: (msg: string) => void): string;

@alfonso-noriega alfonso-noriega changed the base branch from 02-wire-build-config-into-extension-specs to graphite-base/7016 March 26, 2026 13:37
@alfonso-noriega alfonso-noriega force-pushed the add_manifest_to_include_files_step branch from 4b9cf0a to cd40db3 Compare March 26, 2026 14:11
@alfonso-noriega alfonso-noriega changed the base branch from graphite-base/7016 to 02-wire-build-config-into-extension-specs March 26, 2026 14:11
@alfonso-noriega alfonso-noriega changed the base branch from 02-wire-build-config-into-extension-specs to graphite-base/7016 March 26, 2026 14:33
Copy link
Copy Markdown
Contributor

@isaacroldan isaacroldan left a comment

Choose a reason for hiding this comment

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

The AI found a few bugs, not sure if all are true, but worth checking


Review assisted by pair-review

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💬 Suggestion: This module contains several non-trivial private functions (stripAnchorPrefix, buildRelativeEntry, resolveManifestPaths, lastKeySegment, containsUnresolvedPath, shallowMerge) tested only indirectly through integration tests. Other modules in this directory (copy-by-pattern, copy-source-entry, copy-config-key-entry) each have their own test files. Edge cases in the helpers (e.g., stripAnchorPrefix when key doesn't start with anchor prefix, buildRelativeEntry with deeply nested paths, resolveManifestPaths with mixed resolved/unresolved values) are harder to isolate when failures surface only as cryptic manifest content mismatches.

Suggestion: Consider adding a generate-manifest.test.ts with focused unit tests for the key helpers. Either export them (like was done for tokenizePath and getNestedValue) or test them through generateManifestFile with targeted inputs that exercise specific branches.

@isaacroldan isaacroldan force-pushed the add_manifest_to_include_files_step branch from cd40db3 to b93a5ff Compare March 31, 2026 14:00
alfonso-noriega and others added 5 commits March 31, 2026 12:08
- Add generateManifest boolean to IncludeAssetsConfigSchema (default false)
- Add anchor and groupBy fields to ConfigKeyEntrySchema for grouped manifest entries
- Track copy operations in pathMap to resolve output-relative paths
- Deduplicate source paths to prevent indexed suffixes on shared files
- Use sequential copy loop to prevent findUniqueDestPath race conditions
- resolveManifestPaths walks manifest tree using pathMap; non-path strings pass through unchanged
- 33 tests passing
- Rename short identifiers (k, v, p) to satisfy id-length rule
- Add eslint-disable for intentional no-await-in-loop in sequential copy loop
- Add null guard for head in buildRelativeEntry to fix TS18048
- Run prettier fixes
- Update copy-config-key-entry.test.ts: use result.filesCopied instead of
  result (function now returns {filesCopied, pathMap}); fix fileExists mock
  in file copy tests so findUniqueDestPath resolves on first attempt
- include-assets-step.ts: rename (e) => entry to satisfy id-length rule
- generate-manifest.ts: use interface instead of type alias

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…l entries contribute to manifest

- Rename generateManifest → generatesAssetsManifest per isaacroldan's suggestion
- All entry types (pattern, static, configKey) now contribute their copied output
  paths to manifest.json when generatesAssetsManifest is true
- pattern/static paths appear under a top-level "files" key
- anchor/groupBy behavior for configKey entries unchanged
- copyByPattern and copySourceEntry now return {filesCopied, outputPaths}
- Add anchor/groupBy JSDoc to ConfigKeyEntrySchema

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ests

- Remove `preserveStructure` field from ConfigKeyEntrySchema and all call sites
- copy-by-pattern: always use relativePath (was the default=true behaviour); remove
  no-match warning; update log to `Included N file(s)`
- copy-config-key-entry: remove preserveStructure branching; update log to `Included '...'`
- copy-source-entry: remove stale preserveStructure destructure
- Tests: rewrite copy-config-key-entry.test.ts (was broken from conflict resolution);
  delete removed-feature tests in copy-by-pattern and copy-source-entry;
  update log message assertions; fix outputPaths prefix in copy-source-entry test

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants