Integrate the initial First Draft CLI

[raghubetina]

Summary

• establish the source-executed, dependency-free firstdraft command shell and audited package boundary
• initialize private local sketch/0.19 Foundation Plan drafts with client-minted Project identity
• push exact Plan bytes through the conditional whole-document API while preserving opaque ETags and honest recovery behavior
• generate UUIDv7 identities for independently mutable authored subjects without reading local state or using the network

Integration boundary

This PR preserves the four logically discrete commits previously reviewed in the component stack. It introduces no behavior beyond those slices and adds no authentication, pull, reconciliation, publication, or Compilation command.

The component PRs remain open as the slice-level review history:

• Scaffold the auditable CLI package #1 — auditable CLI and package scaffold
• Initialize local Foundation Plan drafts #2 — local Foundation Plan initialization
• Push local Foundation Plans conditionally #3 — conditional exact-byte Plan push
• Generate Foundation Plan subject IDs #4 — Foundation Plan subject identity generation

They will remain open until this integration PR has completed hosted review and CI.

Verification

• pinned Node.js 24.18.0: npm run check
• typecheck, lint, and formatting checks
• 60 unit and integration tests
• exact package-content verification
• offline installed-package smoke test

[raghubetina]

Technical review

A note on scope first. The body positions this as an integration of #1 through #4, so a merge-shaped review would be the natural approach. Those four opened before this watcher's cutoff and were never reviewed here, so nothing in the diff has been read before. This is a full review of the source rather than a check that four reviewed slices survived integration.

The branch is also a fast-forward line of four commits rather than a merge, so there is no resolution to audit. What the PR is really asking for is a first read of the whole CLI.

Verification reproduced

npm run check on pinned Node 24.18.0 exits 0. Typecheck, ESLint, Prettier, 60 tests passing, pack:check, and pack:smoke all clean. That matches the claimed 60 tests and the listed gates exactly.

The conditional push is the load-bearing part, and the safety argument holds

Everything else in this PR is scaffolding around one operation: replacing a whole Foundation Plan on the server without ever silently overwriting somebody else's work. The mechanism is a precondition on every request, chosen by whether local state already knows an ETag:

...(state.foundation_plan_etag
  ? { "If-Match": state.foundation_plan_etag }
  : { "If-None-Match": "*" })

I traced the failure paths to see whether any of them can lose a Plan, since that is the property worth having. None can:

• Local read fails. No request is sent at all, and the error text says so.
• Network fails mid-flight. The Plan may or may not have been stored, and local state keeps no ETag. The next push therefore still sends If-None-Match: *, which a server holding a Plan answers with 412. It fails loudly instead of clobbering.
• Server rejects. Several tests assert local state is unchanged byte for byte.
• Server accepts but the state write fails. This is the interesting one, and it is handled rather than ignored. PlanPushStateWriteError carries the recovery state, and the CLI prints it as JSON with an instruction not to push again. A push that ignores that advice sends a stale precondition and gets 412.

So the design's real strength is that the precondition makes every failure mode safe, including the ones the client cannot detect. The recovery path exists to keep the user unstuck, not to prevent data loss, because the conditional header already prevents that.

Two details I want to call out as good rather than merely correct. redirect: "error" means a redirect never carries the If-Match header or the Plan body to another origin. And the client verifies the server's echoed digest against what it sent:

foundationPlan.source_sha256 === sourceSha256

That turns "the server said 200" into "the server stored exactly the bytes I sent", which is a meaningfully stronger claim.

The supporting evidence is unusually well chosen

Three things stood out as the kind of check that catches real regressions rather than restating the implementation.

The UUIDv7 test uses the RFC's own vector. test/uuid-v7.test.js asserts against RFC 9562 Appendix A.6, and it is a genuine external check: timestamp 1645557742000 is 0x017F22E279B0, which is exactly the leading 017f22e2-79b0 of the expected output. The implementation itself is correct on layout, including preserving 74 random bits after the version and variant nibbles are stamped.

scripts/check-pack.js asserts exact tarball contents. Ten paths, compared with deepEqual against a literal list. A stray file added under src/ fails CI, and so does an intended file dropped from files. That is what makes the "audited package boundary" claim checkable rather than aspirational, and it pairs with zero runtime dependencies and source published unbundled.

Errors never echo user input. Tests push canary-secret-option and canary-secret-positional through the argument parser and assert the strings never appear in stderr. For a tool an agent invokes with generated arguments, that keeps stray input out of logs.

The CI matrix is also right in a way that is easy to get wrong. It runs the engines floor of 22.0.0 across Linux, Windows, and macOS, and separately runs the pinned 24.18.0. Claiming >=22 and testing only 24 would have been the usual mistake. Actions are pinned to commit SHAs.

One design observation for agent callers, not a defect

This is a suggestion rather than something wrong, and it only matters because the intended caller is an agent rather than a person.

Five distinct push failures collapse to exit code 1: local read, network, protocol, server rejection, and the state-write failure. Exit 2 is reserved for usage and configuration. The situations call for different responses, though. A network failure is safe to retry. A server rejection needs the Plan fixed. A state-write failure specifically must not be retried until state is repaired, which is exactly what its message says.

The stderr format also varies by case. The state-write failure emits JSON with a machine-readable error: "local_state_not_saved". A server rejection emits the server's own JSON. The local, network, and protocol failures emit prose. So an agent has to try parsing stderr as JSON, then branch on which shape it got, then fall back to matching prose for the remaining three. That last step breaks whenever a message is reworded.

Either distinct exit codes or an error discriminator on every failure would remove the guesswork. Worth deciding before the surface is public, since exit codes are hard to change later.

I should note one hypothesis I checked and discarded. The push tests exercise pushPlan directly for most cases, so I initially suspected the CLI's error-to-exit-code mapping was untested. It is not: test/plan-push.test.js invokes the full runner through an invoke helper and asserts statuses, stderr text, and the recovery JSON end to end. The mapping above is deliberate and covered.

Small notes

saveState writes to a randomly named temporary path with flag: "wx" and flush: true, then renames. If the rename fails, the temporary file stays behind in .firstdraft/. Harmless, since the name is unique per attempt and the error is reported, but it accumulates on a persistently failing filesystem.

isStrongEtag bounds length with Buffer.byteLength, which counts UTF-8 bytes, while an ETag containing obs-text arrives as one JavaScript character per wire byte. The bound is therefore conservative rather than exact. It rejects sooner than 1024 wire bytes, never later, so nothing unsafe follows from it.

[raghubetina]

Lesson: the lost update, and the header that prevents it

Here is a bug that no amount of careful Ruby will fix.

Two people open the same record. Both edit. Both save. The second save wins completely, and the first person's work is gone with no error, no warning, and nothing in the log to suggest anything happened.

10:00  Alice loads the plan
10:01  Bob loads the plan
10:02  Alice saves her changes
10:03  Bob saves his changes    <- Alice's work is now gone

Bob never saw Alice's version. His browser sent the state of the world as of 10:01, and the server took it. This is called the lost update problem, and it is not a race condition in the usual sense. There is no tight timing window. Bob had three full minutes to cause it.

The CLI in this PR pushes an entire Foundation Plan document to the server in one request, so it has this problem in its purest form. Two agents working on the same project would overwrite each other silently. What follows is how it avoids that.

Version numbers, and why HTTP already has one

The instinct from Rails is lock_version, and that instinct is right. Optimistic locking adds an integer column, sends it with the update, and raises ActiveRecord::StaleObjectError if it does not match what the database holds.

HTTP has the same idea built in, and you have probably seen half of it. When a server sends a response it can include an ETag, which is an opaque string identifying that exact version of that resource:

ETag: "a1b2c3"

You most likely know this from caching. Rails' fresh_when and stale? use it so a browser can ask "has this changed since version a1b2c3?" and get a cheap 304 Not Modified back. That is the read side.

The write side uses the same tag and is far less widely used. The client sends the version it is trying to replace:

If-Match: "a1b2c3"

The server compares. If the resource is still at a1b2c3, the write proceeds. If somebody else has changed it, the server refuses with 412 Precondition Failed and changes nothing. It is lock_version with the version living in a header instead of a column.

Note what that buys you: the check and the write happen in one request, on the server, where nothing can slip between them. A client that fetched first and then wrote would have a window between the two. This has no window.

