Drop the .tmp-* rigs, fix the MIT licence, test the render harness, add CI - #45
Open
SandroHub013 wants to merge 4 commits into
Open
Drop the .tmp-* rigs, fix the MIT licence, test the render harness, add CI#45SandroHub013 wants to merge 4 commits into
SandroHub013 wants to merge 4 commits into
Conversation
The five .tmp-* directories were throwaway capture rigs — scouting stills, one-off puppeteer drivers, PNG dumps — committed by accident. 209 files, ~25 MB, none of it referenced by the skill, the site, or any demo. Removed from the tree and added to .gitignore so the next rig stays local. This takes them off the tip. It does NOT shrink the pack: the blobs stay reachable through every commit that ever held them, so the clone is still ~63 MB. Purging the history needs git filter-repo and a force-push, which cannot travel through a pull request — tracked separately.
GitHub classified the repo as 'Other' (NOASSERTION) and showed no licence anywhere, because LICENSE carried a three-line Awwwards trademark appendix below the MIT body. GitHub's licensee matcher scores a candidate against the canonical text and needs a near-exact match; the appendix pushed it under the threshold, so a correct MIT licence read as unrecognised. The MIT body itself was already byte-perfect. Only the appendix is gone, and nothing is lost with it: README.md already states the same disclaimer under 'License'. Year and copyright holder unchanged.
The six harness scripts were untestable by construction: each is a top-level-
await script that launches Chrome on import, with the parsing and the
arithmetic inlined between the puppeteer calls. lib.mjs now holds the pure
half — argument parsing, request routing, frame naming, the fps/percentile
maths, the ledger totals — and the scripts import it, so the tests cover the
code that actually runs rather than a copy of it. The browser-driving parts
are left alone and untested.
94 tests. Every one of them was checked against a deliberate break of the
function it covers: scripts/mutation-check.mjs applies 95 targeted mutations
to lib.mjs and asserts that each test dies under at least one, and that no
mutation passes unnoticed. npm run test:mutation.
Behaviour the extraction pinned down, all of it previously silent:
- an empty or too-short capture reported avg NaN and crashed on
p05.toFixed(); it now says 'no usable frame samples'
- zero-length rAF deltas divided into 1000 and averaged Infinity into the
result, which reads as a flawless run; they are discarded
- the percentile index could run one past the end of the array at p=1
- --frames abc baked a directory of NaN.png; unparsable numeric flags throw
- a failed page.evaluate returned undefined and died inside Buffer.from with
no mention of the frame; dataUrlToBuffer names what came back instead
- the harness servers turned every read error into a bare 404, so a
permission error looked exactly like a missing file and the page simply
never became ready; ENOENT is still a silent 404, anything else is logged
- the servers served '/../../etc/passwd' — posix.normalize resolves a
leading .. against the root, so the obvious guard does not work; segment
depth is now tracked explicitly
- gen-ledger died on the first missing encode; it now names all of them, and
checks that the segment table is a contiguous chain whose declared seconds
match its frame counts
Verified against the real media: the refactored gen-ledger reproduces the
committed ledger.json byte for byte apart from its date stamp.
Also fixed the eight dead bindings ESLint found in demo and harness source
(unused allocations, an unused destructured field, let that never reassigns).
No markup, stylesheet, media or data file under docs/ was touched.
CI runs lint and test on push and pull_request; CodeQL and Dependabot added.
Nothing here touches Pages, which publishes docs/ from a repository setting
rather than a workflow.
no-empty is a warning rather than an error for now: 16 empty catch blocks
remain in the two video-scrubbing demos and how each should behave is still
being decided.
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } catch (err) { | ||
| // ENOENT is the ordinary 404; anything else is a broken harness, and a | ||
| // silent 404 there costs a debugging afternoon (the page just never readies) | ||
| if (err.code !== "ENOENT") console.error(`static server: ${req.url} → ${err.code ?? err.message}`); |
| res.writeHead(200, { "content-type": mimeFor(rel) }); | ||
| res.end(data); | ||
| } catch (err) { | ||
| if (err?.code !== "ENOENT") log(`static server: ${req.url} → ${err?.code ?? err?.message ?? err}`); |
CodeQL flagged three alerts on the new harness code in #45 — one high, two medium. All three are in lib.mjs. Path handling (high). resolveRequestPath refuses traversal in string space, but the handler then pasted the result onto the root and read it. safeJoin now resolve()s both sides and checks the result really sits under the root before anything touches the disk, so the guarantee holds on the filesystem rather than in the parser. The prefix test compares against root + separator: without it a sibling directory that merely shares the root's name (/docs-secret next to /docs) would pass. Two independent guards is the point, not redundancy — the string-level one is easy to get subtly wrong, as the first version of it was. Incomplete escaping (medium x2). scoutName and stopName used String(x).replace('.', '_'), which substitutes the first occurrence only. A number carries at most one dot so the output was correct in practice, but a name that silently keeps a second dot is a file written somewhere unintended. replaceAll, and a test that would notice. 102 tests, 103 mutations, 102/102 killed and no mutation unnoticed. The mutation run caught a real gap while fixing this: swapping safeJoin for plain concatenation went undetected, because resolveRequestPath rejects traversal before the second guard is ever reached. There is now a test that pins the resolution itself.
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.
Three commits. Nothing under
docs/that the site serves was touched: no HTML, CSS, media or data file changed, and all 61.mp4demos are byte-identical.1.
.tmp-*scratch rigs off the tip.tmp-scroll-test,.tmp-miliario,.tmp-site-test,.tmp-site-gl,.tmp-webgl-demo— 209 files, ~25 MB of throwaway capture rigs and PNG dumps, none of it referenced by the skill, the site or any demo. Removed, and.tmp-*/added to.gitignore.This does not shrink the clone. The blobs stay reachable through every commit that ever held them, so the pack is still ~63 MB. Purging the history needs
git filter-repoand a force-push, which cannot travel through a pull request — tracked in #46, with the procedure and the measured result (63.39 → 38.33 MiB).2. LICENSE recognised as MIT again
GitHub classified the repo as "Other" (NOASSERTION), so no licence showed anywhere.
The MIT body was already byte-perfect. The only difference from canonical was a three-line Awwwards trademark appendix below a
---rule. GitHub'slicenseescores a candidate against the canonical text and needs a near-exact match; the appendix pushed a correct MIT licence under the threshold.LICENSE is now byte-identical to canonical MIT. Year and copyright holder unchanged. Nothing was lost with the appendix —
README.md§ License already carried the same disclaimer verbatim.3. Vitest over the soglia render harness
The six scripts were untestable by construction: each is a top-level-await script that launches Chrome on import, with the parsing and arithmetic inlined between the puppeteer calls.
docs/demos/soglia/render/lib.mjsnow holds the pure half and the scripts import it, so the tests cover the code that actually runs rather than a copy of it. The browser-driving parts are deliberately left alone and untested.94 tests, every one verified capable of failing.
scripts/mutation-check.mjsapplies 95 targeted mutations tolib.mjsand asserts each test dies under at least one, and that no mutation passes unnoticed — currently 94/94 killed, 0 survivors.npm run test:mutation.The harness earned its keep twice: it caught 10 tests the first mutation set couldn't kill, and one genuinely weak test (the p5 case had its slow frames pre-sorted, so deleting the
.sort()went unnoticed).Behaviour the extraction pinned down, all of it previously silent:
avg NaNand crashed onp05.toFixed(); it now saysno usable frame samplesInfinityinto the result, which reads as a flawless run; they are discarded--frames abcbaked a directory ofNaN.png; unparsable numeric flags now throwpage.evaluatereturnedundefinedand died insideBuffer.fromwithout naming the frameENOENTis still a silent 404, anything else is logged/../../etc/passwd—posix.normalizeresolves a leading..against the root, so the obvious guard silently does not work; segment depth is now tracked explicitly (the first attempt at this had exactly that bug, and the tests caught it)gen-ledgerdied on the first missing encode; it now names all of them, and checks that the segment table is a contiguous chain whose declared seconds match its frame countsVerified end-to-end: the refactored
gen-ledgerreproduces the committedledger.jsonbyte for byte apart from its date stamp, and the real segment table passes the new contiguity check.Also fixed the eight dead bindings ESLint found in demo and harness source — unused allocations, an unused destructured field, a
letthat never reassigns. All behaviour-neutral.CI
ci.ymlruns lint and test on push and pull_request.codeql.ymlanalyses JavaScript weekly.dependabot.ymlcovers root tooling, both render harnesses and github-actions.No Pages workflow existed and none was added — Pages publishes
docs/from a repository setting, so there was nothing to collide with and nothing here can take it over.Known:
no-emptyis a warning, not an error16 empty
catchblocks remain, 8 indocs/demos/soglia/app.jsand 8 indocs/demos/stacco/app.js. How each should behave is still being decided — classification and definition-of-done in #48.no-emptyignores a block once it holds a comment, so each clears the rule as it is resolved, and the rule goes back toerrorwhen the last one is done.One of them sits next to a real bug that is not fixed here: a failed clip fetch causes an unbounded refetch loop in soglia and permanent silent abandonment in stacco — same line, opposite outcomes. See #47.