docs: gotcha 49 for the origin check, and the inventory is a dated snapshot
CI / Build all bundles (push) Has been cancelled
CI / Test and lint (host) (push) Has been cancelled
CI / wasm32 check (browser bundle crates) (push) Has been cancelled
CI / Browser JS unit tests (node --test) (push) Has been cancelled
CI / Knowledge base consistency (push) Has been cancelled
CI / Lint the harness crate (push) Has been cancelled
CI / Browser harness (OPFS contract + shell) (push) Has been cancelled
CI / Deploy to GitHub Pages (push) Has been cancelled

Gotcha 49 records improvise-kc7n: a websocket handshake is exempt from the
same-origin policy, so no network control can see cross-site hijacking — an
IP allow-list, a private network and loopback binding all answer "may this
client reach the endpoint" and the honest answer is yes. Loopback is the
least protected case, not the safest. Records why all three arms are decided
rather than defaulted, and that none of it is authentication.

The File Inventory line counts were audited: 26 of 61 resolvable entries had
drifted past 10%, several by a third. Refreshed, and relabelled as a dated
snapshot with the method, because false precision in a carefully-checked
document is worse than an obvious estimate. Two traps found while doing it,
both recorded: a path-walking refresh resolves a bare name like lib.rs
against the first matching crate and rewrites the wrong number, and the
ws-server/ entries use a shorthand prefix no such walk resolves at all.

