Skip to content

Three small bugs: Windows launcher temp files, tune panel reset, scroll tracking loops #1002

Closed

Description

@nicolaepocroianu

Found while reviewing the vendored skill (impeccable SKILL.md 4.5.2, scripts VERSION 0.1.14) in a project that installs it with npx skills add pbakaus/impeccable. We keep the files byte for byte so skills-lock.json and npx skills update hold, so we haven't patched them locally. Reporting here instead.

1. Windows launcher: concurrent first runs share temporary files

scripts/impeccable.cmd (around lines 81–85) always stages the download at %cached%.part, and verifies through %cached%.sha256. If two invocations start with an empty cache at the same time, they write, verify and delete the same files and can break each other's download. scripts/impeccable avoids this with a per-process name (.impeccable.part.$$). A per-invocation suffix (for example %RANDOM%), or a lock around the download step, would match.

2. live: reopening the tune panel resets adjustments

openTunePopover() in scripts/live-browser.js (around lines 4982–4984) calls applyParamDefaults(variantEl, params) on every open. A user who adjusts a parameter, closes the panel to look at the page, and reopens it gets the defaults back on the same variant. Resetting only when the variant changes would keep the adjustments.

3. live: startScrollTracking() can run more than one loop

startScrollTracking() (around line 7122) starts a new requestAnimationFrame loop without cancelling an existing one. Arrow navigation while CONFIGURING calls it again (around lines 7147–7149). Each loop overwrites the same scrollRaf, so stopScrollTracking() cancels only the newest; the others keep repositioning the bar and highlight after cleanup. Cancelling scrollRaf at the start of startScrollTracking(), or returning early when one is active, would fix it.

Thanks for the skill.

Activity

  1. mager commented on Oct 11, 2026

    @mager
    Contributor

    I reproduced item 2 (Tune resetting on reopen) and would be happy to take that part if it is available.

    Tested in Chrome 154.0.8037.99 on macOS using the real Live server, the repository's vite8-react-ts fixture, and its deterministic test agent. The source and native engine were at d631a8827; I also checked current main c63b8dd7d, where the relevant Live browser source and test harness files are unchanged.

    Reproduction:

    1. Generate variants for the hero heading and open Tune on variant 1.
    2. Change Lightness from 0.50 to 0.65.
    3. Close Tune: the preview stays at 0.65.
    4. Reopen Tune: both the slider and the actual preview return to 0.50.

    I repeated the browser run successfully. The computed heading color changes from oklch(0.65 0.25 25) back to oklch(0.5 0.25 25), so this affects the rendered result, not just the control display. Switching to variant 2 still initializes its separate Face default to Sans.

    Proposed scope: preserve the current variant's parameter values when reopening Tune and render the controls from those values. Keep the documented reset-on-variant-switch behavior. Removing applyParamDefaults() from the open path alone would be incomplete because buildParamsPanel() also initializes controls from p.default.

    For a PR, I would cover range, steps, and toggle controls (including zero/false), repeated close/reopen, variant/session isolation, parameter publication and restoration paths, and an end-to-end accept after reopening that verifies the tuned value reaches the saved source. Then run the focused tests, default suite, source-first build, and Live E2E checks.

    Would you be happy for me to prepare a focused PR for item 2? I would leave the Windows launcher and scroll-loop items out of that PR.

    AI assistance: reproduction, source investigation, and this comment were prepared with OpenAI Codex under my direction.

  2. nicolaepocroianu commented on Oct 11, 2026

    @nicolaepocroianu
    Author

    @mager sounds very good to me. Thank you.

  3. added
    bugSomething isn't working
    and removed
    needs triageNew or reopened issue awaiting maintainer triage
    on Oct 15, 2026
  4. pbakaus commented on Oct 15, 2026

    @pbakaus
    Owner

    Thanks for the clear report, and @mager for the repro. All three reproduce on main. We'll fix the launcher temp-file collision and the scroll-tracking loop ourselves. @mager, please go ahead with a focused PR for the tune panel (keep values on reopen, reset on variant switch), including a bun run test:live-e2e run.

    (Reply by Claude Code on behalf of @pbakaus.)

  5. pbakaus commented on Oct 23, 2026

    @pbakaus
    Owner

    All three items are fixed: the Windows launcher temp files and the scroll-tracking loop in #1036, and the tune panel keeping values on reopen in #1108 (thanks @mager). (Closed by Claude Code on behalf of @pbakaus.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions