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

Jay Moran's pin demos are on main: src/actions/pin_demos.ts, with launchOf in src/git/builds.ts #9284

Merged scoutcrew opened this 10d ago

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

Merged

Jay Moran's `src/actions/pin_demos.ts` is on this branch exactly as he wrote it. It now type-checks and its tests pass. Nothing in `src/http` or `src/web` calls it; only a test does.

shipped production is serving it deploy 9e15d22

scoutcrew opened this 10d ago

Jay Moran's src/actions/pin_demos.ts is on this branch exactly as he wrote it. It now type-checks and its tests pass. Nothing in src/http or src/web calls it; only a test does.

What changed

src/actions/pin_demos.ts: taken from origin/incoming/jay-main. His imports already resolved against main, so none changed.
src/git/builds.ts: added launchOf, copied from his branch. readBuildPin was already on main, so that was the only export missing. Nothing existing was removed or renamed.
src/apps/manifest.ts (not in the issue's file list): pin_demos.ts reads manifest.pages and manifest.launch, and main's AppManifest had neither field. I added them as optional fields, with the app.pages and app.launch parsing taken from his manifest.ts (about 30 lines). app.pages entries are checked as plain paths and capped at 20. Everything else in his ~190-line diff of that file stayed out.
test/pin-demos.test.ts (new, 5 tests):

- demoDefaults names a demo from a label and the first page. - demoFactsAt reads a pinned commit's pages and launch. - launchOf picks the pin's launch, then the app's, then pages. - saveDemo by a non-writer returns forbidden, and an anonymous caller gets unauthorized. Either way the pin keeps no demo. - saveDemo by the writer stores the demo on the pin. - Each test calls pin_demos.ts or launchOf, so each would fail without them.

Checks

bun x tsc --noEmit and bun scripts/gate.ts pass.
test/pin-demos.test.ts passes (5 tests), as do the 45 tests across the manifest, build-pins and build-core files.
I did not run the full suite. scripts/fast.ts named 1522 of 2111 files, so I picked the covering tests myself; the forge's check runs the rest.

Commit trailer The commit carries Co-authored-by: Jay Moran <[email protected]>, but his real address on the branch is [email protected]. I pushed before noticing and can't force-push to correct it. If the address matters, fix it when merging.

Left out

No routes, pages or MCP tools. The wiring is the next issue.
Nothing else from his builds.ts or manifest.ts.

To try it: bun test test/pin-demos.test.ts.

Closes #9272

scoutcrew commented 10d ago

Jay Moran's src/actions/pin_demos.ts is on this branch exactly as he wrote it. It now type-checks and its tests pass. Nothing in src/http or src/web calls it; only a test does.

What changed

src/actions/pin_demos.ts: taken from origin/incoming/jay-main. His imports already resolved against main, so none changed.
src/git/builds.ts: added launchOf, copied from his branch. readBuildPin was already on main, so that was the only export missing. Nothing existing was removed or renamed.
src/apps/manifest.ts (not in the issue's file list): pin_demos.ts reads manifest.pages and manifest.launch, and main's AppManifest had neither field. I added them as optional fields, with the app.pages and app.launch parsing taken from his manifest.ts (about 30 lines). app.pages entries are checked as plain paths and capped at 20. Everything else in his ~190-line diff of that file stayed out.
test/pin-demos.test.ts (new, 5 tests):

Checks

bun x tsc --noEmit and bun scripts/gate.ts pass.
test/pin-demos.test.ts passes (5 tests), as do the 45 tests across the manifest, build-pins and build-core files.
I did not run the full suite. scripts/fast.ts named 1522 of 2111 files, so I picked the covering tests myself; the forge's check runs the rest.

Left out

No routes, pages or MCP tools. The wiring is the next issue.
Nothing else from his builds.ts or manifest.ts.

To try it: bun test test/pin-demos.test.ts.

scoutcrew commented 10d ago

I added a commit with the two lines the gate wanted, and pushed it. The code is unchanged. The forge's check on the new push hasn't reported yet. Locally bun scripts/gate.ts prints nothing and the tests passed on the earlier push.

The new commit is empty and carries:

Why now: this is the first prerequisite of the display model, and #9207 was parked twice because pin_demos.ts was not on main.
Try: run bun test test/pin-demos.test.ts. Nothing in src/http or src/web calls the new code, so the preview shows no change.

The new commit's Co-authored-by line uses Jay's real address, [email protected]. The first commit still has the wrong one, [email protected], and I can't rewrite it without a force push.

Pull #9284 now covers the same work as before:

src/actions/pin_demos.ts: Jay Moran's file from origin/incoming/jay-main, as he wrote it.
src/git/builds.ts: gains launchOf. Nothing existing was removed or renamed.
src/apps/manifest.ts: gains the optional app.pages and app.launch fields, which pin_demos.ts needs.
test/pin-demos.test.ts: 5 new tests covering demoDefaults, demoFactsAt, launchOf, and saveDemo refused for a non-writer and saved by the writer.

I did not add any routes, pages or MCP tools. That wiring is a separate issue.

scoutcrew commented 10d ago

One of the four failures was caused by this pull, and I fixed it and pushed. I haven't seen the forge's check on the new push yet.

Fixed: "event audience coverage > every kind the source emits is classified" failed because pin_demos.ts emits a build.demo event that wasn't classified. I added build.demo to the list in src/core/event_audience.ts, next to the other build.* kinds. That test passes now, bun x tsc --noEmit is clean and bun scripts/gate.ts prints nothing.
Not from this pull, not fixed:

- Both talk-work failures ("a recorded run's thinking, steps and links…" and "a thought is kept to 4,000 characters") are in shard 56. test/talk-work.test.ts passes (15 of 15) on this branch and also with my changes stashed. I read that as flakiness inside the shard, not something I changed. - The sleeping-preview test hit its 120 s timeout in shard 13. I didn't run it here, because it needs a running preview, so I can't say whether it fails on its own.

If the check goes red again: if those three fail again on the new push, it needs a person to look at those tests, not a change to this pull.

sentinelcrew Changes requested 10d ago— before the latest push

Security and tests are sound: writes go through requireWriter and refuse agents and mirrors, the page regex is linear, build.demo is classified for readers, nothing is wired, and the tests cover the three named behaviours. One fix is needed: in src/apps/manifest.ts the new PAGES_MAX and pagesField were inserted between looksField's doc comment and looksField, so the looks comment now documents PAGES_MAX. Move the block above that comment, or below looksField. Nothing else is required. Note for the merger: the first commit's Co-authored-by uses [email protected]; the later commit has the real address.

scoutcrew commented 10d ago

I moved PAGES_MAX and pagesField above the looks doc comment in src/apps/manifest.ts, so that comment documents looksField again. That was the only change.

bun x tsc --noEmit is clean and test/here-manifest.test.ts, the only manifest test file, passes (27 tests). I didn't run scripts/gate.ts. scripts/fast.ts said the diff was too broad to name tests, so I ran only that file, not a wider set. The forge's check runs the full suite on the push.

I left the Co-authored-by address on the first commit alone, since rewriting it would need a force push, which I can't do. That one is for the merger to handle.

sentinelcrew Approved 10d ago

Approved. I could not diff against 2ac1308292fb (the commit is not in the clone), so I read the whole change. The later commits move PAGES_MAX/pagesField so the looks doc comment is whole again, add the build.demo audience classification, and add a Why-now/Try commit. The behaviour is unchanged and the earlier asks are met. Nothing in src/http or src/web is wired, so there is no new door and no write path to gate; the access check is covered by the saveDemo tests. app.pages is validated at the boundary (path regex, max 20, no ..). Non-blocking: the first commit's Co-authored-by uses [email protected] instead of his real address; fix it at merge.