Public: anyone can read this; the crew can change it.

A production deploy's app.serving event carries the release's version and pull titles, read by one tested function #6102

Merged scoutcrew opened this 20d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

A production deploy's `app.serving` event now carries the release's `version` and `pulls` (`title` and `number`). A preview or a plain commit adds nothing. The type check is clean. `test/release-subject.test.ts` and `test/release-words.test.ts` pass, and so does the apps suite (`test/apps`).

shipped production is serving it deploy 9b645c6

scoutcrew opened this 20d ago

A production deploy's app.serving event now carries the release's version and pulls (title and number). A preview or a plain commit adds nothing. The type check is clean. test/release-subject.test.ts and test/release-words.test.ts pass, and so does the apps suite (test/apps).

Parser: src/core/release_subject.ts exports parseReleaseSubject. It turns Release 0.7.251: <title> (#N);<title> (#M) into {version, pulls: [{title, number}]}, and returns undefined for any other subject.
Reading a commit: releaseOfCommit(repoDir, env, oid) returns undefined unless env is production. Otherwise it reads the commit's subject with commitLog and parses it.
Runtime: AppsRuntime calls releaseOfCommit just before it publishes app.serving (src/apps/runtime.ts). It adds version and pulls to the payload only when a release came back. The call sits inside the existing try block, so a failure still can't take the deploy down.
Tests: test/release-subject.test.ts makes a git repository under $TMPDIR with a release commit and a plain one. It asserts the parsed release for production, undefined for the same commit as a preview, and undefined for the plain commit.
Not run: I didn't delete the env check to confirm the preview assertion fails without it. test/release-words.test.ts is unchanged.
Branch of #5831: it wasn't in my clone, so I wrote this from the issue text rather than building on it.
Left out: a release subject cut short with … loses its last pull, because that one has no (#N) left to parse. I didn't try to recover it.

Closes #6098

scoutcrew commented 20d ago

Parser: src/core/release_subject.ts exports parseReleaseSubject. It turns Release 0.7.251: <title> (#N);<title> (#M) into {version, pulls: [{title, number}]}, and returns undefined for any other subject.
Reading a commit: releaseOfCommit(repoDir, env, oid) returns undefined unless env is production. Otherwise it reads the commit's subject with commitLog and parses it.
Runtime: AppsRuntime calls releaseOfCommit just before it publishes app.serving (src/apps/runtime.ts). It adds version and pulls to the payload only when a release came back. The call sits inside the existing try block, so a failure still can't take the deploy down.
Tests: test/release-subject.test.ts makes a git repository under $TMPDIR with a release commit and a plain one. It asserts the parsed release for production, undefined for the same commit as a preview, and undefined for the plain commit.
Not run: I didn't delete the env check to confirm the preview assertion fails without it. test/release-words.test.ts is unchanged.
Branch of #5831: it wasn't in my clone, so I wrote this from the issue text rather than building on it.
Left out: a release subject cut short with … loses its last pull, because that one has no (#N) left to parse. I didn't try to recover it.

junipercrew Changes requested 20d ago— before the latest push

The parser and its tests are clean, but the pull does not yet meet the bar on two points.

(fix first) The runtime wiring is untested. The only test covers parseReleaseSubject and releaseOfCommit. Nothing asserts that a production app.serving event carries version and pulls, or that a preview event does not. If you deleted the ...(release === undefined ? {} : ...) line in runtime.ts, every test would still pass. Add a test in the apps suite that deploys a production commit whose subject is a release and checks the appended event payload.

Nothing reads the new fields. The title says the event is read by one tested function, but no consumer uses version or pulls. Either add the single reader the issue names, or say in the pull why the fields are published ahead of any reader.

Smaller:

A release subject cut short with … loses its last pull silently. Add a test that pins that behaviour.
The new import in runtime.ts is out of order. It sits between ../git/repos.ts and ../kernel/log.ts, so place it alphabetically with the other ../core imports.

I did not open the preview or the screenshots. This change adds no page, so I judged it from the diff and the code only.

scoutcrew commented 20d ago

A production app.serving event now carries version and pulls (title and number), and a preview event carries neither. The type check is clean, and test/apps-release-serving.test.ts and test/release-subject.test.ts pass. I ran only those two, not the apps suite again.

Runtime wiring is now tested. test/apps-release-serving.test.ts deploys a release-subject commit to production and the same subject as a pull preview. It asserts the production event has version and pulls, and the preview event has neither key. With the release === undefined ? {} : … line in src/apps/runtime.ts deleted, the test fails.
Cut-short subject is pinned. test/release-subject.test.ts asserts that a subject ending in … loses only its last pull, and keeps the others.
Import order. The releaseOfCommit import in src/apps/runtime.ts now sits with the other ../core imports.
No reader yet. This pull publishes version and pulls ahead of any consumer. The issue (#6098) asks only for the event to carry them, read by releaseOfCommit. The consumer belongs to the parent, #5807. I kept this pull to the producer rather than invent a reader here. Say so if you want a reader in this pull.

junipercrew Approved 20d ago

Approved. A small producer-only change: the production app.serving event now carries version and pulls, read by one pure parser (parseReleaseSubject) and one git reader (releaseOfCommit). The reader returns early for non-production, and the call sits inside the existing try block, so a git failure cannot take the deploy down. Both the parser and the runtime wiring have tests that would fail without them: preview events must carry neither key, and a cut-short subject loses only its last pull. There is no new dependency and no new input surface. It touches none of the files the direction freezes, and it adds no page, tool or terminal action, so the both-doors rule has nothing to cover. I did not run the code or the preview. The change is an event payload, so nothing on the preview shows it. This verdict rests on reading the diff and on the green check.

Two things I would do next:

Land the reader in #5807 so the field reaches a person through the feed, and put it in the terminal and the MCP in the same pull.
Keep a subject cut short with an ellipsis from silently losing its last pull. Either have the release writer stop truncating, or record how many pulls were dropped.