Repository navigation
Feature/region capture support - #150
Closed
dimitar-radenkov wants to merge 3 commits into
Closed
dimitar-radenkov wants to merge 3 commits into
dimitar-radenkov wants to merge 3 commits into
Conversation
- Support --help/-h/help and --version/-v/version commands in CLI; print plain text and exit 0, taking priority over other args - Accept short aliases (-m, -s, -f, -r) and --flag=value syntax for all CLI options - Improve CLI usage/help text to document new commands, aliases, and inline value support - Add FfmpegDirectVideoWriterFactory.GetAvailability() to report ffmpeg presence, path, and source without process start - Add MCP resource pointframe://server-info for server version, desktop testing flag, and ffmpeg availability - Update README.md and knowledge-base.md for new CLI commands, flag forms, MCP resources, and usage - Add xUnit tests for CLI parsing, help/version handling, short/inline flag support, and MCP resource responses - Add test collections to prevent parallel test execution when mutating POINTFRAME_FFMPEG_PATH - Add docs-sync tests to ensure CLI/MCP documentation matches code and registered tools/flags, failing CI on drift
- Flags like --monitor, --seconds, --fps, and --redact now return specific usage errors if missing a value, not a generic error. - --fps validates its value is an integer between 1 and 60, with clear error messages for invalid input. - CLI entry point parses args before constructing services, so --help, --version, and usage errors do not depend on service availability. - Refactored CLI logic to separate argument parsing/help/version from service construction and command execution. - Added/expanded unit tests for missing-value errors, --fps validation, help/version handling, and ensuring usage errors do not construct services.
There was a problem hiding this comment.
🟡 Changes recommended
The updated documentation describes JSON fields in camelCase (e.g., success, schemaVersion), but the implementation serializes PascalCase property names, which will mislead API/CLI consumers.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR expands Pointframe’s standalone automation surface (CLI + MCP server) with whole-monitor MP4 recording support and new “status/health” capabilities, while tightening documentation/tooling drift checks via new unit tests.
Changes:
- Add CLI
recordcommand (plus--help/--versionhandling, option aliases, Ctrl+C graceful stop) and packaging support to optionally bundleffmpeg.exe. - Add MCP
get_recording_statustool andpointframe://server-inforesource (version + desktop-testing flag + ffmpeg availability). - Replace the hand-maintained MCP tool catalog with reflection-based discovery and add docs/packaging sync tests to prevent drift.
Knowledge base: updated in this PR; no further knowledge-base changes appear necessary.
File summaries
| File | Description |
|---|---|
| README.md | Updates CLI + MCP feature overview to include recording and new status/health surfaces. |
| Pointframe.Tests/Mcp/PointframeMcpResourcesTests.cs | Tests for new pointframe://server-info resource and regression check for commands resource. |
| Pointframe.Tests/Mcp/DirectRecordingMcpServiceTests.cs | Adds tests for recording status JSON output. |
| Pointframe.Tests/Mcp/DesktopTestingToolContractTests.cs | Asserts tool catalogs include get_recording_status and match tool method registration. |
| Pointframe.Tests/Engine/FfmpegDirectVideoWriterFactoryTests.cs | Tests ffmpeg path discovery/availability reporting. |
| Pointframe.Tests/Engine/DirectRecordingServiceTests.cs | Adds engine-level recording status tests and enforces env-var isolation collection. |
| Pointframe.Tests/Engine/DirectFfmpegPathOverrideCollection.cs | Disables parallelization for tests mutating POINTFRAME_FFMPEG_PATH. |
| Pointframe.Tests/DocsSync/McpToolDocumentationTests.cs | Ensures README docs mention every registered MCP tool. |
| Pointframe.Tests/DocsSync/McpPackagingScriptDocumentationTests.cs | Ensures packaging script expected-tool arrays match the reflective catalog. |
| Pointframe.Tests/DocsSync/CliDocumentationTests.cs | Ensures docs/cli/README.md tracks CLI help text commands/flags. |
| Pointframe.Tests/Cli/CliApplicationTests.cs | Adds CLI parsing + run-path coverage for help/version and new record behavior. |
| Pointframe.Mcp/Tools/PointframeMcpTools.cs | Adds MCP tool get_recording_status. |
| Pointframe.Mcp/Services/DirectRecordingMcpService.cs | Exposes engine recording status through MCP service layer. |
| Pointframe.Mcp/Resources/PointframeMcpResources.cs | Adds pointframe://server-info resource and version resolution logic. |
| Pointframe.Mcp/Resources/PointframeCommandCatalog.cs | Switches to reflection-derived tool identifiers (no hand-maintained lists). |
| Pointframe.Mcp/Models/McpToolResponses.cs | Adds response DTOs for recording status and server-info/ffmpeg availability. |
| Pointframe.Mcp/Models/DirectRecordingResponse.cs | Removes MCP-local DTO (moved to shared engine models). |
| Pointframe.Mcp/Mappers/McpResponseMapper.cs | Maps recording status into structured MCP response types. |
| Pointframe.Engine/Recording/Services/IRecordingServices.cs | Adds GetStatus() to IDirectRecordingService. |
| Pointframe.Engine/Recording/Services/FfmpegDirectVideoWriter.cs | Adds FfmpegAvailability + discovery helper for health checks. |
| Pointframe.Engine/Recording/Services/DirectRecordingService.cs | Implements GetStatus() with locking to expose active session/elapsed time. |
| Pointframe.Engine/Recording/Models/DirectRecordingModels.cs | Adds shared DirectRecordingResponse and DirectRecordingStatus DTOs. |
| Pointframe.Cli/Program.cs | Adds Ctrl+C handling and passes cancellation to CLI runner. |
| Pointframe.Cli/Commands/CliCommandParser.cs | Adds full CLI parsing for record, help/version, aliases, and inline --flag=value. |
| Pointframe.Cli/Commands/CliCommand.cs | Extends command model to carry record parameters. |
| Pointframe.Cli/Application/CliApplication.cs | Adds record execution flow + help/version short-circuit before service construction. |
| packaging/test-mcp-stdio.ps1 | Updates expected tool list to include get_recording_status. |
| packaging/build-cli-package.ps1 | Adds optional -FfmpegPath to bundle ffmpeg into the CLI package. |
| docs/mcp-desktop-testing/README.md | Documents new direct tool identifier get_recording_status. |
| docs/knowledge-base/knowledge-base.md | Updates KB for new CLI/MCP capabilities and reflective tool catalog behavior. |
| docs/cli/README.md | Documents CLI recording support, new flags, and packaging requirements. |
Review details
Suppressed comments (1)
docs/cli/README.md:139
- Same as above: this paragraph refers to a
"success": falseJSON field, but the serialized response uses"Success"(PascalCase).
to standard output. `record` failures that the engine reports as a structured
error (rather than an exception) print a `"success": false` JSON response to
standard output instead, so scripts can parse the failure the same way as a
success.
- Files reviewed: 31/31 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
No description provided.