83 Commits

Include renames

Author	SHA1	Message	Date
timothy	02c82b35ea	fix(610): make the frontmatter READ path dependency-free — CI has no PyYAML ... This is why `decisions lifecycle` went red, and it was NOT the known flake. I came close to dismissing it as one for the second time this session, because an earlier red on another branch genuinely was. The dual-format parser imported PyYAML to read frontmatter. `decisions-guard` does `setup-python` and installs NOTHING, so once the corpus was migrated every record became unparseable there: ModuleNotFoundError, job fails. The same would hit the Husky pre-commit hook and every contributor's machine. Installing PyYAML in CI is the wrong fix: READING happens everywhere -- CI, the hook, every dev -- while WRITING happens once, in a migration a human runs deliberately. So the read path is now dependency-free and only `migrate_decisions_split` (the writer) still imports yaml. A hand-rolled parser is only safe if it provably matches the library that WROTE the files, so `test_frontmatter_reader_matches_pyyaml_on_every_real_record` compares the two field-by-field across all 169 real records (importorskip, so it is skipped rather than failing where PyYAML is absent) with a >100-file guard against near-vacuity. It is narrow by construction: the frontmatter is machine-generated with default_flow_style=False and width=10**9, so every value is a single-line scalar, and the reader bails to None on anything nested. Verified by running all four affected entry points against a shim that makes `import yaml` raise: validate --base/--head, catalog --check, the kickoff guard, and the plain validate the pre-commit hook calls. All exit 0. refs #610	2026-07-25 19:57:22 +02:00
timothy	52786a545c	feat(610): re-key the body-diff guard to key; enforce path<->key; pin the filesystem invariant ... Three Done-when items that were genuinely unfinished. I audited the checklist before ticking it rather than after, and these were not done. 1. BODY-DIFF GUARD RE-KEYED FROM HEADING TO `key`. Heading-keying made a pure RENAME look like a removal plus an unrelated addition, so retitling a record failed CI as an "unlogged removal" -- a trap that has bitten this repo before. Records are now MATCHED by key and REPORTED by heading (a key alone is not enough for a human to find the record). Demotion, which is inherently about a record losing its key, is detected on the heading axis instead, and a demoted record is no longer double-reported as removed. Key-matching is also what lets the comparison work straight through the legacy->frontmatter migration, since `key` survives both the format change and the move between files. Pinned by a test driving the REAL git-backed diff engine: retitle a record, same key, byte-identical prose -> removed/rewritten/demoted all empty. Mutation-verified: restoring heading-keying makes it fail with exactly "a retitle was reported as a removal". 2. PATH <-> KEY ENFORCED BY THE VALIDATOR. The filename is derived from the key, which is what makes one-active-per-key a filesystem property rather than a check -- but only if the two cannot drift. Now an error when they disagree, with legacy multi-record files explicitly exempt (they have no key-derived path to match). 3. ONE-ACTIVE-PER-KEY PINNED AS A FILESYSTEM PROPERTY. Two records sharing a key derive the same path, so the filesystem refuses the second; the migration aborts on a destination collision rather than silently overwriting. Both are now tested, the latter end-to-end through `main()`. Also fixed a vacuous assertion ruff caught in my own new test -- `assert X or True` is always true. Note on verification: my first positive control for the guard reported it NOT firing. That was the probe, not the code -- "load 340" appears in both the `sources:` frontmatter field and the body, and replace(...,1) hit the metadata copy, which is correctly token-free. Re-run against genuine body prose, the guard fires with exit 1. refs #610	2026-07-25 19:53:08 +02:00
timothy	fba5233caf	feat(610): split the decision corpus into one YAML-frontmatter file per record ... 168 records -> docs/decisions/records/<area>/<topic>.md (163 active, 23 dirs) and docs/decisions/archive/<area>/<topic>.md (5 archived). The filename IS the key, so one-active-record-per-key becomes a filesystem property rather than a validator check, and supersession becomes a `git mv`. WHY: the monolith was a concurrency problem before an aesthetic one. A 3,900-line append target made parallel sessions collide -- PR #605 and PR #614 both hit append-vs-append conflicts during routine rebases, and hand-resolving those inside the corpus is exactly the operation the rationale-rewrite guard exists to police. HOW IT IS VERIFIED: a ~170-file diff cannot be meaningfully read, so correctness does not rest on reading it. The parser was taught BOTH formats first, so the body-diff guard parses the old form at the merge-base and the new form at head -- the migration validates itself, no bypass. The proof is a field-level equivalence harness: 168 records before and after, zero lost, zero gained, zero field mismatches, zero rationale bodies differing. Reviewers should scrutinise the harness; it is the actual evidence. What measuring caught that reading would not have: - ~500 lines sit OUTSIDE any record -- decisions.md's lifecycle schema and each topic file's preamble, mostly the only copy. Source files are kept and stripped, never deleted. They also cannot be filed per-area: topic files hold several areas and 4 of 23 areas span several files. - Archive discovery was a non-recursive glob; after the split it found ZERO archived records, surfacing as four bogus "supersedes points to unknown key" errors rather than an obvious failure. - ~32 live docs point into the corpus BY DATE, which the split dangles. Each stripped file now ends with a generated "Records formerly in this file" index, which also rescues the identical breadcrumbs in old issue comments. - decisions.md's "In this file:" list was 97 same-file anchor bullets that the split makes WRONG, not merely stale. Dropped; the generated index replaces them with links that resolve. The equivalence harness now runs against a checked-in FIXTURE, not the live corpus. The earlier version migrated the real tree, which made it a one-shot: the moment the migration landed there was nothing left to move and the tests failed for reasons unrelated to the code. A fixture keeps them testing the SCRIPT rather than the repo's current state. Keys preserved verbatim, warts included: `sched` (12) and `scheduling` (1) remain two directories for one concept. Renaming a key is not a move -- it changes identity, breaks the equivalence proof, and invalidates MemPalace's per-key drawers. Taxonomy normalisation is separate work. refs #610	2026-07-25 19:45:09 +02:00
timothy	8578dc1ca7	fix(610): de-brittle the count assertion; stop conflating index lines with preserved prose ... Both surfaced when main gained two records mid-flight. - test_record_count_is_the_expected_166 hardcoded the total, so it failed the moment a record landed on main -- a merge turning an unrelated test red. The real invariant is before == after; the count only needs to prove the harness isn't parsing a stub corpus, so it is now equality plus a floor. - The migration's "lines preserved" figure silently absorbed the generated where-did-it-go index once that was threaded into the preamble string, jumping 507 -> 759 with no new prose preserved. It now reports the two separately: 514 lines of original prose, plus 245 generated index lines. A number that quietly changes meaning is worse than no number.	2026-07-25 19:08:58 +02:00
timothy	64b65fd2db	feat(610): generated where-did-it-go index on each stripped file ... Live docs point into the corpus BY DATE -- "see `decisions.md` 2026-07-10" -- about 32 such references across 12 files, plus the same form in historical issue comments. The split would dangle every one of them. Each stripped file now ends with a generated "Records formerly in this file" index: date, title, and a link to the record's new path. A reader following a date pointer lands on the file it names and resolves from there. That is far cheaper and less error-prone than rewriting 32 references by hand, and it also covers the issue-comment breadcrumbs, which cannot be rewritten at all. Caught while verifying it: the generated `## Records formerly in this file` heading is itself an H2, so the record parser counted one legacy-unmigrated record per stripped file -- the notice went 0 -> 6. Same treatment as the existing `## Index` section: skip it by name. SKIP_HEADINGS moved to decisions_lib as the single source of truth, since three modules now need it. Found by reading the validator's notice output on a trial migration, not by inspection -- the corpus still validated OK, so nothing else would have flagged it.	2026-07-25 19:08:58 +02:00
timothy	8ec1f527e4	fix(610): catalog path links only for the split layout, not every file under docs/decisions ... The first cut tested `relative_to(TOPIC_DIR)`, which also matches the LEGACY multi-record topic files -- so every record in workflow-process.md et al. lost its anchor and linked to the top of the file instead of to its own record. Caught by `build_decisions_catalog --check` going stale on the unmigrated corpus, not by reading. Narrowed to `RECORDS_DIR in src.parents`, which by construction only matches one-record-per-file.	2026-07-25 19:08:58 +02:00
timothy	1af65b7bee	feat(610): budget counts PROSE, excluding YAML frontmatter ... The line budget exists to bound how much narrative a reader or agent must get through. Under the split each record carries ~11 frontmatter lines plus two fences -- 1789 lines across 166 records -- which are the structured restatement of what used to be one dense backtick line. Counting them inflates the metric without any new knowledge being added. Stated plainly because it flatters the number: this is a change of METRIC, not a consolidation. It re-measures the same corpus, it does not shrink it. Whole-file counting put the migrated corpus at 6837 against a 5600 budget; prose-only puts the same content at ~5048. The consolidation work is still worth doing -- it is simply no longer being signalled by a warning that was partly measuring punctuation. Inert pre-migration: no legacy file has frontmatter, so the branch is never taken and today's number is unchanged.	2026-07-25 19:08:58 +02:00
timothy	ef67be924c	feat(610): teach the validator and catalog the split layout ... Both surfaced in a trial migration against a disposable copy of the real corpus, not from reading: - Archive discovery was a NON-recursive glob. After the split, archived records live at archive/<area>/<topic>.md, so every one of them became invisible -- which surfaced as four bogus "supersedes points to unknown key" errors rather than as an obvious "no archive found". rglob at both sites. - Catalog links: a record is now a FILE, so the link is a plain relative path with no anchor -- nothing to slug, nothing to keep in sync with a heading. The legacy anchor form is kept for records still living inside a multi-record file, so the catalog is correct on either side of the migration.	2026-07-25 19:08:58 +02:00
timothy	d52268f2ed	feat(610): migration script + field-level equivalence harness ... The migration is a MOVE, so correctness is provable rather than reviewable: parse the corpus before, migrate, parse after, assert the Record sets are identical field-for-field with byte-identical rationale. scripts/migrate_decisions_split.py 166 records -> docs/decisions/records/<area>/<topic>.md (active) or docs/decisions/archive/<area>/<topic>.md (superseded/retired), 26 directories. Path is DERIVED from the key, so identity stays the key. Refuses to run if any record lacks a key, and aborts on a destination collision. Source files are KEPT, stripped to their narrative -- not deleted. 506 lines of the corpus sit outside any record: decisions.md's lifecycle-schema header (the status vocabulary, supersession rules, the edit-token contract) and each topic file's preamble explaining why those records exist. For most of it that is the only copy. It also cannot be filed per-area -- topic files hold several areas and 4 of 23 areas span several files -- so the files themselves stay. scripts/tests/test_migration_equivalence.py Runs the real migration against a COPY of the real corpus in tmp_path, never the working tree. Asserts: all 166 records survive with the same keys, every metadata field round-trips, titles round-trip from headings, every rationale body is byte-identical, path matches key, archived records land in the archive wing, the legacy files keep their narrative, and no parseable record is left behind in them. Proven non-vacuous: corrupting one migrated record's prose is caught by the byte-identical check, and deleting one is caught by the survival check. One test-authoring note: an early assertion string-matched "## " to prove no records were left in decisions.md. That is wrong -- the schema header quotes an illustrative "## 2026-07-17 ..." example in prose. Whether records remain is a PARSING question, so the parser-based leftover test is the real invariant.	2026-07-25 19:08:58 +02:00
timothy	e063fd065f	feat(610): dual-format decision-record parser (backtick line + YAML frontmatter) ... First step of the one-file-per-record split. `decisions_lib.parse_text` now dispatches on a leading `---`: the legacy "many ## records per file, metadata on a backtick line" form and the new "one record per file, YAML frontmatter" form both produce the same Record. This is the load-bearing move for the migration. Because the parser understands both, the body-diff guard can parse the OLD form at the merge-base and the NEW form at head -- both keyed on `key` -- so the commit that relocates 166 records validates itself with no one-time bypass and no escape hatch. Details: - Frontmatter uses the SAME field vocabulary as the backtick line (`superseded-by`, `stale-after`), so on-disk names don't shift mid-migration. - YAML `None` (a key with no value) is preserved as "" rather than collapsing to None, keeping the empty-vs-absent distinction `stale-after` depends on (#603). - `active_files()` also walks `docs/decisions/records/**`, so both layouts can coexist while the migration lands. - Malformed frontmatter (unterminated fence, non-dict, YAML error) yields no record rather than raising; the validator then reports it as a missing record.	2026-07-25 19:08:58 +02:00
timothyandClaude Opus 5	e9f4444fec	docs(609): qualify two comments as non-merge, matching the implementation ... Review Low: the module comment and the _edit_trailer_armed summary still said "a commit message ... does arm" / "some commit", while the matcher excludes merges. The decision Rule and catalog already carried the qualification; these two did not. Comment-only. fixes #609 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-25 18:38:58 +02:00
timothyandClaude Opus 5	f00dfde0c5	docs(609): correct the --no-merges rationale — merges are discouraged, not blocked ... Review Low x2, both correct and both the stale-comment class: The claim that prepush-rebase-check.sh forbids merging main into a PR branch is false. That hook refuses a branch that is BEHIND origin/main; a merge makes origin/main an ancestor, so the push is allowed. Merging main in is discouraged by convention only. So --no-merges does cost a real false negative: an author who marks ONLY a conflict-resolving merge commit gets a legitimate rewrite rejected. Keeping --no-merges and stating the trade explicitly -- that failure is loud and costs one extra commit, whereas honoring forge-composed merge bodies disables the guard silently, which is #609 itself. The module comment also still claimed a quoted example cannot arm the guard, which contradicts the residual the decision record now states accurately. Aligned both, and narrowed the record's Rule line from "some commit" to "some NON-MERGE commit" so the stated contract matches the implementation. No logic change -- comments, docstring, record prose and regenerated catalog only. fixes #609 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-25 18:36:07 +02:00
timothyandClaude Opus 5	cc9481f541	fix(609): review fixes — exclude merge commits, unfold folded values, correct the fail-posture doc ... Cross-family review (High + Medium + Low), all three reproduced before fixing: High -- on a pull_request event actions/checkout lands on a synthetic merge commit whose body the forge composes from the PR description, so a description ending in an example marker armed a guard no author armed. Excluded merge commits from the range; merging main into a PR branch is separately forbidden, so no author-written commit is skipped. Medium -- a folded value (`no` + continuation ` yes`) was split into independent lines and the continuation armed on its own, inverting the value the author wrote. Read with `unfold` so the value is judged whole. Low -- the module docstring promised fail-open while marker resolution deliberately fails closed. The posture is right; the docstring was wrong. Documented as the one exception. Both new negative controls verified red against the unfixed matcher. Decision record amended to state the residual honestly rather than overclaim: a quoted example that is the FINAL paragraph of an ordinary commit is a trailer by git's own grammar and does arm. What the change buys is that discussing the marker can no longer disable anything. fixes #609 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-25 18:29:46 +02:00
timothyandClaude Opus 5	c597c49f02	fix(609): arm the decisions body-diff with a git trailer, not a bare substring ... The rationale-edit exemption was a substring test over the whole commit range, so any message containing the literal marker armed it -- including prose explaining why no marker was needed, which is how it fired live in PR #605: a green --base/--head run that was vacuous on the body-diff dimension, in the one PR that hand-resolved a merge conflict inside the corpus the guard exists to police. Now read as an affirmative `Decisions-Edit:` git trailer. Git parses trailers only in the final paragraph, so a quoted example message cannot arm it -- which matters because this commit and its decision record both quote one. A non-affirmative value (`no`) does not arm it either; the retired substring arms nothing and gets a ::warning:: nudge. Tests: negative controls (prose mention, quoted mid-body trailer, `no` value, retired substring) plus positive controls (trailer, uppercase, alongside Co-Authored-By). All four negative controls verified red against the old matcher. fixes #609 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-25 18:21:12 +02:00
timothy	75139bbf31	fix(603): close four defects found in adversarial review of the stale-after fields ... 1. `date.fromisoformat` is not a YYYY-MM-DD validator. On Python >= 3.11 it also accepts ISO basic format ("20270101") and week dates ("2027-W01-1"), so a malformed-looking value passed the blocking check — and which forms parse depends on the interpreter, meaning the same corpus could validate differently on a dev machine and on the runner (pr-checks.yml pins only python-version '3.x'). Knock-on: the catalog's Review-due section sorts on the raw STRING, so an accepted "20270101" sorted AFTER "2027-01-15" ('-' < '0'), contradicting the section's own "sorted soonest-first" text. Gate on ^\d{4}-\d{2}-\d{2}$ first, which fixes both — a fixed-width zero-padded form makes string sort == date sort. 2. A present-but-empty `stale-after:` was collapsed to None by `or None` in the parser and then skipped by a truthiness guard in the validator, so it passed as "absent" — a field that silently never fires, which is the exact failure mode the blocking check exists to prevent. Keep "" distinct from None and test with `is not None`. 3. `test_catalog_is_date_independent` was partly vacuous: with no date in either render, both sides were trivially equal after the .replace(). It did still catch an injected clock-derived marker, but it passed with the feature deleted. Assert the dates are present. 4. The malformed-date check ran only over the active set, exempting archive records. Staleness is moot there, but a typo is still a typo — check both wings. Adds regression tests for each, plus a Review-due row for a topic-file record (pinning the `../decisions.md` vs bare-filename link forms).	2026-07-25 15:22:50 +02:00
timothy	0ad02db651	feat(603): adopt OKF's optional stale-after and Sources decision-record metadata ... Evaluated the Open Knowledge Format (GoogleCloudPlatform/knowledge-catalog okf v0.2, scaccogatto/okf-skills) as a replacement for our decision-record system and rejected it: its conformance rules are deliberately permissive exactly where ours are strict (broken links, unknown types and missing fields must all be tolerated; `deprecated` points at no successor), and its stable identity is the file path, which the breadcrumb rule tells agents not to trust. Adopted two of its optional families instead, additively: - `stale-after: YYYY-MM-DD` on the metadata line — marks a record asserting an outside-world fact as due for re-confirmation. Absolute date, no TTL. - `**Sources:**` in the metadata block — the evidence a record rests on, as distinct from `Signals:` (recall keywords). Neither is required; absence is never an error. A malformed `stale-after` is blocking (it would silently never fire), but a past-due record is only a non-blocking `::notice::` — going stale is the passage of time, not a defect in whatever commit is under test. The catalog's new "Review due" section renders the date only and never a clock-derived verdict, so it cannot drift `--check` red on a calendar boundary with no commit touching the corpus. No backfill: no existing record adopts either field here. fixes #603	2026-07-25 15:22:50 +02:00
timothy	352aa70634	refactor(445): drop e2e-ui.sh's port pre-flight, now owned by e2e-local.sh (#586/#598) ... main's #598 added a port pre-flight to scripts/e2e-local.sh while this branch was in review, and theirs is strictly better than the one I had here: - it probes BOTH bound ports. The app binds a SECOND (streaming) listener that defaults to 8409 regardless of ETV_UI_PORT, so my single-port check could certify a port free while the run still died binding 8409. That also means this PR's CI step (ETV_UI_PORT=8410) was only working because the curl step's server had already been killed — #598's fix, defaulting ETV_STREAMING_PORT to the given port, is what makes it correct rather than lucky. A real latent bug in my work, surfaced by their change. - it REPORTS rather than reaps, which is the #586 rule. Keeping mine would leave two divergent port checks on the same concern. Also records the app's SINGLE-INSTANCE guard, which is independent of ports: a stray instance blocks a run whatever port you choose, so 'pick another port' is not a workaround. Kill the leftover BY PID (#586). Refs #445 #586	2026-07-25 14:29:27 +02:00
timothy	54919e4730	fix(445): pidfile-first cleanup ordering + clear BOOT_PID after wait ... Codex round-3 verdict was BLOCKED @ 29bc5119 with three High findings. All three are real; all three are fixed. They are narrower than the previous rounds — two are sub-millisecond races between adjacent statements — but the fixes are cheap and make the ordering correct rather than lucky. **Recycled-PID kill (the one that genuinely mattered).** `BOOT_PID` stayed set after `wait` had already reaped the launcher. A stale PID is not merely useless: the OS can RECYCLE that number during a long spec run, and cleanup would then SIGTERM an unrelated process. This is the same "never kill something you did not prove is yours" failure that got the earlier port-based reap deleted — it came back in a different disguise. Cleared immediately after `wait` returns. **Cleanup ordering.** cleanup() killed e2e-local.sh BEFORE reading the pidfile, so a signal landing between the server fork and the pidfile write could kill the launcher and lose the only handle on an already-running server. The pidfile is now read FIRST; stopping the launcher is best-effort and never the primary handle. **Fork/publish sliver.** A signal can also land between `... &` and `BOOT_PID=$!`, or between e2e-local.sh's fork and its pidfile write — the pidfile reads empty, then populates microseconds later, and the server is orphaned. cleanup now does a bounded grace re-read (~1s) when it has no PID. Verified this does NOT cost anything on a genuinely server-less abort: a bad ETV_BUILD_CONFIG still exits in 1s rather than sitting out the loop. ## Verification - passing run 3/3; failing run exits 1 - server-less abort exits in 1s (grace loop stays bounded) - targeted SIGTERM: exit 143, no listener left - zero orphaned processes after the full gate Refs #445	2026-07-25 14:04:52 +02:00
timothy	e8ad79623f	fix(445): make the boot wait interruptible so a mid-boot signal can't orphan the server ... Codex round-2 verdict was BLOCKED @ 2a5f26c0 on one High finding, which was correct and is the subtlest defect in this whole change. ## The defect Bash DEFERS a trapped signal while it waits on a FOREGROUND command. The boot was OUT="$(ETV_UI_PORT=... scripts/e2e-local.sh "$CONFIG_DIR")" so a TERM arriving during the (up to 120s) readiness wait did not run `on_signal` until boot completed — and a supervisor escalating TERM->KILL means the trap never runs at all, orphaning the server on its port. The pidfile added in the previous commit only helps IF the trap runs; this is the case where it doesn't. Proved the mechanism in isolation rather than asserting it, since the real boot is only ~2s locally and kept winning the race: foreground command substitution : TERM at t=1s -> trap ran at t=6s (deferred) background job + `wait` : TERM at t=1s -> trap ran at t=1s (prompt) ## The fix Boot in the background and `wait` on it. `wait` is interruptible, so the handler runs immediately; by then e2e-local.sh has already written $ETV_PIDFILE, so cleanup can reap a server whose PID this script has not yet parsed. Cleanup also now stops the backgrounded e2e-local.sh itself, so it cannot sit waiting on a server we just killed, and removes its temp output file. Boot diagnostics are preserved on the failure path (verified with a deliberately bad ETV_BUILD_CONFIG: the underlying "bin/<config> does not exist" error is surfaced, along with the exit status, which is now propagated rather than flattened to 1). ## Verification - mechanism: deferred-vs-prompt trap proven as above - passing run 3/3 green; failing run (--grep ZZZ_NOPE) exits 1 - boot-failure path prints e2e-local.sh's real error and its exit status - targeted SIGTERM: exit 143, no listener, no stray process - curl harness unaffected: 45/45 PASS - no orphaned processes after the full gate (an earlier count of 1 was a graceful shutdown still in flight, not a leak — re-checked clean) Refs #445	2026-07-25 14:04:52 +02:00
timothy	71706f3849	fix(445): silent-pass on spec failure + prove server ownership via pidfile [decisions-edit] ... Second review round. Codex returned BLOCKED @ e50a2624 with three High findings; all three were real and all three are fixed. One of them was severe and was introduced by my OWN previous "fix" commit. ## HIGH 1 — a FAILING spec run exited 0, silently passing CI `if ! npx playwright test; then status=$?; ... exit "$status"; fi` Under `!` negation bash sets `$?` to the LOGICAL NEGATION of the command's status, so inside the failure branch `$?` reads 0 — the script exited 0 on a failing run. Verified: `if ! (exit 42); then echo $?; fi` prints 0. A UI-E2E harness that reports success when its specs fail is worse than no harness. My five green local runs could never have caught this: the bug lives only on the failure path. Introduced by the log-tail improvement in e50a2624. Fixed with `set +e` / read `$?` / `set -e`, then exit that status explicitly. Verified: a deliberately-failing run (`--grep ZZZ_NOPE`) now exits 1. ## HIGH 2 + 3 — the pre-PID fallback needed lsof and only GUESSED ownership The mid-boot fallback reaped "whatever LISTENS on $PORT", which was wrong twice: - it needed `lsof`, which is ABSENT from the CI toolchain image (verified directly in the published image) — so it silently no-opped precisely where it was needed; - it INFERRED ownership from the earlier pre-flight rather than proving it, so a process that grabbed the port after the pre-flight — or a real instance on a shared host — could be killed. Reaping someone else's server is worse than the leak it was meant to fix. Replaced with an opt-in `ETV_PIDFILE`: e2e-local.sh writes the PID the instant it forks, BEFORE its readiness wait, which is exactly the window a mid-boot signal lands in. A pidfile we asked for PROVES ownership, needs no external tool, and works in CI. The port-based kill is gone; the lsof pre-flight remains as a friendly local check only. Verified: the pidfile is populated while still mid-boot (readiness not yet reached), names the real `dotnet ErsatzTV` process (not a subshell — which also re-confirms the `exec` fix), and killing that PID alone frees the port. Also dropped the `seq` dependency inside the trap (shell arithmetic instead), addressing the other reviewer's busybox concern. ## Docs-reviewer finding — my stated reasoning was wrong I justified amending `testing.e2e-local-fresh-config-dir` rather than superseding it partly on "renaming the heading trips CI". That's a true statement that does NOT bear on the choice: a supersession relocates to `archive/` with the heading INTACT (verified: archive/api.md keeps the #72 heading verbatim). Corrected to the actual reasons — the Rule never reversed, and the key is cited from docs/handoffs/chicorytv-issue-queue.md plus two docs/superpowers/ files, which a supersession would aim at an archived, stale-labelled record. ## Gotcha found by accident, now documented A flawed test of mine booted two e2e-local.sh instances concurrently and the first mysteriously failed to become ready. Cause: every run `rm -rf`s and re-copies the SAME build-output wwwroot, so a second run yanks the static files out from under a still-starting first instance. Documented in both the script header and docs/e2e-local.md, because the symptom (readiness timeout, or /app 404ing) looks nothing like a shared-directory race. CI is unaffected — its curl and UI-E2E steps are sequential. ## Budget: filed, not shaved This PR pushes the active decisions corpus 7 lines past its 5600-line soft budget (main was under). I trimmed my records repeatedly and each rewrite recovered ~1 line, because the content is load-bearing; continuing would have meant deleting useful rationale from a new convention record to hit an arbitrary cap. The validator's own remedy is "schedule a consolidation", so that is filed as #595 rather than paid for by starving the record. Non-blocking warning. Also filed #594 for the pre-existing `ci-image-pin` any-hex-length weakness. ## Verification - failing run exits 1 (was 0); passing run still 3/3 green - pidfile written mid-boot, names the real dotnet proc, reap frees the port - SIGTERM mid-run: exit 143, no orphan listener or process - curl harness unaffected: 45/45 PASS; ETV_PIDFILE unset => unchanged behaviour - decisions validator OK; zero orphaned processes after the full gate Refs #445 #533 #594 #595	2026-07-25 14:04:52 +02:00
timothy	0f1951340e	fix(445,533): harden e2e-ui lifecycle + retire stale #533 decision record [decisions-edit] ... Addresses the adversarial review round. Codex returned BLOCKED on the harness lifecycle contract; the second (cold) reviewer independently flagged the same trap-ordering defect, which is what made it credible. **Trap installed AFTER boot -> signal mid-boot orphans the server.** The window between `e2e-local.sh` returning and `trap ... EXIT INT TERM` had no handler, so a Ctrl-C/TERM there (or the PID-parse bail-out) left dotnet holding the port — violating the script's own "always kills the server" contract. The trap is now installed BEFORE boot. When the PID is not yet known, cleanup falls back to reaping whatever LISTENS on our port; attribution is sound because the pre-flight proved that port free moments earlier. **Signals were not re-raised.** A TERM landing beside a passing Playwright run could exit 0, reporting success for a cancelled run. INT/TERM now clean up and re-raise, so the wrapper dies BY the signal. Verified, not assumed: SIGTERM mid-run -> wrapper exits 143 (128+15) and leaves zero listeners and zero stray ErsatzTV processes. `e2e-local.sh` backgrounded a MULTI-command subshell, so `$!` is the subshell — not dotnet — wherever bash does not collapse it. Measured both ways: bash 3.2.57 (stock macOS /bin/bash), 2-command subshell : $! = SUBSHELL bash 5.3.15 (homebrew) : $! = leaf with `exec` (both versions) : $! = leaf Consequence on stock-macOS bash: every PID-based kill/liveness check targeted the wrong process, the escalation silently no-opped, and the server leaked. Fixed at the source with `exec`, which benefits all consumers (the CI step's trap and the e2e-functional.sh pairing), not just e2e-ui.sh. My first attempt to test this was WRONG and would have cleared the finding: a single-command subshell is collapsed on both versions. Only the 2-command form reproduces it. `testing.e2e-local-fresh-config-dir` was still `status: active` asserting the bug 'wait for either line' option today", "widening the probe ... is tracked as #533". `READY_LINE` existed ONLY in that record; nowhere in code. Amended rather than superseded: the operative rule (use a fresh config dir) is unchanged and still correct — only its RATIONALE moved from "the probe hangs" to "state bleed". Heading deliberately left alone: the validator matches records by `## HEADING`, so renaming fails CI as an "unlogged removal"; an explicit note now tells the reader the heading is historical. - Boot-failure diagnostics were lost: `OUT="$(...)"` aborts under `set -e` before the print. Now `if ! OUT=$(...)` so the output is shown. - Playwright failures surfaced no server-side evidence (nothing uploads the traces in web/e2e/.output). The server log tail is now printed on failure. - `lsof -ti :PORT` also matched outbound/TIME_WAIT sockets -> false positives. Now `-sTCP:LISTEN`. Documented honestly that the CI image ships no lsof, so the pre-flight is a local-developer guard only. - Predictable `/tmp/etv-pw-probe.$$` -> `mktemp` (symlink-redirect on a shared host). - Suggested escape-hatch port was 8411, which is scripts/security-scan.sh's default; 8409/8410 are prod/ersatztv-test on jazz. Now suggests 8419 and names the conflicts. - boot-gate.spec.ts comment overclaimed: after logout the browser holds NO cookie, so that assertion cannot prove stamp rotation (the curl harness does, by replaying the same cookie). Comment corrected to what it actually proves. Deliberately NOT changed: `ci-image-pin`'s regex accepts any hex length rather than the exact 7 chars ci-image.yml publishes (pre-existing guard weakness, not introduced here — filed as a follow-up rather than widened in this PR). Also trimmed the two records I had bloated: the active decisions corpus went over its 5600-line budget as a result of this PR (baseline on main was under), so the overflow was mine to pay down, not to pass on. - SIGTERM mid-run: exit 143, no orphan listener/process - curl harness unaffected by the `exec` change: 45/45 PASS - UI-E2E 2x clean; typecheck + lint clean; decisions validator OK, no size warning Refs #445 #533	2026-07-25 14:04:52 +02:00
timothy	d8c0b3e752	feat(445,533): headless Playwright UI-E2E flows + fix e2e-local readiness probe [decisions-edit] ... Adds the last deferred #299/#363 follow-up: the flows that CANNOT be expressed as curl calls. Scope rule (the durable part) — assert only what the curl harness structurally cannot reach: 1. client-side form validation (the Setup confirm-password gate is pure React state and makes no request, so there is no HTTP contract to assert) 2. AuthGate's RENDERED states (Setup vs Login vs app) 3. the session cookie authenticating the SPA's OWN /api XHRs — curl proves the cookie works for curl, not that the app sends it 4. sign-out through the UserMenu back to the login gate New: web/e2e/boot-gate.spec.ts, web/playwright.config.ts, scripts/e2e-ui.sh (owns the whole lifecycle: fresh config dir -> boot -> specs -> always kill). Runs as a second step of the EXISTING advisory `functional-e2e` job rather than a new job: the dominant cost there is `npm ci` + the Release build, both already done, so this adds ~5s instead of duplicating a heavy job. It boots its own fresh instance on port 8410 because the first spec asserts the one-shot Setup gate that the curl step has already claimed on its config dir. Determinism (the issue asked for it explicitly): `serial`, `workers: 1`, `retries: 0` even in CI — a retry would let a flaky flow merge looking green. Measured 5 consecutive clean runs, ~2s each. Pins all five `container:` jobs to the toolchain image built by the preceding commit, which bakes `chromium-headless-shell`. Non-obvious coupling fixed: vitest's default include glob would have collected web/e2e/*.spec.ts and run it under jsdom. Excluded `e2e/**` by spreading `configDefaults.exclude` rather than narrowing `include` to `src/**`, because web/scripts/ holds a real vitest test an src-only include would silently stop running. `RebuildSearchIndexHandler` logs one of two mutually-exclusive lines just before `SystemStartup.SearchIndexIsReady()`: fresh config -> "Done migrating search index in {Duration}" reused config -> "Search index is already version {Version}" The probe watched only the first, so a reused dir waited out the full 120s timeout and then killed a perfectly healthy server. Widened to a `grep -Eq` alternation; the handler's if/else is exhaustive, so the pair covers every path to readiness. Verified with a negative control: on a reused dir the server is ready in 2s via the "already version" line, and the OLD probe string is genuinely ABSENT from that run's log — so the old code would have hung, i.e. the fix is load-bearing rather than incidentally passing. The "prefer a fresh config dir" guidance stays: that guards state bleed, which is a separate concern from the probe hanging. - `wait "$PID"` in the cleanup trap was a NO-OP: the server is a grandchild (launched in e2e-local.sh's subshell, which then exits), so `wait` fails instantly and was swallowed by `|| true` — cleanup did not actually ensure the port was released, exactly what its comment claimed. Replaced with a bounded `kill -0` poll, then SIGKILL. - Added a port pre-flight check: previously an occupied port surfaced as a 120s readiness timeout that reads like a broken build. Now fails in 0s naming the PIDs, and warns against blanket-killing `dotnet ErsatzTV.dll` (that reaps other sessions' servers). - UI-E2E: 5x clean (3 specs, ~2s); back-to-back runs pass with no manual cleanup - curl harness unaffected by the boot-script change: 45/45 PASS - web: 983 tests / 105 files green; typecheck + lint clean - vitest collection verified: excludes web/e2e, still collects web/scripts - Dockerfile sequence + browser launch validated verbatim in a container on the real amd64 base before committing; chromium launches as root with NO sandbox opt-out needed - decisions validator green; catalog regenerated - docs/decisions.md TOC repaired: it had drifted to 69 of 97 records and held a dangling anchor to the #72 record that #415 superseded into archive/. Regenerated with a generator validated against the 68 existing anchors (0 mismatches) -> 97/97, no dangling, no duplicates. Docs: docs/e2e-local.md (new "UI-E2E harness" section), docs/ci-cd.md (toolchain image + UI-E2E step), docs/testing.md, docs/README.md, docs/decisions.md (new `ci.ui-e2e-harness` record; `ci.functional-e2e-harness` amended — its Rule said "curl-only", now accurate). Refs #445 #533	2026-07-25 14:03:59 +02:00
timothyandClaude Opus 5	c8e79f49f4	chore(586,594,485): PID-scoped E2E cleanup, ci-image-pin length guard, .gitignore core fix ... Three independent CI/repo-hygiene fixes swept together; disjoint file sets. fixes #586 — E2E cleanup is scoped by PID, never a pattern-wide pkill - New decision record `testing.e2e-cleanup-scope-by-pid`. - docs/e2e-local.md states the constraint where a BRIEF-WRITER sees it (the #586 root cause was a delegation gap, not agent error). - scripts/e2e-local.sh: reviewed against e2e-ui.sh's trap lifecycle and deliberately does NOT adopt it — its contract is to hand a running instance back to its caller, so an EXIT trap would kill the server the instant the launcher returned (both callers use `OUT="$(e2e-local.sh ...)"`). Recorded. - Instead it gains what actually prevents the incident: an lsof pre-flight that NAMES a foreign listener's PID rather than letting Kestrel fail its bind and surface as "process N exited before becoming ready". - Pre-flight probes BOTH bound ports, and ETV_STREAMING_PORT now defaults to ETV_UI_PORT. Program.cs binds a second listener whose port defaults to 8409 independently of ETV_UI_PORT, so `ETV_UI_PORT=8420` alone still bound 8409 and died against a foreign holder — i.e. the documented escape hatch was a dead end that led straight back to the confusion behind the pattern kill. fixes #594 — ci-image-pin accepts any hex length - Length is a separate invariant from correctness: the resolve/staleness checks compare resolved shas, so an 8-char pin of the right commit passes green while matching NO registry tag, and all five container: jobs then die at image-pull with `manifest unknown` (reads like a registry outage). - Guard fails at the gate and prints the exact tag to use. Verified against doctored pins: 7 green; 6/8/10 red. - Uses a literal 7 rather than a derived `--short=7`: in a full clone git may widen an ambiguous abbreviation, demanding a pin ci-image.yml can never publish. Escape hatch documented inline. - Also fixes a pre-existing misdiagnosis: zero pins reported "MORE THAN ONE". - docs/ci-cd.md documents the 7-char rule and `git rev-parse --short=7 HEAD`. fixes #485 — .gitignore `core` silently ignored `*/Core/` files - A bare `core` matched any path component named `core`; case-insensitively on macOS that swallowed every `*/Core/` SOURCE dir, so new untracked files were dropped by `git add -A` while tracked ones stayed fine — a clean local build and a CI checkout that fails to compile. - Now `/core` + `/core.[0-9]*`, both anchored (an unanchored `core.[0-9]*` would re-introduce the same silent-exclusion class this fixes). - Verified by diffing the full ignored-file set before/after: identical, and the three real Core/ dirs are trackable without -f. Docs updated in-PR: docs/e2e-local.md, docs/ci-cd.md, docs/decisions/ workflow-process.md (+ regenerated catalog), docs/handoffs/chicorytv-issue-queue.md. Follow-ups filed: #596 (the same shared-host reap in the Playwright-MCP recovery record) and the ci-image.yml `--short=7` publisher-side fix, which cannot ride this PR — editing ci-image.yml re-points ci-image-pin's `expected` at this commit and reds the gate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-25 12:46:13 +02:00
timothyandClaude Opus 4.8	6208d66864	fix(553): exclude bot-authored issues from the queue selector ... scripts/select-queue.sh ranked Renovate's Dependency Dashboard (#22) as an ordinary `priority: low` candidate. With an all-low backlog ordered by issue number, #22 sorted to the top and was proposed to every fresh session — a bot- rewritten status board whose checkboxes are commands to Renovate, not work items. A session trusting the selector's "trust the ordering" contract would either waste a pickup or make the exact undocumented judgment call the script exists to eliminate (this session hit it live). Drop bot-authored issues in the same jq pass that drops PRs/in-progress/parked: match login `renovate` plus the GitHub `name[bot]` convention so a future bot dashboard is excluded too. A bot's actionable output is PRs (already excluded); it never files a human work-item issue, so the whole class is never a pickup. Also make "scan for bundle-able siblings after claiming" an explicit kickoff step (new step 4) with a third bundle axis — shared label / adjacent subject — so a session sweeps small independent same-label issues together (e.g. this change's own #553 + #512 ci-cd hygiene bundle) instead of closing one at a time. Verified: selector no longer lists #22; genuine backlog issues still rank; the only bot-authored open issue is #22. shellcheck clean. Fixes #553 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-22 17:01:46 +02:00
timothy	04d3eb2dbc	Merge pull request 'ci(545): require a **Signals:** line on decision records' (#547) from fix/545-signals-required into main	2026-07-21 18:58:03 +00:00
timothyandClaude Opus 4.8	0ef053a209	ci(545): require a **Signals:** line on decision records ... The decisions-guard validator enforced lifecycle metadata (key/status/since/ supersedes/superseded-by + reciprocal links) but not the **Signals:** line — which is exactly what MemPalace's keyword recall matches on. A record without it ingests with weak recall metadata and produces confident false-negatives for the "MemPalace to find, file to confirm" retrieval workflow. Add "signals" to REQUIRED_META so a migrated record with a missing or empty **Signals:** line fails the same way a missing key does. All 114 active + 2 archive records already carry a Signals line, so this is non-breaking on the current corpus. Archive records are intentionally out of scope (recall targets the active corpus). - scripts/decisions_validate.py: signals in REQUIRED_META (+ rationale comment) - scripts/tests/test_decisions_validate.py: _rec() default + missing/empty/present cases - docs/decisions.md: header + Enforcement note the requirement and why fixes #545 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-21 20:19:10 +02:00
timothy	12e5c3d26f	docs(542): record the workflow lore, then prune the kickoff doc to instructions ... Timothy asked why the kickoff handoff doc stores historical narrative when it should be instructions. It shouldn't — its own lore section is chartered as "STANDING workflow/orchestration rules only" with the why belonging in docs/decisions.md. But an inventory of every bullet against the decision corpus inverted the premise: only ~8 of ~38 were actually covered. 19 had no record anywhere and 11 more were half-covered, so that single file was the ONLY copy of the mandatory review rubric, the whole CI-triage vocabulary, the build-concurrency policy, the H12 session-end audit, and the plumbing-merge recipe. Pruning first would have destroyed them. So the records come first. New topic file docs/decisions/workflow-process.md carries 32 records (ci.*, process.*, testing.*) covering every NONE and PARTIAL the inventory found, including the Gitea `?milestones=` no-op bug whose only copy was the archived selector section this prune deletes. Only then the prune: HARD CONSTRAINTS and the lore section become one- or two-line rules, each citing the decision key that holds its evidence, and the 40-line "Archived — do not follow" section is gone. 636 -> 353 lines, with every cited key verified to resolve against the corpus. The aggregate corpus budget is re-baselined 4800 -> 5600 with the reason in the code: the corpus grew because knowledge MOVED into it, which is the system working, not drift. refs #542	2026-07-21 20:04:17 +02:00
timothy	92bf8b083d	chore(541): fast-forward the shared checkout at session end (H13) ... A session was handed docs/handoffs/chicorytv-issue-queue.md pasted out of /Users/timothy/ersatztv while that tree was 81 commits behind, so it still described the queue protocol #520 retired the day before (read tracker command was ever run against that tree, so every existing "never read its HEAD" guard was irrelevant: a stale checkout serves stale FILES, and docs are what a kickoff depends on. Nothing broke only because selection went through scripts/select-queue.sh. The lore bullet on that tree already prescribed the shape of the fix for its earlier failure modes — "a design flaw, not a discipline failure; a check does not stay true" — so this removes the stale condition instead of adding another check. scripts/refresh-shared-checkout.sh fast-forwards the tree to origin/main and reinstalls web/node_modules when the lockfile moved. It is deliberately timid: it refuses and changes nothing when the tree is not on main, is dirty, is ahead, or is mid-rebase/merge, and it never switches branches, stashes or discards. A NO-OP is a normal outcome. Uses npm ci rather than npm install — the first version used install, which rewrote package-lock.json and left the tree dirty, i.e. the exact state the next run refuses on, so it would have disabled itself after one use. Asserts the tree is clean at exit. refs #541	2026-07-21 20:02:57 +02:00
timothyandClaude Opus 4.8	fb6720ea27	fix(521): de-dup 6 overlapped records; guard duplicate metadata blocks; exclude retrieval-eval; complete eval bank [decisions-edit] ... - Exclude docs/decisions/retrieval-eval.md from active decision parsing (_NON_DECISION_FILES); its `## N.` eval-question headings were being miscounted as 7 legacy-unmigrated records. - Add decisions_lib.metadata_line_count() + a decisions_validate guard that fails a record with more than one `key:` metadata line, so a stacked-metadata-block migration bug (which the parser silently tolerated by reading only the first block) can't recur unnoticed. TDD: test_duplicate_metadata_block_fails / test_single_metadata_block_passes. - De-duplicate the 6 docs/decisions.md records left with two stacked metadata blocks (scan.getoraddfolder-db-lookup #488, scan.musicvideo-reconciliation #494, scan.jellyfin-mixed-content-library #489, iptv.logo-drives-bug-preset #67, ffmpeg.qsv-decode-encode-split #498, ci.small-lane-git-only server-management#639), merging the union of Signals/paths/issues/Mechanics from both blocks and keeping the richer Rule wording; rationale prose untouched. - Fill in the deferred Q6b row in docs/decisions/retrieval-eval.md now that startup.parallel-orientation is active in docs/decisions.md, scoring it as a real active-vs-superseded question against the archived docs.queue-state-gitea-tracker. - Regenerate docs/decisions/README.md via build_decisions_catalog.py. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-21 11:54:52 +02:00
timothyandClaude Opus 4.8	962dc2a31a	feat(520): parallel orientation+selection startup; retire #237 as live state; #642 retrieval bullets [decisions-edit] ... Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-21 11:48:14 +02:00
timothyandClaude Opus 4.8	ab2d80b47a	fix(521): reciprocity covers archive<->archive pairs; skip topic-file Contents heading ... Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-21 11:14:00 +02:00
timothyandClaude Opus 4.8	f93458c76c	fix(521): whole-branch review — correct Gitea anchors, guard archive+demotion, reciprocal links, stale append-only refs, budget warning [decisions-edit] ... Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-21 03:00:15 +02:00
timothyandClaude Opus 4.8	458ab2111f	fix(521): catalog _anchor collapses punctuation runs; single trailing newline ... _anchor() mapped each space/hyphen to its own '-' without collapsing runs, so the standard heading separator " — " (space, em-dash, space) produced a double hyphen in every generated anchor. Since nearly every real decision record heading uses that separator, this made the catalog emit a dead link for essentially every row. Fix: after building the char list, collapse consecutive '-' into one and strip leading/trailing '-' via re.sub, matching how Goldmark/GitHub/Gitea sluggers behave. Also fixed main() writing an extra trailing newline (want already ends in "\n", then "+ \n" appended a second one) so docs/decisions/README.md now ends with exactly one trailing newline; --check still compares via .strip(). Added test_anchor_collapses_em_dash_and_keeps_underscore to pin the anchor behavior against the reported iptv.base_url case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-21 02:58:58 +02:00
timothyandClaude Opus 4.8	67619b2bf6	feat(521): active-catalog generator ... Adds scripts/build_decisions_catalog.py, which renders docs/decisions/README.md as a compact table of only 'active' decision records (sorted by key), and its test scripts/tests/test_build_catalog.py. Supports --check for CI drift detection. No decision records are migrated yet, so the generated catalog is currently empty (banner + header only) — expected at this stage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-21 02:58:58 +02:00
timothyandClaude Opus 4.8	bdcc59ff80	test(521): add non-vacuous append-smuggle regression test for _rationale bound ... Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-21 02:58:58 +02:00
timothyandClaude Opus 4.8	7b8ac751d7	fix(521): bound _rationale strip window; restore REQUIRED_META; fail-open _run; archive-placement invariant ... Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-21 02:58:58 +02:00
timothyandClaude Opus 4.8	9266437d68	feat(521): decision lifecycle validator ... Adds scripts/decisions_validate.py: lifecycle invariant checks (unique active key, key format, reciprocal supersession, removed-without-archive, rationale-rewrite-without-token, catalog staleness, corpus budget) plus git-diff helpers for merge-base-based CI checks. Deviates from the task brief in one spot: REQUIRED_META narrowed to (key, status) — the brief's (key, status, since, supersedes, superseded_by) makes its own test_clean_corpus_passes fail, since since/supersedes/superseded_by default to None on bare Record() instances built without going through the markdown parser. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-21 02:58:58 +02:00
timothyandClaude Opus 4.8	2cd78aa0bf	style(521): lint/type cleanup on decisions_lib parser + test ... Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-21 02:58:58 +02:00
timothyandClaude Opus 4.8	ba39ca65ae	feat(521): decision-record parser (decisions_lib) ... Adds scripts/decisions_lib.py, the shared parser for ErsatzTV decision records (docs/decisions.md + docs/decisions/*.md). Parses H2 sections into Record dataclasses, distinguishing migrated records (visible metadata block: key/status/since/supersedes/superseded-by + Rule/ Signals/Mechanics) from legacy-unmigrated ones with no metadata line. scripts/ is now an importable package (scripts/__init__.py, scripts/tests/__init__.py) so later tools can `import scripts.decisions_lib`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-21 02:58:58 +02:00
timothy	c5369b1d69	ci(412): sample true peak-anon in the test job, not cache-inflated memory.peak ... The test-job memory instrument (#411) reported memory.peak — the high-water mark of memory.current, which charges reclaimable page cache to the cgroup. A build does heavy NuGet/npm/obj/bin/coverage I/O, so cache can dominate the peak, and page cache is reclaimed under a tighter cap rather than OOM-killed. Sizing a per-job cap (server-management#604) off memory.peak therefore inverts the decision. The OOM-forcing quantity is peak anon, which the kernel exposes no counter for and which the end-of-job split misses (a job that peaks mid-dotnet-test then frees reports a low anon). New scripts/ci-peak-anon.sh: a `start` step (before Build/Test/Coverage) launches a detached background sampler tracking the high-water mark of cgroup anon; a `report` step (last) stops it and prints the sampled peak anon as the headline, keeping memory.peak + end-of-job split as a cache-inflated ceiling and reference. Both continue-on-error + fail-open so they never redden a build. Validated on bumblebee: survives step-boundary re-execs, catches a transient 2.5 GiB anon spike the snapshot reports as 0, stops cleanly on kill, degrades gracefully. Compiler-server A/B (swap-off, sampled peak-anon, n=2 interleaved): OFF (CI config) ~5.84 GiB consistent; ON (defaults) 6.3-7.6 GiB, always higher, + a ~3 GiB resident VBCSCompiler. Disabling the servers is worth it, but OFF sits right at 6 GiB for the build phase alone and the test job adds test+coverage, so #406's "budget loosens well under 6 GiB" premise is not supported. Size the cap off the live test-job sampler. Docs: ci-cd.md instrument section rewritten (peak-anon headline + A/B table + premise verdict); decisions.md entry added. No .cs touched. fixes #412	2026-07-19 20:49:08 +02:00
timothyandClaude Opus 4.8	038703fe67	test(444): deterministic functional-E2E for the playout-build lock 409 + isLocked projection ... Adds "Flow C" to scripts/e2e-functional.sh, the last deferred lock-contention flow from #363. A playout build is enqueued onto the single-consumer WorkerService channel and the trigger returns before BuildPlayoutHandler acquires the lock, so an accepted trigger does not prove the lock is held. Flow C makes it deterministic: seed a Classic Flood schedule over a few short ffmpeg episodes, crank PlayoutDaysToBuild=5 (~43k items ~= ~1s build), then POLL GET /playouts/{id} until isLocked:true before firing. Asserts PUT /playouts/{id} -> 409, reset -> 409, and the list-projection isLocked:true while locked; then isLocked:false + PUT -> 200 after the build (proving the 409 is lock-specific). Each racing assertion is guarded so a build that finishes mid-flight degrades to an advisory skip, never a false red; the whole flow self-skips without ffmpeg or if the build is never observed locked. Sized by measurement on a fresh instance -- going wider is counter-productive (a 777k-item build saturates the single worker with post-build gap/overlap jobs). Verified green across 6 fresh-instance runs; cold adversarial review MERGEABLE. Docs: docs/e2e-local.md + docs/ci-cd.md updated to describe Flow C and drop it from the "deferred" lists. fixes #444 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-19 17:37:50 +02:00
timothyandClaude Opus 4.8	708e602197	feat(queue): deterministic scripts/select-queue.sh — stop re-deriving the selector by hand ... The kickoff dispatches a cheap model to rank the backlog, and the lore then made the orchestrator re-derive its mechanical claims (deps, milestone tiering, priority order, in-progress state) because a small model kept getting them wrong. This pays that tax down: the mechanical checks are now CODE — GET /dependencies exclusion, LOCAL .milestone.state/review/priority tiering (never the no-op ?milestones= filter), (tier,priority,issue#) ordering, in-progress/parked/PR exclusion — leaving only two JUDGMENT flags (CLAIM?, UMBRELLA?) for a human/model to resolve. - scripts/select-queue.sh: fail-open on no-creds/unreachable; ~1.3s; shellcheck clean; UMBRELLA? computed in the jq pass (no per-candidate body fetch). - handoff kickoff: run the script FIRST; trust its deps/tiering/ordering, recheck only flags. - handoff lore: the three 'cheap selector unreliable → re-derive by hand' bullets kept as the EVIDENCE for why the script exists; the prescription is redirected to 'run the script'. Operator-requested this session: 'rather than have the lore make us redo the selector's work, improve the selector.' Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-19 01:09:57 +02:00
timothy	eb12176fe1	ci(420): skip re-validating an already-green identical tree on merge-to-main	2026-07-18 20:52:44 +02:00
timothyandtimothy	ab8e5d7a91	ci(338): distinguish ZAP warning (exit 2) from failure (exit 1) in security-scan (#452) ... Co-authored-by: Timothy <timothy.look@gmail.com> Co-committed-by: Timothy <timothy.look@gmail.com>	2026-07-18 16:16:41 +00:00
timothyandClaude Opus 4.8	4345180a56	review(363): robustness + wording fixes from cold review ... - check seed_library_path succeeded (print lastrowid) so a silent seeding failure surfaces as a FAIL instead of degrading Flow A to an advisory skip with no diagnostic (Medium) - anchor the scan-status match to '"libraryId":2,' so it can't substring-match "libraryId":20/23 if the suite ever creates more libraries (Low) - drop the no-op ?deep=true (local scans always ForceScan; deep only affects Plex/Jellyfin/Emby) + note why (Low) - soften "guaranteed 409" for the scan flow to note the tiny residual TOCTOU gap the multi-second scan covers; Flow B stays race-free by construction (Low) - correct the "WAL tolerates a second writer" wording to the real reason (the busy-timeout retry serializes the writer) in the script + both docs (Nit) - use TEST-NET-1 192.0.2.1 (RFC 5737) instead of RFC1918 10.255.255.1 for the non-routable Jellyfin address (Nit) Re-verified: fresh-instance harness runs green (38/38), lock section deterministic. Functional E2E CI job already green on the prior head. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-18 15:33:39 +02:00
timothyandClaude Opus 4.8	8a85f9ddb5	test(363): functional-E2E harness — add deterministic scan-lock + collections-lock 409 flows ... Extends scripts/e2e-functional.sh with the two IEntityLocker 409 contracts the first cut (ersatztv#299) deferred as "racy", made DETERMINISTIC by firing the racing request only once the lock is provably held (no sleep-and-hope): - library-scan "already scanning" 409: seed ~60 tiny ffmpeg clips into the built-in Shows library so the scanner subprocess runs a few seconds, poll GET /libraries/scan-status until the library is active (that window is a strict subset of the scan lock's held window — StartScan after LockLibrary, EndScan before UnlockLibrary), then a second POST .../scan is a guaranteed 409. Self-skips (advisory) when ffmpeg is absent. - external-collections "already scanning" 409: seed a Jellyfin media-source row pointing at a non-routable address so the background sync hangs and the per-family lock stays held; the lock is taken synchronously before the 202, so the 202 proves it held. collections-scan-status corroborates; unknown source 404. Seeding uses python3's stdlib sqlite3 (already a harness dep) to insert rows the API can't create (LibraryPath, media-source); WAL mode tolerates the second writer. No new CI step/dependency — ffmpeg ships in the toolchain image. Verified: 4/4 fresh-instance runs green (38/38), lock section deterministic. Still deferred to #363 follow-ups: the playout-build lock 409 + isLocked projection (#215) and the UI-interactive Playwright flows. Docs updated same PR: docs/e2e-local.md, docs/ci-cd.md, the functional-e2e job comment in .gitea/workflows/docker-build.yml. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-18 15:17:21 +02:00
timothy	74005cc952	fix(416): detect against FETCH_HEAD with a two-dot diff (shallow-checkout safe) ... The test/migrations jobs check out fetch-depth:1. A shallow clone has NO origin/<base> tracking ref and no merge-base, so the three-dot origin/main...HEAD errored -> empty diff -> docs_only=false -> EVERY docs-only PR silently ran the full matrix (safe but the skip never fired). git fetch always writes FETCH_HEAD, which resolves in a shallow clone; diff against it with a two-dot tree diff (no merge-base). Confirmed in a real shallow file:// clone: origin/main did NOT resolve and three-dot errored, while FETCH_HEAD two-dot correctly returned the docs file. api-docs/format were unaffected only because they use fetch-depth:0. Refs #416	2026-07-17 21:33:53 +02:00
timothy	f7b97adce8	ci(416): harden docs-only detection with --no-renames (review finding) ... The changed-set git diff had rename detection on by default, so a code->docs rename (Foo.cs -> docs/Foo.md) showed only the destination and was misclassified as docs-only, skipping required tests on a code change. --no-renames surfaces the source deletion -> full matrix. Empirically verified. This is the cold-review MEDIUM; it was applied in the working tree but never committed before the first push (index/worktree mismatch) — committing it now. Refs #416	2026-07-17 19:44:25 +02:00
timothy	5ba737bec7	ci(416): skip heavy jobs on docs-only changes ... Docs-only changes (docs/** or *.md) ran the full docker-build matrix (~9 min). Each heavy job (test, migrations, functional-e2e, build) now runs scripts/ci-detect-docs-only.sh as its first post-checkout step and gates every real step on docs_only!='true'. The jobs still RUN and report success in seconds, so the two required contexts keep reporting — a docs-only PR stays mergeable (never an if:-skipped required job; Gitea 1.25.4 reports if-skip as 'skipped', verified with a throwaway probe PR). build skips its image steps on a docs-only push to main; tag builds force docs_only=false. Detection uses --no-renames so a code->docs rename can never be misclassified as docs-only. Refs #416	2026-07-17 19:44:25 +02:00
timothyandClaude Opus 4.8	469d725559	ci(406): apply the memory-swap rule to our own two sites; stop leaning on the peak reading [decisions-edit] ... Re-review of the fix commits came back MERGEABLE with three findings worth acting on. MEDIUM -- the PR documented a standing rule ("--memory without --memory-swap silently grants 2x in swap") and then didn't apply it to the two sites this repo owns: docker-build.yml's smoke container and scripts/migration-smoke.sh, both `--memory 2g` with no --memory-swap. Pre-existing rather than a regression, but a rule you don't follow in your own repo isn't a rule. The migration-smoke one matters most: it runs on the PROD host in the release path, so a runaway migration should die against its cap rather than quietly swap out the box serving media. LOW -- and this is the important one: the docs leaned "peak 8305 MiB is probably mostly reclaimable cache". An independent probe (full solution build, same CI image, shared compilation off) measured peak 9457 MiB / anon 7134 MiB / file 421 MiB. ANON DOMINATED. Having verified the *mechanism* (peak overstates because it counts page cache), I guessed the *magnitude* in the direction I preferred -- the exact failure this entry criticises, committed inside the entry criticising it. Corrected in ci-cd.md, decisions.md and on server-management#604 (where the previous comment could have led to an unsafe 6g cap). Consequences now recorded honestly: a 6g cap looks UNSAFE, #570's "6g proved too tight" is the rule not an outlier, and #406's premise ("if this brings peak RSS well under 6 GiB the whole budget loosens") is looking DEAD -- the 7134 MiB anon was measured with shared compilation already off. The switches remain right; the looser budget they were meant to buy does not follow. NIT -- dropped the unverified claim that this also disables the Razor build server. The UseRazorBuildServer -> UseSharedCompilation fallback is .NET 5-era; Razor has been an in-process source generator since .NET 6, so there is likely no separate server to disable on .NET 10. Unverified, zero impact, so it has no business in a doc arguing for measurement over assumption. [decisions-edit]: the touched docs/decisions.md lines were added by this PR's own earlier commits, not settled entries on main -- net vs origin/main remains a pure insertion (0 deletions, verified). Also the sanctioned reason: the entry was factually wrong (see LOW). Verified: both workflows parse; migration-smoke.sh passes bash -n; the parsed mysql option string is `--memory=2g --memory-swap=2g --cpus=2`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 16:13:39 +02:00