Skip to content

Generate Foundation Plan subject IDs - #4

Draft
raghubetina wants to merge 1 commit into
codex/plan-pushfrom
codex/plan-subject-id
Draft

Generate Foundation Plan subject IDs#4
raghubetina wants to merge 1 commit into
codex/plan-pushfrom
codex/plan-subject-id

Conversation

@raghubetina

Copy link
Copy Markdown
Contributor

Summary

  • Add firstdraft plan subject-id, which prints one fresh lowercase UUIDv7 and performs no Plan, state, working-directory, or network access.
  • Document the sketch/0.19 continuity rule: retain subject_uuid through renames and same-kind moves, and mint a new value for a replacement concept.
  • Exercise direct dispatch, invalid and help paths, production freshness, and the installed tarball.

Why

The pre-Compilation Skill needs a safe way to obtain IDs before adding independently mutable authored subjects. Keeping generation in this zero-runtime-dependency CLI avoids making the Skill invent randomness or mutate the Plan implicitly.

This command enables local authoring only. The current server slice still rejects nonempty Plans, as documented in firstdraft/skills#1.

Verification

  • Exact Node 22.0.0 compatibility lane: 60 tests, package allowlist, and installed-package smoke pass.
  • Pinned Node 24.18.0 quality lane: typecheck, lint, format, 60 tests, package checks, and audit pass with zero vulnerabilities.
  • Max-effort local review raised three gaps; all were addressed. Re-review approved the complete diff.

The final review also observed that the pre-existing init and push help paths resolve the working directory before parsing. This branch does not regress that behavior and strictly removes working-directory lookup from root and subject-id paths, so I kept that separate from this narrow slice.

Stack

Foundation Plan authors need stable identities before adding
independently mutable subjects, but the Skill should not invent or
persist them itself.

Expose the existing UUIDv7 generator as a pure leaf command with no
project or network access. Document the continuity rules and exercise
both direct and packed-package paths at the minimum runtime.
@raghubetina

Copy link
Copy Markdown
Contributor Author

Review: technical

