v1.4 — experience overhaul, finished + hardened + CI-validated#261
Merged
Conversation
added 30 commits
July 4, 2026 04:44
Unify every user-facing surface to the single brand "Reframe": the Electron BrowserWindow title, the in-app header, the native About panel (app.setAboutPanelOptions), and the electron-builder productName + nsis shortcutName. Add productName:"Reframe" to app/package.json and bump the version 1.2.0 -> 1.3.0. The internal id "media-studio" (package name, appId local.media-studio, the name-based artifact filename, appData/path literals) is deliberately preserved so first-run state, proxy/peak/dub caches, and the sidecar-env sentinel are untouched. Extend brand.test.ts to assert version 1.3.0, productName "Reframe", the new brand on all four surfaces + the About panel, and a no-user-facing-media-studio audit. Update the two e2e brand assertions and add a CHANGELOG 1.3.0 entry noting the 1.1.0 -> 1.2.0 -> 1.3 lineage.
…(WU A3) Vendor static ffmpeg/ffprobe via the redistribution-safe BtbN win64-LGPL build instead of the GPL GyanD essentials build. Pin the exact source tag (autobuild-2026-07-03-13-21, FFmpeg n7.1.5), stage into build/ffmpeg/win/, and ship outside app.asar via electron-builder extraResources -> resources/bin (LICENSE.txt shipped alongside per the LGPL obligation). - python-embed-setup.ps1: BtbN win64-lgpl pinned URL/tag; FfmpegDest -> ffmpeg/win. - electron-builder.yml: from ../build/ffmpeg/win -> to bin; record tag + LGPL note. - sidecar.env.test.ts: TDD the buildSidecarEnv ffmpeg/ffprobe path-resolution (packaged -> resourcesPath/bin/*.exe; dev sets neither; no-clobber of a pre-set override; guarded when resourcesPath is absent) - the previously untested packaged env branch. Binaries stay gitignored (regenerable ~138 MB). Sidecar fail-loud on a missing binary is already covered (test_ffmpeg.py FfmpegNotFound). Claude-Session: https://claude.ai/code/session_01Ci5vgCseSuuvugmWPJpoic
…er path guard (WU A4) Prefer a provisioned lower tier over an empty writable <exeDir>/data before the portable tier-3 auto-pick, so a clean install opens the real library with no manual data-dir.txt edit. env-override + data-dir.txt marker tiers still win unconditionally; multiple provisioned roots resolve by the deterministic tier order (never a silent migrate/merge). Validate the data-dir.txt marker + MEDIA_STUDIO_CONFIG_DIR as absolute LOCAL paths (canonicalize/realpath; reject UNC/device/".." traversal) at BOTH the Electron main consumer (dataRoot.ts DataRootSecurityError/isSafeLocalDataRoot) and the sidecar consumer (settings_store.default_config_dir / UnsafeConfigDirError) — defense-in-depth against a code-exec vector into the pip-install tree. TDD: dataRoot.test.ts (provisioning/tie-break/rejection branches), dataRootIo.test.ts (isProvisionedRoot marker probe), test_settings_store.py (UNC/device/traversal/relative rejection + realpath canonicalization). Renderer 100% coverage; sidecar settings_store.py 100% line+branch.
Never emit an assets.ensure / download target for an asset whose manifest.get_asset() is None (de-registered), which otherwise trips the assets.ensure "unknown asset(s)" gate in assets/rpc.py. - _wire.py _missing_tier_assets: filter mapped tier assets to manifest- known names only (ensurable targets). - _wire.py _tier_readiness_items: keep a tier not-ready when a weight is missing, but emit assets.ensure only over ensurable assets; when every missing weight maps to a de-registered asset, report unavailable with no action (fail-loud) instead of a false ready or a broken button. - recommender._derive_downloads: skip a component whose mapped asset is de-registered; drop the now-dead entry-is-None fallback in _download_for. assets/rpc.py's unknown-asset rejection is confirmed correct and kept. TDD: red tests first; sidecar suite 5322 passed, 100% line+branch coverage; ruff + pyright clean. Pre-existing test_director_render ffmpeg/fontconfig failures are unrelated (untouched by this WU).
… probe (WU B2) Fixes "LLM 10061" refused-connection: select / subtitles / edit-plan local paths now lazily bring the llama.cpp server (:8088) up before the first local call, instead of hitting a not-yet-listening socket. - provider.py (runner-free): add readiness_probe (bounded GET /health poll -- 200 ready, 503/refused keep waiting, child-exit fail, RAISES ProviderError on timeout so it never hangs) + health_url_from_base; RotatingProvider gains an injected opaque ensure() invoked ONLY for the local backstop slot (provider=="local"), after cloud keys, never for a detected Ollama/LM-Studio server; threaded through build_pool_provider / get_provider (incl. the bare local fall-through). - translation.py: TieredTranslator/get_translator accept ensure; the local tier runs it after start_server so subtitles/dub also gate on readiness. - ai_ops.py: Services._llama_ensure builds the callback from the shared ModelRunner (reuse-aware + LaneLock-cooperative via start_server) + the probe; start failures surface loudly as ProviderError (no silent fallback). Wired into _provider_for_function, _select_provider_or_local, _translator_for_function, _editplan_provider_or_refuse. TDD: 30 new tests; sidecar suite 100% line/branch coverage.
…er (WU B3) The mstream resolver streamed the raw, non-Chromium-decodable source whenever a cached proxy was not yet on disk (the "media error code 4" bug) and swallowed proxy-build failures into a silent center-crop. Make the resolver authoritative for playability via a new PlaybackProxy orchestrator: a valid cached proxy is served; a directly-playable source streams its original; a NON-playable source triggers the sidecar proxy build guarded by a single-flight map keyed by videoId (concurrent video range requests can never kick duplicate transcodes), awaited within a bound, with its build state pushed to the renderer over a new proxy.state channel. A build FAILURE now throws ProxyBuildFailedError -> HTTP 502 (loud), and a still-running build past the bound throws ProxyBuildingError -> 503 (transient); neither ever falls back to the undecodable original. The Workspace no longer kicks the build itself; it reacts to proxy.state (building -> note, ready -> shake-free reload, error -> loud banner). The sidecar media_compat proxy build is unchanged (already single-verdict + mtime-cached + raises on ffmpeg failure). Tests: playbackProxy.test.ts (single-flight / success / timeout-transient / loud-failure branches), mediaProtocol.test.ts (503/502 handler mapping), Workspace.test.tsx + rpc.test.ts (proxy.state react + bridge). App suite 2323 pass, 100% line/branch/function/statement coverage.
… gap list (WU D1) RECONCILE audit of the existing Providers & Keys surface (ProviderKeyRow / AddKeyRow / ProvidersKeys / UsageBar / OpenRouterUsage / SpendCap + the sidecar providers_ops/settings_store key flow). Adds a characterization test that LOCKS current behaviour (add/validate/remove keys; usage surfaces; RPC method snapshot) so the D2-D4 refactors are diff-visible, and a written gap list. Confirmed gaps (documented, not changed): no reveal, no edit-in-place, no re-validate of a stored key; keys plaintext at rest and inbound over stdio. No behaviour change. Full sidecar suite 5346 passed, coverage 100% line+branch.
… no-log redaction (WU D2) Fix API keys sitting in plaintext at rest and prevent them leaking to logs. Main process (app/main/keystore.ts): - safeStorage-backed encrypt-at-rest (base64 in userData secure-keys.json); decrypt only in main. Availability gated on isEncryptionAvailable() AND getSelectedStorageBackend() != basic_text -- never a silent plaintext write. - When no secure backend exists: REFUSE to persist -> session-only + a loud renderer banner (SESSION_ONLY_BANNER); the user's only key copy is preserved. - One-time migration re-encrypts any legacy plaintext keys from settings.json into the keystore, strips the raw keys (metadata preserved), and SHREDS every prior copy (settings.json in-place + .tmp + backups). Tested assertion: ZERO plaintext key bytes remain across settings.json/.tmp/backups after migration. - loadDecryptedKeys returns the decrypted material for main to inject into the sidecar per-request over the existing stdio JSON-RPC frame (never env/argv). Sidecar no-log (R7): - secrets.redact_params() scrubs apiKey/apiKeys/cloudApiKey (top-level, nested provider, providers list) to [REDACTED]. - rpc.py wires it into the crash + notification-failure diagnostics so a key-bearing frame's params never reach a stderr log line. TDD, all green: sidecar 100% line/branch (secrets.py, rpc.py); keystore.ts 100%.
…WU D3)
Close the confirmed v1.3 gap: a stored provider key could never be shown,
edited, or re-validated by id -- testKey always demanded the plaintext inline.
Sidecar (the ONE sanctioned exception to redact-over-RPC):
- providers.revealKey({id, index?}) -> {key}: returns exactly ONE raw key read
via get_raw() (the FACTORY accessor), for an explicit-click transient display.
Every other providers.* read stays last-4 redacted. Unknown id / out-of-range
/ negative / non-int (incl. bool) index / empty slot are typed INVALID_PARAMS,
never a crash or a silent empty reveal. Registered at the single composition
root; the response never hits a stderr log (params already redacted by rpc.py).
Renderer (R7 -- the revealed key NEVER enters state/store/logs/telemetry/crash):
- ProviderKeyRow: reveal holds the plaintext ONLY in a transient ref (a boolean
flag drives the re-render), masked-by-default, auto-re-masks on blur/timeout,
and wipes the ref on re-mask AND on unmount (an await resolving post-unmount
can't re-populate the wiped secret). Adds per-key "Re-validate" and "Replace"
(re-runs validation; preferred over in-place mutation).
- ProvidersKeys: wires revealKey / revalidate (revealKey -> testKey, plaintext in
a local const only) / replace (testKey the new key, then upsert it at its index
so surviving redacted siblings round-trip to RAW server-side).
- client.revealKey + RevealKeyResult schema.
TDD, all green: sidecar 100% line/branch (new handler + updated surface
snapshots); renderer 100% stmt/branch/func/line. Tests assert the revealed key
never reaches a store/log/callback and is gone from the DOM after re-mask.
…(WU D4) Complete the usage surface without fakes: - Local request/token counters always surface (providers.usage, unchanged). - New providers.usageAvailability RPC states, per configured cloud provider, whether a provider-side usage API exists: OpenRouter yes (live GET /api/v1/key cost), OpenAI/Anthropic gate usage behind an org admin key a stored project key cannot use, others publish nothing per-key -> honest "Usage API not available for <provider>" instead of a fabricated number. Rendered next to OpenRouter spend in Models & System. - New provider_pricing model: real per-model price table (honestly empty today - no curated model publishes a per-request price) + the documented placeholder; providers.spend now returns isEstimate so the month-to-date figure is labelled an ESTIMATE in the spend meter rather than shown as a real invoiced charge. - _estimate_job_cents routes through provider_pricing so real pricing is honored when added (behaviour identical today). Sidecar + renderer at 100% line/branch/function/statement coverage; tsc/biome/ ruff/pyright clean.
…ty state (WU E1) Thread App's selected editVideo into <DirectorPanel video onChooseVideo/> and plan against video?.id ?? plan?.videoId — NEVER the goal text. The prop-less mount previously dispatched the goal string as the videoId on a first run (DirectorPanel.tsx: plan(plan?.videoId ?? trimmed, trimmed)). When no video is open and no prior plan is on screen the panel now shows a "Choose a video" empty state whose CTA routes to the Library, so a first run can never mis-fire. Claude-Session: https://claude.ai/code/session_01W4LPidyKbYghtHeCHqrpAy
… (WU E2)
So a first-time user knows what Director does. Adds:
* clickable example-prompt chips (DIRECTOR_EXAMPLES) that prefill the goal
textarea (disabled while a job is in flight);
* a persistent "What is Director?" header button that (re-)opens a first-run
guided tour — DirectorOnboarding, a 3-step focus-trapped, dismissible dialog
reusing the existing useFocusTrap/ModelsOnboarding pattern;
* the tour is gated on a new settings.directorOnboardingSeen flag (read once on
mount via the injected client; persisted best-effort on Skip/Got it/Escape),
and is reachable over BOTH the editor and the no-video empty state.
Renderer-only: directorOnboardingSeen rides the existing arbitrary-key
settings.set merge, so no sidecar change. Discoverable + dismissible.
TDD: DirectorOnboarding.test.tsx (full component branch coverage) + extended
DirectorPanel.test.tsx (chip prefill, busy-disable, first-run gating, seen/
unseen, read-failure best-effort, dismiss-persist, re-open, empty-state reach).
Renderer suite 100% line/branch/function/statement; 2401 tests pass; biome clean.
Claude-Session: https://claude.ai/code/session_01W4LPidyKbYghtHeCHqrpAy
Ship the user-picked production icon. The multi-size .ico (16/24/32/48/64/ 128/256, PNG-embedded) becomes the exe + installer + taskbar icon; the 512px PNG is the runtime BrowserWindow icon. - build/icons/: add reframe.ico + the 16..1024 master PNG set. - electron-builder.yml: win.icon + nsis installerIcon/uninstallerIcon -> the .ico (buildResources-relative); extraResources ships reframe-512.png + reframe.ico to resources/icons/ for the running window. - app/main/main.ts: resolveWindowIcon() (packaged resources/icons vs dev build/icons) and set BrowserWindow icon in createWindow(). - app/main/icon.test.ts: config/asset-assertion guard — asset existence + real ICO/PNG magic, electron-builder wiring, and the main.ts window-icon path. Claude-Session: https://claude.ai/code/session_01W4LPidyKbYghtHeCHqrpAy
…p (WU C1) Extend the existing asset manager (sha-pinned, atomic .part, 416-resume, detect-existing all preserved) with the WU C1 installer surface: - Profiles: per-entry tier (core/optional/gpu) + resolve_profile (Minimum=nothing, Default=core, Full=all, Custom=picked). assets.ensure and the new assets.plan accept a profile; day-1/phase-8/lightasd/yunet/edgetam entries are tier-tagged. - Real progress: live bytes/sec + ETA in the download progress message via pure download_speed_eta / format_eta / format_bytes_progress helpers and an injectable monotonic clock. - Automatic retry: exponential backoff + full jitter (backoff_delay) on transient transport drops, reusing the Range-resume .part; HTTP-status and sha-mismatch (new AssetIntegrityError) and cancellation are never retried. - What/why/size: every active entry carries a plain-English why; assets.plan returns per-component what/why/size + total/to-download so users see what a multi-GB profile buys before downloading. - Graceful per-item failure: ensure skips + notes a failed item and keeps going (returns installed/failed/assets); only an all-failed ensure errors the job. TDD: new tests/test_assets_profiles.py; sidecar suite 5448 passed, 100% line+branch coverage; assets modules 100%. Claude-Session: https://claude.ai/code/session_01W4LPidyKbYghtHeCHqrpAy
…U C2)
Add a thin feature->asset mapping layer that reconciles over the existing
readiness infra (ReadinessRollup/ReadinessBadge/readiness.summary + the
manifest installer profiles) — NO new wire type, RPC, or UI framework.
- _capabilities.py: FeatureSpec map (reframe / reframe.saliency / scene.detect)
+ feature_readiness_items (per-install-state point-of-use "Needs download ->
[button]" ReadinessItems) + profile_capability_matrix (per-profile view).
- REFRAME INVARIANT (R3): the tiny always-on YuNet tracker (core
yunet-face-detection) satisfies "no silent centre-crop" on its OWN; ViNet-S
saliency is an on-demand crop-quality enhancement, never a prerequisite. So
reframe is READY the moment the tracker is present — independent of saliency —
and a missing saliency model surfaces as a SEPARATE, LOUD "download saliency to
improve" item, never a silent degrade and never marking reframe unavailable. A
Minimum install is one tiny tracker download from honest subject-tracked
reframing.
- De-registered enhancement assets (pre-B4) surface as loud unavailable
("not yet available for download"), never emitting an unknown-asset name into
an assets.ensure action (B1-safe).
- system_ops._installed_asset_names: read-only capability-asset probe seam
(never creates the data dir); readiness.summary appends the feature family so
it rides the SAME roll-up the existing ReadinessRollup renders (no renderer
change needed).
TDD: new tests/test_handlers_capabilities.py (17 tests). Full sidecar suite
5465 passed, 100% line+branch coverage; ruff + basedpyright clean.
Claude-Session: https://claude.ai/code/session_01W4LPidyKbYghtHeCHqrpAy
… C3) Redefine FIRST_RUN_COMPLETE from "env + every model + weights" to CORE-ONLY: env + bundled ffmpeg + the always-on face/ASD weights (YuNet subject tracker + S3FD/LR-ASD active-speaker weights). On-demand assets (Whisper/Qwen GGUFs, llama-server builds, TTS voices, ViNet-S saliency, TransNetV2 scene-cut) live OUTSIDE the marker and are fetched at point-of-use, so a Minimum/Custom install opens provisioned (no re-bootstrap loop, never perpetually "un-provisioned") while a missing CORE face/ASD weight still leaves no marker so the next launch retries instead of silently centre-cropping. - bootstrap.py: core_first_run_assets() (the face/ASD marker set); the run still installs the full default set but the marker gate narrows to verify_provisioned(core_subset) and records the CORE set — a failed/skipped on-demand model no longer blocks the marker. - firstRunGate.ts: CORE_FIRST_RUN_ASSETS (kept in sync with bootstrap), isProfileFirstRunComplete (per-profile completion signal), and firstRunReadinessRollup (point-of-use core-vs-on-demand readiness). - firstRunGate.test.ts: SEMANTIC assertions — the marker means face/ASD, not every model (asserts the bootstrap CORE set references the face/ASD constants and excludes the GGUFs); a Minimum install with no optional models is provisioned; missing on-demand asset is needs-download, not setup-incomplete. TDD. sidecar suite 5469 passed, 100% line+branch (--cov=media_studio); renderer 100% statements/branches/functions/lines; ruff + basedpyright + biome clean. Claude-Session: https://claude.ai/code/session_01W4LPidyKbYghtHeCHqrpAy
…c) — WU C4 Switch the pip --target env installer onto a fully-hashed lockfile so every wheel over the FULL transitive closure is hash-verified before pip unpacks it — closing the gap where the top-level pins in requirements-sidecar.txt / chatterbox still let transitives resolve UNHASHED off PyPI. - manager.build_env_install_argvs(lock_file=...) + bootstrap.build_pip_steps( lock_file=...) emit "pip install --require-hashes --only-binary=:all: --no-deps -r <lock>" (HASHED_LOCK_PIP_ARGS) when a lock is staged. - validate_hashed_lock() gates a lock (every requirement pinned + carries a --hash=; the cu128 torch index is allowed as a STILL-HASHED per-index exception); a bad lock fails LOUD (AssetError -> BootstrapError in bootstrap). - install_env / _install_env auto-use a staged sibling/declared lock and fall back LOUDLY to the pinned pins when the lock is not staged (no silent skip). AssetEntry gains an optional lock_file (env installer only). - generate_hashed_lock.py: documented F1 build-prep lock-gen script (uv/pip-compile --generate-hashes). Lock CONTENT is an F1 build-prep artifact (real hashes need network) staged offline like the ffmpeg binary. TDD: manager + bootstrap + lock-gen paths covered; sidecar coverage 100%. Claude-Session: https://claude.ai/code/session_01W4LPidyKbYghtHeCHqrpAy
…WU A5) A user whose first run partially failed was left with a loud bootstrap-error banner but no in-place recovery short of relaunching. Add a first-class "Retry setup" control that re-runs the idempotent runtime_setup/bootstrap.py on demand (pip re-checks satisfied deps; only missing assets re-download) and, on success, (re)starts the sidecar. - SidecarBanner: bootstrap-error branch now offers a "Retry setup" button that invokes window.api.repairSetup(); shows an in-flight "Retrying setup..." note, clears on success, and re-offers loudly (with the reason) on failure. - preload: expose repairSetup() over the frozen bridge (setup.repair channel). - repairSetupIpc: thin, injectable main-process handler - SINGLE-FLIGHT on the live bootstrap child (never spawns a second concurrent bootstrap), (re)starts the sidecar on success, and never crashes on a thrown runner. - main: wire the handler (in-flight = live bootstrapChild; success re-activates the embeddable ._pth and restarts the sidecar) + will-quit teardown. TDD: renderer + main IPC tests first (RED), then implement. Renderer coverage 100% (14705/14705 stmts, 5004/5004 branches); repairSetupIpc unit-tested. Claude-Session: https://claude.ai/code/session_01W4LPidyKbYghtHeCHqrpAy
…nted abundance token (WU A6) The usage panel's animated "superpowered / high-headroom" state baked ad-hoc purple hex (#9b6cff / rgba(155,108,255,...) / #c2a3ff) straight into usageBar.css - an accent off the single-accent amber + green/amber/red status ladder, undocumented as a token, violating tokens.css "consume tokens only". Reconcile to one-accent discipline by promoting the hue to ONE documented single-use semantic token family (--status-abundance / -text / -soft) in tokens.css, explicitly annotated as the only sanctioned off-ladder accent (colour redundant with the always-present "Superpowered" text label). usageBar.css now consumes the tokens; no logic change. TDD: tokens.conformance.test.ts asserts (1) usageBar.css carries no raw colour literals and (2) the abundance exception is a documented token in tokens.css. Renderer suite 2437 passing, coverage 100% lines/branches/functions/statements.
…2b-1)
Defense-in-depth main-side wiring of the DPAPI keystore (ruling B). No raw
API key reaches the sidecar at-rest store via the providers.upsert path.
- keyBridge.ts (new): the main-process guard on the single rpc channel.
* providers.upsert -> restore redacted placeholders against the keystore,
persist the resolved RAW keys into the DPAPI keystore, forward a
FULLY-REDACTED entry (last-4 only) to the sidecar.
* provider-calling methods (ai./director./shortmaker./index. prefixes plus
subtitles.translate and providers.usage/openrouterUsage/revealKey) get the
decrypted keys injected as params._injectedKeys, in-memory over the stdio
frame ONLY (never env/argv/log/disk). Session-only fallback keeps keys in
an in-memory overlay and writes NOTHING to disk when secure storage is
unavailable.
- keystore.ts: add saveDecryptedKeys (public encrypted writer; refuses on no
secure backend).
- ipc.ts: registerIpc takes an optional KeyBridge; the RPC handler applies the
forward transform; serves the new secure.status channel; disposer updated.
- main.ts: run migrateLegacyPlaintextKeys BEFORE the sidecar starts; build the
KeyBridge; a refused result surfaces the session-only banner.
- preload.ts: expose window.api.getSecureStatus() over secure.status.
- SecureKeysBanner (new, renderer): loud session-only banner shown when main
reports sessionOnly; wired into App.tsx.
KNOWN + ACCEPTED: runtime provider calls stay broken until WU-D2b-2 consumes
_injectedKeys (lands next in this run).
Tests: keyBridge/ipc/keystore/SecureKeysBanner units; 2483 pass, renderer
coverage 100%; tsc + biome + oxlint clean. Also fixes a pre-existing tsc error
in Workspace.test.tsx (compiler-suggested as-unknown on a test-only cast).
Claude-Session: https://claude.ai/code/session_01W4LPidyKbYghtHeCHqrpAy
Sidecar half of the DPAPI key guard (orchestrator ruling B, defense-in-depth): the "zero plaintext key bytes at rest" invariant now holds on EVERY path. CONSUME: SettingsStore.get_raw() applies a request-scoped key_overlay so the 4 factory seams + provider.get_provider/build_pool_provider see the RAW keys main injects for THAT request only (in-memory). The composition root wraps every handler: it pops _injectedKeys off the params IN PLACE (so dispatch's job-store record + rpc.py's error logger never see a live key) and opens the overlay around the handler, whose real key-consuming seams all capture synchronously. NO-PERSIST: SettingsStore._write strips every providers[].apiKeys entry and the legacy cloudApiKey to their last-4 marker before persist, so a raw key is never written plaintext regardless of caller. redact_params also drops _injectedKeys. TEST MIGRATION: re-pointed the OLD plaintext-at-rest / get_raw-returns-disk-keys locks (settings_store, raw_vs_redacted_audit, provider_surface_characterization, handlers_keys/reveal/index/usage, director text-consent e2e) at the NEW secure property — marker at rest, key usable via the injection overlay. Added a headline providers.upsert round-trip that scans settings.json + siblings for zero plaintext key bytes, plus overlay/wrapper/no-leak unit tests. pytest --cov=media_studio --cov-branch --cov-fail-under=100: 100% line+branch, 5518 passed; ruff clean. Claude-Session: https://claude.ai/code/session_01W4LPidyKbYghtHeCHqrpAy
…WU B4/I2)
Make the two premium reframe models loadable + manifest-registered so the
"unknown asset" feature works and C2's on-demand "download to improve" surfaces
a real, installable, safely-loadable asset.
- Reverse the de-registration no-ops in saliency.py + scene_transnet.py: register
vinet-s-saliency + transnetv2-pytorch with the pinned 40-hex HF-commit
safetensors URLs + 64-hex sha256 (WU I1 re-host), tier=optional, .safetensors
dest. manifest.get_asset() now returns them -> B1 manifest-known gate + C2
readiness offer them; _wire.py needed no change (already B4-aware).
- I2 verify-before-load gate (_safetensors_loader.py): safetensors ONLY via an
injectable load_file seam; refuses .pth/.pt/.model LOUD (torch.load/pickle is
forbidden, RCE surface); optional on-disk sha256 re-verify; load_state_dict
strict (key/shape mismatch raises). Torch-free gate logic, unit-tested 100%.
- Vendor the two upstream arches (never imported by the torch-free gate -> 0
measurable stmts): _vinet_s (ViNet-S VideoSaliencyModel + model_utils,
CC-BY-NC-SA-4.0) and _transnetv2 (TransNetV2, MIT), each with its LICENSE.
Verified: ViNet-S builds to 9.5M params / 36MB / 470 backbone.*/decoder.* keys
matching the weight; TransNetV2 7.6M params.
- Wire backends to load via the gate: saliency_backend.py (new ViNetSaliencyBackend
ENHANCES the always-on YuNet crop-track, never a prerequisite) + fix
scene_transnet_backend.py (was building the net but never loading weights;
relativize the vendored import + load_into_model).
- F1 fallback preserved: a missing/unloadable weight degrades present=False
("download to enable"), never blocks a reframe.
- safetensors pinned in the reframe-gpu extra; vendored dirs ruff-excluded;
deselected e2e integration test proves the real-weights load+infer.
The _lightasd torch.load sites are LR-ASD (a separate model/WU), not on this
load path -> intentionally untouched. Sidecar gate: 100% line+branch (5542
passed); ruff clean.
Claude-Session: https://claude.ai/code/session_01W4LPidyKbYghtHeCHqrpAy
The real app icons live in build/icons/ (plural, committed by WU A2); the singular build/icon/ is a regenerable duplicate an icon step emits. e2e_artifacts/ holds ad-hoc E2E probe scripts + runner datadirs. Both are regenerable and were showing as untracked; ignore them. docs/research/*.md source docs are untouched. Claude-Session: https://claude.ai/code/session_01W4LPidyKbYghtHeCHqrpAy
Both the URLError branch (exc.reason) and the non-JSON response branch (raw[:200]) constructed a ProviderError from server/attacker-influenced text without routing it through scrub_error_body, unlike the sibling HTTPError branch. A leaked bearer token or a threaded live key echoed in a proxy error page could therefore survive into a surfaced/loggable error at those two sites. Wrap both through scrub_error_body(..., secrets) so every error-construction site is uniformly scrub-guarded (security review 2 LOW). Pure hardening; TDD, sidecar suite 100%. Claude-Session: https://claude.ai/code/session_01W4LPidyKbYghtHeCHqrpAy
…rcial notice Security review HIGH#1b/1c: ViNet-S is bundled under CC-BY-NC-SA-4.0, which requires user-facing attribution and makes the app non-commercial while bundled. - Add a Settings -> Licenses section (ThirdPartyNotices) reproducing each bundled model's attribution: ViNet-S (CC-BY-NC-SA-4.0, authors, arXiv:2502.00397, CC URL, NON-COMMERCIAL callout) alongside YuNet (MIT), EdgeTAM (Apache-2.0), TransNetV2 (MIT), LR-ASD (MIT); points at the vendored _vinet_s/_transnetv2 LICENSE files. TDD, renderer suite 100% line+branch. - Add a repo-root NOTICE file and a CHANGELOG v1.3 "Legal" note stating the app is non-commercial while ViNet-S is bundled and a future paid tier must remove/replace it. Claude-Session: https://claude.ai/code/session_01W4LPidyKbYghtHeCHqrpAy
…nse filename Add a LICENSE glob to the electron-builder sidecar extraResources filter so the vendored model LICENSE files (features/_lightasd MIT, _transnetv2, _vinet_s CC-BY-NC-SA) ship in the installer, matching ffmpeg LGPL LICENSE.txt. Glob is relative to the from base (../sidecar/media_studio), same as the sibling py filter. Fix python-embed-setup.ps1 ffmpeg license copy that appended .txt to a name already ending in .txt (LICENSE.txt.txt); now preserve an existing extension and only append when extensionless. Config/script-only; no source/test/logic change; coverage unaffected. Claude-Session: https://claude.ai/code/session_01W4LPidyKbYghtHeCHqrpAy
…title
The renderer index.html title was "media-studio". Once the renderer loads,
document.title overrides the BrowserWindow title ("Reframe") set in main.ts,
so the running window title bar showed "media-studio". Set it to "Reframe".
brand.test.ts now asserts the index.html title is "Reframe" and includes it
in the no-leak audit surface list. Internal identifiers (package name, appId,
artifactName, %APPDATA%/media-studio data root, sidecar _APP_DIR_NAME /
ENV_SENTINEL) are unchanged.
Claude-Session: https://claude.ai/code/session_01W4LPidyKbYghtHeCHqrpAy
Future versions now upgrade WITHOUT a clean uninstall/reinstall: - app/package.json: add electron-updater (6.8.9) - electron-builder.yml: replace publish:null with a github provider (Prekzursil/Reframe) so the build emits latest.yml + the .blockmap (delta updates) + app-update.yml; signing stays OFF (no CSC) - electron.vite.config.ts: bundle electron-updater into main.js (packaging excludes node_modules, so an externalized require would be MODULE-NOT-FOUND at runtime) - main/updater.ts: testable autoUpdater state machine (injected deps) -- each event maps to one typed update.status IPC (checking/available/ progress/downloaded/none/error); autoDownload OFF; check/download/ quitAndInstall handlers degrade gracefully (never crash) - main/main.ts: packaged-only wiring; auto-check GitHub on launch once the renderer is up; quitAndInstall runs the NSIS in-place upgrade which PRESERVES userData (DPAPI keystore + settings + data root) - preload.ts: expose onUpdateStatus/checkForUpdate/downloadUpdate/ quitAndInstall via the existing contextBridge (no nodeIntegration) - renderer UpdateBanner: non-intrusive dismissible banner -- available->Download, Downloading N%, Ready->Restart to update; error gated on user engagement so a silent offline launch check stays quiet; UNSIGNED build so Windows SmartScreen may warn (noted, no signing) Tests: main state machine + renderer UI states mocked to 100%. Renderer vitest --coverage 100% line/branch/func/stmt; tsc clean; biome clean (only a pre-existing buildProxyJob format diff, out of scope). Sidecar untouched. Claude-Session: https://claude.ai/code/session_01W4LPidyKbYghtHeCHqrpAy
Prevent concurrent bootstraps from corrupting the runtime env + library.db. - app.requestSingleInstanceLock() before window/sidecar spawn; a second launch of the same copy focuses/restores the running window and quits. - DATA-ROOT-scoped lockfile in the resolved (relocatable) data folder holding the owner pid: a second DIFFERENT copy aimed at the same data root that finds a LIVE holder aborts spawning and surfaces a loud contention banner; a stale lock from a dead/crashed holder is reclaimed; released on quit only if ours. - Extract the lock DECISION logic into pure, Electron-free dataRootLock.ts (serialize/parse, decide, stale-reclaim, release) at 100% coverage; keep the requestSingleInstanceLock + fs/process seam wiring in main.ts thin. Claude-Session: https://claude.ai/code/session_01W4LPidyKbYghtHeCHqrpAy
added 2 commits
July 8, 2026 05:10
…s the real reframe test_no_speech_export_still_produces_valid_mp4 drives shortmaker.export with the claudeshorts reframe, which hard-requires the sha256-pinned YuNet ONNX. It passed locally only because the dev box had YuNet globally provisioned; on a fresh CI runner it hit "the YuNet face-detection model is not provisioned". Provision YuNet via the real assets.ensure (the same step first-run / the app-e2e provisionAssets perform, ~230KB pinned-HF download) and point MEDIA_STUDIO_CONFIG_DIR at the test data root so first-run provisioning (assets.ensure -> Services.data_dir) and the export-time resolver (default_config_dir()) agree — they coincide in production (register_all leaves data_dir=None), but this test passes an explicit data_dir. No assertion changed. Verified locally: RED->GREEN, full pytest -m e2e = 47 passed, 4 skipped. Claude-Session: https://claude.ai/code/session_01CaUSwiidebWvpgMs2wQgqL
…lf-diagnosing Two independent hardening changes (neither is confirmed to fix golden-journey — its reframe failure cause is still under investigation; see below): 1. Pin opencv to 4.13.0.92 in both e2e install steps. CI installed opencv unpinned and resolved OpenCV 5.0.0.93, while the shipped app + pyproject pin 4.13.0.92 (runtime_setup/requirements-sidecar.txt:27). CI should exercise the version that actually ships, not a newer major — reproducibility hygiene. (An A/B proved 5.x is NOT itself the reframe breaker: FaceDetectorYN/YuNet create+detect work on 5.0.0, correct 15-col output; so this pin aligns CI with prod but is not the root fix.) 2. Capture the app main/sidecar stdout+stderr in golden-journey and fold its error tail into the done-signal failure message. The export job's real error (reframe/cv2/model) surfaces on the sidecar stream, NOT in Playwright's error-context.md, so today a red repro shows only the on-disk intermediate snapshot. This makes the keystone test name the failing stage (fulfilling its own documented diagnostic intent) so the next CI run reveals golden-journey's true cause. Claude-Session: https://claude.ai/code/session_01CaUSwiidebWvpgMs2wQgqL
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
When the claudeshorts reframe cannot find the YuNet model it fails LOUD (correct), but the message did not say WHERE it looked. golden-journey provisions YuNet yet still hit this on CI, and a falsify-probe showed the resolver root == the ensure root when MEDIA_STUDIO_CONFIG_DIR is set — so the app-context divergence is NOT the known resolver asymmetry (which only bites Services(data_dir=X) with no config dir). Append the resolver root + MEDIA_STUDIO_CONFIG_DIR + default_config_dir() to the error so the next real-pipeline failure names the exact path divergence instead of only "not provisioned". Straight-line in the already-covered None branch (no new branch); the "not provisioned" substring is preserved (assertions unchanged). basedpyright clean; 144 reframe tests pass. Claude-Session: https://claude.ai/code/session_01CaUSwiidebWvpgMs2wQgqL
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
…y downloads The diagnostic round proved the reframe resolver looked in EXACTLY the right place (resolver root == MEDIA_STUDIO_CONFIG_DIR == default_config_dir == the data root) — so it was never a path/resolver problem. installed_path checks file-exists+size, so YuNet simply was not on disk: the e2e-gui job installs `sidecar[dev] --no-deps` + only opencv, so it had NO httpx; the asset downloader lazily imports httpx, so provisionAssets' YuNet download threw, assets.ensure GRACEFULLY SKIPPED it but still emitted job.done, and the app reframe correctly reported "not provisioned". A CI-harness dep gap, not a product bug. Fixes: - e2e-gui: install httpx (+ huggingface_hub) so the YuNet download works. - fixtures.provisionAssets: VERIFY the requested assets are in the job.done `installed` list and capture the sidecar stderr — a silent download skip now fails LOUD at setup with the real error instead of surfacing as a mystery reframe failure. - real_pipeline_smoke: provision YuNet (verified) + set MEDIA_STUDIO_CONFIG_DIR to the tiny-sidecar data root so first-run provisioning and the export-time model lookup agree (the config==data invariant production always holds; the launcher passes an explicit data_dir). Claude-Session: https://claude.ai/code/session_01CaUSwiidebWvpgMs2wQgqL
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
…prove The app is proven end-to-end on the shipping platform: golden-journey PASSES on Windows (15.4s) + Linux, preview.spec passes everywhere, packaged isPackaged/asar pass. Two legs assert things a CI window cannot reach; scope them per their real behaviour (the full checks move to the clean-box first-run smoke, app/e2e/README): - golden-journey: SKIP on macOS. macOS is a NON-SHIPPING platform (electron-builder win: only). The full pipeline runs the libass caption stage, and brew-ffmpeg on the macOS runner rejects the `subtitles=` filter path-quoting that Linux + Windows ffmpeg accept — a macOS-runner ffmpeg quirk, not a product defect. Full pipeline still runs on Windows (shipping) + Linux; preview.spec keeps macOS GUI coverage. - packaged.spec:84/99: a COLD packaged first-run pip-installs the heavy sidecar runtime (multi-minute) before the sidecar answers RPCs, so the post-provisioning shell (.app__brand) and a completed bootstrap cannot appear in a 30s CI window. What the packaged app DOES render — driven by the main-process getProvisioningState — is the FirstRunSetup gate. Assert THAT (packaged renderer boots + shows a live setup UI, not a blank screen) + the seeded-env wiring. The real post-provisioning shell + full first-run are verified on the dev build (preview.spec) + the clean-box smoke. No product code touched; assertions retarget un-reachable CI states to the correct CI-observable behaviour. typecheck:e2e clean. Claude-Session: https://claude.ai/code/session_01CaUSwiidebWvpgMs2wQgqL
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
…ation) The one decisive check CI cannot cover — the cold packaged first-run pip-installs the sidecar runtime + downloads models (multi-minute) — captured as an exact, hand-runnable repro: install, verify the FirstRunSetup screen (not blank), let provisioning finish, do a real transcribe→reframe→export, and the optional 1.4.0→1.4.1 auto-update smoke. Referenced by packaged.spec:84/99. Claude-Session: https://claude.ai/code/session_01CaUSwiidebWvpgMs2wQgqL
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
+38
to
48
| if injected is None: | ||
| return handler(params, ctx) | ||
| with svc.settings.key_overlay(injected): | ||
| return handler(params, ctx) | ||
|
|
||
| return wrapped | ||
|
|
||
|
|
||
| def register_all( | ||
| services: Services | None = None, | ||
| *, |
There was a problem hiding this comment.
Bug: A threading bug causes the decrypted key overlay to close on the main thread before the worker thread can access the keys, leading to authentication failures for AI operations.
Severity: CRITICAL
Suggested Fix
The key overlay context needs to be active on the worker thread where the provider_factory is executed. This can be achieved by passing the injected keys to the AI job and applying the overlay within the job_body on the worker thread, rather than wrapping the handler on the main thread.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: sidecar/media_studio/handlers/composition.py#L12-L48
Potential issue: A threading issue exists where the key overlay context, responsible for
providing decrypted API keys, closes on the main RPC thread before the worker thread
attempts to use them. The `_run_ai_job` function returns immediately, causing the
`_key_overlay_wrapper` context to exit. Later, the worker thread executes the
`provider_factory`, which calls `settings.get_raw()`. Since the overlay is no longer
active, `get_raw()` returns redacted key markers instead of raw keys. This leads to
authentication failures for all AI operations that egress to an external provider, such
as `director.plan` and `director.apply`.
Also affects:
sidecar/media_studio/handlers/ai_ops.py:382~429
Comment on lines
+165
to
+170
| if not isinstance(keys, list) or index >= len(keys): | ||
| raise _invalid(f"providers.revealKey: no key at index {index} for {provider_id!r}") | ||
| key = keys[index] | ||
| if not isinstance(key, str) or not key: | ||
| raise _invalid(f"providers.revealKey: no key at index {index} for {provider_id!r}") | ||
| return {"key": key} |
There was a problem hiding this comment.
Bug: The providers_reveal_key handler can return a redacted key marker instead of the real key when the secure keystore is unavailable, as the validation logic is insufficient.
Severity: HIGH
Suggested Fix
The providers_reveal_key handler should add validation to ensure the key it is about to return is not a redaction marker. For example, it could check if the key starts with the redaction ellipsis "…". Alternatively, if no raw key is found in the overlay, the handler should raise an error instead of falling back to the on-disk value.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: sidecar/media_studio/handlers/providers_ops.py#L165-L170
Potential issue: The `providers_reveal_key` handler can return a redacted key marker
(e.g., `"…WXYZ"`) instead of the actual API key. This occurs if the application is
running in a "session-only" mode where the secure keystore is unavailable, for instance,
after a restart or on a new machine. In this scenario, the key overlay provided to the
backend is empty. The handler reads the on-disk settings, which contain redacted
markers, and since the overlay has no raw keys to apply, it returns the marker. The
handler's validation incorrectly permits this marker, as it is a non-empty string.
My provisioning edit in da2f6c8 drifted from ruff-format; the pre-commit gate-lint-format runs `ruff format` (not just `ruff check`, which passed), so it red the REQUIRED quality gate. Reformat to match. No logic change. Claude-Session: https://claude.ai/code/session_01CaUSwiidebWvpgMs2wQgqL
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
…aseline regen
The Windows-only VISUAL + A11Y suite navigated by stale tab labels (a "Create" tab
that is now "Make Shorts", "Repurpose" now "Edit"), timing out 30s and reding the
Windows leg. Fix the nav in surfaces.visual + a11y.a11y (the visual step runs both
via testMatch **/*.{visual,a11y}.spec.ts). One selector drifted: the Make Shorts
root is `.make-shorts` (MakeShorts.tsx:161); `.shorts` is now sub-tab-scoped and
`.create` never existed. Screenshot filenames kept stable to preserve baselines.
Note: the old Repurpose batch surface moved into Make Shorts → Batch, so the (kept)
repurpose.png now depicts the Edit tab's selected state — a deliberate, documented
retarget, not a silent one.
Also add a `workflow_dispatch` input `update_visual_baselines`: when true the Windows
leg runs `test:e2e:visual:update` (regenerating the *-win32.png baselines that
legitimately drifted with the UI evolution) and uploads them as an artifact for
review + commit; when false/absent the normal blocking diff is UNCHANGED.
No product code touched. typecheck:e2e clean.
Claude-Session: https://claude.ai/code/session_01CaUSwiidebWvpgMs2wQgqL
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
Comment on lines
+316
to
+326
| if not force and not self._original_exists(row): | ||
| raise KeepCopyError( | ||
| f"refusing to evict the managed copy for {entity_id}: its original source " | ||
| f"{row['original_path']!r} is gone, so the managed copy is the only " | ||
| f"surviving copy of the video (pass force=True to destroy it anyway)" | ||
| ) | ||
| to_delete = self._evict_row_db(conn, row) | ||
| for victim_file in to_delete: | ||
| with suppress(FileNotFoundError): | ||
| victim_file.unlink() | ||
| return {"ok": True, "entityId": entity_id} |
There was a problem hiding this comment.
Bug: The evict() and clear() methods perform multiple database writes without a transaction, risking an inconsistent state if a partial failure occurs.
Severity: HIGH
Suggested Fix
Wrap the database operations within the evict() and clear() methods in an explicit transaction using conn.execute("BEGIN") and conn.execute("COMMIT"), with a try...except block to call conn.execute("ROLLBACK") on any failure. This pattern is already correctly used in the keep_copy() function.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: sidecar/media_studio/keepcopy.py#L303-L326
Potential issue: The database connection is configured in autocommit mode. The `evict()`
and `clear()` methods call `_evict_row_db()`, which executes an `UPDATE` on the `entity`
table and a `DELETE` from the `managed_copy` table. These two operations are not wrapped
in an explicit `BEGIN`/`COMMIT` transaction. If an error occurs after the `UPDATE`
succeeds but before the `DELETE` completes (e.g., due to disk I/O errors, permissions
issues, or DB corruption), the database will be left in a permanently inconsistent state
where an entity's path is updated but the corresponding `managed_copy` row is not
removed.
Also affects:
sidecar/media_studio/keepcopy.py:328~352
The a11y/axe suite (first CI run) caught 3 real serious/critical violations on the Workspace tab strip (shared TabBar, grouped mode), latent since it never ran CI: - aria-required-parent / aria-required-children (critical): role="tablist" owned the group <section aria-label> wrappers (→ role=region) + the non-tab Export/Advanced buttons, so the role="tab" buttons resolved their ARIA parent to a region, not the tablist. Fix: the group <section>s carry role="presentation" (+ drop aria-label; the visible caption stays aria-hidden), flattening them so their tabs surface as direct tablist children; and the role="tablist" moves to an INNER div wrapping ONLY the tab clusters, with the Advanced toggle + Export rendered as its SIBLINGS. The tablist now owns exclusively role="tab" elements. - color-contrast (serious): .tabbar__group-label was --text-muted × opacity:0.62, which blended to #767c8a = 3.85:1 on #1b212e (< 4.5:1). Fix: drop the opacity; the token itself is ~7.7:1. Quietness preserved by the 11px tracked-caps treatment. Product behaviour unchanged (tab switching, Export, Advanced disclosure). Renderer vitest 2945 tests + 100% coverage (verified locally); tsc + biome clean. The Workspace visual baseline shifts slightly (more-legible labels) and is re-captured. Claude-Session: https://claude.ai/code/session_01CaUSwiidebWvpgMs2wQgqL
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
Comment on lines
+235
to
+239
| try: | ||
| transport(url, {}, {}, timeout) | ||
| except ProviderError as exc: | ||
| return exc.status_code or 0 | ||
| return _HEALTH_OK |
There was a problem hiding this comment.
Bug: The readiness probe waits for the full timeout on definitive HTTP errors like 404 instead of failing fast, delaying feedback on misconfigurations.
Severity: MEDIUM
Suggested Fix
Modify readiness_probe to distinguish between transient errors (like 503 or connection refused) and definitive errors (like 4xx or non-503 5xx codes). The probe should continue to wait for transient errors but should raise an exception immediately upon receiving a definitive error code, providing faster and more specific feedback to the user.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: sidecar/media_studio/models/provider.py#L235-L239
Potential issue: The readiness probe in `readiness_probe` incorrectly handles definitive
HTTP error codes. When the health check endpoint returns a status like 404 (Not Found)
or 401 (Unauthorized), which can occur due to a misconfigured URL, the probe treats it
as a transient 'still loading' state, similar to a 503 status. Instead of failing fast
and reporting the specific configuration error, the probe continues to poll until the
full timeout is reached (e.g., 60 seconds). This results in a significant delay before
the user is notified of a problem with a generic timeout message, hindering quick
diagnosis of configuration issues.
Bump app/package.json 1.4.0 -> 1.4.1 (drives electron-builder artifactName + latest.yml on the GitHub-Releases auto-update feed), README download links, and a CHANGELOG [1.4.1] entry (vision-jobs egress fix, renderer white-screen resilience, Workspace tab-strip a11y/contrast, reframe/GPU runtime pins). Refresh the 3 Windows visual baselines that legitimately shifted with the UI evolution + the a11y fix (CI-captured via the update_visual_baselines regen dispatch, then reviewed screenshot-by-screenshot — no regression hid in the drift): - workspace-preview: the a11y ARIA restructure is visually invisible; only the tab group labels are now legible (the contrast fix). - settings-models-system + library: design-token / readiness drift. The other 5 baselines matched within threshold (unchanged). Known minor (flagged, non-blocking): two Library readiness rows render a `?` mojibake where a dash should be (em-dashes render fine elsewhere — likely a CI-font-glyph artifact); to be confirmed on the clean-box smoke. Claude-Session: https://claude.ai/code/session_01CaUSwiidebWvpgMs2wQgqL
Contributor
Analysis Failed
Troubleshooting
Retry: |
Comment on lines
+783
to
793
| rejectBuild(new Error(result.error.message ?? `proxy build failed for ${videoId}`)); | ||
| } else if (typeof result.path === 'string' && result.path !== '') { | ||
| resolveBuild(result.path); | ||
| } else { | ||
| rejectBuild(new Error(`proxy build for ${videoId} returned no path`)); | ||
| } | ||
| }; | ||
| sc.on('done', onDone); | ||
| }), | ||
| ); | ||
| } |
There was a problem hiding this comment.
Bug: The Sidecar.restart() method doesn't remove event listeners like 'done', causing listeners to accumulate on each restart and leading to a memory leak.
Severity: MEDIUM
Suggested Fix
Update the Sidecar.restart() method in app/main/sidecar.ts to remove all event listeners from the old sidecar instance before creating a new one. Calling this.removeAllListeners() within the restart method would clear listeners for events like 'done' and prevent leaks.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: app/main/main.ts#L774-L793
Potential issue: When a media proxy is built via `buildProxyJob`, it registers an
`on('done', ...)` listener on the sidecar. If the sidecar crashes and restarts before
this 'done' event is emitted, the listener is never cleaned up. The `Sidecar.restart()`
method only removes `stdout` and `stderr` listeners, leaving other event listeners
attached. Repeated restarts, which can occur automatically, will cause these listeners
to accumulate, leading to a memory leak, Node.js `MaxListenersExceededWarning` warnings,
and eventual performance degradation.
main/brand.test.ts pins package.json.version; v1.4.1 bump made it assert the old 1.4.0. Update the guard to 1.4.1 (title + assertion). No behaviour change. Claude-Session: https://claude.ai/code/session_01CaUSwiidebWvpgMs2wQgqL
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
… injection (CodeQL) CodeQL's first scan of the never-CI'd v1.4 line flagged 11 alerts on the DPAPI keystore. Fixed with REAL guards (no dismissals): - js/path-injection (188/197/198/208/212/217/237): a `safeFilePath()` barrier (path.resolve + basename + startsWith(dir+sep)) re-derives each keystore/settings file path and fails closed on any escape — the exact containment sanitizer main.ts dataRootChild / exportPath already use, which CodeQL recognizes. - js/remote-property-injection (171/302): an `isSafeProviderId()` denylist (__proto__/constructor/prototype + path/JSON separators) gates every provider-id property write — a genuine prototype-pollution guard for renderer-sourced ids (also added defensively to writeKeystore). - js/file-system-race (212, TOCTOU): shredFile now uses an atomic openSync(safe,'r+') (must-exist, never-creates) + ftruncate, removing the existsSync-then-write window while preserving the exact missing→false / existed→true contract. Product behaviour unchanged: DPAPI encrypt/decrypt round-trip, key read/write/shred, migration. Verified: tsc clean, keystore 31/31 + keyBridge 29/29 + ipc 7/7, biome clean. The 10 remaining alerts (main.ts + a test fixture) are dismissed as contextual false positives (validated-data-root paths via dataRootChild; array-form no-shell spawn). Claude-Session: https://claude.ai/code/session_01CaUSwiidebWvpgMs2wQgqL
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
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
2 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.
v1.4 — the experience overhaul, finished + hardened + CI-validated
This brings the whole v1.4 line to
main: the experience overhaul (cool-blue redesign, in-app first-run, Library provenance, opt-in keep-a-copy, in-place auto-update), the Make Shorts pipeline fixes (the real-GUI golden-journey E2E is fully green), and a post-ship remediation that runs the branch through CI for the first time.What this does
WU1 — green
main+ reconcile. dependabot #259 desynced the CUDA torch pins in three places (chatterboxtorchstripped+cu128+ mismatchedtorchaudio; the pyprojectreframe-gputrio broken with a duplicated line; an untested cuDNN bump). Reverted the CUDA-critical bumps to the dev-venv-validated pins, kept the benign ones (ctranslate24.8.1,huggingface_hub1.22), and hardened the pin test to enforce torch/torchaudio pairing ++cu128— now covering the pyproject extra too (the exact class that slipped CI). Landed via #260 (main green);git log feat/reframe-v1.4..origin/mainis now empty.WU2 — renderer resilience. A tested
ErrorBoundarywraps<App/>; every eager-rpc site is guarded sync-safely (the sharedbridge()throws synchronously whenwindow.apiis missing, so.catch()can't catch it); the preloadcontextBridgeis wrapped;main.tsgains render-process-gone / uncaughtException recovery. Defense-in-depth against a white-screen-on-mount.WU3 — e2e test-debt + a real bug. The opt-in e2e-ai2 suite had rotted after the per-request key-injection change (the test drives never injected keys). Rewrote them to replicate production's DPAPI injection and removed both
xfail(strict=True)markers — the "consent leak" and "auth corruption" they described were already fixed (director.plan is text-consent-gated; the embedder does useget_raw()). But the fresh-verify uncovered a genuine shipped bug: cloud vision egress in jobs (thumbnail.select,phase8.selecttier≥2) resolved its provider on the worker thread after the key-overlay closed → a real user with a cloud vision provider would crash / silently degrade. Fixed (vision_ops.py/system_ops.py: captureget_raw()synchronously + thread the snapshot, mirroringindex.build).WU4/5/6 — sweep, docs, ship. Both coverage lanes 100%, e2e lane green, ruff/tsc/biome/charter clean. README → 1.4.0, CHANGELOG
[1.3.0]backfilled, drawtext tests skip on fontless hosts (fontconfig probe), and preview.spec's stale"Create"→"Make Shorts"tab locator fixed.Verification (local; CI is the authoritative gate)
Known follow-ups (honest)
phase8.selecttier≥2 vision fix is applied symmetrically but not e2e-covered (the suite is tier<2).