Copilot-assisted commits: selected snapshots and validated Git transactions [2/5] by sergiou87 · Pull Request #22976 · desktop/desktop · GitHub
Skip to content

Copilot-assisted commits: selected snapshots and validated Git transactions [2/5] - #22976

Open
sergiou87 wants to merge 5 commits into
sergiou87-assisted-commit-uxfrom
sergiou87-assisted-commit-transactions
Open

sergiou87 wants to merge 5 commits into
sergiou87-assisted-commit-uxfrom
sergiou87-assisted-commit-transactions

Conversation

@sergiou87

@sergiou87 sergiou87 commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

Description

Layer 2 of 5, stacked on #22971. The base is sergiou87-assisted-commit-ux at b79922ca5ad766f3bee0cbf13dbfdd8e76e8a35b, which includes development at bebc062caafcf818993b9f53397b976d33484231; this PR still targets the preceding layer, not development directly.

⚠️ For a better understanding of this PR (and the whole feature) I recommend checking the docs introduced in this PR, mainly docs/technical/assisted-commit-process.md ⚠️

This adds the standalone plan-first transaction backend. Desktop captures selected content, validates the entire proposed plan and every cumulative tree, then creates only the planned local commits. Copilot receives opaque snapshot change IDs; it cannot supply executable paths, patches or blobs, and this layer contains no SDK/model calls.

  • Immutable selected-only snapshots preserve file/line selections, raw bytes, type/mode, original full HEAD/ref and real index. Selected text hunks from the same file can be committed separately in either order; indivisible changes are atomic or fail with a documented unsafe condition.
  • Whole-plan validation enforces exact-once exhaustive ownership and rejects unknown/injected fields, IDs, duplicates, omissions, blank titles, empty groups and unintended empty deltas. All cumulative trees are prepared before the first published commit and the final tree must equal exactly the frozen selection. Single-commit fallback requires an explicit whole-selection message/reason; invalid payloads never silently succeed.
  • Capture uses isolated indices/configuration/attributes and owned copies of selected bytes for native index-aware conversion. The real index remains untouched during analysis/execution. Native clean/process filters retain original-root context, and ordinary Git's indexed CRLF semantics are preserved.
  • Guarded local execution reuses message/trailer/sign-off/skip-hook and native hook-interception conventions for every commit. Cooperative cancellation waits for in-flight Git. Ref CAS, complete HEAD identity, parent/tree/index verification and parent-owned locks protect unrelated history and staging.
  • Recovery removes only this run's commits, preserves current working-tree bytes and original index, exposes original-selection reconciliation data, and retains explicit retry/finalization capabilities when interference or resource cleanup blocks recovery. No hard reset, checkout, stash, amend, push or existing-history rewrite.
  • Failed snapshot capture or pre-execution disposal exposes an opaque cleanup-only token through AssistedCommitError.cleanupToken. retryAssistedCommitCleanup retries removal of identity-checked private temporary resources without reading or changing HEAD, the index or working-tree files. Repeated failures retain the same capability; successful cleanup is idempotent. Existing executor rollback/recovery tokens and original failure causes remain intact.

Public APIs and lifecycle/support/unsafe cases are documented in docs/technical/assisted-commit-transactions.md. Original manual commit/staging/parser behavior and layer 1's availability/callback gate remain unchanged. Shared existing helpers only gain optional environment plumbing; unrelated issues already present in development are not fixed.

Scope boundary: backend scaffolding only. The constrained planner belongs to layer 3; AppStore/Dispatcher/UI run/progress/cancel/hook recovery wiring belongs to layer 4; opt-in push/retry belongs to layer 5. No real assisted callback is wired here.

Stack

Merge in order, starting with 1. Each PR builds on the preceding PR.

  1. #22971 - Repository-scoped assisted commit UX
  2. #22976 - Selected snapshots and validated Git transactions (this PR)
  3. #22977 - Constrained Copilot-assisted commit planning
  4. #22986 - Awaited local runs and recovery
  5. #22991 - Optional push and push-only retry

Validation

Command Result
corepack yarn --frozen-lockfile Passed after inheriting upstream manifest/lockfile changes; tracked manifests and configuration unchanged.
corepack yarn test app/test/unit/git/assisted-commit-cleanup-test.ts app/test/unit/git/assisted-commit-test.ts app/test/unit/git/assisted-commit-hooks-test.ts app/test/unit/git/commit-test.ts Passed on the cleanup-fix tree: 180/180, including 17 new cleanup-capability cases and existing transaction, hook and manual commit coverage.
corepack yarn test app/test/unit/git/assisted-commit-test.ts app/test/unit/git/assisted-commit-hooks-test.ts app/test/unit/git/commit-test.ts app/test/unit/git/refreshable-credentials-test.ts app/test/unit/account-token-lifecycle-test.ts app/test/unit/accounts-store-test.ts app/test/unit/credential-sessions-test.ts app/test/unit/refresh-token-consumers-test.ts app/test/unit/copilot-sdk-auth-test.ts app/test/unit/stores/copilot-store-credentials-test.ts app/test/unit/ui/commit-message-test.tsx app/test/unit/ui/copilot-animation-test.tsx Previously passed after the development rebase: 369/369, including transaction, ordinary manual commit, upstream OAuth/credential-session and inherited UI cases.
corepack yarn test Before the cleanup fix: 2,459 passed, 2 skipped, 4 bare-fixture failures; the same four failures were independently reproduced using exact pinned-parent source without this layer.
GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=safe.bareRepository GIT_CONFIG_VALUE_0=all corepack yarn test Passed on the cleanup-fix tree: 2,480/2,482, two existing skips, zero failures.
corepack yarn lint:fix Passed.
corepack yarn lint Passed with current upstream lint tooling.
corepack yarn tsc --noEmit --pretty false Passed with upstream TypeScript 6.0.3, including dependency declarations; no skipLibCheck required.
corepack yarn exec -- markdownlint docs/technical/assisted-commit-transactions.md --config .markdownlint.js --rules .markdownlint-rules.js Passed.
corepack yarn test:e2e:build:unpackaged Passed: production unpackaged Electron build and full Sass checks.
git diff --check Passed before cleanup-fix publication.

The four plain-suite bare-fixture failures (three push fixtures and one bare-repository detection fixture) are caused by the CLI-injected safe.bareRepository=explicit policy. They were independently reproduced from an archive of exact pinned parent b79922ca5ad766f3bee0cbf13dbfdd8e76e8a35b, without this layer; the same four parent tests pass with the process-only override. No global or repository configuration was changed. These baseline issues remain out of scope and unfixed. The former dependency declaration conflicts are resolved upstream; the current plain strict TypeScript check passes.

There are 155 new real-Git/fault tests beyond the lower layer. Coverage includes partial/full selections and same-file offset/reordering, actual trees/full SHAs/counts and working-tree/index byte equality, binary/rename/delete/new/mode/path/EOL cases, hostile plans, selected versus unselected mutation, cancellation boundaries/in-flight Git, real declined/ignored hooks, unintended hook staging, ref/index interference, unborn/detached/linked worktrees, and repeatable owned-resource cleanup/recovery. Cleanup-specific cases cover capture failures before a snapshot exists, planner/validation/cancellation failures, repeated disposal failures, idempotence, forged capabilities, active-operation exclusion and replaced temporary roots without changing repository content.

Screenshots

Not applicable: this layer changes no UI surface and wires no assisted executor callback.

Release notes

Notes: no-notes

@sergiou87
sergiou87 added this pull request to stack #22992 October 9, 2026 08:14
@sergiou87 sergiou87 changed the title Copilot-assisted commits 2/5: selected snapshots and validated Git transactions Copilot-assisted commits: selected snapshots and validated Git transactions [2/5] Oct 15, 2026
@sergiou87
sergiou87 force-pushed the sergiou87-assisted-commit-transactions branch from d08f99b to 8e8dfea Compare October 15, 2026 14:45
Copilot AI balanced review requested due to automatic review settings October 15, 2026 14:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Cross-platform ref/index recovery requires human review, and pre-execution cleanup retry remains unresolved.

