Begin Stipple/Vue migration with isolated typed studies UI - #6
Closed
hyperpolymath wants to merge 189 commits into
Closed
hyperpolymath wants to merge 189 commits into
hyperpolymath wants to merge 189 commits into
Conversation
… a better place one less R module at a time.
…nd working cutadapt -> vsearch-dada merge on default settings.
…for DADA2 and VSEARCH
… of previously run stages. Created a versatile project structure.
….it was getting messy and altogether not worth it for now.
#27) ## The bug The strip set on `main` leaves **71.58 MiB** behind while reporting success. Measured in a scratch clone: `269.43 MiB -> 81.13 MiB`, with every check green — including `VESPA/Multiplex blobs remaining (expect 0): 0`, printed **truthfully** — and 71.58 MiB of that pool's content still present under `inputs/fastq/`. ## Why, structurally `git rev-list --objects` emits each object **exactly once**, paired with **one** of the paths it is reachable under. Those 2 blobs lived at both `data/Multiplex_pool/…` and `inputs/fastq/…`. The census that chose the strip set saw only the first name, so `inputs/` **never appeared in it at all**. Deleting the path did not delete the content, and the post-strip grep for the deleted path returned 0 because it could not have returned anything else — that check is a tautology, not evidence. ## The fix | | before | after | |---|---|---| | pack | 269.43 MiB | **9.17 MiB** | | heaviest remaining | `inputs/fastq` 71.58 MiB | `data/MiSeq_SOP` 7.71 MiB — the **live fixtures**, the correct floor | - adds `inputs` and `logs_*.zip` (a committed CI log artefact no path class covered) - prints the heaviest **remaining** path aggregates after the rewrite, because a path strip can never prove a blob is gone — the blob may have a second name. The predicted figure was ~21 MiB and the run produced 81 MiB; that gap was the entire signal, and nothing in the named checks would have raised it. The HEAD-tree-identity assertion is kept and still passes — it answers a different question (did the strip set catch a *live* file) and was never able to detect dead content that was missed. Still never runs on its own: refuses without `--i-have-read-the-warnings`, and the force-push commands remain printed instructions inside a heredoc. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
…assable Every bench script is run by the `test` job, which carries no `continue-on-error` at job or step level, so any one of them failing is fatal to `Julia 1.12.5 / ubuntu-24.04` -- the only honest gate on this repo and the one blocking upstream PR #6. All of these calls were unreachable until the bench steps were wired into CI, so each failed the first time it ever ran. They failed SERIALLY: the first script aborted the step, masking the next, so CI could only reveal one per ~21-minute cycle. All seven were instead run locally under julia 1.12.6 until clean, twice each -- once creating a baseline and once comparing against it, which is the path CI takes. Six of the seven fixes are in one file. Corrected against the definitions in `src/analysis/analysis.jl` and against the unit tests, which call every one of these functions correctly: table_loading filtered_df 7 args -> 4 (:130) duckdb_aggregation aggregate_by_taxon 4 args -> 6 (:141) duckdb_aggregation venn_taxa_present invented a `[g1, g2]` group-list parameter no method has ever taken; a Venn is assembled by calling it once per group (:161) duckdb_aggregation bar_chart passed (labels, counts, names); counts belongs third, and the matrix is (segments x samples) per :403 and test_analysis.jl:35 duckdb_aggregation taxa_bar_chart arguments 2 and 3 transposed (:475) duckdb_aggregation alpha_chart passed a 5th `groups` argument that no method accepts (:312) permanova_nmds alpha_boxplot `Vector{Any}` matched neither concrete method, and `metric=` is not a keyword of either. This file's own header says it measures "alpha_boxplot with significance", so `annotate_significance=true` is what was meant (:734, :840) `bench/analysis_config/benchmark.jl` failed for a different reason: it is built on BenchmarkTools (`@benchmarkable`, `BenchmarkGroup`, save/load/median) but BenchmarkTools was in neither Project.toml nor Manifest.toml, and CI never installs it -- so that step has never once succeeded since it was wired in. Pinned rather than fetched at CI time, because an unpinned network fetch would defeat the committed Manifest. The resolve is purely additive: BenchmarkTools 1.8.0 plus the Profile stdlib, with no existing package version changed. Its baseline.json is deliberately NOT committed. The other five baselines are tracked, but those numbers came from whichever host produced them; adding a sixth from a laptop would make this machine's timings the reference. CI regenerates it per run and exits 0, which matches the workflow's own statement that the deltas are informational. `bench/results/` is a run artifact and is now ignored alongside the existing `frontend/bench/results/` rule. The resolve also rewrote `julia_version` in the Manifest from 1.12.5 to 1.12.6 -- the julia this laptop actually runs, despite mise.toml pinning 1.12.5. That line is asserted by test/unit/test_install_pins.jl:112 against config/defaults/tool_versions.yml, so leaving it would have turned a second gate red. It is restored to 1.12.5 by hand; Pkg only warns on that field disagreeing with the running julia, it does not gate. Verified by mutation: with 1.12.6 the suite reports 94 pass / 1 fail at :112 ("1.12.5" == "1.12.6"), with 1.12.5 it reports 95/95. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
`cancel-in-progress: true` applied to every event. The Julia job runs ~31 min, and `main` was merged to at 14:25, 14:28 and 14:43 on 2026-09-21. Each run was cancelled 17-18 min in by the next push, so three consecutive commits to `main` produced no test verdict of any kind -- the suite was never once allowed to finish. The red that everyone was reading as "CI is broken" was, for those commits, no result at all. Cancelling remains correct for a pull_request: a verdict on a superseded head is worthless. So the flag is now conditioned on the event rather than removed. This does not rescue an upstream PR whose head keeps moving -- JoshuaJewell#6's head IS this fork's `main`, so its runs are cancelled by the head update itself, not by this setting. Only letting `main` settle for ~31 min does that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
…ssable (#28) ## Why The Julia job (`Julia 1.12.5 / ubuntu-24.04`) is the only gate on this repo carrying no `continue-on-error` at job or step level — verified by `awk 'NR>=78 && NR<=445 && /continue-on-error/'` returning nothing. It is also the gate blocking upstream `JoshuaJewell#6`. The original blocker (`test/integration/test_server.jl:44`) is **fixed and verified**: run `35614055917` shows `Server smoke tests` by name and `1727 passed` with no Fail column. What that fix uncovered was a second wall of failures sitting behind it, in the bench scripts. ## The trap: serial unmasking The `test` job runs seven bench scripts sequentially. The first one to fail aborts the step and **hides every later one**. `bench/duckdb_aggregation` alone held six broken calls, each visible only once the previous was fixed — so CI could surface at most one defect per ~21-minute cycle. Running the suite locally exposed all of them in a single sitting. Every one of these calls failed *the first time it ever ran*. The bench scripts were only recently wired into CI, and code that is never executed cannot drift-check itself. The unit tests have been calling the same functions correctly the whole time. ## The eight fixes Corrected against the definitions in `src/analysis/analysis.jl` and against the unit tests, which call every one of these functions correctly. | script | function | defect | |---|---|---| | `table_loading` | `filtered_df` | 7 args → 4 (`:130`) | | `duckdb_aggregation` | `aggregate_by_taxon` | 4 args → 6 (`:141`) | | `duckdb_aggregation` | `venn_taxa_present` | invented a `[g1, g2]` group-list parameter no method has ever taken; a Venn is assembled by calling it once per group (`:161`) | | `duckdb_aggregation` | `bar_chart` | passed `(labels, counts, names)`; counts belongs third, and the matrix is (segments × samples) per `:403` and `test_analysis.jl:35` | | `duckdb_aggregation` | `taxa_bar_chart` | arguments 2 and 3 transposed (`:475`) | | `duckdb_aggregation` | `alpha_chart` | passed a 5th `groups` argument no method accepts (`:312`) | | `permanova_nmds` | `alpha_boxplot` | `Vector{Any}` matched neither concrete method, and `metric=` is not a keyword of either (`:734`, `:840`) | | `analysis_config` | — | built on BenchmarkTools, which was in neither `Project.toml` nor `Manifest.toml` | `bench/analysis_config/benchmark.jl` is structurally built on BenchmarkTools (`@benchmarkable`, `BenchmarkGroup`, save/load/median) and CI never installs it, so **that step has never once succeeded since it was wired in**. Pinned rather than fetched at CI time, because an unpinned network fetch would defeat the committed Manifest. The resolve is purely additive: BenchmarkTools 1.8.0 plus the Profile stdlib, **no existing package version changed** (`git diff -U0 Manifest.toml | grep '^-version'` → 0 lines). ### One trap the resolve set, caught by mutation `Pkg.resolve` also rewrote the Manifest's `julia_version` from 1.12.5 to **1.12.6** — the julia this laptop runs, despite `mise.toml` pinning 1.12.5. That line is asserted by `test/unit/test_install_pins.jl:112` against `config/defaults/tool_versions.yml`, so shipping it would have turned a *second* gate red. Restored by hand (Pkg only warns on that field; it does not gate) and verified by mutation: with 1.12.6 the suite reports 94 pass / 1 fail at `:112` (`"1.12.5" == "1.12.6"`); with 1.12.5, 95/95. ## Concurrency (second commit) `cancel-in-progress: true` applied to every event. `main` was merged to at 14:25, 14:28 and 14:43 on 2026-09-21; each run was cancelled 17–18 min into a ~31 min job by the next push, so **three consecutive commits produced no test verdict of any kind**. The red everyone was reading as "CI is broken" was, for those commits, no result at all. Cancelling stays correct for a `pull_request` — a verdict on a superseded head is worthless — so the flag is conditioned on the event rather than removed. ⚠ This does **not** rescue upstream #6, whose head *is* this fork's `main`: its runs are cancelled by the head update itself, not by this setting. Only letting `main` settle for ~31 min does that. ## Verification - All seven bench scripts run clean locally, **twice each** — once creating a baseline, once comparing against it, which is the path CI takes. - Tracked baselines for the five original categories were **not** modified by any run. - `bench/analysis_config/baseline.json` is deliberately **not** committed: the other five came from whichever host produced them, and adding a sixth from a laptop would make this machine's timings the estate reference. CI regenerates it per run and exits 0, matching the workflow's own statement that the deltas are informational. - Repo hygiene is **enforcing** on this PR (a `pull_request` reads `github.repository` as the BASE repo, which is this fork). All three scripts run clean locally: `check-spdx` OK 265 files, `check-format` OK 311 files, `check-lint` OK. Both commit subjects pass the conventional-commit regex. - `test_install_pins.jl`: 95 passed / 95 total. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
hyperpolymath
added a commit
to hyperpolymath/MetaManifold-WebUI
that referenced
this pull request
Sep 21, 2026
…ssable (#28) ## Why The Julia job (`Julia 1.12.5 / ubuntu-24.04`) is the only gate on this repo carrying no `continue-on-error` at job or step level — verified by `awk 'NR>=78 && NR<=445 && /continue-on-error/'` returning nothing. It is also the gate blocking upstream `JoshuaJewell#6`. The original blocker (`test/integration/test_server.jl:44`) is **fixed and verified**: run `35614055917` shows `Server smoke tests` by name and `1727 passed` with no Fail column. What that fix uncovered was a second wall of failures sitting behind it, in the bench scripts. ## The trap: serial unmasking The `test` job runs seven bench scripts sequentially. The first one to fail aborts the step and **hides every later one**. `bench/duckdb_aggregation` alone held six broken calls, each visible only once the previous was fixed — so CI could surface at most one defect per ~21-minute cycle. Running the suite locally exposed all of them in a single sitting. Every one of these calls failed *the first time it ever ran*. The bench scripts were only recently wired into CI, and code that is never executed cannot drift-check itself. The unit tests have been calling the same functions correctly the whole time. ## The eight fixes Corrected against the definitions in `src/analysis/analysis.jl` and against the unit tests, which call every one of these functions correctly. | script | function | defect | |---|---|---| | `table_loading` | `filtered_df` | 7 args → 4 (`:130`) | | `duckdb_aggregation` | `aggregate_by_taxon` | 4 args → 6 (`:141`) | | `duckdb_aggregation` | `venn_taxa_present` | invented a `[g1, g2]` group-list parameter no method has ever taken; a Venn is assembled by calling it once per group (`:161`) | | `duckdb_aggregation` | `bar_chart` | passed `(labels, counts, names)`; counts belongs third, and the matrix is (segments × samples) per `:403` and `test_analysis.jl:35` | | `duckdb_aggregation` | `taxa_bar_chart` | arguments 2 and 3 transposed (`:475`) | | `duckdb_aggregation` | `alpha_chart` | passed a 5th `groups` argument no method accepts (`:312`) | | `permanova_nmds` | `alpha_boxplot` | `Vector{Any}` matched neither concrete method, and `metric=` is not a keyword of either (`:734`, `:840`) | | `analysis_config` | — | built on BenchmarkTools, which was in neither `Project.toml` nor `Manifest.toml` | `bench/analysis_config/benchmark.jl` is structurally built on BenchmarkTools (`@benchmarkable`, `BenchmarkGroup`, save/load/median) and CI never installs it, so **that step has never once succeeded since it was wired in**. Pinned rather than fetched at CI time, because an unpinned network fetch would defeat the committed Manifest. The resolve is purely additive: BenchmarkTools 1.8.0 plus the Profile stdlib, **no existing package version changed** (`git diff -U0 Manifest.toml | grep '^-version'` → 0 lines). ### One trap the resolve set, caught by mutation `Pkg.resolve` also rewrote the Manifest's `julia_version` from 1.12.5 to **1.12.6** — the julia this laptop runs, despite `mise.toml` pinning 1.12.5. That line is asserted by `test/unit/test_install_pins.jl:112` against `config/defaults/tool_versions.yml`, so shipping it would have turned a *second* gate red. Restored by hand (Pkg only warns on that field; it does not gate) and verified by mutation: with 1.12.6 the suite reports 94 pass / 1 fail at `:112` (`"1.12.5" == "1.12.6"`); with 1.12.5, 95/95. ## Concurrency (second commit) `cancel-in-progress: true` applied to every event. `main` was merged to at 14:25, 14:28 and 14:43 on 2026-09-21; each run was cancelled 17–18 min into a ~31 min job by the next push, so **three consecutive commits produced no test verdict of any kind**. The red everyone was reading as "CI is broken" was, for those commits, no result at all. Cancelling stays correct for a `pull_request` — a verdict on a superseded head is worthless — so the flag is conditioned on the event rather than removed. ⚠ This does **not** rescue upstream #6, whose head *is* this fork's `main`: its runs are cancelled by the head update itself, not by this setting. Only letting `main` settle for ~31 min does that. ## Verification - All seven bench scripts run clean locally, **twice each** — once creating a baseline, once comparing against it, which is the path CI takes. - Tracked baselines for the five original categories were **not** modified by any run. - `bench/analysis_config/baseline.json` is deliberately **not** committed: the other five came from whichever host produced them, and adding a sixth from a laptop would make this machine's timings the estate reference. CI regenerates it per run and exits 0, matching the workflow's own statement that the deltas are informational. - Repo hygiene is **enforcing** on this PR (a `pull_request` reads `github.repository` as the BASE repo, which is this fork). All three scripts run clean locally: `check-spdx` OK 265 files, `check-format` OK 311 files, `check-lint` OK. Both commit subjects pass the conventional-commit regex. - `test_install_pins.jl`: 95 passed / 95 total. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
Six SonarCloud vulnerabilities in new code, all in shell and workflow steps. Each was verified rather than assumed, because three of the four rules have a failure mode where the "fix" breaks the build instead of hardening it. S6505 bun install without --ignore-scripts ci.yml:61, ci.yml:255, start.sh:14 S6506 wget may follow a redirect to http ci.yml:176 S6506 curl | sh does not pin the scheme install.sh:78 S8549 cargo build without --locked ci.yml:488 Verification, in the order the risk demanded: - --ignore-scripts also suppresses the ROOT package's own preinstall, postinstall and prepare hooks, not just those of dependencies. frontend/ package.json declares none of the three and sets no trustedDependencies, and `bun install --frozen-lockfile --ignore-scripts && bun run build` was run to completion: 539 installs, no changes, built in 17.36s, rc=0 on both. ci.yml:255 sits in the `test` job -- the only gate in this workflow without continue-on-error -- so breaking it would have removed the one honest signal. - --locked FAILS when no Cargo.lock exists, which would have turned a passing step into a hard error. hyperpolymath/cicd-squabbler does carry one at the pinned ref 6be52e34 (blob 2ffb575f), confirmed via the contents API before the flag was added. - --https-only lets wget follow https->https but refuses a downgrade, where --max-redirect=0 would break the step outright if the mirror ever adds a redirect. The URL resolves HTTP 200 with no redirect today, so the flag is inert now and load-bearing later. Not a Sonar finding, but the reason --locked matters here: ci.yml:488 is the cicd-squabbler build inside the `Gate triage` job, which is currently red on upstream PR #6. An unlocked dependency resolve is exactly the class of drift that makes such a job fail for reasons unrelated to the repo under test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
…at zero The `Install R packages` step rebuilt all 79 packages from source on every run and kept nothing when it failed. Measured on two consecutive runs of ci.yml: run 35614055917 Install R packages 10m 08s run 35612508961 Install R packages 13m 21s That is the largest single cost in the workflow, and it was paid in full every time -- including when the restore died partway, which discarded every package that had already built. Two things caused it. renv's cache is content-addressed and would have made the work reusable, but `renv::restore` runs under sudo, so the cache landed in root's home rather than anywhere durable; and the workflow had no actions/cache step for R at all -- only Julia had one (line 127). Nothing survived the runner. The fix pins RENV_PATHS_CACHE to /opt/renv-cache and caches that directory: - The key carries github.run_id so every attempt writes a NEW entry and progress accumulates across failures; restore-keys then picks the most recent entry matching the prefix. A changed renv.lock falls through to the second key and still re-uses every package whose version did not move. - The save step is `if: always()` deliberately. actions/cache saves only on success, which would discard exactly the partial progress this cache exists to keep: the packages that DID build before a failure are the ones the next attempt must not build again. - The cache is written by root, so a separate `if: always()` step makes it readable before it is packed. - `sudo env VAR=...` is used rather than `sudo -E` or a bare VAR=value prefix, so the variable reaches Rscript without depending on the runner's sudoers env_reset policy -- which the existing comment in this step already notes resets the environment. Reordering the downloads was considered and rejected: renv derives install order from the dependency graph, so a package cannot be moved ahead of the packages it links against. Caching is what makes the retry cheap, not ordering. actions/cache is pinned to 0057852bfaa89a56745cba8c7296529d2fc39830 (v4 == 4.3.0), dereferenced to a commit rather than trusting the tag ref, matching the SHA-pinning discipline the rest of this workflow follows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
renv wraps its download counter in ESC[?25l / ESC[?25h and rewrites the line in place. A captured Actions log is not a terminal, so the rewrite never lands: the counter sticks at "(0/79) Downloading: SummarizedExperiment, gtable, ..." for the full ten to thirteen minutes of the restore, and the residue "25l25h25l" is all that survives of the escape sequences. The step looks hung when it is working. Setting R_CLI_DYNAMIC=false and TERM=dumb makes cli emit one line per update, so the log shows real progress -- and, more usefully, a step that genuinely stops making progress becomes distinguishable from one that is merely slow. Both variables are passed through `sudo env` alongside RENV_PATHS_CACHE for the same reason that one already is: the runner's sudoers policy resets the environment, so a step-level `env:` block alone does not reach Rscript. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
SonarCloud typescript:S1082 flags a non-interactive element carrying an onClick as keyboard-inaccessible: a div or span takes no focus, so the control exists for a mouse and for nothing else. Seven such controls become <button type="button">, which brings focus, Enter/Space and the correct role for free. The visual result must not change, so BUTTON_RESET carries the one set of properties a UA applies to a button and to no other element -- appearance, background, border, margin, padding, font inheritance, colour and text alignment. Resetting them in one shared constant rather than per call site keeps the next conversion from having to rediscover which of them matter. Disclosure controls additionally gain aria-expanded, which a div never carried and which a screen reader needs to announce the collapsed state. 581 frontend tests pass; tsc --noEmit is clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
… applied Completes SonarCloud typescript:S1082 across PipelineStages and RunView, and replaces the inline BUTTON_RESET introduced in the previous commit, which was wrong in a way that only shows up on elements carrying a CSS class. An inline style beats a class, so spreading the reset into `style` silently overrode the very rule it sat beside: .label lost `font-weight: 500` and `font-size: .9rem` to `font: inherit`, and .statValue lost `padding: 0 2px` and its colour. The reset is now a `:where(button.btn-reset)` rule in the global sheet. :where() matches at zero specificity, so any author rule beats it while the user agent's button chrome still loses -- an author rule outranks a UA rule regardless of specificity. That is the one mechanism that can strip UA styling without outranking the component's own. Four sites needed structure rather than a tag swap: - Two controls were interactive only under a condition (a stage with config, a run with stats). They render as a <button> when the handler would do something and a plain element when it would not: a focus stop that does nothing on Enter is worse for a keyboard user than no control. - Two config rows already contained removeBtn, a real <button>. A button cannot nest, so the row stays a container and the clickable region becomes its sibling. labelEl moved from <div> to <span display:block> because a button may only contain phrasing content. - The table pill had a delete <span onClick> nested inside the select <button> -- invalid HTML, and the delete was mouse-only. Select and delete are now sibling buttons inside a span carrying the .btn pill styling, which is element-agnostic, so the rendering is unchanged. Disclosures gained aria-expanded; the multi-select toggle gained aria-haspopup="listbox". 581 frontend tests pass, tsc --noEmit clean, check-spdx/format/lint all OK. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
The Julia gate failed at "Test analysis-config category" with 8 errors, all
UndefVarError: `Epistemic` not defined in `Main`.
That step runs one file on its own:
julia --project=. -e 'using Test; using MetaManifold; include(...)'
`using MetaManifold` does not bring submodules into scope, which the file's
own header already recorded -- someone hit this for AnalysisConfig and fixed
it with the `:` form. Two testsets added later reach for Epistemic and
CladeCumulus, and neither was added to that import.
It passes in the full suite because sibling test files load both submodules
into Main first, so the missing import is invisible there and surfaces only in
the isolated step. CladeCumulus was the next failure queued behind Epistemic:
its 18 tests never ran at all, because the LoadError aborted the file first.
Verified by running the CI command verbatim: 8 errored becomes Epistemic
module 14/14 and CladeCumulus 18/18, exit 0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
…equest
config.json is fetched at runtime rather than built in, so whatever it holds in
"apiBase" reached fetch() in client.ts and the EventSource in events.ts verbatim.
SonarCloud flagged all three sinks (tssecurity:S5144, S8476, S8480); they share a
single source, so a single cleansing point closes all three and events.ts needs
no change.
A split deployment legitimately points at another origin ("https://bioserver:8080"),
so this cannot be narrowed to same-origin. What it does instead is refuse any
scheme but http/https -- javascript:, data:, blob: and file: are the dangerous
ones -- and rebuild the value from parsed URL components rather than passing the
string through. Rebuilding is the substantive part: it drops embedded
credentials, query and fragment, and normalises path traversal. Anything
unparseable falls back to same-origin, which is what a missing config.json
already did.
Five tests pin the behaviour, including the four refused schemes and the
credential/query/fragment strip. The existing same-origin identity test is
unchanged and still passes: 581 -> 586 tests, +5 exactly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
…t on PATH
The bench harness recorded the checkout's commit with
`execSync('git rev-parse --short HEAD')`. That resolves the git binary through
PATH, so whichever git came first ran with this process's privileges
(typescript:S4036) -- and a benchmark harness has no need of a subprocess at
all.
Reading the plaintext files git already maintains is safer, faster, and works
with no git installed. headCommit() handles the four shapes HEAD takes: a
detached SHA, a symbolic ref to a loose ref file, a symbolic ref present only in
packed-refs (which is what a fresh shallow CI clone gives you), and a linked
worktree, where .git is a FILE pointing at the real gitdir and refs live in the
common dir rather than beside HEAD. It walks up from the module's own directory
rather than trusting cwd, because the harness is run from frontend/ by
`just bench` and from the repo root by CI.
Verified by running the harness: the artifact's environment.commit is 2faf5f4,
identical to `git rev-parse --short HEAD`, and a start directory under no
repository returns "unknown" rather than throwing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
…ranch
R/_renv_dependencies.R exists so renv can see four library() calls that are
never executed for their effect. The conventional idiom for that is
`if (FALSE) { ... }`, which is dead code to every static analyser
(SonarCloud rdre:S1145). The rule is right about what it sees, so the fix is to
stop writing a dead branch rather than to suppress the finding.
A function that is defined and never called says the same thing: renv parses the
whole file either way. Verified with renv::dependencies() against both forms --
each returns exactly dada2, dplyr, tibble, vegan -- and again against the file
in place.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
SonarCloud's PR analysis measured 6.7% duplication on new code against a 3% limit, and the duplicated block was the one I had just written: five test bodies whose entire content was `expect(sanitiseApiBase(x)).toBe(y)` repeated. By token count that is genuine duplication, not a false positive. A table says it once. Every one of the seventeen assertions is preserved verbatim -- the file still makes exactly 18 expect() calls, as it did when the same cases were spread across five tests -- and each row is still registered as its own named test, so a failure names the precise case rather than pointing at a block of five. The comments that explained each group now sit above the rows they describe. Verified: 598 pass / 0 fail / 3367 expect() calls across 16 files, against a 581-pass baseline -- exactly +17, matching the seventeen rows. Run alone the file reports 18 tests and 18 expect() calls, which is the presence check: a test file that fails to load would subtract its tests from the pass count while adding nothing to the fail count. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
sanitiseApiBase ended with url.pathname.replace(/\/+$/, ''). SonarCloud typescript:S8786 measured that pattern as super-linear: on a long run of slashes the engine backtracks quadratically. That matters here more than it would elsewhere. This function exists to bound an untrusted value -- config.json's apiBase, which prefixes every fetch() and the EventSource -- so a hostile config.json is exactly the input it must survive, and a long run of slashes is free to write. A sanitiser that is itself a denial-of-service vector contradicts its own reason for existing. Replaced with an explicit index walk, which is linear and needs no reasoning about an engine's backtracking. A test row pins the behaviour on 5000 trailing slashes -- a correctness check of the new path on long input, not a timing assertion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
SonarCloud typescript:S3776 measured headCommit's cognitive complexity at 26 against a limit of 15. The function's own doc comment already named the four shapes HEAD can take, so the seams were written down before the code was split along them: findGitPath, resolveGitDir, resolveCommonDir and resolveRef. Behaviour is unchanged -- verified by running the bench and comparing the environment.commit it emits against git rev-parse --short HEAD at run time, which also confirms it resolves THIS checkout rather than walking up into a parent repository. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
…or them Two roles added while fixing typescript:S1082 were themselves flagged by typescript:S6819, which asks for the native element instead: - Toast: <div role="status"> is <output>. It is a flex item, so its inline display is blockified anyway; set explicitly rather than relied on. - DataTable: the filter dropdown carried role="presentation" only to justify an onClick that called stopPropagation(). That handler existed because the dropdown renders inside a <th> whose own onClick sorts the column. Moving the guard to the <th> -- skip the sort when the click came from inside [data-dropdown] -- removes the handler and the role together, and puts the condition where the behaviour it protects lives. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
typescript:S6772. The span is byte-identical to the one on origin/main; converting the enclosing div to a button in 5bbe143 is what brought the adjacency into new-code scope, so the finding is this branch's to answer. A bare text node adjacent to an element leaves it ambiguous whether a space renders. Here none should -- the span carries marginLeft: 8 -- so the text is now an explicit string expression saying so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
typescript:S7758, minted by the previous commit's own cure -- the second time in this branch that fixing one rule produced another (S1082 -> role="presentation" -> S6819 was the first). Taken rather than deferred because it is provably behaviour-preserving here: charCodeAt and codePointAt differ only when the index sits on a HIGH surrogate, and neither result can equal 47, so the === 47 test is identical in every case. The 5000-slash row and the rest of the file's 19 cases still pass. This is the last cure on this branch. Anything SonarCloud attributes to it from here becomes an issue with acceptance criteria, not another round -- a cure that mints a finding is a loop, and it needs a stop. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
`_read_databases` and `_read_primers` both answer `json_error(400, ...)`: the route's own verdict that a malformed `config/databases.yml` or `config/primers.yml` is an operator-fixable input, not a server fault. Logging that at `@error` with a full backtrace reports a routine bad file as if the server had crashed, and the backtrace pointed into YAML.jl's parser rather than at the offending line, so it carried no diagnostic value either. The reason string is kept -- `reason=sprint(showerror, e)` is what actually names the YAML defect. Only the backtrace goes. Deliberately NOT changed: the three 500-class sites, which stay at `@error` with their backtraces -- src/server/routes/analysis_config.jl:144 src/server/routes/annotations.jl:318 src/server/routes/composition.jl:370 A 500 is the route saying it did not expect this, and there the backtrace is the only thing separating a data problem from a code bug. Dropping it would be muting a diagnostic rather than fixing one. `analysis_config.jl` already draws this line itself: its `ArgumentError` branch returns 400 with no logging at all, while everything else logs at `@error` and returns 500. `src/server/routes/pipeline.jl:520` is untouched -- it is the silent-degradation shape tracked in #31, not a severity question. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
…s returning
Two defects with the same shape: a control that is documented as enforcing
something it cannot enforce.
## The commit-msg hook could never have run
`docs/compliance/standards-alignment.md:49` claimed commit conventions were
"Enforced ✅" by `.githooks/commit-msg`. That hook was committed **mode 100644**.
Git refuses to execute a non-executable hook, so it had never run for anyone,
ever -- and it failed to *nothing*, with no error, rather than to a visible
exit 126.
It was also wired nowhere. `core.hooksPath` is local config and cannot be
committed, and the only place it appeared was a line in CONTRIBUTING.md, so it
was unset in every clone that had not read that line. Mine included.
Fixed rather than documented around:
- `git update-index --chmod=+x` on the hook, so git will run it;
- a `just hooks` recipe, made a dependency of `just bootstrap`, so enablement
follows from bootstrapping instead of from remembering;
- CONTRIBUTING.md points at `just hooks` rather than the raw git incantation.
The doc row is now split honestly: CI is *the gate* (binding on this repo,
advisory upstream), the hook is *local pre-flight* and marked ⚠ opt-in per
clone, because that is what a per-clone setting can be and no more.
## Nothing stopped the 269 MiB coming back
`.gitattributes` had no fastq or LFS rules. After the history rewrite reclaims
~96.6% of the pack, anyone could have re-added the same data the next day.
`scripts/check-blob-hygiene.sh` is the guard, with **one implementation and two
callers** -- `.githooks/pre-commit` and a new CI `Blob hygiene check` step. That
is deliberate: a hook and a CI check that re-implement one rule drift apart, and
the drift is invisible because both keep reporting success.
The primary rule is a **4 MiB size ceiling, not a path list**. A path list can
only forbid paths somebody already thought of, and that is exactly how the
Multiplex pool survived a history rewrite -- it was reachable under a second
path (`inputs/fastq/`) that the census had never enumerated, because
`git rev-list --objects` pairs each object with only one of its paths. Path
rules are kept, but as better error messages rather than as the gate.
The six `data/MiSeq_SOP/run_[AB]/*.fastq.gz` fixtures (0.6-2.6 MiB, whitelisted
at .gitignore:294-304) are allowlisted by glob. The ceiling sits above the
largest of them (2,723,348 B) with headroom.
## Verified by mutant, not by a green run
A guard that passes proves nothing until it refuses something. Five
reintroduction attempts, each refused:
uncompressed .fastq · a 5 MiB blob · a node_modules/ path ·
an inputs/ second-path copy · a logs_*.zip CI artefact
and two negative controls admitted: an allowlisted 2.6 MiB fixture, and an
ordinary source file. The positive control over the live tree examines 335
files, sees all 6 fixtures, and passes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
…mode `git diff --cached --diff-filter=AM` reports a rename as a single R entry, so the AM filter dropped it entirely. An oversized blob already in the index could therefore be moved to a new path and sail past the pre-commit hook without check_one() ever being called on it. This was a real hole, not a theoretical one. Measured against a 5 MiB blob renamed with `git mv`: pre-fix hook: "blob hygiene: ok" rc=0 <- false green fixed hook: refuses the 5 MiB blob, rc=1 `--no-renames` decomposes the rename into D + A, and the A is examined like any other addition. The CI `--tree` mode was never affected -- it walks the whole tree rather than a diff -- and that is precisely why the gap was invisible: one of the two callers stayed correct, so nothing ever reported a problem. This is the drift the one-implementation-two-callers design was meant to prevent, and it appeared anyway because the two callers ask the index and the tree different questions. Controls run (4/4 correct): - 5 MiB blob renamed -> REFUSED (was admitted) - allowlisted fixture renamed out of it -> REFUSED - ordinary small file renamed -> admitted - fixture renamed within the allowlist -> admitted Also dates the "verified by mutant" cell in standards-alignment.md. It was an undated one-off manual battery presented as ongoing status -- the same shape as a step name asserting a comparison the script never makes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
The guard could report success without looking at a single file.
$ scripts/check-blob-hygiene.sh --tree refs/heads/no-such-branch
fatal: Not a valid object name refs/heads/no-such-branch
blob hygiene: ok
rc=0
`git ls-tree` on an unresolvable ref writes to stderr and emits no paths. A CI
`run:` step does not fail on stderr, so the loop body never executed and the
script fell through to its success line. A typo in the ref would have produced a
permanently green gate that inspected nothing -- and a tree of 337 files and a
tree of 0 files printed byte-identical output.
Three changes:
- resolve the ref with `git rev-parse --verify "$ref^{tree}"` BEFORE walking it
- count what was examined, and in --tree mode refuse when the count is zero
- print the count, so a vacuous run is visible in the log rather than inferred
No emptiness guard in --staged mode: a commit that only DELETES files
legitimately stages zero additions, so zero is a real state there. The
asymmetry is deliberate and commented.
Controls (7 designed + 1 corrected):
bad ref -> rc=2 FATAL (was rc=0 "ok")
empty tree, valid object -> rc=2 FATAL (was rc=0 "ok")
real tree, 337 files -> rc=0, count matches `git ls-tree | wc -l`
nothing staged -> rc=0 (legal)
oversized blob renamed -> rc=1
fixture renamed out of allowlist -> rc=1
uncompressed .fastq added -> rc=1
small file / fixture moved in-allowlist -> rc=0
This is the defect class the repo already had a note about -- a gate whose
success is indistinguishable from never having run. It was shipped in the guard
that was written to prevent exactly that.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
The preceding commit branches on `squabble fetch` exit 3 for "the base branch has no required_status_checks rule". That code did not exist at the pinned revision, so the fix was inert: rc=2 fell into the `*` arm and hard-failed exactly as before. An ordering trap, and one that looks identical to a working fix from the diff alone. hyperpolymath/cicd-squabbler#99 merged at 2026-09-21T18:55Z and is what introduced `FetchError::NoGate` and `ExitCode::from(e.exit_code())`. Re-pinning onto its merge commit is what makes the preceding commit live. The window also carries #94..#98 -- CI permissions indentation, two DEED grammar gate fixes, an actions.lock reconcile, and a harden-runner bump. None touches squabble's CLI behaviour. The pin comment now records the FLOOR rather than just the value, because moving this pin backwards would silently revert the exit-code handling below without touching a line of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WPSJ7fBhVAMcpSffCBWUDo
Author
|
Closing in favour of #7, which is the same content as a reviewable diff. This PR became undiffable through no change in its content. It was opened from That is not repairable here: the pre-rewrite head #7 is one commit whose tree is byte-identical to Apologies for the churn. 🤖 Generated with Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.