Add manifest-driven scaffolding with create and scaffold-cleanup commands#97
Add manifest-driven scaffolding with create and scaffold-cleanup commands#97ericelliott wants to merge 67 commits intomainfrom
create and scaffold-cleanup commands#97Conversation
Implements the `npx aidd create` epic with manifest-driven scaffolding: - `aidd create [type|URI] <folder>` resolves named scaffolds, HTTP/HTTPS URIs (with remote-code warning + confirmation), and file:// URIs. Reads SCAFFOLD-MANIFEST.yml and executes run/prompt steps sequentially; supports `--agent <name>` (default: claude) for prompt steps; warns about remote code and halts on step failure; leaves .aidd/scaffold/ in place and suggests scaffold-cleanup on completion. - `aidd scaffold-cleanup [folder]` removes the .aidd/ working directory and reports gracefully when nothing exists to clean up. - `lib/scaffold-resolver.js` with full unit test coverage. - `lib/scaffold-runner.js` with parseManifest (gray-matter YAML parsing) and runManifest with injectable execStep for isolation. - `lib/scaffold-cleanup.js` with unit tests. - `ai/scaffolds/scaffold-example` — minimal E2E test fixture that inits npm, sets vitest test script, and installs riteway/vitest/playwright/ error-causes/@paralleldrive/cuid2 at @latest. - `ai/scaffolds/next-shadcn` — default named scaffold stub with README describing the intended Next.js + shadcn/ui setup. - `bin/create-e2e.test.js` — E2E tests covering scaffold creation, AIDD_CUSTOM_EXTENSION_URI file:// override, scaffold-cleanup, and --agent flag passthrough. https://claude.ai/code/session_01VxNxs3Ly9UQh9TZpJpjG63
- Fix command injection in scaffold-runner: prompt steps now use spawn([agent, promptText]) array form instead of shell interpolation, so prompt content cannot inject shell commands - Fix extension.js execution to also use array spawn (node, path) - Add 30s network timeout to defaultFetchText to prevent indefinite hangs - Extract isHttpUrl/isFileUrl predicates and DEFAULT_SCAFFOLD_TYPE constant in scaffold-resolver for readability and to eliminate repeated string checks - Remove dead _noFetch variable from scaffold-resolver test - Fix fragile cwd-default test in scaffold-cleanup using vi.spyOn(process, cwd) instead of depending on ambient filesystem state - Mark npx-aidd-create-epic as DONE and log activity https://claude.ai/code/session_01VxNxs3Ly9UQh9TZpJpjG63
|
@cursor please /review |
|
✅ No technical debt markers (TODO/FIXME) in new code 9. Specific Issues SummaryCritical: High:
Medium: Low: 10. Actionable RecommendationsMust Fix (Before Merge):
// In lib/scaffold-resolver.js
import { createError } from "error-causes";
// Replace all throw new Error() with:
throw createError({
name: 'ScaffoldError',
message: 'Remote extension download cancelled by user.',
code: 'USER_CANCELLED',
uri: effectiveType
});
import { URL } from 'url';
const validateHttpUrl = (urlString) => {
try {
const url = new URL(urlString);
const hostname = url.hostname;
// Block private IP ranges
if (
hostname === 'localhost' ||
hostname.startsWith('127.') ||
hostname.startsWith('10.') ||
hostname.startsWith('192.168.') ||
hostname.startsWith('169.254.') ||
hostname.match(/^172\.(1[6-9]|2[0-9]|3[0-1])\./)
) {
throw createError({
name: 'InvalidScaffoldUri',
message: 'Private IP addresses are not allowed',
code: 'PRIVATE_IP_BLOCKED',
uri: urlString
});
}
return true;
} catch (err) {
throw createError({
name: 'InvalidScaffoldUri',
message: 'Invalid URL format',
code: 'INVALID_URL',
uri: urlString,
cause: err
});
}
};
// Use before fetching
if (isHttpUrl(effectiveType)) {
validateHttpUrl(effectiveType);
// ... rest of logic
}Should Fix (Post-Merge):
/**
* Resolves a scaffold source to local file paths.
* Supports named scaffolds, file:// URIs, and HTTP(S) URIs.
* @param {Object} options - Configuration options
* @param {string} [options.type] - Scaffold type: name, file://, or http(s):// URI
* @param {string} options.folder - Target folder path
* @param {string} [options.packageRoot] - Package root directory
* @returns {Promise<{manifestPath: string, extensionJsPath: string, readmePath: string}>}
*/
11. Vision Alignment✅ The PR aligns well with the vision document:
Final VerdictStatus:
Overall Quality: 8.5/10
Test Coverage: 10/10 - Comprehensive and well-written Code Quality: 9/10 - Clean, functional, follows project standards (minus error-causes) Security: 7/10 - Good injection prevention, but SSRF risk and missing integrity checks The implementation is solid and the test coverage is exemplary. Once the two security issues are addressed, this will be an excellent addition to the codebase. |
Define ScaffoldCancelledError, ScaffoldNetworkError, and ScaffoldStepError
in lib/scaffold-errors.js using the errorCauses() factory so every call-site
that builds a handler is exhaustiveness-checked at setup time.
- scaffold-resolver: throw createError({...ScaffoldCancelledError}) on user
cancellation; wrap fetchAndSaveExtension failures in ScaffoldNetworkError
- scaffold-runner: defaultExecStep now rejects with createError({...ScaffoldStepError})
so spawn failures carry a typed cause
- bin/aidd.js create action: replace generic catch with handleScaffoldErrors()
for typed branching (cancelled → info, network → retry hint, step → manifest hint)
https://claude.ai/code/session_01VxNxs3Ly9UQh9TZpJpjG63
There was a problem hiding this comment.
Pull request overview
Implements manifest-driven project scaffolding for the aidd CLI by adding create and scaffold-cleanup subcommands, along with the supporting resolver/runner/cleanup libraries, scaffold fixtures, and test coverage.
Changes:
- Added scaffold source resolution (named scaffolds,
file://,http(s)://with confirmation) and a manifest runner forrun/promptsteps. - Integrated new
createandscaffold-cleanupsubcommands into the CLI. - Added scaffold fixtures (
scaffold-example,next-shadcn) plus unit + E2E test suites.
Reviewed changes
Copilot reviewed 17 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tasks/npx-aidd-create-epic.md | Marks the epic status as DONE. |
| lib/scaffold-runner.js | New manifest parser + step runner (run/prompt + optional extension.js). |
| lib/scaffold-runner.test.js | Unit tests for manifest parsing and execution ordering/failure behavior. |
| lib/scaffold-resolver.js | New resolver for named/file/http scaffolds with caching under .aidd/scaffold. |
| lib/scaffold-resolver.test.js | Unit tests for named/file/http resolution, confirmation, and env var defaulting. |
| lib/scaffold-cleanup.js | New cleanup helper to remove .aidd/ working directory. |
| lib/scaffold-cleanup.test.js | Unit tests for cleanup behavior and default folder handling. |
| bin/aidd.js | Adds create (with --agent) and scaffold-cleanup subcommands. |
| bin/create-e2e.test.js | E2E tests for create, env defaulting, --agent, and cleanup. |
| ai/scaffolds/index.md | Adds index entry for scaffold directory. |
| ai/index.md | Links to ai/scaffolds/ from the top-level AI index. |
| ai/scaffolds/scaffold-example/README.md | Documents the scaffold-example fixture scaffold. |
| ai/scaffolds/scaffold-example/SCAFFOLD-MANIFEST.yml | Defines manifest steps for the E2E fixture (npm init/pkg/test/install). |
| ai/scaffolds/scaffold-example/index.md | Generated directory index for scaffold-example. |
| ai/scaffolds/scaffold-example/bin/index.md | Generated directory index for bin/. |
| ai/scaffolds/next-shadcn/README.md | Documents default scaffold (stub). |
| ai/scaffolds/next-shadcn/SCAFFOLD-MANIFEST.yml | Placeholder manifest with a prompt step. |
| ai/scaffolds/next-shadcn/index.md | Generated directory index for next-shadcn. |
| activity-log.md | Adds an entry describing the new commands and modules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
The previous implementation fetched only README.md, SCAFFOLD-MANIFEST.yml, and bin/extension.js over HTTP(S). This prevented scaffolds from carrying arbitrary file trees, templates, package.json dependencies, config files, bin scripts, etc. Replace fetchAndSaveExtension + manual HTTP(S) fetch with a git clone (defaultGitClone) that clones the full repo into <folder>/.aidd/scaffold/. The injectable `clone` parameter keeps tests fast and network-free. Also drop the now-unused http/https node built-ins and FETCH_TIMEOUT_MS. https://claude.ai/code/session_01VxNxs3Ly9UQh9TZpJpjG63
1. YAML document-start marker silently drops all steps (medium severity)
The gray-matter wrapper trick ("---\n" + content + "\n---") caused
gray-matter to see two consecutive --- lines and parse empty frontmatter
when a manifest author included the standard --- marker. Replace with
matter.engines.yaml.parse(content), a direct call to js-yaml's load()
which handles --- markers correctly.
Test added: "parses steps when manifest begins with YAML document-start marker"
2. npx aidd create (no args) crashes with TypeError (high severity)
Both positionals were optional, so Commander allowed zero arguments.
folderArg became undefined and path.resolve(cwd, undefined) threw.
Flip the arg order to <folder> [type] — folder is now required and
Commander rejects the call before the action runs.
Also note: the defaultFetchText res.resume() socket-leak issue reported
separately is moot — defaultFetchText was removed in the previous commit
when HTTP fetching was replaced with git clone.
https://claude.ai/code/session_01VxNxs3Ly9UQh9TZpJpjG63
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 20 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ent GitHub release workflow
TDD: added failing tests first, then implemented to pass.
- scaffold-errors.js: add ScaffoldValidationError (code SCAFFOLD_VALIDATION_ERROR)
- scaffold-runner.js: parseManifest now validates that steps is an array of plain
objects with at least one recognized key (run/prompt); throws ScaffoldValidationError
with a descriptive message on any violation instead of silently iterating wrong values
- scaffold-runner.test.js: 5 new tests covering string steps, object steps, bare-string
items, unrecognized-key items, and the error message content
- scaffold-verifier.js + scaffold-verifier.test.js: new verifyScaffold() function
(8 tests) that checks manifest existence, valid YAML, valid step shapes, and
non-empty steps list — returns { valid, errors } for clean reporting
- bin/aidd.js: add `verify-scaffold [type]` subcommand; update `create` error handler
to cover ScaffoldValidationError
- scaffold-example/SCAFFOLD-MANIFEST.yml: add release-it install + scripts.release step
so generated projects have a release command out of the box
- scaffold-example/package.json: new file so scaffold AUTHORS can release their scaffold
as a GitHub release with `npm run release`
- tasks/npx-aidd-create-epic.md: update requirements — GitHub releases instead of git
clone for remote scaffolds; add verify-scaffold, steps validation, and scaffold
author release workflow sections
- docs/scaffold-authoring.md: new guide covering manifest format, validation, the
distinction between npm's files array (npm publish only) and GitHub release assets,
and how to publish a scaffold as a GitHub release
https://claude.ai/code/session_01VxNxs3Ly9UQh9TZpJpjG63
…rst, folder second
The previous fix made <folder> required (correct) but accidentally swapped the
positional order to `create <folder> [type]`. Every E2E test, both scaffold
READMEs, the epic requirements, the activity log, and the docs all call the
command as `create [type] <folder>` (e.g. `create scaffold-example my-project`).
With the wrong order, `create scaffold-example test-project` would bind
folder="scaffold-example" and type="test-project", creating a directory named
after the scaffold and trying to resolve a nonexistent scaffold named after the
folder — completely backwards.
Restores `.command("create [type] <folder>")` with action `(type, folder, opts)`.
https://claude.ai/code/session_01VxNxs3Ly9UQh9TZpJpjG63
…full e2e in pre-commit
Argument parsing:
The previous [type] <folder> definition caused Commander to assign the
single-argument form (create my-folder) to `type`, leaving folder missing.
Restore [typeOrFolder] [folder] with manual validation so all three calling
patterns work correctly:
create scaffold-example my-project → type=scaffold-example, folder=my-project
create my-project → type=undefined (env/default), folder=my-project
create → explicit 'missing required argument' error
Pre-commit hook:
Changed from `npm run test:unit` to `npm test` so e2e tests run on every
commit. The arg-order regression slipped through exactly because e2e was
excluded from the hook. Full suite is ~50s but catches integration bugs.
https://claude.ai/code/session_01VxNxs3Ly9UQh9TZpJpjG63
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 25 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The internal [typeOrFolder] [folder] signature (needed to work around Commander's left-to-right arg assignment) showed both as optional in --help. Add a .usage() override and .addHelpText() so the displayed usage reads `[options] [type] <folder>` with an Arguments section that explicitly marks <folder> as required and shows four calling-convention examples. https://claude.ai/code/session_01VxNxs3Ly9UQh9TZpJpjG63
|
@ericelliott I've opened a new pull request, #99, to work on those changes. Once the pull request is ready, I'll request review from you. |
* Initial plan * fix: use @paralleldrive/cuid2 in task requirements for consistency Co-authored-by: ericelliott <364727+ericelliott@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: ericelliott <364727+ericelliott@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
… commit Add syncRootAgentFiles() to lib/agents-md.js — a strict content-equality sync that overwrites AGENTS.md or CLAUDE.md whenever their committed content differs from the current agentsMdContent template (created, updated, or unchanged). Wire it into the bin/aidd.js --index handler so the pre-commit hook regenerates both files in the same pass that regenerates ai/**/index.md. Extend the git add line in .husky/pre-commit to stage AGENTS.md and CLAUDE.md automatically, exactly as it does for the index files. Export SyncFileResult from lib/agents-md.d.ts and add three unit tests covering the created / unchanged / updated paths.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| code: "FILESYSTEM_ERROR", | ||
| message: "File system operation failed", | ||
| name: "FileSystemError", | ||
| }; |
There was a problem hiding this comment.
Duplicate error definitions across modules are fragile
Low Severity
ValidationError and FileSystemError objects in symlinks.js are manually duplicated from the errorCauses() definitions in cli-core.js, using identical code and name strings. If the codes or names in cli-core.js ever change, the copies in symlinks.js will silently drift, causing handleCliErrors to fail to dispatch errors from createSymlink to the correct handler. A shared error-definitions module would eliminate this coupling risk.
Additional Locations (1)
| const __filename = fileURLToPath(import.meta.url); | ||
| const __dirname = path.dirname(__filename); | ||
|
|
||
| const VERIFY_SCAFFOLD_DIR = path.join(AIDD_HOME, "scaffold"); |
There was a problem hiding this comment.
Exported VERIFY_SCAFFOLD_DIR constant is never imported externally
Low Severity
VERIFY_SCAFFOLD_DIR is exported from scaffold-verify-cmd.js but never imported by any other module or test file. The only consumer of this module (bin/aidd.js) imports only runVerifyScaffold, and the test file scaffold-verify-cmd.test.js also skips it. This is dead exported API surface that adds confusion about the module's public contract.
Use yaml.JSON_SCHEMA to prevent unsafe YAML tags (e.g. !!binary,
!!js/function, !!timestamp) from being deserialized. Matches the
same fix already applied in scaffold-runner.js.
A malformed or tagged config file still fails gracefully — the
existing catch block returns {} as before.
The error message previously used a generic string that gave no indication of which path was checked. Updated to include the full manifestPath so users know exactly where the file was expected. Also strengthened the corresponding test to assert that the error contains the actual manifest path, not just the word 'not found'.
Scaffold authors should use run steps in SCAFFOLD-MANIFEST.yml to execute arbitrary commands instead of relying on a dedicated extension.js entry point. This removes extension.js from the resolver, runner, and create flow, and cleans up all related tests, docs, and epic references. - Remove extensionJsPath from resolveNamed, resolveFileUri, downloadExtension - Remove extensionJsPath param and execution block from runManifest - Remove extensionJsPath from runCreate -> runManifest call - Remove extension.js from docs/scaffold-authoring.md file layout - Remove extension.js requirement from tasks/npx-aidd-create-epic.md - Remove extension.js test cases and mock props from all test files
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| console.log(chalk.green(`✅ ${action} ${file}`)); | ||
| }); | ||
|
|
||
| process.exit(0); |
There was a problem hiding this comment.
Sync and exit code run after failed index generation
Medium Severity
When generateAllIndexes fails, the else branch calls process.exit(1) but doesn't return. The syncRootAgentFiles call and process.exit(0) at lines 151–158 sit outside the if/else block, so they execute after the error path. Since process.exit doesn't synchronously halt the current tick, syncRootAgentFiles runs against a failed state, and process.exit(0) can override the intended non-zero exit code.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| .filter(({ action }) => action !== "unchanged") | ||
| .forEach(({ file, action }) => { | ||
| console.log(chalk.green(`✅ ${action} ${file}`)); | ||
| }); |
There was a problem hiding this comment.
--index command silently overwrites user-customized agent files
High Severity
The --index handler now calls syncRootAgentFiles(targetPath), which overwrites AGENTS.md and CLAUDE.md with the built-in template whenever their content differs. This is intended only for the aidd repo's own pre-commit hook to keep committed copies in sync, but npx aidd --index is a documented user-facing command. Any user who customized their AGENTS.md or CLAUDE.md (via ensureAgentsMd's careful append logic, or manually) will have those customizations silently destroyed when they run npx aidd --index.
Additional Locations (1)
…document release script The SCAFFOLD-MANIFEST.yml already installs release-it@latest and configures scripts.release="release-it", but the README omitted both. Add release-it to the dependency list and a note about npm run release. Also add missing unit test (lib/scaffold-example-readme.test.js) asserting the README documents release-it and the release script, and add e2e assertions in bin/create-e2e.test.js for release-it dev-dep installation and scripts.release configuration to close the coverage gap.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| .filter(({ action }) => action !== "unchanged") | ||
| .forEach(({ file, action }) => { | ||
| console.log(chalk.green(`✅ ${action} ${file}`)); | ||
| }); |
There was a problem hiding this comment.
syncRootAgentFiles overwrites user-customized agent files
High Severity
syncRootAgentFiles is called unconditionally in the --index code path, which is a public CLI option (npx aidd --index). Unlike ensureAgentsMd, which carefully preserves user customizations by only appending missing directives, syncRootAgentFiles unconditionally overwrites AGENTS.md and CLAUDE.md with the template whenever content differs. Any user who runs npx aidd --index on their project will lose all customizations to these files. The function was designed for the AIDD repo's own pre-commit hook but is exposed through the shared --index flag.
Additional Locations (1)
| code: "FILESYSTEM_ERROR", | ||
| message: "File system operation failed", | ||
| name: "FileSystemError", | ||
| }; |
There was a problem hiding this comment.
Duplicated error definitions across symlinks.js and cli-core.js
Low Severity
ValidationError and FileSystemError are manually re-declared as plain object literals in symlinks.js with the same code, message, and name values as the canonical definitions created by errorCauses() in cli-core.js. If the error codes or messages ever change in cli-core.js, these copies would silently drift out of sync, potentially causing handleCliErrors dispatch failures. Exporting the error definitions from cli-core.js and importing them here would eliminate the duplication.
Overview
This PR implements the
npx aidd createepic by adding two new CLI subcommands that enable manifest-driven project scaffolding:npx aidd create [type|URI] <folder>— Scaffolds new projects from named scaffolds, local file:// URIs, or remote HTTPS URIsnpx aidd scaffold-cleanup [folder]— Removes temporary.aidd/working directories after scaffoldingNew Requirements
Key Changes
New Modules
lib/scaffold-resolver.js.aidd/scaffold/AIDD_CUSTOM_EXTENSION_URIenvironment variable for custom defaultslib/scaffold-runner.jsrunsteps as shell commandspromptsteps via agent CLI (default:claude) with proper argument escapingbin/extension.jsafter all manifest steps if presentlib/scaffold-cleanup.js.aidd/directory from scaffolded projectsCLI Integration
Updated
bin/aidd.jsto add:createcommand with--agentflag for specifying the AI agentscaffold-cleanupcommand for post-scaffold cleanupScaffolds
Added two scaffold fixtures:
ai/scaffolds/scaffold-example/— E2E test fixturenpm init -yai/scaffolds/next-shadcn/— Default scaffold (placeholder)Tests
Added comprehensive test suites:
lib/scaffold-resolver.test.js(378 lines) — Tests named scaffolds, file:// URIs, HTTP/HTTPS URIs, remote code warnings, and environment variable handlinglib/scaffold-runner.test.js(329 lines) — Tests manifest parsing, step execution, error handling, and extension.js invocationlib/scaffold-cleanup.test.js(70 lines) — Tests directory removal and not-found casesbin/create-e2e.test.js(292 lines) — End-to-end tests forcreateandscaffold-cleanupcommands with real file I/OSecurity Considerations
Test Plan
All changes are covered by automated tests:
https://claude.ai/code/session_01VxNxs3Ly9UQh9TZpJpjG63
Note
High Risk
Executes scaffold-defined commands and agent prompts, and adds remote download/extraction and config writing logic; despite validation and HTTPS/confirmation safeguards, this expands the security-sensitive surface area.
Overview
Adds a manifest-driven scaffolding workflow to the
aiddCLI via newcreate,verify-scaffold,scaffold-cleanup, andset create-uricommands, including support for named scaffolds,file://scaffolds, and HTTPS-only remote scaffolds with explicit user confirmation.Introduces a YAML-based
SCAFFOLD-MANIFEST.ymlrunner (runshell steps +promptsteps executed via a configurable agent) with stricter validation/safe YAML parsing, plus GitHub repo URL resolution to latest release tarballs and a.aidd/working directory cleanup flow.Updates installation flow and docs to include Claude Code support (
--claude+.claudesymlink), automaticCLAUDE.mdcreation, new agent skill/aidd-fix, scaffold authoring documentation, and adjusts the pre-commit hook to run unit tests only (E2E run manually).Written by Cursor Bugbot for commit f355dc3. This will update automatically on new commits. Configure here.