1 open finding
What changed in this PR

Adds the standalone transaction backend for assisted commits, building on layer 1’s UI foundation without enabling model calls, UI execution, or push.

Changes:

  • Captures selected-only snapshots and validates complete commit plans.
  • Adds guarded local execution, cancellation, rollback, and resource cleanup.
  • Extends hook/message environment support and adds tests and lifecycle documentation.
File Description
docs/​technical/​assisted-commit-transactions.md Documents lifecycle and safety boundaries.
app/​test/​unit/​git/​assisted-commit-hooks-test.ts Tests native hooks and failure recovery.
app/​test/​helpers/​assisted-commit.ts Adds transaction test helpers.
app/​src/​models/​assisted-commit.ts Defines snapshot and plan contracts.
app/​src/​lib/​hooks/​with-hooks-env.ts Passes captured hook configuration.
app/​src/​lib/​hooks/​hooks-proxy.ts Preserves captured Git parameters.
app/​src/​lib/​hooks/​get-repo-hooks.ts Supports environment-aware hook discovery.
app/​src/​lib/​git/​update-ref.ts Adds guarded ref transactions.
app/​src/​lib/​git/​owned-file-lock.ts Tracks lock ownership and cleanup.
app/​src/​lib/​git/​interpret-trailers.ts Supports isolated trailer formatting.
app/​src/​lib/​git/​index.ts Exports assisted-commit APIs.
app/​src/​lib/​git/​assisted-commit/​state.ts Tracks private transaction state.
app/​src/​lib/​git/​assisted-commit/​snapshot.ts Captures frozen selected content.
app/​src/​lib/​git/​assisted-commit/​progress.ts Defines progress and cancellation controls.
app/​src/​lib/​git/​assisted-commit/​plan.ts Validates ownership and cumulative trees.
app/​src/​lib/​git/​assisted-commit/​index.ts Exposes backend lifecycle APIs.
app/​src/​lib/​git/​assisted-commit/​git.ts Implements isolated Git and integrity helpers.
app/​src/​lib/​git/​assisted-commit/​execute.ts Executes commits and manages recovery.
app/​src/​lib/​git/​assisted-commit/​error.ts Defines typed errors and recovery information.
app/​src/​lib/​format-commit-message.ts Supports captured formatting environments.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +1067 to +1070
@sergiou87
sergiou87 force-pushed the sergiou87-assisted-commit-transactions branch 2 times, most recently from b29f3d4 to 538a13c Compare October 16, 2026 11:51
Capture immutable selected-content snapshots and validate complete commit plans before publishing local commits. Materialize cumulative trees in isolated Git state and guard cancellation, hook failure, ref/index ownership, and resource cleanup with explicit recovery capabilities. Keep planner, UI orchestration, and push integration for later layers.
Match exact physical fault targets across Git/native path spellings and directory aliases, and assert injections are exercised. Compare filter cwd/worktree identity instead of short-versus-long directory spellings while preserving quoted paths and persistent filter protocol coverage. Leave production transaction guards and unrelated baseline issues unchanged.
Assert helper termination and actual unsuccessful exit before checking parent-owned HEAD locks. Accept Dugite's numeric failure result on Windows as well as POSIX signal rejection, without swallowing assertions or weakening transaction ownership checks.
@sergiou87
sergiou87 force-pushed the sergiou87-assisted-commit-transactions branch from 538a13c to a921bb0 Compare October 19, 2026 16:38
Expose an opaque cleanup-only capability when capture or pre-execution disposal fails, including before a snapshot callback exists. Retain owned temporary-directory identity, reject active or concurrent cleanup, and keep disposal retries idempotent without Git operations. Preserve original failure causes and existing executor recovery tokens; cover lifecycle, ownership and error paths with real-Git tests.
@sergiou87
sergiou87 marked this pull request as ready for review October 21, 2026 12:49
@sergiou87
sergiou87 requested a review from a team as a code owner October 21, 2026 12:49

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants