Teach Skill to wait for Plan analysis

[raghubetina]

Summary

• require a bounded current-analysis wait after every verified Plan push
• branch on validated analysis status and stop safely at failure and concurrency boundaries
• allow one well-founded issues repair per approval while preserving identity and private state
• pin the merged CLI contract and add eval evidence for status polling

Evidence boundary

This teaches the client-side contract only. The matching First Draft AnalysisRun API is still pending, so valid
server-backed eval outcomes are not yet end-to-end evidence. Compilation remains unavailable.

Verification

• sh script/check (13 tests)
• node script/check-cli-contract.mjs <firstdraft/cli at 74e3d42>
• exact source runner and freshly packed CLI passed

Part of firstdraft/firstdraft#133.

[raghubetina]

Technical review

The consumer side of firstdraft/cli#8, teaching the Skill to wait for analysis and branch on what comes back.

Verification reproduced

sh script/check                              → 13 tests pass
node script/check-cli-contract.mjs <cli>     → exit 0
npm audit --audit-level=low                  → 0 vulnerabilities

The pinned baseline 74e3d42 is cli#8's merge commit and is currently the tip of that repository's main.

The cross-repo binding still bites. Pointed at the previous merged CLI commit it exits 1, so a rename on the CLI side would fail this repository's CI rather than surfacing to an agent at runtime. The check also grew to exercise the new surface, driving the real CLI runner with an injected fetch to produce project_not_pushed, status_unavailable, analysis_changed, and wait_timed_out from the actual implementation.

The evidence boundary is stated honestly

Worth quoting, because it is an unusual thing to volunteer:

This teaches the client-side contract only. The matching First Draft AnalysisRun API is still pending, so valid server-backed eval outcomes are not yet end-to-end evidence.

So the CLI can poll, the Skill knows how to react, and nothing has ever talked to a real analysis endpoint because one does not exist yet. Saying that plainly in the PR that adds the client is the right call, and it keeps the eval outcomes from reading as proof of a working pipeline.

An earlier finding is closed

For the record, since I raised it on #6: the three discovery strings that promised "Create and deploy a Rails app for web, iOS, and Android" now describe what the Skill actually does. short_description reads "Experimental First Draft Plan authoring and diagnostics", the SKILL.md description leads with "Experimental and in development", and the default_prompt describes authoring and review with an explicit local-first stance.

That fix is already in main from earlier work rather than being part of this PR, which only refines the prompt's trailing clause. Noting it because it closes the loop rather than to credit it here.

Finding: the one status code with opposite advice is not bound

test/repository.test.mjs lists eight plan status codes and asserts the documentation covers each. The cross-repo contract check exercises six of them against the real CLI. Two are absent, and one of those matters:

invalid_server_response in check-cli-contract.mjs: 0
status_unavailable      in check-cli-contract.mjs: 1

Those two are a matched pair, and cli#8's README gives them deliberately opposite guidance:

error	Guidance
status_unavailable	"retry the GET a bounded number of times"
invalid_server_response	"retrying unchanged will not repair the mismatch"

The retryable half is verified against the real CLI. The do-not-retry half is documented in diagnostics-and-recovery.md:95, asserted to be documented by the repository's own test, and never checked against the thing that emits it.

The consequence is bounded rather than dangerous, because the Skill's fail-closed rule catches an unrecognized code and stops. But stopping is what it does for any unknown output, so a silently dead branch costs exactly the actionable distinction this design was built to provide: an agent that could have said "the server's response is malformed, retrying will not help" instead says "something unknown happened."

The fix is small given what is already there. The check injects fetchFunction rather than running a server, so returning a response that violates the status contract is a few lines beside the existing cases. server_rejected is the other uncovered code and would come along for roughly the same effort.

[raghubetina]

Lesson: building the client before the server exists

This PR teaches an agent how to wait for an analysis to finish and what to do with each result. There is one detail that makes it interesting: the server endpoint it talks to has not been built.

That sounds backwards. It is a real technique, it has real benefits, and it has one failure mode that ruins it if you are not careful.