The code-review skill's gate list now runs BOTH spellings of
check-comment-runs.py. The flag selects rather than adds, so --check-evidence
never runs the comment cap and does not say so; three separate agents hit it
in one campaign (improvise-ijql).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
Edward Langley
2026-09-20 21:36:29 -07:00
parent 384f55c636
commit 95839fce43
4 changed files with 66 additions and 35 deletions
+14
View File
@@ -117,6 +117,9 @@ invariant(no_input_line_is_dropped,'The Markdown renderer covers exactly the sub
invariant(a_table_cell_boundary_is_an_unescaped_pipe_outside_a_code_span,'splitRow walks the row rather than split(\"|\"), because this corpus documents a file format whose own quoting syntax is |Time Period| and a cell containing one would be shredded into extra columns -- a row that still parses, so no column count could detect it. Latent when fixed, which is the point: the trigger is one ordinary sentence away in a document whose subject is pipe quoting.').
invariant(code_spans_stand_aside_for_emphasis,'Inline rendering replaces code spans with placeholders before matching emphasis, because this corpus routinely opens ** on one side of a code span and closes it on the other; matching per segment silently drops that bold with no visible symptom. Strong is matched before emphasis with a body that may contain a single * pair, because [^*]+ cannot match **a *b* c** and the leftover * then pairs across the bold -- which turned one sentence of gotcha 33 into three italic paragraphs.').
invariant(key_matching_is_variant_equality_never_unification,'scripts/kb-merge.pl compares clause keys with same_key/2 (copy_term + numbervars), never by unifying them. Unification matched too much -- a non-ground key probe(X) matched probe(a) and deleted it -- and MUTATED the incoming clause, binding X to a, so the clause written was not the clause supplied; the round-trip verify cannot see that, since it compares the file against an intended list already corrupted by the same unification (improvise-dj1l). A RULE keys on the whole clause, because one head shape with different bodies is ordinary Prolog. check_total/2 asserts the arithmetic the tool already PRINTED beside the delta size.').
invariant(duplicate_category_sections_merge_in_pass_one,'Two ## Category: sections naming one category are ONE category, concatenated by ParsedFile::push_category in pass 1 (improvise-349m). Merging in pass 2 was not idempotent: build_category creates its own section item-groups before its own > lines, while format_md writes every item before every > line, so the reparse built the other order. push_category destructures PCategory so a fourth field cannot be silently dropped on the duplicate path. No grammar change was needed -- this is the tree-walker, not the format, and duplicate headers stay legal.').
invariant(where_and_is_share_a_word_boundary,'where_clause, inline_where and is_expr are all compound-atomic with sp+ after the keyword (improvise-r08u). Non-atomic, pest auto-inserts WHITESPACE and SUM(Rev WHEREAS x = y) reads as a WHERE on a category AS. Note compound-atomicity ALONE refuses that input, since AS x cannot be one identifier, so the residue sp+ actually closes is WHEREAS=y -- a negative test needs both shapes.').
invariant(evaluate_aggregated_takes_a_named_state,'Model::evaluate_aggregated takes Aggregation::{Exact, OverHidden}, not none_cats: &[String] (improvise-5jm5). The list was consulted once, for emptiness, in every revision since Axis::None was introduced -- the aggregation runs over key.0 via matching_values. Aggregation::of reduces at the boundary and the regression guard is the SIGNATURE. recompute_formulas and aggregate_raw keep &[String] because they walk the elements.').
doctrine(bug_first,'write a failing test demonstrating the bug before fixing it').
doctrine(unit_over_e2e,'prefer small unit tests over integration/e2e').
doctrine(suite_speed,'Two numbers, because the ambiguity between them is what let this drift: NO SINGLE TEST BINARY may exceed 2 seconds, and the WORKSPACE WALL CLOCK is the sum of them. Measured at 22d43b2: 1309 tests over 33 binaries, ~2.2s wall, longest single binary 1.42s -- so the workspace is already past 2s and the rule that bites is the per-binary one. Re-MEASURE rather than re-assert: cargo test --workspace prints a \"finished in\" per binary. Excludes compilation, and excludes harness/ (Playwright, minutes, its own CI job).').
@@ -240,6 +243,13 @@ doctrine(a_careful_comment_is_weak_evidence_that_anything_checks_it,'The compani
doctrine(a_prepare_step_is_not_evidence_until_deleted,'A test-fixture prepare step is claimed load-bearing far more often than it is. Measured 2026-09-20 on every_layout_geometry_effect_is_visible_to_the_narrow_fingerprint: of four prepare steps documented as \"the reason the test is not vacuous\", only DeleteView and ShowItem are. ToggleGroup passes without one because ToggleGroup::apply calls toggle_group_collapse UNCONDITIONALLY -- the membership check gates only reset_cursor -- and TransposeAxes step is literally a no-op, because View::on_category_added already assigns the first regular category Row and the second Column. The check is cheap and nobody runs it: delete each prepare in turn and require the case to fail. A prepare documenting realistic input is fine; a prepare DESCRIBED as carrying the test and not carrying it is an entry that will be cited. Note the orchestrator brief asserting the replacement claim was itself wrong on two of the four, so this is not a failure of the original author alone.').
doctrine(a_precondition_asserted_after_the_act_asserts_nothing,'An assertion naming a fixture precondition must run BEFORE the code under test, or the code under test establishes it. Proven 2026-09-20 on selecting_the_page_already_selected_invalidates_no_cursor: \"the fixture was not on Q1\" ran after a SetPageSelection to Q1, so it could never see a fixture that started elsewhere. The property was still guarded -- a Q2 fixture failed -- but at a DIFFERENT assertion, under a message about renumbering. That split is the failure mode: the guard survives, the explanation drifts, and the next reader trusts the sentence rather than the assertion.').
doctrine(an_evidence_entry_that_measurement_contradicts_is_worse_than_none,'context/test-evidence records WHY a test can fail, so a mechanism stated there is load-bearing prose and will be cited. Proven 2026-09-20: the entry for every_layout_geometry_effect_is_visible_to_the_narrow_fingerprint claimed a dirty=false clear stopped the preparation satisfying the assertion. Removing the clear AND blinding document_config to 0 left all THIRTEEN cases reading false -- dirty is raised only inside App::apply_effects and no prepare runs an effect, so no preparation can set it. Nothing checks an evidence claim against the code: check-comment-runs.py --check-evidence verifies only that each heading names a test that exists. Re-run an evidence entry injection when you touch the test it describes. Probing this particular test needs the assert replaced by a print, because the loop aborts at case 1 under any injection blinding document_config -- which is probably how the wrong claim survived.').
doctrine(a_grammar_guard_must_be_generatable,'A pest rule whose only guard is a negative lookahead is a rule the grammar-walking generator can violate: the generator reads the grammar through pest_meta and SKIPS PREDICATES, so a caret-WHERE followed by !bare_char emitted WHEREa=\"\" and generated_formula_parses went red. An explicit separator (sp+) is generatable where a lookahead is not. This is why WHERE took IS\'s sp+ rather than the !bare_char improvise-r08u proposed, at the cost of one accepted spelling: a category may no longer abut the keyword. Applies to improv.pest equally, and constrains every future change to either grammar in this repo.').
doctrine(a_roundtrip_equality_is_blind_to_a_loss_both_sides_share,'fmt(parse(x)) == fmt(parse(fmt(parse(x)))) tests STABILITY of the printed form and says nothing about PRESERVATION. Demonstrated on improvise-349m: injecting a merge that DROPS a duplicate section left the whole improvise-io suite green at 2000 proptest cases while an item and its group line vanished from the document, because both operands lose the same thing. Main was unstable-but-lossless and the test could not tell that from stable-and-lossy. A preservation assertion must run BEFORE the equality. The sharpest form of check_the_checker yet seen here: the fault that survives is the one that changes BOTH OPERANDS.').
doctrine(could_not_reproduce_and_does_not_reproduce_are_different_claims,'Filing the second having earned only the first is how a real defect gets closed. On improvise-gfvj an agent concluded SortData does not reorder; the mechanism was real, and the cause was that ToggleRecordsMode pushes effect::SortData as its FIRST effect, so any records-mode fixture built by pressing R has a pre-sorted store BY CONSTRUCTION. The failure to reproduce was a property of the fixture, not of the code. Before writing \"does not reproduce\", name what would have to be true for the bug to exist and check THAT -- a fixture that cannot reach the condition reports the condition absent. Sibling of a_benchmark_fixture_that_cannot_reach_the_slow_branch_measures_the_fast_one.').
doctrine(a_script_with_two_modes_has_two_gates,'scripts/check-comment-runs.py runs the comment-run cap bare and the evidence cross-check under --check-evidence, and NEITHER MODE RUNS THE OTHER. Every wave brief this campaign, and the gate list in the code-review skill, named only --check-evidence -- so the cap was effectively unrun locally and caught only by CI. It caught one: an enum inserted between Model doc comment and its derive, leaving the enum carrying the comment and Model with none. When a gate grows a flag, the unflagged invocation stops being run by everyone who copied the command, and a gate nobody invokes is indistinguishable from a gate with nothing to catch.').
doctrine(an_injecting_reviewer_needs_its_own_tree,'An adversarial reviewer given leave to inject faults RACES THE AUTHOR if they share a worktree: on wave F the reviewer subagent injections landed in the author tree mid-gate and the author own run reported a failure for a fault it had not written. Everything was restored cleanly, but the diagnosis cost real time and could have been read as a genuine regression. A reviewer that injects gets its own worktree or a scratch copy, always. Related and separately costly: the session SCRATCHPAD IS SHARED across wave agents, and one agent overwrote another pr-body.md mid-task -- unique filenames, or a per-agent subdirectory.').
doctrine(who_may_connect_is_two_questions,'Ask of any access control whether it constrains the CLIENT or what may DRIVE the client. A websocket handshake is exempt from the same-origin policy, so an address allow-list, a private network and loopback binding all answer \"may this client reach the endpoint\" and answer YES to a cross-site hijack: the request genuinely comes from an allowed client, driven by a page the user never intended to trust. Only Origin separates them. Loopback is the LEAST protected case, having no proxy in front of it at all -- the intuition that it is the safe default is exactly backwards (improvise-kc7n).').
doctrine(a_line_count_in_a_document_is_a_dated_snapshot,'repo-map File Inventory line counts were audited 2026-09-21: 26 of 61 resolvable entries had drifted past 10 percent, several by a third. They are useful as \"is this file big\" and useless as a figure to cite, which is how false precision enters a document that is otherwise carefully checked. Refreshed and labelled with the date and the method. Two traps found while doing it: a naive path-walking refresh resolves a bare name like lib.rs or render.rs against the FIRST matching crate and silently rewrites the wrong number (it reported lib.rs 561 -> 8), and the ws-server/ and improvise-worker-server/ entries use a shorthand prefix no such walk resolves at all, so they drift silently and must be checked by hand.').
work_done('improvise-8tk',bug_fix,'ZZ/:wq quit even when the save failed, silently discarding the model. Save now aborts the effect batch on failure; save_to_current_path/1 returns Result<String,String>; three tests in ui/effect.rs.').
work_done('improvise-907',bug_fix,'Import wizard step machine redesign: validation_error() gate at ReviewProposals (no-category and over-limit), history-stack previous() bound to Left on every step, and Done made a real confirmation step holding the built workbook. Superseded improvise-gfq, improvise-o1c, improvise-c6j.').
work_done('improvise-6sq',wont_do,'Merging EditOrDrill and EnterEditAtCursorCmd into one EnterEdit{mode} is not safe: they differ in WHEN the cell value is read, not in a mode parameter. Added ui/app.rs::tests::i_and_o_diverge_on_an_empty_records_grid as executable evidence.').
@@ -339,6 +349,10 @@ decision('improvise-864a',skip_a_superseded_pages_deploy,'Three merges within 20
decision('improvise-9c8c',gate_the_comment_cap_last,'The CI step enforcing the two-line comment cap and the test-evidence keys was held back until every unit of the comment wave had merged, so that the gate\'s first green run was evidence rather than vacuous.').
decision('improvise-kh8',landing_page_at_the_root,'USER RULING 2026-09-20: the landing page should be served at the root of the repository and link to the in-browser demo. Supersedes the agent\'s proposed landing_page_beside_the_app, which argued for the app at the root with the landing page at /landing/ on the grounds that the live app is the better front door; that argument is recorded as the reason the alternative was considered, and it was overruled. web/assemble.sh puts docs/index.html at the site root and moves the app bundle (index.html, app.js, style.css, worker.js, demo.improv, pkg/, worker-pkg/) under /app/, moving the two bundle dirs WITH the app so every relative path in the generated glue stays correct. The non-obvious cost: harness/tests/shell.spec.js navigates to /index.html six times, so a path move at the root silently retargets the whole shell spec at a different page. The deployed arrangement IS what the harness serves (improvise-vvl3), which is the property that makes the harness trustworthy and also the property that makes a layout change a test change.').
decision('pr82-fingerprint-guards-2026-09-20',a_test_reads_the_view_its_effect_writes,'App::view() resolves ViewState::active_view (this client) and Workbook::active_view() is the DOCUMENT DEFAULT; gotcha 29 makes them deliberately free to differ since improvise-jytm. A test asserting what a geometry effect did must read App::view(), the view the effect writes through view_mut(). They agree in a single-client fixture, so the mismatch is latent and survives review easily -- found only because a sibling guard added beside it used the other accessor, one test reading one view through two accessors.').
decision('improvise-l8rh',ambiguous_date_is_a_known_limit,'Ruled by the user 2026-09-20: leave detect_date_format first-match. A day-first default only moves which half of the world imports silently wrong; the honest fix surfaces the ambiguity and changes FieldProposal shape plus the wizard step, which is deliberate work rather than a patch. Tolerable because improvise-7yx made it deterministic. Pinned by an_ambiguous_date_is_read_american_first with a_day_first_only_value_settles_the_ambiguity as the discriminating arm.').
decision('improvise-kc7n',origin_default_is_loopback_not_the_projects_own_page,'improvise-ws-server refuses a handshake whose Origin is neither loopback nor named by --allow-origin. Loopback is the default because the shipped local recipe serves the page from http://localhost:8080 while the socket is on another loopback port, so the one case worth optimising for must need no flag. The deployed Pages origin is NOT hard-coded: it can only reach a plain-ws server through a TLS proxy, already a multi-step setup carrying an Origin allow-list, so one flag is an honest cost there where a hard-coded default would be a permanent hole for every other deployment.').
decision('improvise-kc7n',an_absent_origin_is_allowed_and_two_are_not,'Absent is DECIDED, not defaulted: every browser sends Origin on a handshake unconditionally, so absent is a non-browser client (curl, a native client, the crate own tests) that an address check already answers, and refusing it would break the repo own instrument for nothing. TWO Origin headers are refused, because HeaderMap::get is first-wins and a proxy that APPENDS rather than sets leaves two -- answering from the first lets the second be anything. An Origin that is not valid text is refused on the same ground. The residual cost is stated rather than guarded: a proxy that STRIPS Origin turns the check off, so the proxy own allow-list stays in README as defence in depth.').
decision('improvise-kc7n',an_origin_check_is_not_authentication,'It answers what page is DRIVING a client, never WHO is connecting, and anything that is not a browser page sends no Origin and is unaffected. improvise-kpnr is untouched by it and still open. Agent recommendation on the PR thread: a token in the URL, ws-server only, because new WebSocket(url) takes no headers so the URL is the one place a page can put a credential DELIBERATELY rather than ambiently; basic auth is ruled out on evidence, since a page cannot answer a challenge on a socket handshake.').
wire_carries(command,both_directions,'user intent up, projection commands down').
wire_carries(snapshot,on_connect_only,'improvise-q08: full ViewState + render cache filled from current viewport').
wire_carries(command,both_directions,'user intent client->server, projections server->client').