Skip to content

Teach Skill to compile valid Plans - #10

Merged
raghubetina merged 1 commit into
mainfrom
codex/skill-plan-compile
Jul 31, 2026
Merged

Teach Skill to compile valid Plans#10
raghubetina merged 1 commit into
mainfrom
codex/skill-plan-compile

Conversation

@raghubetina

Copy link
Copy Markdown
Contributor

Summary

  • teach create-full-stack-app to compile an unchanged analyzer-valid Plan only after separate user approval
  • keep the first local compiler workflow honest about its one-Entity scalar-Field boundary and lack of deployment
  • define closed recovery for every plan compile result without direct requests, private-state access, or unsafe retries
  • add behavioral evals and an exact cross-repository contract against the merged CLI source and a freshly packed executable

Contract

Approval to author, push, analyze, or correct a Plan never implies approval to compile it. Compilation requires:

  • a successful push and terminal valid analysis observed in the current workflow;
  • no subsequent local Plan edit;
  • explicit user approval for Compilation and a named absent destination; and
  • an installed CLI that exposes 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-scripts
  • sh script/check — 14 repository tests
  • node 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 executable
  • npm run check in the pinned CLI checkout — 111 tests plus type, lint, format, package, and installed-package smoke checks

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.
@raghubetina
raghubetina force-pushed the codex/skill-plan-compile branch from f749012 to 110cb20 Compare July 31, 2026 02:24
@raghubetina

Copy link
Copy Markdown
Contributor Author

Technical review

The consumer side of firstdraft/cli#9, and the strongest cross-repository verification in this series so far.

Verification reproduced

sh script/check                              → 14 tests pass
node script/check-cli-contract.mjs <cli>     → exit 0

The pinned baseline 36f1292 is cli#9's merge commit and is currently the tip of that repository's main. Pointed at the previous merged CLI commit the contract check exits 1, so the binding still bites.

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 plan compile:

invalid_arguments, invalid_configuration, invalid_output_path, local_input_unreadable,
project_not_pushed, project_not_valid, analysis_not_found, analysis_failed,
analysis_changed, compilation_changed, compilation_start_rejected,
compilation_status_unavailable, compilation_wait_timed_out, invalid_compilation_status,
invalid_artifact, materialization_failed

The other six belong to plan init, push, and status.

My earlier finding is closed

On #9 I noted the contract check covered six of eight status codes, and that the missing invalid_server_response was the "do not retry" half of a matched pair whose retryable counterpart was verified. Both it and server_rejected are now exercised against the real CLI.

The adversarial block is the best thing here

This 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 0o4755 on a generated binary would be a privilege-escalation vector, and it is not the sort of thing you think to test unless you have been bitten.

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 precisely

The contract section is worth reading closely because it says something easy to get wrong:

Approval to author, push, analyze, or correct a Plan never implies approval to compile it.

Then it enumerates what compilation additionally requires: a successful push and a terminal valid analysis observed in the current workflow, no subsequent local edit, explicit approval, a named absent destination, and a CLI that exposes the command.

"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 valid result from a previous session describing different bytes. Without the second, a user could approve compilation, edit the Plan, and get an artifact of something they never reviewed.

The single carve-out is well chosen. After invalid_output_path the user may name a different absent path, and the body notes the CLI proves that failure occurred before any network request, so retrying cannot duplicate a mutation.

No findings

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

@raghubetina

Copy link
Copy Markdown
Contributor Author

Lesson: yes to one thing is not yes to everything

One sentence in this PR is worth more than the fifteen hundred lines around it:

Approval to author, push, analyze, or correct a Plan never implies approval to compile 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 widens

You start with a check that means one thing.

def edit?
  user.admin?
end

Then 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 like

This Skill does not just say compilation needs approval. It says what has to be true:

  • a successful push and a terminal valid analysis observed in the current workflow
  • no subsequent local edit
  • explicit approval for compilation specifically
  • a named destination that does not already exist
  • a CLI that actually exposes the command

Two of those are subtle enough to be worth pulling out.

"Observed in the current workflow" rules out relying on a valid result from an earlier session. That analysis described bytes that may no longer exist. Freshness is part of the permission, not a separate concern.

"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 version

You have seen the mechanism even if you have not named it.

form_authenticity_token scopes a form submission to a session. A stolen form from another site does not carry it, which is the whole of CSRF protection.

Signed URLs carry an expiry, because "you may download this" was never meant to be forever.

Optimistic locking with lock_version is exactly the "no subsequent edit" rule in database form: your update carries a claim about what you were editing, and the write fails if somebody else changed it.

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 safe

One exception here is instructive. If compilation fails with invalid_output_path, the user may name a different path without starting over.

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 habit

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

@raghubetina
raghubetina merged commit e24b438 into main Jul 31, 2026
2 checks passed
@raghubetina
raghubetina deleted the codex/skill-plan-compile branch July 31, 2026 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant