Skip to content

Push local Foundation Plans conditionally - #3

Draft
raghubetina wants to merge 1 commit into
codex/plan-initfrom
codex/plan-push
Draft

Push local Foundation Plans conditionally#3
raghubetina wants to merge 1 commit into
codex/plan-initfrom
codex/plan-push

Conversation

@raghubetina

Copy link
Copy Markdown
Contributor

Summary

  • add firstdraft plan push for argument-free, whole-document Foundation Plan submission
  • send the exact local bytes with If-None-Match: * on create and replay the complete server ETag with If-Match on replacement
  • pin the first accepted API origin and validator in private local state so neither can be silently redirected or reconstructed
  • validate response status, media type, Project identity, exact-source digest, diagnostics, ETag syntax, and size before changing state
  • replace state atomically and emit verified recovery state if a successful request cannot be recorded locally
  • document ambiguous post-send failures honestly; the CLI does not trust a header-only validator or infer one from the Plan digest

Verification

  • npm run check on the pinned Node 24.18.0: typecheck, lint, format, 56 tests, package allowlist, and packed-package smoke test
  • exact minimum Node 22.0.0: 56 tests, package allowlist, and packed-package smoke test
  • test suite with a developer-style loopback FIRSTDRAFT_API_URL already present in the environment
  • live cross-repository process probe against firstdraft/firstdraft#162: create at graph version 1, conditional replacement at graph version 2 with ETag rotation, and source-addressed 422 diagnostics with local state unchanged

Stack

Current boundary

The server slice still accepts only the empty starter Plan shape. Authentication, pull/reconciliation, non-empty import, and publish remain later work. If a request is accepted but its response cannot be fully verified, local state stays unchanged and manual recovery may be necessary until a reconciliation endpoint exists.

Send the exact local Plan representation with a create-or-match
precondition so agents cannot silently overwrite a newer remote
head.

Pin the accepted API origin and opaque ETag in private local state,
validate response identity before updating it, and report ambiguous
transport outcomes without inventing recovery data.
@raghubetina

Copy link
Copy Markdown
Contributor Author

Review: technical

Reviewed the single commit a732fb2..af33be3. The claim that matters here spans two repositories, so I booted firstdraft/firstdraft@4d956a5 on loopback and drove this CLI against it rather than a stub. Nothing to fix.

The live round trip

Server: the #162 head, RAILS_ENV=test, real PostgreSQL, on http://127.0.0.1:3199. Client: bin/firstdraft.js from this branch, in a scratch directory.

Step Result
plan init then plan push 201, outcome: "created", graph_version: 1
state after create project_id, api_url, and the complete foundation_plan_etag recorded
edit the Plan, push again 200, outcome: "updated", graph_version: 2, ETag rotated
push the identical bytes again 200, ETag unchanged, graph_version still 2
push a Plan with entities 422 foundation_plan.import.unsupported_bootstrap_content at /application/entities, exit 1, local state byte-identical
push with a tampered stored validator 412 problem+json, exit 1

The two failure rows are the ones worth having: a rejected Plan and a stale validator both leave state.json untouched, so a failed push cannot desynchronize the client from the server.

Also verified

  • npm run check passes on the pinned Node 24.18.0: typecheck, lint, format, 56 tests, the package allowlist, and the packed-package smoke test. The test count matches the body.
  • Redirects are refused rather than followed. I put a 302 redirector in front of the API and pushed at it. The request fails with redirect: "error", and the message is honest about what it cannot know: "The Plan may have been accepted; local state was not changed." That is the correct thing to say when a request left the process and the response was never validated.
  • The origin is pinned, not merely defaulted. After a successful push records api_url, running with a different FIRSTDRAFT_API_URL exits 2 with a configuration error instead of quietly sending the document somewhere else.
  • Only a strong validator is accepted. STRONG_ETAG_PATTERN admits %x21, %x23-7E, and %x80-FF between quotes and has no W/ branch, so a weak validator from a server is refused rather than stored. That is the right pairing with the server side, where If-Match uses strong comparison, so a stored weak validator could only ever produce a 412 later.
  • Success is validated, not assumed. Before touching local state the client requires the exact expected status (201 when no ETag is stored, 200 when one is), application/json, a syntactically strong ETag within a size bound, and a body whose project.id equals the ID it sent and whose foundation_plan.source_sha256 equals the digest of the bytes it just uploaded. A response that agrees about everything except which Project it describes is refused.

Design notes

  • Recomputing the digest locally and comparing it to the server's is the detail that makes the rest trustworthy. The client never adopts a validator for a representation it cannot prove the server received, which is what stops a proxy or a confused server from pinning state to bytes nobody sent.
  • PlanPushStateWriteError carrying recoveryState is the honest handling of the one genuinely unrecoverable case: the server accepted the document and the local write failed. Printing the state the user needs beats either silently dropping it or pretending the push failed.
  • The 30-second timeout, the 2 MiB response cap, and the 1 MiB Plan cap are all injectable or constant, and the Plan cap matches the server's own body limit, so an oversized document fails locally instead of consuming a request.

Small

  • STRONG_ETAG_PATTERN spells the upper range as \x80-\xff, which in a JavaScript regex means U+0080 to U+00FF rather than raw bytes. That lines up with how fetch exposes header values, so it is correct here, and it is the kind of thing worth a comment if the pattern ever moves somewhere that handles buffers.
  • Both plan init and plan push now read state written by the other. A shared loader for state.json would keep the two commands' notions of the file shape from drifting, though at two commands it is not yet worth it.

@raghubetina

Copy link
Copy Markdown
Contributor Author

Lesson: what this PR actually did

The client half of the conditional PUT contract. The reusable ideas are validating a success response before believing it, and being honest when you genuinely cannot tell whether a write happened.

The one-sentence version

firstdraft plan push sends the exact bytes of your local Plan to the server, using If-None-Match: * the first time and replaying the server's ETag with If-Match afterwards, and it updates local state only after checking that the response describes the document it actually sent.

Two repositories, one contract

The server side (firstdraft/firstdraft#162) added conditional replacement: an unconditional PUT is refused, creation requires If-None-Match: *, and replacement requires an If-Match naming the version you read. This PR is the other half.

The client keeps three things in .firstdraft/state.json: the Project ID it minted, the API origin it first talked to, and the complete ETag the server last returned. Which conditional header it sends follows directly from whether that ETag exists:

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

I checked the whole loop against a real server rather than a stub: init and push created the Project at graph version 1, editing the Plan and pushing again returned 200 with a rotated ETag at version 2, pushing the identical bytes again changed nothing, and a Plan the server rejects came back as a 422 pointing at /application/entities with local state untouched.

Do not believe a 200

The habit worth stealing is what happens between "the server said 200" and "update local state". A less careful client stores whatever ETag came back. This one refuses the response unless all of these hold:

  • the status is exactly the one implied by the request it made (201 when creating, 200 when replacing);
  • the content type is application/json;
  • the ETag is syntactically a strong validator, within a size bound;
  • project.id in the body equals the Project ID it sent;
  • foundation_plan.source_sha256 in the body equals the SHA-256 the client computed over the bytes it just uploaded.

That last check is the interesting one. The client hashes the file itself and compares against the server's claim. If those disagree, something in the middle changed the bytes, or the response describes a different document, and adopting that ETag would pin local state to a version the user never wrote. Comparing a value you computed against a value you were told is a cheap way to detect a whole class of "the network was not transparent" problems.

The same skepticism applies to which server it is talking to. redirect: "error" means a 302 is a failure rather than a silent hop to another origin, and once a push succeeds, the API origin is pinned in state. Running later with a different FIRSTDRAFT_API_URL is a configuration error rather than a quiet re-target. I tested both by putting a redirector in front of the API and by changing the URL after a successful push.

The failure you cannot resolve

Every network client has one genuinely ambiguous case: the request went out and no valid response came back. The write may or may not have happened.

Most tools paper over this. This one says it plainly:

Could not complete the First Draft request. The Plan may have been accepted; local state was not changed.

Both halves matter. "May have been accepted" is the truth. "Local state was not changed" tells you what the tool did about it, which is nothing, because guessing either way would be worse: recording an ETag it never validated would corrupt the next push, and pretending failure would hide a change that landed.

There is a rarer cousin handled the same way. If the server accepts the document and the local state write then fails, the push genuinely succeeded but the client cannot record it. Rather than swallow that, the error carries the state that should have been written so the user can save it by hand.

The general rule: when you cannot determine what happened, say so and change nothing. An honest "I do not know" is more useful than a confident wrong answer, because the user can act on it.

Why the client refuses weak validators

ETags come in strong and weak forms, and the client's pattern for a valid ETag has no W/ branch at all, so a weak validator from a server is rejected rather than stored.

That is not fussiness. The server compares If-Match using strong comparison, as RFC 9110 requires for lost-update protection. A weak validator stored locally could therefore never authorize a replacement; every later push would come back 412 with no obvious cause. Refusing it at the moment it arrives turns a confusing future failure into an immediate, local one.

That is the theme running through this command. Every check happens as early as possible and as close as possible to the thing being checked, so failures name their cause instead of surfacing three steps later as something unrelated.

@raghubetina

Copy link
Copy Markdown
Contributor Author

The first portable consumer scaffold is now firstdraft/skills#1. It uses the reviewed init/push capabilities, never installs or upgrades the CLI automatically, requires explicit network intent, and stops before creating authored subjects until the next CLI slice exposes subject-ID generation.

@raghubetina

Copy link
Copy Markdown
Contributor Author

The next stacked CLI slice is #4. It adds the pure firstdraft plan subject-id command on top of this push branch, with no Plan, state, working-directory, or network access. It does not change the conditional PUT contract in this PR.

@raghubetina

Copy link
Copy Markdown
Contributor Author

Server capability has advanced without changing this CLI transport contract. firstdraft/firstdraft#190 adds atomic import and replacement for Entities, short_text Fields, and Field or system-Field Primary Descriptors; firstdraft/firstdraft#192 expands that conditional PUT to boolean, date, datetime, decimal, integer, language_code, long_text, short_text, time_zone, and url. The empty-starter-only boundary in this PR body is superseded by that pending stack. plan push needs no change because it already sends exact bytes and relays verified success or diagnostics.

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