Add ui record command for MP4 capture#599
Merged
Merged
Conversation
Adds a new `winapp ui record` command that captures a window or element region to H.264 MP4 using Windows Graphics Capture and Media Foundation, with duration, FPS, max-edge downscale, and optional screen capture for overlays/popups. Also updates the generated CLI schema and UI automation skill/docs to surface the new command. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new winapp ui record subcommand that captures a target window (or a single element's region) to an H.264 MP4. Frames are grabbed via Windows Graphics Capture (with PrintWindow / screen-DC fallbacks) and encoded incrementally through Media Foundation's IMFSinkWriter, so long recordings never buffer in memory. It fits into the existing UI-automation surface alongside ui screenshot, giving agents/tests video evidence rather than only stills.
Changes:
- New recorder pipeline:
UiRecordCommand,UiAutomationService.Record(capture/crop/scale loop),WgcCapture.Record(persistent frame grabber), andMp4SinkWriterEncoder(Media Foundation encode), plusRecordOptions/RecordCaptureResult/UiRecordResultmodels and newNativeMethods.txtMF projections. - Wiring: registered in
HostBuilderExtensions,UiCommand, theIUiAutomationServiceinterface, plus test stubs/fakes and newUiCommandTests.Recordunit tests. - Docs/schema: regenerated
cli-schema.jsonand updateddocs/usage.md, the hand-written skill fragment, and the two generatedSKILL.mdmirrors.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/winapp-CLI/WinApp.Cli/Commands/UiRecordCommand.cs |
New command: option parsing, input validation, JSON envelope (validation uses the wrong error code). |
src/winapp-CLI/WinApp.Cli/Commands/SharedUiOptions.cs |
Adds --duration-sec, --fps, --max-edge options. |
src/winapp-CLI/WinApp.Cli/Services/UiAutomationService.Record.cs |
Core capture/crop/scale/encode loop and element-rect resolution (WGC-failure fallback diverges from screenshot). |
src/winapp-CLI/WinApp.Cli/Services/WgcCapture.Record.cs |
Persistent free-threaded FrameGrabber that caches the latest frame. |
src/winapp-CLI/WinApp.Cli/Services/Mp4SinkWriterEncoder.cs |
Media Foundation H.264 encoder with deterministic COM release and partial-file cleanup. |
src/winapp-CLI/WinApp.Cli/Services/RecordModels.cs |
RecordOptions / RecordCaptureResult DTOs. |
src/winapp-CLI/WinApp.Cli/Helpers/UiJsonContext.cs |
Adds UiRecordResult for AOT-safe JSON. |
src/winapp-CLI/WinApp.Cli/Helpers/HostBuilderExtensions.cs |
Registers the command handler. |
src/winapp-CLI/WinApp.Cli/Commands/UiCommand.cs |
Adds record as a ui subcommand. |
src/winapp-CLI/WinApp.Cli/Services/IUiAutomationService.cs |
Adds RecordAsync to the interface. |
src/winapp-CLI/WinApp.Cli/NativeMethods.txt |
Adds Media Foundation P/Invoke + interface projections. |
src/winapp-CLI/WinApp.Cli.Tests/UiCommandTests.Record.cs |
Command validation + JSON envelope tests. |
src/winapp-CLI/WinApp.Cli.Tests/FakeUiServices.cs / UiSessionServiceTests.cs |
Fake/stub RecordAsync implementations. |
docs/cli-schema.json, docs/usage.md, docs/fragments/skills/.../ui-automation.md, .github/plugin/.../SKILL.md, .claude/.../SKILL.md, scripts/generate-llm-docs.ps1 |
Schema + docs/skill updates for the new command. |
Notes for the author (files outside this PR, so not inline-commentable): per repo conventions, docs/ui-automation.md should gain a ### record section under ## Commands (it documents every other ui subcommand), and .github/plugin/agents/winapp.agent.md "Key subcommands" list should include ui record.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Build Metrics ReportBinary Sizes
Test Results✅ 3058 passed, 2 skipped out of 3060 tests in 762.3s (+95 tests, +116.9s vs. baseline) Test Coverage✅ 83.5% line coverage, 77% branch coverage · CLI Startup Time40ms median (x64, Updated 2026-07-16 02:15:04 UTC · commit |
nmetulev
reviewed
Jul 6, 2026
nmetulev
left a comment
nmetulev
left a comment
Member
There was a problem hiding this comment.
🤖 AI-generated review — produced by GitHub Copilot CLI at a maintainer's request. Every finding comes from an automated, multi-dimensional review plus a hands-on build-and-test of this branch in an isolated worktree. Treat
file:linereferences as pointers to verify, not gospel. Nothing was pushed. Posting as a comment (no approve / request-changes).
Verdict: 🟡 SHIP-WITH-CHANGES — The core encoder (Windows.Graphics.Capture + Media Foundation, no external deps) works well; the blockers are agent-ergonomics and generated-file sync, not the capture path.
What I actually ran
Built WinApp.Cli (Debug, ~3.5s, 0 warnings); 6/6 Record unit tests pass. Recorded Notepad via -w <hwnd>:
- 3s @15fps → valid MP4 (
ftyp/mp42, h264, 45 frames, 62 KB) ✅ --max-edge 640downscale ✅ correct aspect (1858×1166 → 640×402)- element-region crop ("File" item, 62×48) ✅
--capture-screen→mode":"screen"✅- error paths:
missing_app✅, bad HWND ✅, unwritable path →E_ACCESSDENIED✅
Encoding is a pure Windows stack: WGC (Direct3D11CaptureFramePool, works while occluded) → Media Foundation IMFSinkWriter (incremental H.264, no buffering) → SkiaSharp for crop/scale. Blocks until done; partial files auto-deleted on failure; no temp litter.
Must-fix (High)
- H1 — wrong error codes.
--duration-sec/--fps/--max-edgevalidation emitsCodeInternalError("internal_error") instead ofCodeInvalidArguments(UiRecordCommand.cs:61,67,73). Confirmed at runtime:--fps 0→{"error":{"code":"internal_error",...}}. An agent that treatsinternal_erroras transient will retry-loop on a bad argument. - H2 — agent file not updated.
ui recordis absent from.github/plugin/agents/winapp.agent.md(both the decision tree and the command reference). Copilot agents that rely on that file won't discover the command. - H3 — npm bindings not regenerated.
src/winapp-npm/src/winapp-commands.tshas no typeduiRecord(). Runnpm run generate-commandsand include the result.
Medium / Low
- M1
--capture-screenhelp says "Implies--focus" butrecordhas no--focusoption (SharedUiOptions.cs:86). - M2
mode":"screen"is used for both explicit--capture-screenand the silent WGC-init-failure fallback (UiAutomationService.Record.cs:70,102) — consumers can't detect degradation. Suggest a distinct"screen-fallback". - M3 WGC→screen fallback, element-not-found, and cancel-mid-record paths are untested.
- M4 (agent safety) default
--duration-sec 0records until Ctrl+C — an agent that omits it hangs forever with no signal. Recommend a safe non-zero default (e.g. 30s) with0as an explicit opt-in. - L1
-ohelp reads "e.g., screenshot" on a video command.
Fit & recommendation
Nicely implemented and native (no bundled ffmpeg, no downloads). The honest caveat is audience: agents can't parse H.264, so record is more a human/CI-evidence tool than an agent-perception tool — for state checks, looped screenshot is usually better. It also needs an interactive desktop + WGC-capable GPU, so it's hard to cover in CI. Worth shipping for evidence / UI-regression capture once M4 and the generated-file sync (H2/H3) land.
…s & npm sync Addresses AI review of PR #599: invalid_arguments for bad args; --duration-sec default 30 (0=until Ctrl+C); distinct screen-fallback mode; accurate --capture-screen/-o help; add ui record to agent decision file; regenerate cli-schema, skills and npm bindings; tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
nmetulev
added a commit
that referenced
this pull request
Jul 14, 2026
…ndependent multi-model (#613) ## What & why Evidence-driven improvements to the hand-written `pr-review` skill (`.github/skills/pr-review/`). Grounded in **5 real past pr-review runs** mined from the local session store — in every one the user had to *separately* ask for real testing, simplicity/necessity, or a genuine second-model opinion, and the skill's own rules ("No build/test execution", "no fixes", "stdout only") fought that intent. Concrete signals these changes address: - **pester5 (`fix/pester5-count`)** — confidence in the missing `exit 1` only came from *running* Pester 5.7.1, not from reading the diff; it also surfaced a GH Actions vs Azure DevOps `$ErrorActionPreference` divergence and a "will this regress?" question a static pass couldn't answer. - **UI-input verbs (#599–603)** — hands-on found #602's streaming watch writing a **completely empty output file** mid-run, and the user explicitly asked whether a new verb "earns its complexity over just improving `wait-for`" ("can merge" ≠ "should merge"). Turn 1 also hit a **"Policy hook failed"** sub-agent abort. - **crash-stacks (`zt/549`)** — the **JsProvider cold-cache version drift** was invisible to unit tests and only showed up for a freshly-installed user; found via E2E + red-team, not diff review. This is a **DRAFT** for review — no behavior in the repo build/tests changes (the skill is hand-written and not in any auto-generation pipeline). ## The 9 changes 1. **New "Validate findings for real" phase** (SKILL.md step 6, before Report): build the branch, install/run the CLI **as a user would** (npm pack+install, built MSIX, or global tool — explicitly *not* `dotnet run`/dev mode), scaffold a throwaway app (may delegate to `winui-dev`), exercise each changed command, then **confirm or drop** each critical/high finding with runtime evidence and record what couldn't be validated. Replaces the old "No build/test execution" rule. Every finding is tagged **validated** vs **static-only**. 2. **New `necessity-and-simplicity` dimension** (#5): does it earn its complexity? in scope for winapp's mission? smaller / fewer commands / staged? pros & cons of shipping vs *not*. Uses the real "can merge ≠ should merge", "earns its complexity", and "fills a need" framings. Registered everywhere. 3. **Strengthened `alternative-solution`**: require ≥1 **concrete named alternative** (in-repo helper *or* ecosystem tool/pattern/API) with a tradeoff, backed by a quick search — and softened the guidance that suppressed out-of-scope/necessity critiques. 4. **Reworked `multi-model`**: independent pass from the **raw diff + actual code files** (not the consolidated findings, which anchored it into a rubber-stamp), **records which model family ran**, and supports 2–3 families (opus/gemini/gpt) for high-risk PRs. 5. **Regression + threat-model rigor**: explicit **regression** analysis in `correctness` (defect vs test-gap vs intended change); a **threat-model checklist** + red-team in `security` (input-injection / process-invocation / credentials-secrets / signing / supply-chain); static-only findings tagged "needs runtime confirmation". 6. **Multi-PR / multi-target mode** (trackable sub-agent per PR) + **sub-agent fault-tolerance** — a blocked/policy tool call must not abort the sub-agent, and the orchestrator retries a dead one (from the real "Policy hook failed" incident). 7. **Opt-in follow-through**: AI-**labelled** PR review comment, a test-setup / prerequisites handoff checklist, and (only on explicit request) a follow-up fix PR. **Default stays stdout-only / no-fix.** 8. **Contract + report format**: findings carry a **Validation** status; the Coverage block gains the new dimension, a **regression** line, and a **validation** line. 9. **Internal consistency**: dimension counts, tables, fan-out list, and both examples all reflect **9 dimensions + the Validate phase**. ## Files - `SKILL.md` — new Validate phase, multi-PR mode, fault-tolerance, opt-in follow-through, updated rules/report/coverage/examples, renumbered to 9 dims. - `dimensions/necessity-and-simplicity.md` — new. - `dimensions/alternative-solution.md`, `multi-model.md`, `correctness.md`, `security.md`, `_shared-contract.md` — updated per above. All changes are confined to `.github/skills/pr-review/`. --------- Co-authored-by: Nikola Metulev <711864+nmetulev@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Part A — design change (default duration 0 + stdin stop + liveness): - A1: --duration-sec default 30 → 0 (record until stopped); updated option description to explain Ctrl+C / stdin stop semantics - A1b: UiRecordCommand description reflects new default-0 behaviour - A2: LinkedCancellationTokenSource threads Ctrl+C and stdin monitor through the same cancellation path so the MP4 is always finalized on graceful stop; StdinStopMonitor helper (testable, background thread) watches stdin when durationSec==0 && Console.IsInputRedirected — newline always stops, immediate EOF within 1 s grace is ignored (no-stdin footgun guard), EOF after grace stops; interactive users use Ctrl+C unchanged - A3: Human path "until" text updated; JSON path emits recording-started liveness event to stderr before the capture loop Part B — review findings: - H1: revert all generated version stamps 1.0.0 → 0.4.1 (cli-schema.json, plugin.json, SKILL.md YAML frontmatter, winapp-commands.ts comment) - M1: path resolution (GetFullPath + CreateDirectory) moved inside try/catch; failure emits structured invalid_arguments error + clean logger message - M2: UiElementNotFoundException thrown when selector given but not found; caught in command handler → element_not_found error; no silent fallback to whole-window recording - M3: --duration-sec > 86400 rejected with invalid_arguments; totalFrames computed with long arithmetic (no int overflow); loop also breaks on wall-clock deadline for bounded recordings - M4: Mp4SinkWriterEncoder writes to a temp sibling (random GUID name) and atomically moves temp→final only in Complete(); Dispose() deletes only the temp, never the final path — pre-existing files at the output path survive any recording failure - M5: WgcCapture.FrameGrabber accepts fps and throttles GPU→CPU readback via Environment.TickCount64 — arrivals faster than the sampling interval are discarded before the expensive copy; fps passed through StartGrabber - M6: tests updated — default duration now expects 0; EmitsInvalidArguments tests check logger message in ConsoleStdErr; new tests for element_not_found, invalid output path, pre-existing file preservation, liveness event not on stdout, and all StdinStopMonitor scenarios (newline stops, empty line stops, immediate EOF in grace ignored, EOF after grace stops, text line stops); EmitsInvalidArguments renamed from the old comment-only versions - L1: ui-automation skill fragment and full guide updated with record default-0 semantics, stdin stop mechanisms, --capture-screen, screen-fallback mode, JSON liveness event, element_not_found, and new troubleshooting rows Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
…cal selector resolution; npm uiRecord duration guard; docs default-0 sync; encoder move-state fix; liveness test fix; npm-usage regen H1 (readiness handshake): emit liveness event and arm StdinStopMonitor only after RecordAsync fires the onRecordingStarted callback (first frame captured, encoder live). Liveness now written via parseResult.InvocationConfiguration.Error (= ConsoleStdErr in tests) so tests capture it without Console.SetError. Fixed parallel-test safety in OutputCapture.Dispose — borrowed _stdOutWriter is no longer disposed, preventing Console.Error closure in concurrent tests. H2 (small-element letterbox): Added MfH264MinWidth/Height = 64 constants. ComputeTargetSize returns a 4-tuple (EncoderW, EncoderH, DisplayW, DisplayH). ProcessFrame pads frames to encoder minimum with black letterbox when content is smaller, preserving aspect ratio. 32x24 elements now produce valid MP4s without COM 0xC00D36B4 failures. H3 (canonical selector resolution): Replaced TryResolveElementRect (FindFirst, silent first- match) with FindSingleElementAsync call — same path used by ui click/hover. Ambiguous selectors return structured error with slug suggestions; no-selector path records whole window unchanged. H4 (npm uiRecord guard): Added 'ui record' -> '_uiRecordGenerated' to FN_NAME_OVERRIDES in generate-commands.mjs so the public name is reserved. Hand-written ui-record-guard.ts exports the guarded uiRecord that throws a clear Error for missing/zero/negative durationSec. Guard survives any npm run generate-commands invocation. H5 (docs default-0): Fixed stale 'Default duration is 30 seconds' in winapp.agent.md. Ran generate-llm-docs.ps1 + sync-claude-plugin.ps1 — .claude/ mirrors updated. Zero stale 30- second text remains anywhere in the repo. M1 (move-state tracking): Separated _finalized and _fileMoved state in Mp4SinkWriterEncoder. Dispose checks !_fileMoved (not !_finalized) so temp files are cleaned up if File.Move fails, with the pre-existing destination file left untouched. M2 (liveness test): Fixed test to handle WriteIndented=true multi-line JSON via regex match count instead of single-line filter. Updated assertions to use ConsoleStdErr.ToString() and parse the JSON object by bracket boundaries. Fixed CA2213 in OutputCapture with SuppressMessage. M3 (npm-usage.md): Ran generate-docs.mjs — npm-usage.md now documents uiRecord() (guarded) and UiRecordOptions. Fixed unused 'mock' import in ui-record-guard.test.ts (tsc error). Test results: 29/29 CLI tests pass; 183/183 npm tests pass. Zero version churn (0.4.1). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
H1 — StdinStopMonitor: replace wall-clock grace with encoder-readiness Task.
- StdinStopMonitor.MonitorCore now takes a Task readyTask instead of (TimeSpan
grace, Func<TimeSpan> getElapsed). Stdin is read eagerly; the stop is applied
only after readyTask completes (encoder initialized), preventing both the
immediate-EOF-runs-forever bug and the round-2 Cancel-before-encoder race.
- UiRecordCommand: StdinStopMonitor.Start now called BEFORE RecordAsync with a
TaskCompletionSource readyTcs; OnRecordingStarted calls readyTcs.TrySetResult().
- Tests: replaced wall-clock grace tests with readiness-Task tests; added
EofBeforeReady and NewlineBeforeReady latching tests (deterministic, no sleeps).
M1 — Mp4SinkWriterEncoder constructor failure orphans temp file: added
File.Delete(_tempPath) in the constructor catch block. Added an injectable
s_testFaultAfterTempCreate seam (creates the temp file then throws) inside the
try block before any MF calls, so the test exercises the actual catch-block
cleanup path without MF hardware. Moved MFStartup inside the try block so the
seam fires cleanly and mfStarted is false when MF was never started.
M2 — _uiRecordGenerated unguarded export: generate-commands.mjs now skips the
function body for underscore-prefixed names (only the Options interface is
emitted, needed for type-import by the guard). generate-docs.mjs skips
underscore-prefixed exports from the public docs. ui-record-guard.ts no longer
imports _uiRecordGenerated; it builds CLI args directly via callWinappCliCapture.
Regenerated winapp-commands.ts and npm-usage.md — _uiRecordGenerated is neither
exported nor documented.
M3 — ComputeTargetSize floors thin crops causing aspect distortion: replaced
EvenClamp (floor) with EvenRound (nearest-even via Math.Round AwayFromZero).
Example: 300x10 with maxEdge=100 now produces displayH=4 (25:1 aspect, 17%
error) instead of displayH=2 (50:1 aspect, 67% error). Added two thin-aspect
downscale tests asserting aspect error < 20% and all dims even and >= encoder min.
M4 — docs/usage.md still said default 30: changed to 0. No other stale mentions
found.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
H1 (REGRESSION): Redirected-stdin stop monitor now arms whenever stdin is
redirected (Console.IsInputRedirected), regardless of --duration-sec. Removed
the erroneous durationSec==0 gate in UiRecordCommand. Added Handler seams
(s_isInputRedirectedOverride, s_stdinOverride) for testing. Added
FakeUiAutomationService.RecordShouldWaitForCancellation flag. New tests:
Record_NonZeroDuration_WithRedirectedStdinEof_StopsEarlyAndExitsZero and
Record_NonZeroDuration_NoRedirectedStdin_DurationDeadlineWins.
M1 (docs): Updated docs/ui-automation.md stop-mechanism prose to describe the
readiness-latching behavior; removed the stale '1-second grace window' language.
M2 (ProcessFrame pixels): Added deterministic pixel tests for ProcessFrame:
black padding, centered content, crop extraction, out-of-bounds clamping, and
thin-aspect padding. All tests use solid-color source frames.
M3 (ambiguous_selector code): Added UiAmbiguousSelectorException class;
UiAutomationService.FindSingleElementAsync now throws it instead of
InvalidOperationException on ambiguous matches. Added CodeAmbiguousSelector
constant to UiJsonError. Added AmbiguousSelector helper to UiErrors. Added
catch in UiRecordCommand to emit 'ambiguous_selector' code. Updated test to
use UiAmbiguousSelectorException. New test
Record_AmbiguousSelector_EmitsAmbiguousSelectorCode asserts the JSON code.
M4 (pool resize): FrameGrabber now tracks _poolSize and calls pool.Recreate()
in OnFrameArrived when ContentSize changes. Guards against zero/invalid sizes.
Added structural unit test WgcCapture_SizeChangeDetection_ResizeTriggersRecreate.
M5 (item closed): FrameGrabber subscribes to GraphicsCaptureItem.Closed and
sets _isClosed. Recording loop in RecordAsync checks grabber.IsClosed and
breaks immediately to finalize partial video rather than padding with stale
frames. Unsubscribes on Dispose. Added test
Record_WindowClosedMidRecording_FinalizesGracefullyWithPartialVideo.
M6 (durationSec required): ui-record-guard.ts now defines UiRecordOptions as
Omit<Generated, 'durationSec'> & { durationSec: number } (required). Function
signature is uiRecord(options: UiRecordOptions) (no default). Added
_uiRecordWithCapture injectable helper for testing. index.ts exports stricter
type to shadow the generated optional one. npm-usage.md regenerated.
M7 (success-path test): Extracted buildUiRecordArgs() and _uiRecordWithCapture()
from ui-record-guard.ts. Tests verify exact CLI args, cwd forwarding, returned
result, and the public export is the guarded wrapper.
L1 (EvenFloor): ComputeTargetSize now uses EvenFloor for the long edge when
maxEdge is set, ensuring the longest display edge is ≤ maxEdge (nearest-even
can round up past the cap). Short edge still uses nearest-even. New tests
verify long edge ≤ maxEdge for odd and even cap values.
L2 (option-shaped selectors): buildUiRecordArgs now places all named options
first and emits the selector after a -- argument terminator so a selector like
'--capture-screen' is parsed as positional, not a flag. Tests verify the
terminator placement and that leading-dash selectors are safe.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
H1: guard StdinStopMonitor cancel callback against disposed CTS - Add volatile _stdinMonitorStopped flag to Handler - Set flag before linkedCts.Dispose() (ordering guarantee) - Wrap Cancel() in try/catch(ObjectDisposedException) (defense in depth) - Add 5-iteration AppDomain.UnhandledException probe test - Add [DoNotParallelize] to UiCommandTests to fix preexisting static-field race that caused intermittent test-runner hangs (s_isInputRedirectedOverride stomped across concurrent tests; now serialised like other flag-sensitive suites) H2: whole-window WGC crop uses current frame dimensions per frame - isWholeWindowWgc flag: effectiveCropW/H = sw/sh per frame (not first-frame stale) - Encoder dims stay fixed; ProcessFrame letterboxes/scales into fixed output - Test: ProcessFrame_WholWindowWgc_GrownFrame_FullFrameVsStaleSubrect M8: drain cached frame before honoring IsClosed break - TryGetLatest() + WriteFrame before finalize ensures non-empty output - Test: ProcessFrame_ClosedItemDrain_ProducesValidEncoderSizeOutput M9: clamp short edge to EvenFloor(maxEdge) in ComputeTargetSize - Math.Min(EvenRound(short*scale), evenMaxEdge) prevents short edge rounding up past cap - Tests: ExactSquare/NearSquare/BroadInvariant covering square + near-square inputs M10: dispose WGC frame before pool.Recreate - frame.Dispose(); frame = null before _pool.Recreate(...) - Structural ordering test M1: document ambiguous_selector error code - Added to docs/ui-automation.md Error codes section - Added to skill fragment troubleshooting table Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
…kill regen)/L1(status text)/L2(doc wording)/L3(npm guard)
H1: UiAutomationService.Record.cs — WGC init failure no longer silently falls back to screen
capture. New EnsureWgcFallbackConsented() helper throws a clear error when --capture-screen
was NOT requested (privacy guard); proceeds with a warning when it was explicitly requested.
3 unit tests added covering: without --capture-screen throws, with --capture-screen proceeds,
and error message mentions --capture-screen.
M4: UiRecordCommand.cs — --max-edge validation changed from 'must be >= 0' to 'must be 0
(unbounded) or >= 64 (encoder minimum)'. Rejects values like 1 that are below the encoder
floor. 3 new tests: max-edge=1 → invalid_arguments; max-edge=64 → ok; max-edge=0 → ok.
Updated existing invalid-max-edge test assertion for new message.
M5: UiRecordCommand.cs — gate 'Recording …' progress MarkupLine on !json && !quiet. The
logger-level path (--quiet sets LogLevel.Warning) already suppressed LogInformation; this
fixes the direct ansiConsole.MarkupLine that bypassed the logger. 2 new tests added.
M6: ran generate-llm-docs.ps1 + npm generate-commands/generate-docs; reverted 1.0.0→0.4.1
version churn; verified generated .github/plugin/skills/winapp-cli/ui-automation/SKILL.md
now contains ambiguous_selector and sync-claude-plugin.ps1 -Check passes.
L1: UiRecordCommand.cs — isStdinRedirected computed once before the status message; when stdin
is NOT redirected show only 'Ctrl+C'; when redirected show 'Ctrl+C, newline/EOF on stdin'.
2 new tests added for the text branches.
L2: docs/ui-automation.md line 258 — 'fails if selector is missing' corrected to 'fails if
the selector doesn't match' (omitting selector = whole-window capture, not failure).
L3: ui-record-guard.ts — guard upgraded from (durationSec > 0) to finite-integer [1, 86400].
Now rejects NaN, ±Infinity, non-integers (1.5), and values outside [1, 86400]. Same guard
applied to _uiRecordWithCapture. 5 new npm tests added.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
…allback doc/mode alignment) M1 - numeric arg validation ordering: Move all numeric/range argument validation (--duration-sec, --fps, --max-edge) to run BEFORE the missing-target check in UiRecordCommand. Previously, a command like 'ui record --max-edge 1 --json' (no target) returned 'missing_app' instead of 'invalid_arguments'. Now invalid argument values are caught regardless of whether a target is supplied. Add regression tests exercising the full command path with no target: --max-edge 1 and --max-edge 63 must yield invalid_arguments; --max-edge 64 and --max-edge 0 must pass validation and yield missing_app. M2 - screen-fallback mode alignment: The screen-fallback mode is dead code: WGC runs only when --capture-screen is NOT passed, so EnsureWgcFallbackConsented always throws in that catch block (captureScreen is always false there). Remove the unreachable 'mode = "screen-fallback"' assignment and update docs to reflect the three real reachable modes: wgc, printwindow, screen. Update docs/ui-automation.md, docs/usage.md, docs/fragments/skills/winapp-cli/ ui-automation.md, generated SKILL.md files, and the stale test that referenced screen-fallback (updated to test the consented --capture-screen path instead). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
Round-8 removed the unreachable screen-fallback capture mode but a few descriptions still listed it with the old "silently fell back" wording, which contradicts the H1 privacy guard (WGC-init failure now fails fast unless --capture-screen is passed). Align the RecordModels.Mode doc comment and the hand-written + mirrored agent docs to the real reachable modes: wgc, screen, printwindow. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
…evel window) + L1 (npm guard null/undefined) H1 — UiAutomationService.Record.cs: when a selector resolves to an element in a popup/owned window (different HWND from the session main window), the capture pipeline now retargets BOTH the capture surface and the crop origin to that element's actual top-level window (via GetAncestor GA_ROOT), instead of silently clamping the element's screen rect against the main window frame and recording a truncated sliver. - Add ResolvePopupCaptureHwnd() static helper (injectable GetAncestor/GetWindowRect delegates for unit-testing without Win32): detects popup case, calls GetAncestor to get GA_ROOT, updates captureOriginLeft/Top/srcWidth/srcHeight to popup window rect. - In RecordAsync: after element resolution, call ResolvePopupCaptureHwnd; if hwnd changed, retarget WGC grabber (restart on popup hwnd), PrintWindow hwnd, or screen-DC origin accordingly; then compute crop using retargeted origin. - Add 5 unit regression tests covering: same-window no-retarget, null WindowHandle no-retarget, popup retarget to correct HWND + origin + size, crop math gives true element dimensions (not sliver), GA_ROOT=session no-retarget. - Smoke-verified: popup button at screen (835,301) 250x125 — after fix captured as width=250 height=126 mode=wgc; before fix would have been a 1-pixel sliver. Normal whole-window + main-window-element selectors unaffected (no regression). L1 — ui-record-guard.ts: guard both uiRecord() and _uiRecordWithCapture() against null/undefined options before accessing options.durationSec; throws the same clear documented range error (mentioning durationSec) instead of a raw TypeError. Add tests: uiRecord(undefined) and uiRecord(null) each throw Error with durationSec in the message, not 'Cannot read properties of undefined/null'. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
…/pad; preserve small-element padding) When a resolved element's rect has no positive-area intersection with the capture surface (entirely offscreen or positioned outside its window), the previous code clamped the position to the surface edge and padded the 1-pixel sliver to the encoder minimum, recording garbage pixels at exit 0. Fix: add IsElementOffscreen() static helper that computes the intersection of the element rect with the capture surface (after ResolvePopupCaptureHwnd retarget) and throws UiElementOffscreenException before any clamp or encoder-min padding when the intersection is zero/negative or the element dimensions are degenerate (<= 0). UiRecordCommand catches UiElementOffscreenException and emits element_not_found (reusing existing code — no schema/doc changes needed) with an actionable 'offscreen' message. Legitimately small elements that DO intersect the capture surface (e.g. a 10x10 on-screen control) are unaffected: the intersection check passes and the existing clamp + encoder-min padding applies unchanged. Regression tests added for IsElementOffscreen() geometry (12 cases covering zero-area, negative, fully outside, partially clipped, small-but-on-surface, non-zero origin) and for the command-level UiElementOffscreenException handler (exit 1, 'offscreen' message, no output file written). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
…WND (H1 r10)
The UIA main-tree element-resolution path stamps WindowHandle = session.WindowHandle
onto every resolved element (UiAutomationService.cs stamping sites). A windowed
popup/dialog reachable via the main UIA tree therefore had WindowHandle == sessionHwnd,
causing ResolvePopupCaptureHwnd to early-return with no retarget fired. The recorder
then captured the main window instead of the popup (Paint "Save As" repro: session HWND
37555028, popup real HWND 299830604 — only "ElementFromHandle(stored HWND 37555028)"
logged, no retarget, 1161-byte MP4 of wrong surface).
Fix: add DeriveGeometryCaptureHwnd, a new geometry-derived retarget path that runs in
RecordAsync when the stamped-handle path did not fire (captureTargetHwnd == sessionHwnd).
It calls WindowFromPoint(element center) → GetAncestor(GA_ROOT) to derive the true
top-level window, then retargets when the derived window:
1. is non-zero
2. differs from the session HWND
3. belongs to the same process (same-process gate prevents retarget to an unrelated
overlapping window from another app if the center is occluded)
The helper uses the same injectable-seam pattern as ResolvePopupCaptureHwnd so it unit-
tests without live Win32. When retargeting, it updates captureOriginLeft/Top/srcWidth/
srcHeight from GetWindowRect — the same downstream machinery (WGC restart / PrintWindow
HWND update / screen-DC origin) fires via the existing if-captureTargetHwnd-!= block.
The existing popup-search retarget (stamped WindowHandle != session) is unchanged.
Tests added (DeriveGeometryCaptureHwnd):
- MainTreePopup_RetargetsToPopupWindow: H1 regression — stamped path skips, geometry
path detects popup, returns popup HWND, updates origin/size
- GenuineInWindowElement_NoRetarget: derived root == session HWND → no retarget
- SameProcessGate_DifferentProcess_NoRetarget: different PID → no retarget
- WindowFromPointReturnsZero_NoRetarget: no window under element center → no retarget
- GaRootResolvesToSessionHwnd_NoRetarget: child HWND inside session window → no retarget
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
…ate (#599 r12) Addresses review-599-r11 blockers: - C1: use strict equality (===) in ui-record-guard.ts null checks so the npm ESLint eqeqeq build gate passes. - H1: only arm the stdin-EOF stop monitor when --duration-sec is 0 (record until Ctrl+C). Timed recordings now run their full wall-clock duration instead of truncating after the first frame when stdin is a closed/redirected handle (e.g. `< nul`). - H2: derive an element's true top-level window from its UIA native-window ancestor (ResolveElementTopLevelHwnd + DeriveElementCaptureHwnd) instead of z-order hit-testing (WindowFromPoint). Following the real UIA parent chain means an overlapping same-process window is never mistaken for the element's window, fixing the r11 regression where a same-process occluder redirected capture to the wrong window. Removed the now-unused WindowFromPoint P/Invoke. Replaces the 5 DeriveGeometryCaptureHwnd_* tests with 3 DeriveElementCaptureHwnd_* tests (popup retarget, session-ancestor no-retarget incl. overlap-safety, no-native-ancestor no-retarget). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
…599 r13) Addresses review-599-r12 validated Mediums: - M1: the default output name used only second precision (recording-yyyyMMdd- HHmmss.mp4), so two concurrent recordings that both omit -o collided on one path — the encoder opens the file exclusively and the loser failed with UnauthorizedAccessException. Append a GUID suffix so default names are unique. - M2: the retained H1 regression test still asserted the pre-r12 behavior (redirected-stdin EOF cancels a TIMED recording) using RecordShouldWaitForCancellation, so after the r12 gate (monitor arms only when durationSec == 0) the fake blocked forever and the test hung. Replaced it with a test proving a timed recording never reads redirected stdin and completes via its own duration deadline. Static-only Mediums M3 (WGC resize aspect-ratio distortion) and M4 (test file exceeds the size limit) are filed as follow-up issues, not looped. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
zateutsch
approved these changes
Jul 15, 2026
zateutsch
left a comment
zateutsch
left a comment
Contributor
There was a problem hiding this comment.
This worked well for me last night - no major UX concerns.
If the implementation ends up being stable, then lgtm.
…default filename now includes GUID - Note that element-scoped recording of XAML windowed popups (flyouts, teaching tips, tooltips backed by Xaml_WindowedPopupClass) may capture the underlying main window; workaround: record the whole window or use `ui screenshot --capture-screen`. Tracked in #646. - Update default output filename docs to recording-<timestamp>-<guid>.mp4 to match the GUID-suffixed default (collision-safe concurrent recordings). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
…tionable Windows-N encoder error (#599 M12) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
…#599 M12 follow-up) 0xC00D36B0 is MF_E_PLATFORM_NOT_INITIALIZED (not MF_E_NOT_AVAILABLE) and cannot occur here since MFStartup runs first. Keep only the two canonical missing-H.264 codes: MF_E_TOPO_CODEC_NOT_FOUND (0xC00D5212) and MF_E_INVALIDMEDIATYPE (0xC00D36B4). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
Fit processed frames into the fixed encoder canvas without stretching when a resized WGC frame changes aspect ratio. Add a pixel test that verifies mismatched crop/display aspect produces black letterbox bars and a centered content band. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
Version the WGC latest-frame cache and remember the last encoded version so the close-drain path only writes a newly arrived cached frame. Add direct coverage for the drain-version decision. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
Serialize free-threaded frame callbacks with disposal so D3D resources cannot be released while a frame copy is in flight. Dispose now marks the grabber disposed, removes handlers, and tears down native resources under the callback drain lock. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
Capture screen-record frames directly into the fitted encoder frame instead of first allocating a native-size screen bitmap and only then downscaling. Add a focused size-plan regression test for --max-edge. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
Split the large record test file into themed partials, exercise encoder temp cleanup via the real publish failure path, and route WGC/stdin regression tests through production helpers. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
Add gated interactive composited-content record coverage, exercise uiRecord through the public npm package entrypoint, and guard the hand-written record argument list against generated option drift. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
Resolve the UiSessionServiceTests.cs conflict by taking main's rewritten, comprehensive test suite (a superset that subsumes the branch's two older tests); the ui-record-specific RecordAsync surface lives in production and FakeUiServices.cs, which merged cleanly. Build 0/0; UiSessionService + Record + UiCommandTests = 374 passed, 1 gated-skip. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
…-writer change 599 commit 0e4dd84 deliberately changed OutputCapture.Dispose to NOT dispose the borrowed inner writer (correct dispose semantics + fixes parallel record-test breakage from closing a shared stderr stream). main's OutputCapture_Dispose_ DisposesInnerWriter test still asserted the old dispose-inner behavior and was merged in unchanged, so it failed on CI (expected ObjectDisposedException, none thrown). Update the test to assert the borrowed writer is NOT disposed, matching the deliberate production behavior. Was the only failing test in the CI full suite. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
nmetulev
approved these changes
Jul 16, 2026
nmetulev
added a commit
that referenced
this pull request
Jul 16, 2026
… pen/touch) PR #599 (ui record) merged to main after this branch. Resolved 4 conflicts: - UiJsonError.cs: union error codes (no_target/injection_unsupported from #600 + ambiguous_selector from #599). - scripts/generate-llm-docs.ps1: union $SkillCommandMap ui-automation list to include ui record AND ui touch/ui pen. - docs/cli-schema.json, src/winapp-npm/src/winapp-commands.ts: regenerated from the merged CLI (both command sets present; version pinned 0.4.1, zero churn). Regenerated all autogenerated docs/skills/.claude mirror + npm wrapper; both generated-file parity checks pass. Build 0/0; PointerInputFrame 27 pass/2 skip, Record 50 pass/1 skip, 0 failures. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
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.
Summary
winapp ui recordto capture a window or element region to H.264 MP4--capture-screenfor overlays/popupsWhy
This adds first-class video evidence to the UI automation surface so scenarios can be recorded and reviewed instead of relying only on still screenshots.