feat!: add Git version control for editing projects - #2164
Conversation
**Feature Branch**: `speckit/005-project-git-versioning`
Session 2026-07-28: creation default on, save/close-only snapshots, fresh history on Save As, auto LFS when detected, guidance-only when Git is absent.
**Branch**: `speckit/005-project-git-versioning` | **Date**: 2026-07-28 | **Spec**: [spec.md](./spec.md) Also adds research.md, data-model.md, quickstart.md, and contracts/ (version-control-service, git-cli-invocation, coordinator-lifecycle).
**Input**: Design documents from `docs/specs/005-project-git-versioning/`
Preserve the loaded appVersion/minAppVersion on plain resave (a newer Beutl no longer dirties every project it opens; migrations advance the version via Project.MarkAsMigrated), and pin JSON serialization to LF on all platforms so cross-OS diffs stop churning. BREAKING CHANGE: Project.Serialize now writes the persisted appVersion/minAppVersion instead of unconditionally stamping the running app version, and project JSON now always uses LF line endings (Windows projects produce a one-time full-file diff on next save). Affected: Beutl.Core.
Scene Include/Exclude entries were written with OS-native separators, so a Windows-saved exclude list silently failed to match on macOS/Linux. Normalize to '/' on write and on read, and compare with normalized relatives.
Replace RandomFileNameGenerator with ElementFileNaming ({Id:N}.belm, -{index} on collision, matching the AgentToolkit convention) at all GUI element-creation sites, so element file names are meaningful and reproducible. Existing files are never renamed.
VersionControlConfig (enable-for-new-projects, auto-commit toggles, git path override, LFS policy, large-media threshold) wired into GlobalConfiguration, plus the English/Japanese UI strings for the upcoming version control surface.
GitInstallationLocator (ordered probe with macOS CLT-stub check, 2.23 floor, config override, LFS probe), GitCliRunner (no shell, GIT_TERMINAL_PROMPT=0/GIT_OPTIONAL_LOCKS=0/LC_ALL=C, timeout, cancellation kills the process tree, stale-lock event), RepositoryWatcher (500ms debounce, .git/.beutl/*.tmp exclusion), domain models, and the GitCliVersionControlService core (serialized operations, porcelain-v2 status parsing incl. conflicts, watcher-driven StatusChanged) with a real-git test fixture. Also registers the 005 active-feature context (CLAUDE.md, .specify/feature.json) and marks T001-T011 done.
… service InitializeAsync (idempotent git init -b main, generated .gitignore/.gitattributes with optional LFS tracking, local lfs install, initial commit with the Beutl-Snapshot: init trailer), CommitAllAsync (clean-tree skip, pathspec-scoped add -A, kind trailer, SkippedNoIdentity for unattended auto commits), and repo-local identity get/set. Repository transitions in place after init and the watcher is created lazily.
VersionControlCoordinator owns the per-project service lifecycle and hooks explicit Save/Save All and the new async ProjectService.Closing event; the create-project dialog gains a git-tracking checkbox (default from VersionControlConfig), the File menu/palette gain Enable Version Control backed by the compiled-binding identity dialog, and EditViewModel exposes IProjectVersionControlService to tool tabs. Includes a headless E2E test proving one snapshot per dirty save and none when clean.
… service Paged history with Beutl-Snapshot trailer parsing, per-commit changed files, capped unified diffs, pathspec-scoped restore (git restore --source + git clean -fd, ignored files preserved), branch create/switch, and the IVersionControlRestoreCoordinator seam for UI-driven restore.
VersionControlTabExtension (DockAnchor.Right) with status header, incrementally loaded commit list with localized kind badges, changed-files pane and colored diff view (compiled bindings); the coordinator implements the confirm -> safety snapshot -> close -> restore -> commit -> reopen cycle with an export-running guard and failure recovery, plus Restore to new branch. Headless E2E covers exact restore, pre-restore reachability, export refusal, and branch restore.
Repository discovery via git rev-parse --show-toplevel with a consent-required exception before using an enclosing repository; every status/add/log/restore/clean call stays pathspec-scoped to the project directory so foreign and sibling files are never staged, restored, or cleaned. Adds conflicted-state lockout (mutating members throw VersionControlConflictedException while reads keep working), a conflict-marker scanner for project files, and consent-gated stale index.lock recovery in GitCliRunner.
…s in the shell The Version Control tab collapses to per-OS install guidance when git is missing or too old and menu commands disable; the coordinator refreshes availability, runs the enclosing-repository consent flow, offers one-click stale-lock removal, and warns before opening projects containing conflict markers. Headless conflict scenarios included.
…service GetBranchesAsync/CreateBranchAsync via for-each-ref, single-origin remote management, cancellable PushAsync with stderr progress, ff-only PullFastForwardAsync with RemoteOpResult mapping (Success/AuthFailed/Diverged/Offline/Failed), the renamed IProjectVersionControlCoordinator seam covering the full UI surface, and the one-time LFS quota notice / non-blocking large-media warning policies. Real-git tests run against a local bare remote.
…ell UI Commit message box + Commit Version command (identity flow, NoChanges feedback), branch dropdown with the dirty-prompt safety-snapshot close/switch/reopen cycle and failure recovery, remote URL/Push/Pull with progress + cancel and localized divergence/auth/offline dialogs; pull runs through the coordinator cycle. No merge, rebase, reset, or force operation is exposed anywhere.
…teness checks 1000-write burst produces a bounded number of git status calls (measured 1, bound 25) with no watcher self-feedback; a single property edit commits exactly its element file (SC-002); 500-element snapshot and 200-commit history land far inside the SC-003 budgets (505 ms / 43 ms). Adds the missing version-control ContextCommandDefinitions and de-duplicates palette registrations.
…nd record the T039 gate results The palette-enumeration assertion predated the T037 ContextCommandDefinition registration; the palette now reaches the command via FindContextCommand.
Onboarding: in-tab Enable Version Control call-to-action for untracked projects, a Download Git link in the unavailable state, and localized empty-state hints for history/changes/diff. History: relative timestamps with absolute tooltips, per-kind badge colors, and a restore action bar for the selected version. Mistake-proofing: branch switching now requires an explicit Switch button (selection reverts on cancel/failure), remote controls live in a collapsed Expander, and ahead/behind show as compact arrow badges.
Below 600px the tab switches to a full-width drill-down: the history list fills the panel and selecting a version navigates to a detail view (back row with short SHA, restore action bar, changed files, full-width diff) that preserves the selection on return. At 600px and above the two-column layout with the splitter remains. The history, changes, and detail-header panes are extracted into shared compiled-binding sub-views used by both roots, and the threshold lives in an independently tested helper.
…textual SplitButton The primary action follows the workspace state — Commit when changes exist, Pull (down-count) when behind, Push (up-count) when ahead, a disabled Up to date when clean and synced, Publish branch… (set remote + push) when clean with no remote, and Cancel while a remote operation runs. The chevron flyout always lists Commit / Push / Pull / Set remote… / Cancel with proper enablement, and the inline remote URL row plus the Remote Expander are gone — Set remote… opens a prefilled dialog via a testable ViewModel callback. Shared across the narrow and wide layouts.
…n Control tab The coordinator publishes a provisional untracked service before async repository discovery finishes, and a tab restored from the saved dock layout kept that first instance forever, showing 'not tracked' until reopened. EditorService now exposes the service as a read-only reactive seam (published internally by the coordinator so consumers cannot cast it writable), EditViewModel serves both the current value and the observable, and the tab ViewModel rebinds with full state rebuild and old-service detachment on every service transition — which also fixes the same staleness when switching projects with the tab open.
The Version Control tab header loses its branch ComboBox, Switch, and New Branch controls; the window title bar (shared MainView chrome on all three platforms) gains a compact branch widget - glyph + current branch + ahead/behind arrows - visible only while a tracked project is open with git available. Clicking it opens a flyout listing local branches (current one checked; choosing another runs the existing confirm + safety-snapshot switch cycle) plus New branch. The widget stays interactive inside the drag area via AppWindow.AllowInteractionInTitleBar.
Replaces the stock-button list with a JetBrains-style popup: a Current branch header with icon and ahead/behind chips, a live case-insensitive branch filter that resets on open (with an empty-result hint), a Branches section of flat hover-highlight rows with a Fluent checkmark and bold emphasis on the current branch inside a capped-height scroll area, and a matching New branch footer row with an Add icon. All colors come from theme resources so light and dark both render correctly.
…ching styling The widget moves from the right-side title-bar cluster into the breadcrumb row (project > file, then branch) and adopts the breadcrumb's LiteButtonStyle plus its compact flyout metrics (4px presenter padding, 240px minimum width, 32px transparent action rows) so the two read as siblings.
The filter TextBox, its ViewModel state, the two resource strings, and the filter tests are removed; the flyout lists branches directly.
…ranch popups The scene-selection flyout's presenter and row ControlThemes move into Beutl.Controls/Styles.axaml as CompactFlyoutPresenterTheme / CompactFlyoutItemButtonTheme, and both popups reference them, so the branch flyout's whitespace is identical by construction: 16px text inset, 32px rows, 4px rhythm, same separators. Current-branch checkmark and the New-branch icon become trailing so labels align with scene rows.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff40b911ca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Make initialization identity atomic, serialize Git configuration reactivation with close and public operations, harden repository hygiene and Git input validation, and preserve extension-provided project migration reporting. Avoid snapshots after partial Save All failures and add deterministic lifecycle and safety regressions. BREAKING CHANGE: Beutl.Editor's InitOptions is now internal because initialization is an internal backend concern; external callers must use IProjectVersionControlCoordinator.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
tests/Beutl.UnitTests/Editor/VersionControl/GitCliVersionControlServiceTests.cs (1)
457-496: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename the test to match the exercised API and assert the recorded LFS calls.
The test name states
EnsureRepositoryHygieneAsync, but the body callsservice.InitializeAsync. The test also builds aRecordingLfsRunnerand never assertsLfsInstallCalls. Rename the test to describe initialization, or callEnsureRepositoryHygieneAsync. Add an assertion onrunner.LfsInstallCalls, or drop the recording runner.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Beutl.UnitTests/Editor/VersionControl/GitCliVersionControlServiceTests.cs` around lines 457 - 496, Rename EnsureRepositoryHygieneAsync_keeps_user_Lfs_overrides_after_the_managed_block to describe the exercised InitializeAsync behavior, and add an assertion verifying the expected runner.LfsInstallCalls value. Keep the existing attribute-file and Git check-attr assertions unchanged.tests/Beutl.HeadlessUITests/VersionControlRestoreTests.cs (3)
2941-2948: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffPrefer an internal test hook over reflection on
_configurationActivationQuiesced.The test replaces a private field of
VersionControlCoordinatorthrough reflection. This couples the test to a private implementation detail and can leave the coordinator in a state the production code never produces. TheAssert.That(readinessField, Is.Not.Null)check catches a rename, but it does not protect against a change in how the field is used. Expose aninternalseam on the coordinator for the quiescence signal, and use that seam here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Beutl.HeadlessUITests/VersionControlRestoreTests.cs` around lines 2941 - 2948, Replace the reflection-based assignment of _configurationActivationQuiesced in the affected test with an internal test hook on VersionControlCoordinator that exposes the quiescence signal for controlled setup. Update the test to assign synchronousReadiness through that hook, preserving the existing synchronization behavior without accessing private fields.
2685-2693: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCollect factory-created backends in a thread-safe collection.
createdBackendsis aList<PullCycleTestBackend>. The service factory runs on coordinator activation threads, and the test body readscreatedBackends.Countand enumerates the list from the test thread.List<T>is not safe for concurrent add and read, so this can produce a torn count or a stale view and make the assertions flaky. Use aConcurrentBag<PullCycleTestBackend>or guard the list with a lock. The same pattern appears at Line 2777 (publishedServices), Line 3103, and Line 3470.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Beutl.HeadlessUITests/VersionControlRestoreTests.cs` around lines 2685 - 2693, Replace the thread-unsafe List collections used for createdBackends and the corresponding publishedServices, createdBackends, and publishedServices instances at the other occurrences with a thread-safe collection such as ConcurrentBag<T>. Update any affected initialization or enumeration code while preserving the existing factory additions and test assertions.
5254-5254: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard
InitializationOptionsagainst concurrent access.
InitializeAsyncadds toInitializationOptions, and tests read the list by index from the test thread.List<T>does not support concurrent add and read. Use a lock or a concurrent collection so the identity-retry assertions stay deterministic.Also applies to: 5397-5397
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Beutl.HeadlessUITests/VersionControlRestoreTests.cs` at line 5254, Protect the shared InitializationOptions collection used by InitializeAsync and the test assertions from concurrent access. Update the InitializationOptions declaration and every add/index-read in the affected test flows to use consistent synchronization or a suitable concurrent collection, while preserving deterministic identity-retry assertions at both referenced locations.tests/Beutl.UnitTests/Editor/VersionControl/NestedRepositoryTests.cs (1)
568-579: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the symbolic-link test helpers into the shared base fixture. Three fixtures that all derive from
RealGitTestRepositorynow declare identical platform-aware symlink helpers, so the sameAssert.Ignorepolicy is duplicated in three places and can drift.
tests/Beutl.UnitTests/Editor/VersionControl/NestedRepositoryTests.cs#L568-L579: removeCreateFileSymbolicLinkOrIgnoreand the neighbouringCreateDirectorySymbolicLinkOrIgnore, and call the base-class helpers instead.tests/Beutl.UnitTests/Editor/VersionControl/GitCliVersionControlServiceTests.cs#L2219-L2230: removeCreateFileSymbolicLinkOrIgnoreand call the base-class helper.tests/Beutl.UnitTests/Editor/VersionControl/GitCliRunnerTests.cs#L643-L656: removeCreateDirectorySymbolicLinkOrIgnoreand call the base-class helper. Add both helpers asprotected staticmembers ofRealGitTestRepository.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Beutl.UnitTests/Editor/VersionControl/NestedRepositoryTests.cs` around lines 568 - 579, Centralize the platform-aware symlink helpers in RealGitTestRepository as protected static members, preserving their existing Assert.Ignore behavior. In tests/Beutl.UnitTests/Editor/VersionControl/NestedRepositoryTests.cs:568-579, remove both local helpers and use the base helpers; in tests/Beutl.UnitTests/Editor/VersionControl/GitCliVersionControlServiceTests.cs:2219-2230, remove CreateFileSymbolicLinkOrIgnore and call the base helper; in tests/Beutl.UnitTests/Editor/VersionControl/GitCliRunnerTests.cs:643-656, remove CreateDirectorySymbolicLinkOrIgnore and call the base helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Beutl.Editor/VersionControl/GitCliVersionControlService.cs`:
- Around line 3589-3594: Update CommitAllCoreAsync and the related flow around
FindIgnoredExistingRequiredProjectPathAsync so the pre-commit ignore probe uses
only the bounded required hygiene paths and representative media paths produced
by GetRequiredProjectRelativePaths, rather than enumerating every file beneath
resources. Remove or bypass the full-tree EnumerateRequiredProjectFiles walk,
and ensure inaccessible directories cannot cause the save to fail; alternatively
cache its result and invalidate it through RepositoryWatcher.
---
Nitpick comments:
In `@tests/Beutl.HeadlessUITests/VersionControlRestoreTests.cs`:
- Around line 2941-2948: Replace the reflection-based assignment of
_configurationActivationQuiesced in the affected test with an internal test hook
on VersionControlCoordinator that exposes the quiescence signal for controlled
setup. Update the test to assign synchronousReadiness through that hook,
preserving the existing synchronization behavior without accessing private
fields.
- Around line 2685-2693: Replace the thread-unsafe List collections used for
createdBackends and the corresponding publishedServices, createdBackends, and
publishedServices instances at the other occurrences with a thread-safe
collection such as ConcurrentBag<T>. Update any affected initialization or
enumeration code while preserving the existing factory additions and test
assertions.
- Line 5254: Protect the shared InitializationOptions collection used by
InitializeAsync and the test assertions from concurrent access. Update the
InitializationOptions declaration and every add/index-read in the affected test
flows to use consistent synchronization or a suitable concurrent collection,
while preserving deterministic identity-retry assertions at both referenced
locations.
In
`@tests/Beutl.UnitTests/Editor/VersionControl/GitCliVersionControlServiceTests.cs`:
- Around line 457-496: Rename
EnsureRepositoryHygieneAsync_keeps_user_Lfs_overrides_after_the_managed_block to
describe the exercised InitializeAsync behavior, and add an assertion verifying
the expected runner.LfsInstallCalls value. Keep the existing attribute-file and
Git check-attr assertions unchanged.
In `@tests/Beutl.UnitTests/Editor/VersionControl/NestedRepositoryTests.cs`:
- Around line 568-579: Centralize the platform-aware symlink helpers in
RealGitTestRepository as protected static members, preserving their existing
Assert.Ignore behavior. In
tests/Beutl.UnitTests/Editor/VersionControl/NestedRepositoryTests.cs:568-579,
remove both local helpers and use the base helpers; in
tests/Beutl.UnitTests/Editor/VersionControl/GitCliVersionControlServiceTests.cs:2219-2230,
remove CreateFileSymbolicLinkOrIgnore and call the base helper; in
tests/Beutl.UnitTests/Editor/VersionControl/GitCliRunnerTests.cs:643-656, remove
CreateDirectorySymbolicLinkOrIgnore and call the base helper.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 461b4ac6-9c25-4b18-8afb-68e22877b0a5
📒 Files selected for processing (17)
docs/specs/005-project-git-versioning/research.mdsrc/Beutl.Core/Project.cssrc/Beutl.Core/ProjectItem.cssrc/Beutl.Editor/VersionControl/GitCliRunner.cssrc/Beutl.Editor/VersionControl/GitCliVersionControlService.cssrc/Beutl.Editor/VersionControl/VersionControlModels.cssrc/Beutl.ProjectSystem/ProjectSystem/Scene.cssrc/Beutl/Services/VersionControlCoordinator.cssrc/Beutl/ViewModels/MenuBarViewModel.Files.cstests/Beutl.E2ETests/Scenarios/ProjectPersistenceTests.cstests/Beutl.HeadlessUITests/ShutdownPipelineTests.cstests/Beutl.HeadlessUITests/VersionControlRestoreTests.cstests/Beutl.HeadlessUITests/VersionControlSaveTests.cstests/Beutl.HeadlessUITests/VersionControlTabViewTests.cstests/Beutl.UnitTests/Editor/VersionControl/GitCliRunnerTests.cstests/Beutl.UnitTests/Editor/VersionControl/GitCliVersionControlServiceTests.cstests/Beutl.UnitTests/Editor/VersionControl/NestedRepositoryTests.cs
🚧 Files skipped from review as they are similar to previous changes (10)
- src/Beutl.Core/ProjectItem.cs
- src/Beutl.ProjectSystem/ProjectSystem/Scene.cs
- docs/specs/005-project-git-versioning/research.md
- tests/Beutl.HeadlessUITests/VersionControlTabViewTests.cs
- src/Beutl.Core/Project.cs
- tests/Beutl.HeadlessUITests/ShutdownPipelineTests.cs
- src/Beutl/ViewModels/MenuBarViewModel.Files.cs
- src/Beutl.Editor/VersionControl/GitCliRunner.cs
- src/Beutl.Editor/VersionControl/VersionControlModels.cs
- src/Beutl/Services/VersionControlCoordinator.cs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b67633e6b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d58e2ff669
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ce5fb2f10
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1dcdd3353c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Make pull recovery durable across restarts, verify marker and transition ownership at every apply boundary, and keep project open/close serialization transactional. Harden Git process, branch, environment, LFS, path, and UI integration contracts with regression coverage. BREAKING CHANGE: Beutl.Editor IProjectVersionControlCoordinator implementations must now provide pending-recovery notifications, listing/actions, and typed ProjectRecoveryResult dispositions. Beutl CreateNewProjectViewModel now requires ProjectService, IProjectVersionControlInitializer, and an identity callback, while MenuBarViewModel requires IProjectVersionControlSession.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c664b1e9a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f964e76dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
No TODO comments were found. |
Minimum allowed line rate is |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d932f8e472
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Validate branch names at each point of use, refresh repository metadata watchers, keep portable packages free of reserved metadata and links, and bound conflict-marker scans. Serialize menu command initialization and retain output-operation leases for the full lifetime of render jobs. BREAKING CHANGE: In Beutl.AgentToolkit, RenderTools now requires an IOutputOperationLeaseProvider. RenderJobManager.Enqueue now requires a caller-owned output lease; ownership transfers to the job after a successful enqueue.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b9bb2670f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Restore initialization state when initial snapshots fail, detect external repository operations before mutations, and classify local-ahead pulls correctly. Reject imported packages containing portable .git metadata aliases and harden repository discovery parsing.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6fc3daedcb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Preserve referenced symbolic-link resources in project packages, retain paged history across unchanged status refreshes, and align Git LFS policy with every advertised media extension.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84531ec3d2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Reject external Git operations across pull transitions and rollback, preserve literal Unix paths, atomically update local identity, reject CLI-special branch names, and keep BatchMode for default OpenSSH variants.
Cancel and await every accepted render job before project close, including lease release when cancellation callbacks fail. BREAKING CHANGE: In Beutl.AgentToolkit, RenderJobManager no longer implements IDisposable. Use DisposeAsync or await using so disposal cancels and drains all accepted render jobs before releasing their output-operation leases.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd9f3167d6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Jobs are serialized (single-flight) because all stills share the one RenderThread and each export | ||
| // builds its own graphics context; concurrent background renders would race those resources. | ||
| public sealed class RenderJobManager : IDisposable | ||
| public sealed class RenderJobManager : IAsyncDisposable |
There was a problem hiding this comment.
Fresh evidence beyond the existing threads is that the exact reviewed commit df9e12713581080ea7865cc7a4a3f64b74d94e1d has a feat!: subject and a ## Breaking changes body section, but no literal BREAKING CHANGE: footer, even though this line changes the public RenderJobManager contract from IDisposable to IAsyncDisposable and the commit changes other public constructors. Add the required footer with the migration and affected projects so Conventional Commits release tooling does not omit it.
AGENTS.md reference: AGENTS.md:L62-L62
Useful? React with 👍 / 👎.
| if (ShouldSkipEntry(file, isDirectory: false)) | ||
| continue; |
There was a problem hiding this comment.
Fresh evidence beyond the earlier linked-media-asset thread is that this skips structural project files too: when the opened .bep, a referenced .scene, or a .belm is a symlink (or lies beneath a linked directory), the staging copy omits it before ExportAsync reopens the temporary project at line 80. The later ExternalResourceCollector relocation cannot repair that ordering, so export either fails outright or reloads the staging project without the missing item; retain arbitrary-link exclusion while materializing referenced project files before the temporary restore.
Useful? React with 👍 / 👎.
| await _versionControlInitializer.InitializeCurrentProjectAsync( | ||
| _requestIdentityAsync, | ||
| CancellationToken.None); |
There was a problem hiding this comment.
When tracking is selected and initialization throws—for example because an initial commit hook rejects the commit, a repository lock appears, or a Git command fails—this callback lets the exception escape after the project dialog has already closed and the project was successfully created. CreateProject only handles its own persistence errors, while the menu's enable-version-control path explicitly catches and passes initialization failures to ex.Handle(); handle this call similarly so the user receives an error instead of a faulted or unhandled create command.
Useful? React with 👍 / 👎.
| int bufferedLength = overlapLength + read; | ||
| if (buffer.AsSpan(0, bufferedLength).IndexOf(s_conflictMarkerBytes) >= 0) |
There was a problem hiding this comment.
Fresh evidence beyond the earlier linked-file scanner thread is that this byte search accepts <<<<<<< anywhere in a serialized project file, without requiring a line boundary or the matching separator/end markers. A legitimate text property such as "Text": "<<<<<<< ours" therefore triggers the conflict-warning notification every time the project opens even though the JSON and Git state are valid; restrict detection to actual conflict-marker lines and preferably require the corresponding ======= and >>>>>>> markers.
Useful? React with 👍 / 👎.
| return await _versionControlCoordinator.RestoreToNewBranchAsync( | ||
| commit.Sha, | ||
| branchName.Trim(), | ||
| CancellationToken.None); |
There was a problem hiding this comment.
When the user starts “restore to new branch” for project A and the published service is replaced while the branch-name prompt is open, this continuation never checks _serviceRevision, _service, or _disposed and invokes the global coordinator with A's commit against the now-current project B. If both projects are in the same enclosing repository, A's SHA is valid there, so accepting the subsequent confirmation can restore B's files from the wrong commit; bind the prompt to the originating service generation and cancel or reject it after a rebind.
Useful? React with 👍 / 👎.
Reapply LFS hygiene when the setting changes, including across internal project transitions, and preserve custom pre-push hooks by falling back to the non-LFS policy when local Git LFS installation fails.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e2d3000bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| string? remoteUrl = await RequestRemoteUrlAsync( | ||
| HasRemote.Value ? RemoteUrl.Value : null); |
There was a problem hiding this comment.
When project A is replaced while the remote-URL dialog is open, this continuation does not check _serviceRevision, _service, or _disposed, and SetRemoteAsync therefore applies A's submitted URL to the coordinator's now-current project B. The Publish Branch path then proceeds to PushAsync, potentially pushing B to a remote selected for A; capture the originating service generation and abort after the prompt if the binding changed.
Useful? React with 👍 / 👎.
| string? branchName = await RequestNewBranchNameAsync(); | ||
| if (_disposed | ||
| || lifetimeToken.IsCancellationRequested |
There was a problem hiding this comment.
When the user opens the new-branch prompt for project A and project B becomes current before the prompt returns, rebinding cancels only _serviceBindingCancellation; this continuation checks the view-model lifetime token instead and invokes the global coordinator for B. Accepting the prompt can consequently create a branch, safety-commit dirty files, and clear undo state in the wrong project, so validate the captured service revision after awaiting the prompt.
Useful? React with 👍 / 👎.
| public MenuBarViewModel( | ||
| ProjectService projectService, | ||
| EditorService editorService, | ||
| IProjectVersionControlSession versionControlSession) |
There was a problem hiding this comment.
Fresh evidence relative to the earlier threads is that the exact reviewed commit 5f8a9af54859319d96b134f5c102b0327dcd664c is a real commit with a feat!: subject but no literal BREAKING CHANGE: footer, although this line makes the public MenuBarViewModel constructor incompatible. The ## Breaking changes body section is not a Conventional Commits footer; add the required footer describing migration and affected projects so release tooling exposes the change.
AGENTS.md reference: AGENTS.md:L62-L62
Useful? React with 👍 / 👎.
| private static readonly string[] s_lfsAttributeLines = | ||
| s_supportedMediaExtensions | ||
| .Select(static extension => | ||
| $"resources/**/*{CreateCaseInsensitiveGlob(extension)} " | ||
| + "filter=lfs diff=lfs merge=lfs -text") |
There was a problem hiding this comment.
When a project references an in-project media file under the root or a directory such as assets/clip.mp4, this generated policy does not match it because every LFS pattern is restricted to resources/**. GetLargeMediaPath repeats the same restriction, so even with the media extension supported the file receives neither LFS filtering nor the configured large-media warning and is committed directly into history; apply both policies to supported media anywhere inside the project scope.
Useful? React with 👍 / 👎.
| List<string> lockPaths = | ||
| [ | ||
| Path.Combine(gitDirectory, "index.lock"), | ||
| Path.Combine(gitDirectory, "HEAD.lock"), | ||
| ]; |
There was a problem hiding this comment.
After an interrupted fetch leaves a stale lock such as .git/refs/remotes/origin/main.lock, pull continues to fail but this recovery scan can only return index.lock, HEAD.lock, or the checked-out local-branch lock. A Git 2.43 reproduction reports cannot lock ref 'refs/remotes/origin/main' followed by the standard stale-process guidance, which makes RepositoryLockFailed fire while GetRecoverableRepositoryLock still returns null; safely recognize the lock path reported for remote-tracking refs so the existing recovery UI can remove it.
Useful? React with 👍 / 👎.
Description
Affected areas
Beutl.Engine(rendering / scene / animation / media)Beutl.ProjectSystem(project / document persistence)Beutl.Editor,Beutl.Editor.Components,Beutl.Controls)Beutl.Extensibility(extensions / plugins)Beutl.NodeGraphBeutl.FFmpegIpc/Beutl.FFmpegWorkerBeutl.ApiBeutl.Core,Beutl.Configuration,Beutl.Language)Breaking changes
Project.Serializenow preserves persistedappVersionandminAppVersionduring a plain save instead of always stamping the running Beutl version. Migration code advances these fields only after rewriting persisted content.IProjectVersionControlServiceis now query-only and no longer exposes mutation or disposal operations. Mutations are coordinated throughIProjectVersionControlCoordinator; the backend and transaction surfaces are internal.IProjectVersionControlInitializer.InitializeCurrentProjectAsyncnow acceptsFunc<CancellationToken, Task<GitIdentity?>>, and the exact operation token is forwarded to the callback.CommitResult.Committednow carries aCommitRevision(KnownorUnavailable) so a durable commit is not reported as failed when its SHA cannot be observed afterward.SnapshotKind.RecoveryandRemoteOpResult.RepositoryDirtydistinguish compensating commits and actual cleanliness-precondition failures.VersionControlPolicyNoticeis now an internal discriminated union.MenuBarViewModelnow requires aVersionControlCoordinatorconstructor argument.CreateNewProjectViewModelnow accepts theIProjectVersionControlInitializerabstraction for Git availability and project initialization.Test plan
dotnet format Beutl.slnx --no-restore --verify-no-changes(3,269 files checked, no formatting changes).claude/scripts/check-gpl-mit-boundary-diff.sh --files <all changed files>dotnet build Beutl.slnx -c Debug --no-restore(0 errors; 64 existing warnings)dotnet test Beutl.slnx -m:1 -c Debug -f net10.0 --no-build --settings coverlet.runsettings --logger "console;verbosity=normal"Fixed issues / References
Summary by CodeRabbit