9eb83538fe2e1091ef4bae52dcd4345aac543759
31 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Edward Langley
|
7fb6960cf8 | WIP: drop the serde derives only the legacy loader used; docs (improvise-tt22) | ||
|
Edward Langley
|
ffe8aed6b4 |
improvise-c5ip, improvise-udt2: exit 1 on a refusal, never on an abort
The headless exit code read `last_batch_aborted()`, which answers "did a chain stop", not "did the request happen" — wrong both ways. `commit-cell-edit` at bottom-right lands its write, saves, and then `EnterAdvance`'s `AbortChain` exited 1 (c5ip); `add-item` into a missing category refuses without aborting and exited 0 (udt2). `App` gains a per-batch `batch_refused`, reset beside `batch_aborted` and written only by `App::refuse(msg)`, which sets the status and the flag together; commands reach it through `effect::refuse`. `main.rs` exits 1 on `last_batch_refused()` only. Not a severity on `status_msg`: that would be portable ViewState and would go stale across its ~60 writers. Strays into ui/app.rs (four small additions) — flagged in the hand-back. Unconverted refusals are filed as improvise-i3o5; headless `q` as improvise-ep5o. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze |
||
|
Edward Langley
|
487f0b6fef |
improvise-up8g: a headless run's extent is fixed, not read off the terminal
`run_headless_commands` read `crossterm::terminal::size()`, and the page size decides where `page-scroll` lands, so one script wrote a different record in a different window — and CI (no tty, 80x24 default) disagreed with a shell. Pin it to 80x24 in `HEADLESS_EXTENT`. The test drives two sizes through LINES/ COLUMNS with the child detached by `setsid`, and asserts the same record. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze |
||
|
Edward Langley
|
c13466f1d9 |
fix(cli): the headless commands were silent whether they worked or not
improvise-q914 and improvise-j6x8 are one bug seen twice, and the second
measured it: `run_headless_commands` applied effects and never read
`app.view_state.status_msg`, setting `exit_code` only for parse errors. A
successful command and a refused one therefore produced byte-identical
output — nothing — and exit 0 for both. `improvise script` is the same
function, so it shared the hole exactly.
Three decisions, argued in `context/code/cli.md`:
**Everything to stderr.** The tempting split — stdout for a report, stderr
for a refusal — cannot be built honestly, because `status_msg` is a bare
`String` with no severity and both candidate classifiers are disproved by
code already in the tree. "A refusal aborted" is false: `AddItem` writes its
`Err` to the status and does not abort, and that is q914's own example. "A
refusal is anything that spoke" is false too: `cmd/fit.rs` reports the seed
that reproduces a fit on **success**. Guessing between them is the adjacent
check design-principles §7 names.
**Exit 1 on a parse error or an aborted batch, and no further.** Both are
real flags. A refusal that only writes a status is now printed but still
exits 0, because nothing can classify it; the README says so plainly rather
than over-promising, and closing it needs a severity on `ViewState`, which
is a wire change reaching the browser.
**No quiet flag.** Silence is never the default and `2>/dev/null` already
exists.
Also: a run that changed nothing no longer rewrites the document. Saving
unconditionally normalised it — `1e20` to its twenty digits — which turns a
hand-edited fixture into a silent no-op. The gate accumulates
`model_state.dirty` per command rather than reading it once at the end,
because `effect::Save` clears the flag and `[add-item, w elsewhere]` would
otherwise drop the edit.
And the three clap doc strings advertised JSON, which the parser refuses.
`tests/headless_cli.rs` pins all of it at the process boundary, where an
exit code and a stream exist. Six of its twelve tests fail at
|
||
|
Edward Langley
|
90ccfb0154 |
feat: attach the assembled site to every release, and let the page name its build
improvise-oujb. A tagged release carried four native tarballs and nothing for the web; the site shipped only by ci.yml's pages-deploy, on a push to main. Pages keeps tracking main (user, 2026-09-22: "I like Pages showing the latest main for now"), so this is a CAPTURE, not a second deployment: the tarball is a self-hostable copy of the site as of the tag and a rollback target, not what a visitor to the published site is running. pages-deploy is untouched. Mechanism is dist's own [[dist.extra-artifacts]], so it survives `dist init`: `dist plan` under the pinned 0.30.4 lists improvise-web.tar.gz beside the four platform tarballs and `dist generate` leaves release.yml byte-identical. The build command is scripts/build-web-release.sh, which calls web/build.sh and web/assemble.sh -- the same two scripts ci.yml and a developer run, so there is no second way to build the bundle. Its toolchain pins come from scripts/web-toolchain-versions.env, which ci.yml's build job now reads too. `github-build-setup` was measured and does NOT help: it injects into build-local-artifacts only, which is the wrong job for a global artifact. The page now names the build it was assembled from in #build-version, stamped by web/assemble.sh with the same token as the bundles and verified by the same scripts/assemble-stamp.sh gate. An unassembled tree honestly reads "unstamped". Verified by running `dist build --artifacts=global` locally: the script ran, the site assembled, and dist placed improvise-web.tar.gz in target/distrib. Not verified: a real tag push, which would require cutting a tag. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
3542d6f717 |
fix(tooling): a leaf is not an identity, and a convenience flag was a bypass
Both blockers from the adversarial review of #83, both in the mode this PR rewrote, both producing a pass while the gate did not do what its prose said. 1. cargo_test_names reduced every listed path to its last segment. Six leaf names collide in this workspace and two are cited in evidence, so a heading was satisfiable by a test in another module answering another question -- renaming ui::project::tests::a_load_that_fails_is_not_a_rebase, the one its entry describes, left the checker at exit 0 with the ui::host namesake answering for it. Paths are kept whole; a bare leaf resolves only when unique, and an ambiguous one is REFUSED with both candidates named. _RUST_NAME accepts `::`, so a qualified heading is a claim rather than prose, and a citation is matched on the leaf so qualifying a heading cannot orphan its own test. The two colliding entries are qualified. Neither half suffices alone: rename one of a colliding pair and the leaf becomes unique again, which is why qualification is the fix and the refusal is what forces it. 2. --rust-tests called cargo_test_names with one argument, so the --ignored subtraction never ran, and it printed a byte-identical success line. It now requires --rust-tests-ignored (one alone exits 2) and the line names its source. Also, from the same review: a missing context/test-evidence/ exited 0 while 170 citations pointed at nothing (early return, now gone); the doc-site "runs last" check accepted a trailing `echo` that merely NAMED the generator, so it asks for an invocation; bundle_freedom gains an arm for a transitive `import "../app.js"`, which reaches the bundle while naming nothing forbidden; and two prose overclaims are corrected -- the prose-heading count is a convenience rather than a guard (git diff is the mechanism), and the ratio rule says "above the run MEDIAN", which is defensible where "above the others" is false against second place. scripts/check-ci-graph.py makes claim 6's rule a gate: pages-deploy must `needs` every other job in ci.yml. The rule was stated in the last commit and a zz-new-check: job added during review was still ungated with nothing noticing. README now states the rule and lists no jobs, the second copy being what went stale twice. Every fault re-injected and observed; figures in context/test-evidence/tooling.md rather than only here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
29cdab1a4a | Merge remote-tracking branch 'gh/main' into worktree-agent-a97453f7fa65d51b9 | ||
|
Edward Langley
|
d100639348 |
fix: check Origin at the handshake, so a page cannot drive someone's session
improvise-kc7n. A websocket handshake is exempt from the same-origin policy — there is no CORS preflight — so any page a user visited could open a socket to improvise-ws-server from that user's own allowed address and read and edit every document served. An address check cannot see it: the request genuinely comes from an allowed client, and only Origin says what is driving it. Loopback is the least protected case, having no proxy in front of it at all. `src/origin.rs` is the decision and `transport::admitted` its one caller, which refuses the upgrade with a 403 rather than accepting and ignoring. The default allows pages served from loopback — which is the shipped local recipe, page on `http://localhost:8080` and socket on another loopback port — and refuses everything else; `--allow-origin` names the deployed page, which reaches the server through a TLS proxy and is not loopback. An absent Origin is allowed and decided rather than defaulted: no browser omits it, so absent is a non-browser client, and refusing those would break this crate's own tests while buying nothing that --bind does not already buy. Five edits, five guards, each reverted alone against the named test: - origin.rs permits() present arm -> a_foreign_origin_is_refused - origin.rs permits() None arm -> a_handshake_with_no_origin_is_accepted - origin.rs is_loopback() -> a_handshake_from_a_loopback_page_is_accepted - transport.rs handshake callback -> a_handshake_from_a_foreign_origin_is_refused - main.rs --allow-origin wiring -> allow_origin_widens_the_policy_and_nothing_else_does `transport::serve` takes the policy rather than defaulting one, so a host cannot forget to pass it. The argument is context/code/clients.md §Origin and the evidence context/test-evidence/clients.md; README no longer says the server never looks, and the proxy's Origin allow-list is now defence in depth. This is not authentication — it answers what page is driving a client, never who is connecting. improvise-kpnr is untouched and still open. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
4552165a93 |
fix(ci): pages-deploy needs every check job, and README names them all (improvise-x7cq)
kb was the one job absent from the needs list, on the unstated ground that kb.pl is not shipped by the site. harness-lint's crate is not shipped either and was in the list, so the ground proved too much. A failing knowledge-base consistency check means main is broken, and publishing from a broken main is what the list exists to stop. The rule that does not go stale is "every check job", so that is what the comment says. README named three of the seven, which read as a promise the other four were ungated -- and it is the shape gotcha 48 is about: a fact stated in two places where changing one is an edit rather than a grep. It now names the rule and the list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
debad21556 |
docs: run the gate CI runs, and bindings.rs is run
Two one-line corrections the wave's last review surfaced, both about a claim being stated in more than one place. .claude/skills/code-review/SKILL.md told every reviewer and every wave agent to run `cargo clippy --workspace --all-targets` with no `-D warnings`. Clippy exits 0 on warnings, so the committed local gate was green on exactly the input CI is red on. This is not one person's shell mistake, it is the instruction: a reviewer following the skill as written would report clean and be wrong, which happened on PR #71. Also adds --no-fail-fast, since fail-fast hides blast radius. README.md was a sixth copy of "bindings.rs is never run", which PR #70 corrected in five places -- its grep missed README, and PR #72 then rewrote the sentence four lines above and carried the wrong half forward. Neither PR is at fault individually; it is an interaction between two branches, and it is the exact failure the corrected fact is a monument to. Plus doctrine(a_weak_check_and_a_strong_one_agree_until_the_defect): five instances this wave, one mistake -- a check written against the failure you imagined rather than the outcome the user sees. The weak and strong checks agree on every input anyone tried and diverge only on the input that is the defect. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
f9580a33af |
docs: what the harness now reaches, and the evidence for each assertion
The engine-coverage and measurement passages in context/code/clients.md, the boundary table (four boundaries become six), and README's caveat -- which named the grouped and multi-level DOM as never executed, and it now is. A fact stated in more than one place is a grep, not an edit (gotcha 48). context/test-evidence/clients.md records, per test, what fault was injected to prove it can fail: the blanked help keys, the five grid-frame injections, the 404'd casts and the dead front-door link, and the mis-wired item-row click -- plus the two-command test-name diff that found that last one. Playwright: 26 passed + 3 skipped, 34.4s / 33.7s (was 17 + 3, 15.1s); the landing page's cast playback is most of the difference. node --test web/tests 26, cargo test --workspace 1394, both unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
3c9668f9e2 |
ci: build the bundles once, and take the harness lint off the critical path
Measured on run 35535225637 (push to main, caches warm). The critical path was the browser job at 263s plus an 11s deploy; everything else finished inside it. Its decomposition: harness lint 37s, Playwright browser download 50s, `./harness/build.sh` 110s, `npx playwright test` 50s. Three wasm-pack builds ran in that 110s, and two of them — the client and the worker bundle — were the same two `pages-build` ran in its own 68s step, in a parallel job with a separate rust-cache key (`add-job-id-key` defaults on). Same work, twice per run. - One `build` job now runs `./harness/build.sh` (all three bundles plus the `harness/serve` tree) and then `./web/assemble.sh site $GITHUB_SHA` from the pkg dirs it just produced. It uploads `harness/serve`, the two raw bundles, and the Pages artifact. `pages-build` is folded into it. - `browser` downloads `harness/serve` whole and needs no Rust toolchain and no wasm-pack. Downloading the tree rather than reassembling it keeps one answer to "what does the harness serve" (improvise-vvl3): `harness/build.sh`. Verified locally that the two assemblies differ in exactly the four cache-bust-stamped files and nothing else — the .wasm bytes are identical. - `harness-lint` is its own job. It gated nothing the harness needs and cost 37s of the critical path, and `cargo fmt --all` run from inside `harness/opfs-contract` formats the WHOLE outer workspace (114 files, measured) despite that crate's empty `[workspace]` table, so a rustfmt violation anywhere in the repo failed the *Browser harness* job. `-p opfs-contract` scopes it to the one file it is about. - Playwright's browser download is cached on the resolved package version. - rust-cache in `build` and `harness-lint` covers `harness/opfs-contract`'s own target dir, which the default single-workspace config never did. No gate is weakened. `pages-deploy` still needs every check that could previously fail it; `harness-lint` is named explicitly because it used to be a step inside `browser`. `kb`, `wasm` and `test` gain no dependency on `build` — none of them consumes an artifact. On improvise-864a: the gap between the Pages artifact upload and the deploy narrows from ~182s to ~115s on this run's shape, so the window in which a superseding run can expire it is smaller. The race is not closed, and not widened. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Edward Langley
|
2ccd21f3e8 |
docs: the landing page is the front door, the app moves to /app/
User ruling on improvise-kh8, overturning the arrangement the previous commit
shipped: "The landing page should be served at the root of the repository and
link to the in-browser demo." The argument for the other arrangement is left
standing in the ticket rather than deleted — it is overruled, not wrong to have
considered — and the root is now `docs/index.html`.
The app moves under `app/` WHOLE, both bundle directories included. Every path
in `app.js` and `worker.js` is relative, so moving the bundle together keeps
every one of them correct and the wasm-pack generated glue untouched;
`worker.js`'s `fetch("demo.improv")` resolves against the worker's own URL, so
the seed follows for the same reason. The four `stamped` guards in
`assemble.sh` follow the files: they are the only check that the cache-bust
substitution worked, and a guard aimed at a path nothing writes fails in the
direction that looks like success.
`harness/tests/shell.spec.js` navigates to `/index.html` six times, including
the `?ws=` and `?doc=` variants, and every one of those now lands on the
landing page. All six are repointed at `/app/index.html`. That file is outside
this unit's declared scope and is the second deliberate stray in this PR.
VERIFIED AGAINST THE ASSEMBLED TREE, not against `docs/` directly, because a
path move is what re-arms the trap this page already caught once. With real
bundles from `web/build.sh`:
* `npx playwright test tests/shell.spec.js --project=chromium` — 6 passed,
with `/app/{index.html,app.js,style.css,worker.js,demo.improv}` and both
`?v=` stamped bundles all served 200;
* the four casts play 37 lines each at the site root, no failed request and
no page error;
* the full journey: load the root, click the call to action, land on `/app/`,
and the grid draws out of the in-page worker — 12 cells, all editable,
a "Revenue" header, `#status` reading "in-page worker · demo.improv".
The call-to-action check was proved able to fail by pointing the link at
`./aap/`: 404, while the page still looks entirely correct.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Edward Langley
|
161e36ea60 |
docs: a landing page, served beside the app rather than in front of it
improvise-e61. Vanilla, single file, 192 lines, no build step: the four casts improvise-d4w recorded, played by asciinema-player pinned from jsdelivr. improvise-kh8 recorded a collision to resolve before starting — Pages already serves the standalone web app at the root, so a landing page either replaces it, sits beside it, or is dropped. It sits beside it at `/landing/`, because kh8's own note is right that the live app is the better front door, and a page the deploy never copies is a file nobody can reach. `web/assemble.sh` copies it, so the browser harness serves the same tree the deploy publishes and there is still one answer to "what ships". The player config deviates from the tickets, deliberately. improvise-d4w's description and improvise-e61's criteria both say 100x30; every cast header says 120x37, so the recordings never complied with their own ticket. A 30-row player clips the bottom seven rows of a 37-row cast, which is exactly where the tile bar and the status line are, so the page declares the geometry the casts actually have. e61 also asked for the casts to be checked against the current UI before embedding. They were: the tile bar's short_label vocabulary and the mode line still match, but the casts predate improvise-2zq's `+cat` column by five months and every one of them opens on the recorder's shell prompt. Filed as improvise-2oxt rather than silently embedded as current. Verified by driving the page in Chromium: four players construct, each renders 37 terminal lines after play, no failed request and no page error — against both `docs/` directly and the assembled `site/landing/`. Counting the players alone was proved worthless first: with autoPlay false the cast is not fetched until the click, so a page whose every cast path 404s constructs four players and throws nothing. Breaking `./casts/` to `./cats/` passes the construction check and fails the play check with four 404s. improvise-bgtf carries that as a harness spec, since nothing in CI asserts any of it yet. README: `cargo install improvise` was false — the crate is not on crates.io — and the Pages description named two of the three gates a deploy waits on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Edward Langley
|
f12a2d3517 |
docs: reconcile the docs with the code after wave 1
An independent review checked every factual claim in the docs against the code at |
||
|
Edward Langley
|
f91c5a77db |
docs: an allow-list is access control, not authentication
Copilot's seventh pass, reporting no new open findings; this is the one carried item that is a real defect and it is cheap. The closing note called the IP and Origin allow-lists "the auth above", in a section whose opening sentence is that the server has NO AUTHENTICATION OF ANY KIND. That muddies the exact distinction the section is built on: an allow-list answers WHICH CLIENTS AND WHICH ORIGINS MAY CONNECT, never WHO is connecting. Calling it auth invites a reader to think the hole is closed when what has been added is a perimeter. Checked every other use of the word in the section rather than editing the one line flagged, per the grep rule this PR earned: "no authentication of any kind" (the server), "HTTP basic auth" (genuinely authentication, and being ruled out), "neither authenticates anything by default" (the tunnels) and `--basic-auth` (a product flag) are all correct as written. One site, one fix. The distinction is also exactly what improvise-kpnr has to decide, so the note now says so. 1193 passing, unchanged; no code touched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Edward Langley
|
858a46e1a8 |
docs: qualify the loopback claim, in both directions
Copilot's sixth pass, and it is my own round-4 failure repeating: I wrote that "loopback is not a boundary a browser enforces" and that ANY open page can reach ws://127.0.0.1 -- an absolute where the evidence supports a qualified claim. Same shape as asserting a WS handshake carries no credential. CHECKED RATHER THAN TAKEN, and the finding is itself overstated. Copilot says "current Chromium gates WebSocket access from a public origin to localhost behind Local Network Access". Chrome's own documentation (developer.chrome.com /blog/local-network-access) says the opposite for the case at hand: WEBSOCKETS WERE NOT YET GATED when the permission prompt began shipping, with coverage planned. So the mitigation exists and is rolling out; whether it covers THIS socket, in a given browser at a given version, is exactly what neither of us can state and what no test here can settle. The text now says that, and keeps two gaps that are structural rather than version-dependent: LNA is a Chromium feature, so other browsers need not behave alike; and it engages only for a PUBLIC origin, so the locally-served page this paragraph is about falls outside it entirely. The threat is kept, the reassurance is not offered, and the durable fix is named -- the server checking Origin, which it does not do. Grepped the same claim through the rest of the section, per the rule this PR earned: the two remaining "any hostile page" statements are about the REMOTE wss:// endpoint, which LNA does not gate at all (it gates public->local), so they are correct unqualified and were left alone. The distinction is the whole point of the fix. 1193 passing, unchanged; no code touched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Edward Langley
|
ae199d431a |
docs: an address check cannot see a cross-site socket
Copilot's fifth pass, and the first finding in this PR that is a genuine security hole rather than a wrong sentence. Verified: there is NO reference to Origin anywhere in crates/improvise-ws-server -- the server accepts every handshake. A WebSocket is EXEMPT FROM THE SAME-ORIGIN POLICY; there is no CORS preflight on a handshake. So any hostile page an allowed user visits can open a socket to the endpoint FROM THAT USER'S OWN ALLOWED ADDRESS and read and edit the document. The IP allow-list I recommended last round is precisely the control that cannot see this, because by then the request really does come from an allowed client. Only the Origin header separates them. This is the same shape as the previous three rounds and it is worth naming as such: I reasoned about WHO MAY REACH the endpoint and not about WHAT MAY DRIVE the browser that reaches it -- one more conclusion carried without its reasoning. "Who may connect" is two questions, and the recipe answered one. So: * the prose states the two questions and says an Origin allow-list is needed IN ADDITION TO, never instead of, whichever control is chosen; * the Caddyfile carries both checks, with the second commented as explicitly not redundant with the first; * the `header` matcher syntax is checked against caddyserver.com/docs/ caddyfile/matchers (`header <field> [<value> ...]`, negatable inside a named matcher) rather than written from memory -- the round-3 rule; * and the LOCAL pairing paragraph is corrected too, because the same fact applies there and I had called it unreservedly fine: loopback is not a boundary a browser enforces, so any page open in that browser can reach ws://127.0.0.1:8787 while the server runs. The server-side half -- ws-server should check Origin itself, since a proxy is not present in the local case at all -- is reported for filing rather than built here. It is a change to a crate this unit does not own. 1193 passing, unchanged; no code touched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Edward Langley
|
1c98aa0929 |
docs: ambient credentials versus challenge-response, which is the real rule
Copilot's fourth pass, and it caught this text CONTRADICTING ITSELF three paragraphs apart -- in prose I wrote in a single commit while fixing the previous round's contradiction. The Caddyfile comment said "the browser cannot send a credential on a WebSocket handshake". That is FALSE, and the same section already said so: the Cloudflare Access bullet explains that Access works by a session COOKIE the handshake carries. A WebSocket handshake is an HTTP request; the browser attaches ambient credentials to it automatically. The true rule, which is sharper than the one it replaces and explains both halves instead of one: AMBIENT credentials are carried -- cookies for that host above all, which is why an identity-aware proxy works at all. CHALLENGE-RESPONSE cannot be satisfied -- the page can set no header (there is no Authorization argument to `new WebSocket`) and can answer no prompt (a handshake surfaces no login dialog and fails to script as a bare connection error). So basic auth is still ruled out, for a REASON rather than by an over-broad invariant, and Access is no longer an unexplained exception to it. Two consequences carried through the rest of the section, on the grep discipline the last round earned: * mTLS is no longer asserted to fail. The certificate IS offered at the TLS layer rather than by the page; only the selection UI is doubtful, and that is now marked as not verified here rather than stated. * "a query parameter is the one credential new WebSocket can carry" restated the same over-broad claim in miniature, and becomes "the one place the page can put a credential DELIBERATELY -- everything else it sends is ambient." 1193 passing, unchanged; no code touched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Edward Langley
|
f4600eb277 |
docs: stop giving ngrok a recipe I have not run
Copilot's third pass, on the flag I introduced LAST round while fixing the flag I introduced the round before. Two consecutive rounds of writing an access control into the README without running it is exactly the doctrine committed in |
||
|
Edward Langley
|
5c1fefb4f1 |
docs: an access control the browser cannot satisfy is not a control
Copilot's second pass, confirmed: the basic_auth recipe from the previous commit CANNOT WORK. web/app.js opens `new WebSocket(WS_URL)` -- one argument, a URL -- and the browser WebSocket API cannot set request headers, while a WebSocket handshake shows no login prompt. So Caddy basic_auth in front of it is a 401 the page can neither answer nor explain. That is worse than the unauthenticated recipe it replaced, because it READS AS PROTECTION. The first round's lesson was that documentation instructing an action carries the action's consequences; this round is sharper and is the one worth keeping: A RECIPE IS CODE, and it needs the same "would this actually run" test the Rust gets. Neither my review pass nor the fix asked it of a Caddyfile. Note it is the same fact that already killed native wss:// in this same document -- a WS handshake has no interstitial, which is why a self-signed cert is unusable. The argument was two sections away and did not transfer. So the section now sorts controls by whether a browser can satisfy them at all, since that is the property that decides it: * basic auth and client-cert mTLS: named as NOT working, with the reason. * IP allow-list: the worked example, via Caddy's documented remote_ip/abort. Needs no credential from the page. * private network (Tailscale/WireGuard): named; needs no credential either. * cloudflared + Cloudflare Access: named and explicitly NOT written out as a recipe, because it turns on a session cookie from a prior interactive login and third-party-cookie rules, which cannot be verified without a browser. Documenting a flow I cannot test is the trap improvise-hwpi named. `ngrok http --basic-auth` carried the identical bug and is corrected to --cidr-allow, which is the same address check. Says plainly that a URL-borne token is what would make this work browser-side -- a query parameter being the one credential `new WebSocket(url)` can carry -- and that the server has no such capability: improvise-kpnr (ID read back). And states that the Caddyfile has not been executed here, on the same terms as the existing browser-JS caveat. 1184 passing, unchanged; no code touched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Edward Langley
|
16471d7def |
docs: the proxy is the open door, not the closed one (Copilot review)
Both Copilot findings on PR #21 applied. The first is the serious one and it is a defect in documentation this PR itself introduced. `improvise-ws-server` has NO authentication -- a deliberate MVP limit its own module doc states -- so anyone who can open a socket to it can rewrite every document it serves. Loopback binding is the only thing standing between that and the network, and terminating TLS in front of it REMOVES EXACTLY THAT PROTECTION. The previous text not only omitted the warning, it actively reassured the reader: the `--bind` note explained that keeping loopback means the port "is not reachable from anywhere else", which is true of the port and false of the deployment, because the proxy is public. TLS buys confidentiality, not access control, and the two read alike in a recipe. So the warning leads the section, the Caddyfile carries `basic_auth` with a comment saying it is not optional, the `--bind` note now says what it does and does not buy, and the tunnel line says cloudflared/ngrok print public URLs and authenticate nothing by default -- an unguessable URL is not a control. Second finding: the watchdog comment was ambiguous in a file whose only verification is reading, which makes an ambiguous comment a real defect. It said a drop before the first frame "leaves the watchdog armed", meaning the state on ARRIVAL; it reads as a claim about the state after the call, which is the opposite of what `setShellFailure` does. Reworded to say the call disarms, why the opened path needs that (the watchdog is armed only after `transport.ready` resolves, i.e. on `open`), and that the never-opened path makes it a no-op. No open ticket covers server auth -- improvise-j31x is the multi-document registry and is closed -- so nothing is cited here and one is proposed in the hand-back rather than an ID invented. 1184 passing, unchanged. `node --check` clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Edward Langley
|
d120596a10 |
docs: the supported way to reach a TUI-hosted document over https
Per the user's ruling on improvise-o8fa: document the reverse-proxy path properly, implement no TLS. The existing text already named the symptom correctly -- an https origin refuses `ws://` as mixed content, `improvise-ws-server` has no TLS, so `wss://` is unobtainable from it -- but it gestured at "a reverse proxy or a tunnel" and stopped, which is not something a reader can follow. So README gains a worked Caddy example (Caddyfile, both commands, and the `?ws=` value to reload with), the nginx difference that actually bites (`proxy_http_version 1.1` plus the Upgrade/Connection headers, without which the handshake is answered as an ordinary request), the cloudflared/ngrok one-liners, and the note that `--bind` should be left at loopback behind a proxy on the same host. It also says the cost rather than papering it: the documented path to sharing involves a second piece of software, which is a poor answer for a tool whose selling point is being one binary. And it says what that cost buys against -- the standalone worker deployment is the point of the Pages site and needs no server at all, and `http://localhost` pairing needs none of this either. Native `wss://` is recorded as DECLINED WITH REASON rather than left as a gap. The substance is the certificate, not rustls: a self-signed cert makes the browser refuse the socket with no usable error, because a websocket handshake has no interstitial to click through. A real cert or mkcert are the only pleasant options and both are a second piece of software too -- so the choice is not one binary versus two, it is two either way, with one option additionally shipping a TLS stack in a pivot-table binary. web/index.html is the second home for the same fact and is updated to point at the worked example rather than restating it (gotcha 48: a fact stated twice is a grep). Both were checked for text the correction left stale. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Edward Langley
|
94d26d8705 |
fix(web): default the socket to TLS, and cache-bust the deployed bundles
?ws= now takes a host:port and the page supplies the scheme: wss:// when it
is itself served over https, ws:// when over http. An explicit ws:// from an
https page is UPGRADED rather than passed through, because an https origin
refuses it as mixed content -- so passing it through can only produce a
failure that reads like 'the server is not running'. The scheme is not the
caller's to get wrong.
And the deploy now stamps ?v=<sha> onto both bundles. Pages serves everything
max-age=600 and the wasm-pack filenames carry no content hash, so for ten
minutes after a deploy a returning visitor runs the PREVIOUS build -- which
made a shipped fix look like it had not shipped, twice, during the first real
browser debugging session.
BOTH halves are stamped, because a fresh glue file does not pull a fresh
.wasm: the glue locates it with new URL('..._bg.wasm', import.meta.url), and
a URL built against a base does not inherit the base's query. Stamping only
the import specifier would refresh the wrapper and keep the stale binary --
exactly the failure this is here to end.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EoW5BB87ZZdm96SvBTWawH
|
||
|
Edward Langley
|
4e1a79fe5f |
docs(web): from https the address must be wss://, and the server does not speak it
The page and the README told the reader that a ws:// address is blocked from an https origin, and then gave them a ws:// address. improvise-ws-server has no TLS at all -- no rustls, no native-tls, nothing -- so wss:// is not obtainable from it and the documented pairing could not work from the deployed site. Both now say the address must be wss:// from the deployed page, that the server speaks plain ws:// only so TLS has to be terminated in front of it, and that the ws:// form works when the page is served over http://localhost. The missing capability is filed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EoW5BB87ZZdm96SvBTWawH |
||
|
Edward Langley
|
bf02472d89 |
fix(web): an async message listener does not serialize, and a watchdog must not overwrite a real error
Three findings from the review-only PR #13, which put 28 already-merged
commits in front of a second reader, plus two comments that had gone false.
worker.js — `self.onmessage` was `async` and awaited `handle()`, which awaits
`server.flush()`. An async listener does NOT serialize: the event loop may
start the next `message` while an earlier one is suspended at an await, so two
`on_message`/`flush` pairs interleave and a later frame's replies can be posted
before an earlier frame's. That breaks the one rule the file names first — a
`GridUpdate::Cells` that overtakes its labels is DROPPED by the client's fold,
not replayed (gotcha 33) — and the pre-start `pending` drain had the same race
twice over, since `server` was assigned before the loop that drained it, so a
message arriving mid-drain overtook the frames it was queued behind. Every
frame is now chained onto one tail promise, which also makes "arrived before
start() finished" an ordinary case of the same rule instead of a second path.
app.js — the handshake watchdog fires 8s after the handshake and writes "the
peer did not answer". A peer that failed LOUDLY has already said something more
precise (worker `error`, `messageerror`, socket `error`), and the watchdog then
overwrote it with the vaguer guess. Those three paths now disarm it:
`setShellFailure` clears the timer rather than merely pre-empting it. The
watchdog is for silence only, which is what it was always for.
README — the "In the browser" section still said the deployed page needs a
server and that the in-page worker "is not built yet". Both landed in the same
unreviewed batch (improvise-9x6, improvise-djm), so the README described the
published site as a client with nowhere to connect while index.html described
it correctly. Rewritten, and it now states the caveat the commits state: no
line of that JavaScript has ever run.
Two comments that had gone false with it, and for the same reason — a follow-up
commit updated one copy and not the others. `web/app.js`'s keyboard note said
key resolution was "text entry, and nothing else" and that upstream keys would
work "until improvise-bpq2 lands"; bpq2 landed, moved the grid, panel and tile
cursors to the client, and settled that the host must not know the keymap.
`Host::handle`'s `ClientFrame::Key` arm said the same thing;
|
||
|
Edward Langley
|
188f3e94ef |
ci(pages): build the wasm bundle and publish web/ to GitHub Pages
improvise-d31's finished artifact depends on improvise-9x6 (the worker-hosted server) through improvise-djm. The *pipeline* does not, and building it now is what stops the delivery half from being a mystery when the worker lands: the swap from websocket transport to worker transport then changes what is bundled, not how it is built and shipped. Two jobs in ci.yml rather than a second workflow. `pages-build` runs on every push and pull request, so a change that breaks `web/build.sh` or the bundle fails on the PR that made it. `pages-deploy` runs only on a push to main and `needs: [test, wasm, pages-build]`, so nothing that failed `cargo test` or the existing wasm32 gate can reach the published site — the deploy depends on those checks instead of repeating them. Deliberately not path-filtered. The bundle is improvise-wasm-client plus improvise-protocol, improvise-core and improvise-formula, so a filter meaning "skip when the bundle cannot have changed" would have to name most of the workspace, and getting it wrong leaves a stale site with nothing saying so. wasm-pack is a pinned release tarball, v0.15.0 — the version `nix develop` now supplies — so the local run that produced 346 KB of wasm (120 KB gzipped) is evidence about the CI run. The devShell was missing wasm-pack entirely: the `wasm` job only runs `cargo check`, which does not need it, so nothing had ever required the one tool that turns a passing check into a loadable page. What this cannot do yet, said on the page itself rather than routed around: web/app.js is a thin client that holds no model, so a copy served from a static host paints an empty grid until it is pointed at an improvise-ws-server with ?ws=. The notice is static HTML, not something app.js decides, because it is true on every load and a page that cannot reach a server cannot be trusted to explain why. README says the same and names the deployed URL. Verified: cargo test --workspace 1134 passed / 0 failed, clippy clean, fmt clean, actionlint clean, ./web/build.sh succeeds and the assemble step produces a 388 KB site whose every reference resolves inside it. NOT verified: the workflow has never run — GitHub Pages is not enabled on the repository (GET /repos/fiddlerwoaroof/improvise/pages is 404), which is improvise-3gy. The deploy job asks configure-pages to enable it; if the token may not, that step is where the run fails and the fix is the repository setting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EoW5BB87ZZdm96SvBTWawH |
||
|
Edward Langley
|
08df85664e |
feat: add examples/demo.csv and examples/demo.improv
40-row CSV with obviously-fake sales data (fictional companies like Acme Corp, Wonka Industries, Cyberdyne Systems). demo.improv generated via headless import with Profit formula and a default view showing Region+Product on rows, Date_Month+Measure on columns. Added the import command to the README quick-start section. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
Edward Langley
|
d3d1df0be2 |
docs: expand README with concrete details
Add dedicated sections for the data model (formula examples), views and axes (tile mode, records mode, drill-down), the .improv file format (annotated example), import wizard and headless scripting, and the command/effect architecture. Link docs/design-notes.md from the "Why" section. Update build instructions to use `nix build .`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
Edward Langley
|
861b142dec |
chore: change license to Apache-2.0
Update both README and Cargo.toml. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
Edward Langley
|
769a5eb443 |
docs: write README for Show HN launch
All 10 sections per the launch plan: title, pitch, demo GIF placeholder, Lotus Improv context, quick start, key bindings, installation (Nix/crates.io/ prebuilt), codebase overview, expectations disclaimer, license. Under 100 lines, no badges or TOC. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |