Teach Skill to compile valid Plans - #10
Conversation
Make Compilation a separately approved action while letting the CLI own mutation, polling, artifact verification, and publication. Pin the merged CLI contract and cover every result so agents fail closed without private-state access or unsafe retries.
f749012 to
110cb20
Compare
Technical reviewThe consumer side of firstdraft/cli#9, and the strongest cross-repository verification in this series so far. Verification reproducedThe pinned baseline The "all 16 compile error envelopes" claim checks out. The script exercises 22 distinct codes in total, and exactly 16 of them are reachable from The other six belong to My earlier finding is closedOn #9 I noted the contract check covered six of eight status codes, and that the missing The adversarial block is the best thing hereThis is the part I did not expect, and it is worth other projects copying. The check feeds the real CLI a malicious compilation artifact and asserts what happens to the disk. Three shapes: { label: "traversal", artifactPath: "../traversal-escape.rb", ... }
{ label: "absolute", artifactPath: null, /* an absolute path */ ... }
{ label: "mode", artifactPath: "bin/unsafe", mode: 0o4755, ... }and then, for each: assertErrorEnvelope(result, 1, "invalid_artifact", [...]);
assert.equal(existsSync(path.join(directory, "generated")), false);
if (escapedPath !== null) assert.equal(existsSync(escapedPath), false);Two assertions rather than one. The CLI must reject and nothing may reach the filesystem, including outside the destination. A rejection that still left a partial tree, or wrote the escaped file before noticing, would fail here. I audited those same defenses in the CLI on #9 by reading the code. This turns that reading into a regression test, owned by the consumer, pinned to an exact revision. If someone weakens the CLI's path validation later, this repository's CI fails. That is a considerably better arrangement than trusting a review from three hours ago. The setuid case is a good instinct too. An artifact requesting mode The block that follows covers internal integrity: a wrong file digest, a wrong manifest digest, and mismatched provenance, each expected to be refused. The approval model is stated preciselyThe contract section is worth reading closely because it says something easy to get wrong:
Then it enumerates what compilation additionally requires: a successful push and a terminal "Observed in the current workflow" and "no subsequent local edit" are the two clauses doing real work. Without the first, an agent could rely on a The single carve-out is well chosen. After No findingsNothing worth reporting. One note for the record: my first attempt at counting the exercised codes used a narrow pattern, came back with 11, and looked like it contradicted the body. It was my regex missing call shapes rather than a real gap, which I confirmed before writing any of this down. |
Lesson: yes to one thing is not yes to everythingOne sentence in this PR is worth more than the fifteen hundred lines around it:
That is a rule about consent, and it is the kind of thing that gets skipped because the alternative feels helpful. The user already said yes to four things. Surely they meant yes to the fifth? No. And the reason is worth internalizing, because you will write this bug in a Rails app long before you write it in an agent. How permission quietly widensYou start with a check that means one thing. def edit?
user.admin?
endThen someone needs bulk import. Same permission, roughly. Then export. Then a "sync" that writes to a partner system. Then a destructive re-index. Nobody decided that "can edit" should mean "can push data to a third party." It happened one reasonable-looking step at a time, and now a permission somebody granted for one purpose authorizes six. The agent version is faster and less visible. A user says "yes, send it to the server for diagnostics." An agent that treats that as general approval might push, then compile, then deploy, all under one yes. Each step looked like a continuation of the last. The user approved the first one. What a scoped approval looks likeThis Skill does not just say compilation needs approval. It says what has to be true:
Two of those are subtle enough to be worth pulling out. "Observed in the current workflow" rules out relying on a "No subsequent local edit" closes the gap between approval and action. A user approves compiling what they just reviewed. If the Plan changes in between, the approval no longer refers to anything real. That second one is the general form: an approval is attached to a specific state of the world, and it expires when that state changes. Most consent bugs are a failure to notice the world moved. The Rails versionYou have seen the mechanism even if you have not named it.
Signed URLs carry an expiry, because "you may download this" was never meant to be forever. Optimistic locking with And in a policy object, the discipline is to name the action rather than the role: # Widens silently
def manage?
user.admin?
end
# Each capability decided on purpose
def edit? = user.admin?
def export? = user.admin? && user.confirmed_data_agreement?
def destroy? = user.owner?More lines. Every one of them a decision somebody made rather than inherited. The carve-out, and why it is safeOne exception here is instructive. If compilation fails with That is safe for a specific reason: the CLI proves that failure happened before any network request. Nothing was started, so nothing can be duplicated by trying again. The approval still refers to the same Plan and the same intent, only the destination changed. Compare that with retrying after an ambiguous failure, which this design refuses. There the request may already have taken effect, and retrying could compile twice. The test for whether an exception is safe is not "is this annoying" but "does the original approval still describe what will happen." Here it does. After an ambiguous mutation it does not. The habitWhen you write a permission check, ask what it will mean in a year after four people have added features near it. Name the specific action rather than a role. Attach approvals to the state they were given for. Let them go stale when the state changes. And when you do allow a retry, be able to say why the original yes still applies. Being asked twice is a small cost. Discovering that one yes authorized something the user never considered is not. |
Summary
create-full-stack-appto compile an unchanged analyzer-valid Plan only after separate user approvalplan compileresult without direct requests, private-state access, or unsafe retriesContract
Approval to author, push, analyze, or correct a Plan never implies approval to compile it. Compilation requires:
validanalysis observed in the current workflow;plan compile.The CLI owns the single conditional start request, pinned polling, artifact verification, and atomic materialization. The Skill stops on every handled failure except that a user may explicitly choose a different absent path after
invalid_output_path, which the CLI proves occurred before any network request.The current smoke boundary is deliberately narrow: one Entity using supported scalar Fields, generated locally. This does not claim arbitrary Foundation Plan support, deployment, production readiness, or mobile clients.
External baseline
The Skill pins merged CLI commit
36f1292. The contract check verifies that exact revision before importing its source runner and before packing and installing the executable.Verification
npm ci --ignore-scriptssh script/check— 14 repository testsnode script/check-cli-contract.mjs <cli-checkout>— all 16 compile error envelopes, success, privacy, hostile artifact paths and modes, internal integrity failures, materialization cleanup, and a fresh packed executablenpm run checkin the pinned CLI checkout — 111 tests plus type, lint, format, package, and installed-package smoke checks