Reviewed the single commit af33be3..0681afd (parent confirmed as #3). Nothing to fix.

Verified

  • npm run check passes: typecheck, lint, format, 60 tests, package allowlist, and the installed-tarball smoke test, matching the body.
  • The generated IDs are correct RFC 9562 v7 values. Five successive calls all parse as version 7 with variant bits 10, are lowercase, are distinct, and carry non-decreasing timestamps. Time ordering is the property the identity design actually depends on, so it is worth asserting rather than assuming from the generator's name.
  • The command really does no local work. Running it in an empty directory with no .firstdraft prints an ID and leaves zero files behind.
  • The working-directory claim holds, and it is a genuine refactor rather than a description. getCwd is now a function defaulting to process.cwd, and cwd ?? getCwd() is evaluated inside the init and push branches only. The subject-id branch never reaches it, so the process's working directory is never resolved on that path. That matters for the intended caller: an agent may invoke this from any context, including one where the working directory is unreadable, and an ID generator has no business caring.
  • The cross-repo gate now has something behind it. Scaffold full-stack application Skill skills#1 instructs an agent to check whether firstdraft plan --help lists subject-id before creating a new subject, and to refuse to invent a UUID if it does not. The help output here lists it, so the capability probe the Skill performs will now succeed against this branch and continue to fail safely against a released CLI that predates it.

Design notes

  • Keeping ID generation in the CLI rather than in the Skill is the right division, for the reason the body gives: a Skill that mints its own randomness is a Skill that has to be trusted with it. Prose instructing an agent to "generate a UUIDv7" would in practice produce whatever the agent felt like, including copied example values, which is exactly what the Skill's wording forbids.
  • Documenting the continuity rule beside the command (retain subject_uuid through renames and same-kind moves, mint a new one for a replacement concept) puts the rule where someone reaches for the tool. That rule is the whole reason these IDs exist, and it is easy to lose when the command is one line of output.
  • Leaving the pre-existing eager working-directory resolution in the init and push help paths alone, and saying so in the body rather than fixing it silently, keeps this slice narrow. Worth doing eventually, since --help should not need a readable working directory either, but not here.

@raghubetina

Copy link
Copy Markdown
Contributor Author

Lesson: what this PR actually did

One command that prints one ID. The ideas worth keeping are why a tool should do less than it could, and why "generate an ID" belongs in code rather than in instructions.

The one-sentence version

firstdraft plan subject-id prints a fresh UUIDv7 and does nothing else: no files read, no files written, no network, not even a look at the current directory.

Why this needs to exist at all

Every independently mutable thing in a Foundation Plan carries a subject_uuid. That ID is how First Draft knows, across two versions of a document, that a renamed Entity is the same Entity. Get it wrong and the system either loses history or merges two unrelated things.

So when an agent adds a new Entity to a Plan, it needs a new ID. The Skill in the sibling repository could have said "generate a UUIDv7". That instruction has a predictable failure mode: an agent asked to produce a random value may produce something that looks right, or copy one from a nearby example, and both corrupt the identity model quietly.

Which is why the Skill instead says to check whether the CLI offers subject-id, use it if present, and explicitly not to invent a UUID or copy an example one otherwise. This PR is the command that check is looking for.

The transferable idea: when correctness depends on a value being generated properly, put the generation in code you can test, and have the instructions call it. Prose is a bad random number generator.

Doing less on purpose

The interesting engineering here is subtraction. The command's help text says it "reads no files and makes no network request", and the code makes that true in a way worth looking at.

Before this change, the CLI resolved the working directory when dispatching any plan subcommand. That is the natural way to write it, because two of the three subcommands need it. This PR changes cwd from a value into a function and calls it only inside the branches that need it:

if (argv[0] === "init")  return runPlanInit({ ..., cwd: cwd ?? getCwd() });
if (argv[0] === "push")  return runPlanPush({ ..., cwd: cwd ?? getCwd() });
if (argv[0] === "subject-id") return runPlanSubjectId({ /* no cwd at all */ });

I confirmed the behavior: run it in an empty directory and it prints an ID and creates nothing.

Why bother, when process.cwd() almost always works? Because the caller here is an agent, which may invoke the command from a context where the working directory has been deleted or is not readable. A command whose entire job is to print a random value should not be able to fail for a reason unrelated to printing a random value. Every input a program touches is a way it can break, and the shortest path to reliability is touching fewer things.

That principle generalizes past this command. When you add a feature to an existing dispatcher, check whether the shared setup code at the top is work your new path actually needs. Shared setup is convenient and it quietly widens the failure surface of everything downstream of it.

Verifying an ID generator

Testing "it returns a UUID" is easy and nearly worthless. The properties that matter here are specific, so I checked them directly on five successive outputs:

  • each parses as version 7 (the timestamp-ordered variety, not the random v4);
  • the variant bits are 10, as RFC 9562 requires;
  • all are lowercase, since the server's pattern and the database check constraint both expect that form;
  • all are distinct;
  • their embedded timestamps are non-decreasing, which is the entire point of choosing v7 over v4.

That last one is the property the identity design actually relies on, and it is the one a naive implementation gets wrong while still producing something that passes a "looks like a UUID" regex. If you ever review an ID generator, ordering and version bits are where to spend your attention.

@raghubetina

Copy link
Copy Markdown
Contributor Author

The structured-authoring path enabled here now has pending server support: firstdraft/firstdraft#190 imports Entities, short_text Fields, and Field or system-Field Primary Descriptors, and firstdraft/firstdraft#192 expands Fields to ten bounded scalar kinds. The body statement that the server rejects all nonempty Plans is therefore superseded by that stack. This PR remains correctly scoped to subject-ID generation and needs no code change.

@raghubetina

Copy link
Copy Markdown
Contributor Author

Cross-repository dogfood now exercises the exact packed artifact from 0681afd with firstdraft/skills#2 at e868210 and firstdraft/firstdraft#192 at cd727c8.

A fresh Skill-guided author used plan init, ran plan subject-id exactly 11 times for one Entity and ten scalar Fields, and used plan push to create a real Project through loopback HTTP into an isolated Postgres database. Controller assertions confirmed all minted UUIDv7s reached the relational graph unchanged and exact submitted bytes became the stored Head. A later rename used no new IDs and retained every database row identity.

The packed CLI also handled both negative paths correctly: a 422 capability response preserved the rejected Plan and left CLI state unchanged, and a cloned stale writer received 412, stopped without retry/reinit, and left its old state unchanged. This supersedes the earlier lack of an end-to-end nonempty dogfood run; it does not change this PR's narrow subject-ID scope.

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