fix(skills): implement-design — guarantee Chromium cleanup on throw (CodeRabbit on omator#266) - #337
Draft
evanharmon1 wants to merge 1 commit into
Draft
fix(skills): implement-design — guarantee Chromium cleanup on throw (CodeRabbit on omator#266)#337evanharmon1 wants to merge 1 commit into
evanharmon1 wants to merge 1 commit into
Conversation
Wrap the per-theme sampling loop in try/finally so a throw from page.goto/page.evaluate (e.g. the preview server not being up yet) can no longer skip browser.close() and leak the launched Chromium process. This script is rerun repeatedly during Phase 5 iteration, so a leak per failed run accumulates. Addresses CodeRabbit finding on ponderousdev/omator#266 (measure-rendered-contrast.mjs — guarantee browser.close()). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
What
CodeRabbit raised 8 findings on the vendored design-handoff skill copy in
ponderousdev/omator#266
(
.claude/skills/design-handoff/**, v3.24.0 vintage). Vendored copies arepinned + drift-checked, so they are never hand-edited downstream — agreed
findings are fixed in the canonical skill here (now
ai/skills/frontend/implement-design/**after the design-handoff → implement-designrename in #108) and reach consumers via the next release → pin bump →
task sync:skills.Extends the earlier batch from #86 (which was merged before this arrived).
Change
Exactly one finding was genuinely new against the current canonical skill:
assets/measure-rendered-contrast.mjs— wrap the per-theme sampling loopin
try/finallyso a throw frompage.goto/page.evaluate(e.g. thepreview server not up yet) can no longer skip
browser.close()and leak thelaunched Chromium process. Relevant because the script is rerun repeatedly
during Phase 5.
The other 7 omator findings were already satisfied by the current canonical
skill (mostly from #86's batch, plus the earlier hardening):
$rootexists → exit 2, andrc>1fails closed,rc==1= clean*reserved for public non-credentialed static assets, origin allowlist for anything authenticatedtmsearch.uspto.gov, notes TESS retired 2023build && preview, "never the dev server"Verification
task verifygreen (612 skills tooling tests pass)node --checkclean on the edited.mjs; exec bit preservedFix ships at
769aa27b843dfd15f33a9c806246e0759976e312.🤖 Generated with Claude Code