feat(scaffold): add create, verify-scaffold, scaffold-cleanup, and set commands

[ericelliott]

Summary

Adds manifest-driven project scaffolding to the aidd CLI via a single new
lib/scaffold-commands.js module. bin/aidd.js gains one import and one call.

All remote scaffolds are downloaded to ~/.aidd/scaffold — a single, predictable
location regardless of which command triggered the download.

New commands

• npx aidd create [type] <folder> — scaffold from a named scaffold, file://, or https://
• npx aidd verify-scaffold [type] — validate a scaffold manifest before running it
• npx aidd scaffold-cleanup — remove ~/.aidd/scaffold after scaffolding
• npx aidd set create-uri <uri> — persist a default scaffold URI to ~/.aidd/config.yml

New modules

Module	Responsibility
lib/scaffold-commands.js	CLI registration — single entry point
lib/scaffold-resolver.js	Resolves named, file://, and https:// sources; always downloads to ~/.aidd/scaffold
lib/scaffold-runner.js	Parses and executes SCAFFOLD-MANIFEST.yml steps
lib/scaffold-verifier.js	Pre-flight manifest validation
lib/scaffold-cleanup.js	Removes ~/.aidd/scaffold
lib/scaffold-create.js	Orchestrates the create flow
lib/scaffold-verify-cmd.js	verify-scaffold command handler
lib/scaffold-errors.js	Typed error definitions
lib/aidd-config.js	Reads/writes ~/.aidd/config.yml

Changes to bin/aidd.js

Exactly two lines added — one import, one call:

import { registerScaffoldCommands } from "../lib/scaffold-commands.js";
// ...
registerScaffoldCommands(cli);

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **High Risk**
> Adds a new code path that downloads and executes scaffold-provided commands from remote sources, touching security-sensitive network and filesystem behavior. Although guarded by confirmation/validation and covered by tests, failures or edge cases could lead to unintended code execution or user-home directory side effects.
> 
> **Overview**
> Adds manifest-driven scaffolding to the `aidd` CLI by registering new subcommands: `create` (with `--agent`), `verify-scaffold`, `scaffold-cleanup`, and `set create-uri`, including user config persisted to `~/.aidd/config.yml`.
> 
> Introduces a scaffold resolution/execution pipeline that supports named scaffolds, `file://` sources, and remote `https://` sources (with confirmation), downloads remote scaffolds into `~/.aidd/scaffold`, auto-resolves bare GitHub repo URLs to latest release tarballs (optional `GITHUB_TOKEN` auth), validates `SCAFFOLD-MANIFEST.yml` structure, and runs `run`/`prompt` steps safely.
> 
> Adds bundled scaffolds (`next-shadcn` stub and `scaffold-example` fixture), a scaffold authoring guide, and extensive unit + e2e tests around argument parsing, validation, cleanup behavior, and dependency installation.
> 
> <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit ec14b159f42e8280f90154e5671d6fb7e39150ce. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

[cursor]

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.}

[cursor]

Case-sensitive URL checks inconsistent with argument parser

Low Severity

The URL detection helpers isHttpUrl, isInsecureHttpUrl, and isFileUrl in scaffold-resolver.js use case-sensitive startsWith checks (e.g., url.startsWith("https://")), while resolveCreateArgs in scaffold-create.js uses a case-insensitive regex (/^(https?|file):\/\//i). If a user passes a mixed-case URL like HTTPS://example.com as the type argument with a folder, resolveCreateArgs accepts it, but resolveExtension fails to recognize it as a URL and falls through to the resolveNamed path, producing a confusing "SCAFFOLD-MANIFEST.yml not found" error instead of a proper network/validation flow.

Additional Locations (1)

• lib/scaffold-create.js#L28-L31

 

[janhesters]

Instead of asserting each key one at a time, why not have one big expected key that is full json? This means we can condense many of the asserts here into one.

[janhesters]

1. Collapse the 7 devDependency tests into a parameterized loop or single assertion. This eliminates ~80 lines of near-identical code and makes adding new expected dependencies a one-line change.

2. Isolate scaffold-cleanup tests from real home directory. Consider using XDG_DATA_HOME or an env var to redirect ~/.aidd to a temp directory during tests. Touching real user paths is risky.

Medium priority:

3. Add error-path tests: What happens when the target directory already exists? When the scaffold name is invalid? When the file:// URI doesn't exist? These are the most likely real-world failure modes.

4. Verify --agent actually invokes the agent. Capture stdout from the echo agent and assert it contains the prompt string "hello world" — otherwise the test only proves the directory was created, not that the agent ran.

Low priority:

5. Consider replacing exec with execFile in tests to avoid shell interpolation (and recommend the same for the production CLI if it uses exec).

6. The beforeAll/afterAll pattern is pragmatically fine for the slow scaffold-example tests, but the beforeEach/afterEach in scaffold-cleanup and --agent could be refactored to use a local setup() + onTestFinished pattern to follow the RITEway principles (explicit - no beforeEach etc.).

[janhesters]

Should we refactor this as a setup function with onTestFinished and call it in each test to keep them explicit and avoid shared mutable state?

[janhesters]

1. [High] Remove @ts-nocheck — Fix type errors with targeted assertions instead of blanket suppression. This is the single most impactful improvement.
2. [Medium] Avoid boolean-derived assertions — actual: agentOption !== undefined tests shape, not behavior. Consider testing the option's properties directly (e.g., assert on the option's long value or flags string) rather than a boolean existence check.
3. [Low] Mocking note — The five vi.mock calls are purely defensive. If registerScaffoldCommands could be refactored to accept action handlers via dependency injection, the mocks could be eliminated entirely. Not urgent, but worth keeping in mind for future refactors.

[janhesters]

Again here, should we collapse assertions like this on one expected object?

[janhesters]

1. Remove dead code: Delete the unused __filename binding.

2. Add tarball integrity verification: After downloading, verify a SHA-256 checksum if one is available in the release metadata. This addresses A03/A08.

3. Consistent error typing: Consider wrapping errors from defaultResolveRelease and defaultDownloadAndExtract with createError and the appropriate error types at the point of creation, not just at the catch site in resolveExtension.

4. Guard double-reject in tar extraction: Use a settled flag or Promise.withResolvers() to ensure only the first error triggers rejection:

   let settled = false;
   const settle = (fn) => (...args) => { if (!settled) { settled = true; fn(...args); } };
   child.on("close", (code) => { ... settle(resolve)() });
   child.on("error", settle(reject));
   child.stdin.on("error", settle(reject));

5. isGitHubRepoUrl edge case: A URL like https://github.com/owner/repo/ (trailing slash) passes because filter(Boolean) strips the empty segment. This is correct behavior, but a URL like https://github.com/owner/repo.git would also pass with parts.length === 2 — is that intentional? If not, consider stripping .git suffixes or explicitly checking. Make sure we have unit tests that explicitly define the behavior here.

6. Consider adding a JSDoc for resolveExtension — it's the main public API of this module, and its options object has many parameters with non-obvious interactions (fallback chain, DI overrides).

[janhesters]

Similar here.

One of the most common "slop" that agents produce is too granular tests. Meaning, if they have an object:

const expectedValue = {
foo: true,
bar: "hello"
}

They frequently test:

assert({
// ...
expected: expectedValue.foo
});

etc. instead of

assert({
// ...
expected: {
foo: true,
bar: "hello"
}
});

We should probably add a constrain to our review & implementation skills / rules for that.

[janhesters]

Same here, need to collapse the "slop" tests.

[janhesters]

Is it intentional to NOT use error-causes here?

[janhesters]

Same as above. Can be made explicit with onTestFinished.

[janhesters]

@ericelliott Also going forward, we should probably break these 4k+ LoC PRs down.