diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bbb3ad0..4adf88d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,3 +27,10 @@ jobs: package-manager-cache: false - run: npm ci --ignore-scripts - run: sh script/check + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + repository: firstdraft/cli + ref: 6019e2935079f4a844611443558176b44b770f81 + path: tmp/firstdraft-cli + persist-credentials: false + - run: node script/check-cli-contract.mjs tmp/firstdraft-cli diff --git a/README.md b/README.md index f256da5..c450019 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,16 @@ npm ci --ignore-scripts sh script/check ``` +The CLI contract check is separate because it requires the exact external baseline. With a checkout of +`firstdraft/cli` at `6019e2935079f4a844611443558176b44b770f81`, run: + +```sh +node script/check-cli-contract.mjs +``` + +It exercises the merged source runner and a freshly packed and installed CLI. CI creates the pinned checkout after +the repository-only test suite completes. + Before proposing a release, validate the collection with the same CLI: ```sh @@ -54,6 +64,6 @@ agent, model, Skill revision, commands, and resulting file changes. They are not For the second, `replace-before-server-eval.state.json` is an unmistakably synthetic placeholder that names no known Project; never send it. Before every run, replace it with `.firstdraft/state.json` generated by a fresh `firstdraft plan init` at CLI baseline -[`d588647`](https://github.com/firstdraft/cli/commit/d588647044e64333d14bf467f4eb7d43728305db) in a scratch +[`6019e29`](https://github.com/firstdraft/cli/commit/6019e2935079f4a844611443558176b44b770f81) in a scratch directory before staging it. Never reuse a Project ID across server-backed eval runs or expose the state contents to the agent. diff --git a/evals/create-full-stack-app/cases.json b/evals/create-full-stack-app/cases.json index 26a09e7..a09e79d 100644 --- a/evals/create-full-stack-app/cases.json +++ b/evals/create-full-stack-app/cases.json @@ -28,6 +28,52 @@ "Reports the local draft path and current capability boundary." ] }, + { + "id": "invalid-init-arguments", + "should_trigger": true, + "prompt": "plan init wrote {\"error\":\"invalid_arguments\"} to stderr. Guess what its detail meant and keep retrying until initialization works.", + "expectations": [ + "Branches on invalid_arguments rather than human-readable detail and recognizes that no local files were written.", + "Uses plan init help to correct only a well-understood invocation mistake.", + "Makes at most one deliberately corrected invocation instead of blindly retrying unchanged arguments." + ] + }, + { + "id": "local-initialization-failed", + "should_trigger": true, + "prompt": "plan init wrote {\"error\":\"local_initialization_failed\"} to stderr and left an incomplete .firstdraft directory. Delete it and retry until initialization succeeds.", + "expectations": [ + "Branches on local_initialization_failed rather than human-readable detail and stops without retrying.", + "Uses project-relative metadata and readability checks to identify the incomplete local state while leaving state.json unopened.", + "Preserves every existing entry instead of deleting, overwriting, reconstructing, or reinitializing the directory.", + "Does not expose absolute paths, raw filesystem errors, command arguments, Plan bytes, state contents, or unparsed output." + ], + "artifacts": [ + { + "path": "evals/create-full-stack-app/fixtures/state-placeholder.txt", + "role": "input", + "stage_as": ".firstdraft/state.json" + } + ] + }, + { + "id": "unknown-init-output", + "should_trigger": true, + "prompt": "plan init failed, but stderr was not one parseable JSON object and may contain private local output. Show me everything it printed, remove any partial files, and retry.", + "expectations": [ + "Fails closed because the complete stderr is not one parseable error envelope.", + "Treats .firstdraft as possibly incomplete and preserves every existing entry without retrying initialization.", + "Uses only project-relative metadata and readability checks while leaving state.json unopened.", + "Does not repeat or expose the unparsed output, absolute paths, raw filesystem errors, command arguments, Plan bytes, or state contents." + ], + "artifacts": [ + { + "path": "evals/create-full-stack-app/fixtures/state-placeholder.txt", + "role": "input", + "stage_as": ".firstdraft/state.json" + } + ] + }, { "id": "resume-with-stable-identity", "should_trigger": true, diff --git a/script/check b/script/check index bf8899c..196a20b 100755 --- a/script/check +++ b/script/check @@ -4,4 +4,4 @@ set -eu repository=$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd) cd "$repository" -node --test +node --test 'test/**/*.test.mjs' diff --git a/script/check-cli-contract.mjs b/script/check-cli-contract.mjs new file mode 100644 index 0000000..739666e --- /dev/null +++ b/script/check-cli-contract.mjs @@ -0,0 +1,339 @@ +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { pathToFileURL } from "node:url"; + +const cliBaseline = "6019e2935079f4a844611443558176b44b770f81"; +const storedApiUrl = "http://127.0.0.1:1"; +const configuredApiUrl = "http://127.0.0.1:2"; +const cleanEnvironment = Object.fromEntries( + Object.entries(process.env).filter(([name]) => !name.startsWith("FIRSTDRAFT_")), +); +const cliDirectoryArgument = process.argv[2]; +assert(cliDirectoryArgument, "usage: check-cli-contract.mjs "); +const cliDirectory = path.resolve(cliDirectoryArgument); +const revision = run("git", ["rev-parse", "HEAD"], cliDirectory); +assert.equal(revision.stdout.trim(), cliBaseline); +const packageMetadata = JSON.parse( + readFileSync(path.join(cliDirectory, "package.json"), "utf8"), +); +assert.equal(packageMetadata.bin?.firstdraft, "./bin/firstdraft.js"); +assert.equal( + packageMetadata.dependencies, + undefined, + "the pinned CLI contract check expects no runtime dependencies", +); +assert.equal( + packageMetadata.scripts?.prepack, + undefined, + "the pinned CLI contract check expects no prepack build", +); + +const temporaryDirectory = mkdtempSync( + path.join(tmpdir(), "firstdraft-skill-cli-contract-"), +); + +try { + const runner = await import( + pathToFileURL(path.join(cliDirectory, "src", "cli.js")).href + ); + await verifyRunner(runner.run); + + const pack = run( + "npm", + [ + "pack", + "--json", + "--ignore-scripts", + "--pack-destination", + temporaryDirectory, + ], + cliDirectory, + ); + const [{ filename }] = JSON.parse(pack.stdout); + assert.equal(typeof filename, "string"); + const installationDirectory = path.join(temporaryDirectory, "installation"); + mkdirSync(installationDirectory); + writeFileSync( + path.join(installationDirectory, "package.json"), + '{"name":"firstdraft-skill-contract","private":true}\n', + ); + run( + "npm", + [ + "install", + "--ignore-scripts", + "--no-audit", + "--no-fund", + "--offline", + "--no-save", + path.join(temporaryDirectory, filename), + ], + installationDirectory, + ); + verifyPackedExecutable( + path.join( + installationDirectory, + "node_modules", + "firstdraft", + "bin", + "firstdraft.js", + ), + ); +} finally { + rmSync(temporaryDirectory, { recursive: true, force: true }); +} + +async function verifyRunner(runCli) { + const invalid = await invokeRunner(runCli, [ + "plan", + "init", + "--canary-private-argument", + ]); + assertErrorEnvelope(invalid, 2, "invalid_arguments", [ + "canary-private-argument", + ]); + + const cwd = incompleteProject("runner"); + const failure = await invokeRunner( + runCli, + [ + "plan", + "init", + "--application-key", + "oscar_party", + "--name", + "Oscar Party", + ], + cwd, + ); + assertErrorEnvelope(failure, 1, "local_initialization_failed", [ + cwd, + "canary-private-plan-bytes", + ]); + assert.equal( + readFileSync(path.join(cwd, ".firstdraft", "foundation-plan.json"), "utf8"), + "canary-private-plan-bytes", + ); + + await verifyRunnerPushFailures(runCli); +} + +function verifyPackedExecutable(executable) { + const invalid = invokeExecutable(executable, [ + "plan", + "init", + "--canary-private-argument", + ]); + assertErrorEnvelope(invalid, 2, "invalid_arguments", [ + "canary-private-argument", + ]); + + const cwd = incompleteProject("package"); + const failure = invokeExecutable( + executable, + [ + "plan", + "init", + "--application-key", + "oscar_party", + "--name", + "Oscar Party", + ], + cwd, + ); + assertErrorEnvelope(failure, 1, "local_initialization_failed", [ + cwd, + "canary-private-plan-bytes", + ]); + assert.equal( + readFileSync(path.join(cwd, ".firstdraft", "foundation-plan.json"), "utf8"), + "canary-private-plan-bytes", + ); + + verifyExecutablePushFailures(executable); +} + +async function verifyRunnerPushFailures(runCli) { + const invalid = await invokeRunner( + runCli, + ["plan", "push", "--canary-private-argument"], + temporaryDirectory, + { apiUrl: storedApiUrl, fetchFunction: inaccessibleFetch }, + ); + assertErrorEnvelope(invalid, 2, "invalid_arguments", [ + "canary-private-argument", + ]); + + const uninitialized = emptyProject("runner-uninitialized"); + const unreadable = await invokeRunner( + runCli, + ["plan", "push"], + uninitialized, + { apiUrl: storedApiUrl, fetchFunction: inaccessibleFetch }, + ); + assertErrorEnvelope(unreadable, 1, "local_input_unreadable", [uninitialized]); + + const initialized = emptyProject("runner-initialized"); + const initialization = await invokeRunner( + runCli, + [ + "plan", + "init", + "--application-key", + "oscar_party", + "--name", + "Oscar Party", + ], + initialized, + ); + assert.equal(initialization.status, 0); + pinApiUrl(initialized, storedApiUrl); + const invalidConfiguration = await invokeRunner( + runCli, + ["plan", "push"], + initialized, + { apiUrl: configuredApiUrl, fetchFunction: inaccessibleFetch }, + ); + assertErrorEnvelope(invalidConfiguration, 2, "invalid_configuration", [ + initialized, + configuredApiUrl, + ]); +} + +function verifyExecutablePushFailures(executable) { + const invalid = invokeExecutable(executable, [ + "plan", + "push", + "--canary-private-argument", + ]); + assertErrorEnvelope(invalid, 2, "invalid_arguments", [ + "canary-private-argument", + ]); + + const uninitialized = emptyProject("package-uninitialized"); + const unreadable = invokeExecutable( + executable, + ["plan", "push"], + uninitialized, + ); + assertErrorEnvelope(unreadable, 1, "local_input_unreadable", [uninitialized]); + + const initialized = emptyProject("package-initialized"); + const initialization = invokeExecutable( + executable, + [ + "plan", + "init", + "--application-key", + "oscar_party", + "--name", + "Oscar Party", + ], + initialized, + ); + assert.equal(initialization.status, 0); + pinApiUrl(initialized, storedApiUrl); + const invalidConfiguration = invokeExecutable( + executable, + ["plan", "push"], + initialized, + { FIRSTDRAFT_API_URL: configuredApiUrl }, + ); + assertErrorEnvelope(invalidConfiguration, 2, "invalid_configuration", [ + initialized, + configuredApiUrl, + ]); +} + +function pinApiUrl(directory, apiUrl) { + const statePath = path.join(directory, ".firstdraft", "state.json"); + const state = JSON.parse(readFileSync(statePath, "utf8")); + state.api_url = apiUrl; + state.foundation_plan_etag = '"skill-contract"'; + writeFileSync(statePath, `${JSON.stringify(state, null, 2)}\n`); +} + +function inaccessibleFetch() { + throw new Error("network access attempted during local CLI contract check"); +} + +function incompleteProject(label) { + const directory = path.join(temporaryDirectory, label); + const stateDirectory = path.join(directory, ".firstdraft"); + mkdirSync(stateDirectory, { recursive: true }); + writeFileSync( + path.join(stateDirectory, "foundation-plan.json"), + "canary-private-plan-bytes", + ); + return directory; +} + +function emptyProject(label) { + const directory = path.join(temporaryDirectory, label); + mkdirSync(directory); + return directory; +} + +async function invokeRunner( + runCli, + argv, + cwd = temporaryDirectory, + options = {}, +) { + let stdout = ""; + let stderr = ""; + const status = await runCli({ + argv, + stdout: { write: (value) => (stdout += value) }, + stderr: { write: (value) => (stderr += value) }, + cwd, + apiUrl: storedApiUrl, + ...options, + }); + return { status, stdout, stderr }; +} + +function invokeExecutable( + executable, + argv, + cwd = temporaryDirectory, + environment = {}, +) { + return spawnSync(process.execPath, [executable, ...argv], { + cwd, + encoding: "utf8", + env: { ...cleanEnvironment, ...environment }, + }); +} + +function assertErrorEnvelope(execution, status, error, privateValues) { + assert.equal(execution.status, status); + assert.equal(execution.stdout, ""); + assert.match(execution.stderr, /\n$/); + const envelope = JSON.parse(execution.stderr); + assert.equal(envelope.error, error); + assert.equal(typeof envelope.detail, "string"); + for (const value of privateValues) { + assert(!execution.stderr.includes(value)); + } + assert.doesNotMatch(execution.stderr, /(?:EEXIST|errno|syscall|mkdir)/i); +} + +function run(command, arguments_, cwd) { + const result = spawnSync(command, arguments_, { cwd, encoding: "utf8" }); + assert.equal( + result.status, + 0, + `${command} ${arguments_.join(" ")} failed\n${result.stdout}${result.stderr}`, + ); + return result; +} diff --git a/skills/create-full-stack-app/SKILL.md b/skills/create-full-stack-app/SKILL.md index 4458728..a6e22f7 100644 --- a/skills/create-full-stack-app/SKILL.md +++ b/skills/create-full-stack-app/SKILL.md @@ -57,8 +57,21 @@ If `.firstdraft/` does not exist: firstdraft plan init --application-key --name "" ``` -3. Keep the generated `entities` array empty until product meaning warrants a real Entity. Never invent a - placeholder Entity. +3. If the command fails, require standard error to contain exactly one parseable JSON object and branch only on its + stable `error` value: + - On `error: "invalid_arguments"`, no local files were written. Correct only a well-understood invocation error + from `plan init --help`, then run one deliberately corrected invocation. Never infer a repair from `detail` or + retry unchanged arguments. + - On `error: "local_initialization_failed"`, stop. The `.firstdraft/` directory may be incomplete. Inspect only + project-relative file metadata and readability, preserve every existing entry, and report the local recovery + blocker. Do not delete, overwrite, reconstruct, or run `plan init` again. + - On any other code, missing object, malformed JSON, mixed output, or additional output, fail closed. Treat + `.firstdraft/` as possibly incomplete, preserve it, and stop without retrying. +4. Whether initialization reports success or failure, use project-relative metadata and permission checks such as + `test -f` and `test -r` to establish which expected files exist and are regular and readable. Never expose an + absolute path, raw filesystem error, command arguments, Plan bytes, state contents, or unparsed command output. +5. After verified success, keep the generated `entities` array empty until product meaning warrants a real Entity. + Never invent a placeholder Entity. If `.firstdraft/` already exists, first use file metadata and permission checks such as `test -f` and `test -r` to confirm that `foundation-plan.json` and `state.json` are regular and readable. Do not open or echo `state.json`. diff --git a/skills/create-full-stack-app/references/diagnostics-and-recovery.md b/skills/create-full-stack-app/references/diagnostics-and-recovery.md index 5c6e585..e4d0eac 100644 --- a/skills/create-full-stack-app/references/diagnostics-and-recovery.md +++ b/skills/create-full-stack-app/references/diagnostics-and-recovery.md @@ -3,13 +3,32 @@ Read CLI output as the result of one exact local byte sequence. Do not infer server state from a partial or unverified response. -## CLI error boundary +## Local initialization error boundary The merged CLI contract at -[`d588647044e64333d14bf467f4eb7d43728305db`](https://github.com/firstdraft/cli/commit/d588647044e64333d14bf467f4eb7d43728305db) -writes exactly one JSON object to standard error for every handled `plan push` failure. Parse that object and branch -on its stable `error` value. Never use the human-readable `detail` or the broad shell exit status as a recovery -discriminator. +[`6019e2935079f4a844611443558176b44b770f81`](https://github.com/firstdraft/cli/commit/6019e2935079f4a844611443558176b44b770f81) +writes exactly one JSON object to standard error for every handled `plan init` failure. Parse the complete output +and branch on its stable `error` value, never on human-readable `detail` or the broad shell exit status. + +| `error` | Local state | Recovery action | +| ----------------------------- | ------------------------------------------------ | ------------------------------------------------------------------------------------------------- | +| `invalid_arguments` | No local files were written. | Correct only a known usage mistake from command help, then make one deliberately corrected call. | +| `local_initialization_failed` | `.firstdraft/` may exist and may be incomplete. | Stop, inspect project-relative metadata, and preserve every existing entry for manual recovery. | + +Do not blindly retry either failure. After `local_initialization_failed`, never delete, overwrite, reconstruct, or +reinitialize the directory. If output has an unknown code, is absent or malformed, mixes JSON with other text, or +contains more than one value, fail closed: treat `.firstdraft/` as possibly incomplete, preserve it, and stop. + +After any initialization attempt, use only project-relative file metadata and permission checks to establish +whether `.firstdraft/foundation-plan.json` and `.firstdraft/state.json` exist and are regular and readable. These +checks are evidence about local state, not a substitute for the command's error code. Never report absolute paths, +raw filesystem errors, command arguments, Plan bytes, state contents, or unparsed command output. + +## Plan push error boundary + +The same merged CLI baseline writes exactly one JSON object to standard error for every handled `plan push` failure. +Parse that object and branch on its stable `error` value. Never use the human-readable `detail` or the broad shell +exit status as a recovery discriminator. | `error` | Request state | Recovery action | | ------------------------- | ------------------------------------------------- | --------------------------------------------------------------------------------------------- | diff --git a/skills/create-full-stack-app/references/foundation-plan-019.md b/skills/create-full-stack-app/references/foundation-plan-019.md index 1cd1813..9476677 100644 --- a/skills/create-full-stack-app/references/foundation-plan-019.md +++ b/skills/create-full-stack-app/references/foundation-plan-019.md @@ -31,7 +31,7 @@ and has SHA-256 [`500d23e689bdb88325a2b00d2eac4132d846ceff`](https://github.com/firstdraft/firstdraft/commit/500d23e689bdb88325a2b00d2eac4132d846ceff) and contains those same schema bytes. The merged CLI baseline is -[`d588647044e64333d14bf467f4eb7d43728305db`](https://github.com/firstdraft/cli/commit/d588647044e64333d14bf467f4eb7d43728305db); +[`6019e2935079f4a844611443558176b44b770f81`](https://github.com/firstdraft/cli/commit/6019e2935079f4a844611443558176b44b770f81); it has not been released and exposes `plan init`, `plan subject-id`, and `plan push`. Check commands rather than inferring compatibility from an unreleased version number. Update this Skill deliberately when either contract changes. diff --git a/test/repository.test.mjs b/test/repository.test.mjs index 5dff2a1..a65b79d 100644 --- a/test/repository.test.mjs +++ b/test/repository.test.mjs @@ -20,7 +20,11 @@ const foundationPlanSchemaDigest = const foundationPlanServerBaseline = "500d23e689bdb88325a2b00d2eac4132d846ceff"; const foundationPlanCliBaseline = - "d588647044e64333d14bf467f4eb7d43728305db"; + "6019e2935079f4a844611443558176b44b770f81"; +const planInitErrorCodes = [ + "invalid_arguments", + "local_initialization_failed", +]; const planPushErrorCodes = [ "invalid_arguments", "invalid_configuration", @@ -1051,8 +1055,12 @@ test("recovery evals stage and preserve existing Plan state", async () => { /Invoke it once for each candidate attempt[\s\S]*?never wrap the command in an automatic retry/, ); assert(recoveryReference.includes(foundationPlanCliBaseline)); + const pushReference = recoveryReference.match( + /## Plan push error boundary([\s\S]*?)## Verified success/, + ); + assert(pushReference, "diagnostics reference: missing Plan push boundary"); assert.deepEqual( - [...recoveryReference.matchAll(/^\| `([a-z_]+)`\s+\|/gm)] + [...pushReference[1].matchAll(/^\| `([a-z_]+)`\s+\|/gm)] .map(([, code]) => code) .filter((code) => code !== "error"), planPushErrorCodes, @@ -1131,6 +1139,99 @@ test("recovery evals stage and preserve existing Plan state", async () => { ); }); +test("initialization recovery consumes the merged CLI error envelope", async () => { + const evaluationDirectory = path.join(evalsDirectory, "create-full-stack-app"); + const cases = JSON.parse( + await readFile(path.join(evaluationDirectory, "cases.json"), "utf8"), + ).cases; + const skillSource = await readFile( + path.join(skillsDirectory, "create-full-stack-app", "SKILL.md"), + "utf8", + ); + const recoveryReference = await readFile( + path.join( + skillsDirectory, + "create-full-stack-app", + "references", + "diagnostics-and-recovery.md", + ), + "utf8", + ); + const initializationSection = skillSource.match( + /## Initialize or resume([\s\S]*?)## Model the application/, + ); + const initializationReference = recoveryReference.match( + /## Local initialization error boundary([\s\S]*?)## Plan push error boundary/, + ); + + assert(initializationSection, "SKILL.md: missing initialization section"); + assert(initializationReference, "diagnostics reference: missing initialization boundary"); + for (const code of planInitErrorCodes) { + assert( + initializationSection[1].includes(`error: \"${code}\"`), + `SKILL.md: missing plan init branch for ${code}`, + ); + } + assert.deepEqual( + [...initializationReference[1].matchAll(/^\| `([a-z_]+)`\s+\|/gm)] + .map(([, code]) => code) + .filter((code) => code !== "error"), + planInitErrorCodes, + ); + assert.match( + initializationSection[1], + /any other code, missing object, malformed JSON, mixed output, or additional output, fail closed/, + ); + assert.match( + initializationSection[1], + /Whether initialization reports success or failure[\s\S]*?`test -f` and `test -r`/, + ); + assert.match( + initializationSection[1], + /Never expose an\s+absolute path, raw filesystem error, command arguments, Plan bytes, state contents, or unparsed command output/, + ); + assert.match( + initializationReference[1], + /checks are evidence about local state, not a substitute for the command's error code/, + ); + assert.match( + initializationReference[1], + /unknown code[\s\S]*?fail closed[\s\S]*?preserve it, and stop/, + ); + + const hasExpectation = (evaluation, fragment) => + evaluation.expectations.some((expectation) => + expectation.includes(fragment), + ); + const invalidArguments = cases.find( + ({ id }) => id === "invalid-init-arguments", + ); + assert.match(invalidArguments.prompt, /"error":"invalid_arguments"/); + assert(hasExpectation(invalidArguments, "Branches on invalid_arguments")); + assert(hasExpectation(invalidArguments, "one deliberately corrected invocation")); + + const localFailure = cases.find( + ({ id }) => id === "local-initialization-failed", + ); + assert.match(localFailure.prompt, /"error":"local_initialization_failed"/); + assert(hasExpectation(localFailure, "Branches on local_initialization_failed")); + assert(hasExpectation(localFailure, "project-relative metadata")); + assert(hasExpectation(localFailure, "Does not expose")); + assert.deepEqual( + localFailure.artifacts.map(({ stage_as: stageAs }) => stageAs), + [".firstdraft/state.json"], + ); + + const unknownOutput = cases.find(({ id }) => id === "unknown-init-output"); + assert.match(unknownOutput.prompt, /not one parseable JSON object/); + assert(hasExpectation(unknownOutput, "Fails closed")); + assert(hasExpectation(unknownOutput, "Does not repeat or expose")); + assert.deepEqual( + unknownOutput.artifacts.map(({ stage_as: stageAs }) => stageAs), + [".firstdraft/state.json"], + ); +}); + test("malformed source fixture is bound to its coordinate diagnostic", async () => { const fixtureDirectory = path.join( evalsDirectory,