Why anyone would do this

Two teams need to agree on how a feature works. The usual order is server first, then client, and it has a cost you have probably felt: by the time the client team touches it, the shape is fixed. Every awkwardness in the response is now expensive to change, and you route around it in client code forever.

Writing the client first inverts that. You discover what the client actually needs while changing it is still free.

Look at what this Skill discovered by trying to use an endpoint nobody has built. It needs the analysis to carry an identity, so a poller can tell "your analysis finished" from "a different analysis finished." It needs a graph_version, so it can tell which Plan the result describes. It needs statuses distinguishable enough to branch on. It needs the difference between "the response was garbage" and "the network failed" to be visible, because one is worth retrying and the other is not.

Those are requirements on the server, found by writing the consumer. If the server had shipped first, some of them would have been missing and the client would have papered over the gaps with guesswork.

This is the honest version of what people mean by contract-first or API-design-first. Write down the interaction, let both sides build against it, and discover the disagreements early.

The failure mode

Here is what goes wrong. You write the client. You write tests. The tests pass, because they run against your own idea of what the server will send. Everything is green, and you start believing the feature works.

Then the server ships, and it returns "state" instead of "status", or an ISO 8601 timestamp where you assumed epoch seconds, or a 202 where you expected 200. Every one of your passing tests was testing your imagination.

Green tests against a fake server tell you your client is self-consistent, not that it is correct.

Which is why the sentence in this PR is the most important thing in it:

This teaches the client-side contract only. The matching First Draft AnalysisRun API is still pending, so valid server-backed eval outcomes are not yet end-to-end evidence.

They wrote down that the proof is partial. Nobody reading those green evals will mistake them for a working pipeline. That costs one sentence and prevents the exact misunderstanding this technique invites.

How to do it without lying to yourself

Write the contract down somewhere both sides read. Not in your client's code. A schema, a shared document, an example response committed where the server team will see it. In this repository the equivalent is a reference document plus a script that checks the real CLI matches it.

Say which parts are verified against reality. "Verified against the real CLI" and "verified against a response I made up" are different claims and should not sit in the same list without a label.

Plan the day the real thing arrives. The moment the server exists, run the client against it. Not a mock, the actual endpoint. That is when you find out whether the contract survived contact.

Fail closed on anything unexpected. Since you are guessing, guess safely. This Skill treats any unrecognized status as "stop and report" rather than assuming it resembles a case it knows. When your assumptions are wrong, and some will be, that turns a wrong action into a halt.

The everyday version

You do not need two teams for this to be useful.

Next time you integrate a third-party API, write the client and its tests before wiring it up. You will discover what you actually need from the response, and you will notice the mismatch when you finally call it for real, which is exactly when you can still choose a different provider.

Same when you build a UI against an endpoint a colleague is writing. Stub it, build the screen, and you will find the missing field in an afternoon rather than a week after the endpoint is frozen.

Just keep a clear line in your head between "my code works against what I imagined" and "my code works." Both are worth having. Only one of them is worth telling other people about.

[raghubetina]

Resolved in 2788cae.

The cross-repository contract check now drives both previously uncovered status paths through the real CLI pinned at
74e3d42:

• a malformed 200 application/json response emits the closed invalid_server_response envelope with its HTTP
  status;
• a valid 404 application/problem+json response emits server_rejected with an exactly whitelisted problem
  response and strips an unknown canary extension.

Both cases assert one fetch, byte-identical private state, and absence of the project path, pinned origin, private
ETag, and response-body canaries from stderr.

Proving the shared server_rejected code also exposed an instruction ambiguity. Status failures are now explicitly
hard stops whose validated payloads are report-only context, while every repair/retry rule is scoped beneath a
failed firstdraft plan push and cannot override a later plan status --wait stop.

Verification:

• sh script/check — 13 tests pass
• node script/check-cli-contract.mjs /Users/sandbox2/code/firstdraft/cli — passes at 74e3d42
• npm audit --audit-level=low — 0 vulnerabilities
• git diff --check — passes