The trick for creating something that does not exist yet

If-Match handles updates, but the very first push is different. There is no version to match, and the thing you want is "create this, but only if nobody beat me to it."

The header for that reads oddly the first time:

If-None-Match: *

* means "any version at all," so the condition is "proceed only if no version exists." First push succeeds with 201 Created. If another client got there first, you get 412 instead of quietly flattening their work.

So the CLI picks its precondition based on whether it has seen an ETag before:

...(state.foundation_plan_etag
  ? { "If-Match": state.foundation_plan_etag }
  : { "If-None-Match": "*" })

Two lines, and every write is now conditional.

The part that took me a while to appreciate

Storing the ETag locally after a successful push introduces an obvious worry. What if the push succeeds and then writing the local file fails? The disk is full, or the directory went read-only. The server has your new Plan, and your machine has no record of it.

The tempting reaction is that this needs careful handling to avoid corruption. It does not, and the reason is worth sitting with.

Your local state still holds the old ETag, or no ETag at all. So your next push sends a precondition that is now stale, and the server answers 412. You are stuck, but nothing is damaged. The precondition protects you even in the situation where your client has lost track of reality, which is exactly when you would otherwise be most dangerous.

That changes what the error handling is for. It is not preventing data loss, because the header already did. It is getting the user unstuck, which is why the CLI prints the ETag it received and tells you not to push again until you have repaired local state:

{
  "error": "local_state_not_saved",
  "detail": "The Plan was accepted, but its ETag could not be saved. Do not push again until local state is repaired.",
  "recovery_state": { "foundation_plan_etag": "\"a1b2c3\"", "...": "..." }
}

The same reasoning covers a network failure mid-request. You genuinely cannot know whether the server got it. You do not need to know, because whichever happened, your next conditional push either works or gets refused. It never destroys anything.

That is the general shape worth taking away. Design so that "I do not know what happened" and "nothing bad can happen" are compatible. A system that requires the client to know the truth will eventually be wrong about it.

Where this shows up in your Rails work

Any form that edits a record somebody else might also be editing has this problem, and lock_version is usually the cheapest fix. Add the column, put it in a hidden field, and rescue ActiveRecord::StaleObjectError to re-render the form with a message rather than a 500.

If you are building a JSON API, the HTTP version is worth knowing because it is standard and clients already understand it. Return an ETag on show, honor If-Match on update, and answer 412 when it does not match. A client that ignores the header keeps working exactly as before, so you can adopt it without breaking anyone.

The general rule: a blind overwrite is only safe when you are the only writer. As soon as that stops being true, and it usually stops being true earlier than expected, you want the write to carry a claim about what it expects to be replacing.

[raghubetina]

Thanks — Skills #6 makes this a concrete consumer contract rather than hypothetical polish. I agree the Skill should not have to distinguish recovery branches by matching English prose.\n\nI do not think this should amend the integration PR. The current behavior remains fail-closed and preserves conditional-write safety; changing the output contract while integrating the four existing commits would obscure that review. The first post-integration CLI slice should make plan push failures uniformly machine-readable, followed by the corresponding Skill update.\n\nA bounded contract:\n\n- Keep the current exit-status classes: 0 for success, 1 for operational or remote failure, and 2 for usage or configuration.\n- Emit exactly one JSON object to stderr for every handled plan push failure.\n- Give that object a stable top-level error discriminator, with a small vocabulary such as invalid_arguments, invalid_configuration, local_input_unreadable, request_outcome_unknown, server_rejected, and the existing local_state_not_saved.\n- Use request_outcome_unknown for both transport ambiguity and a success-shaped response that could not be verified. Its recovery rule is stop and reconcile; do not infer acceptance or retry.\n- Have server_rejected include the HTTP status and a validated server problem or diagnostic payload when safe. Keep recovery_state only on local_state_not_saved.\n- Never echo argv, local Plan bytes, raw network errors, or unvalidated response bodies.\n- Exercise every branch through the full CLI runner and the packed artifact.\n\nThat lets Skills #6 key both damaged-local-files and ambiguous-outcome behavior entirely on stable codes. I am leaving #5 unchanged and unmerged.