Skip to content

fix(review): support unborn repository reviews - #305

Merged
Alan-TheGentleman merged 5 commits into
Gentleman-Programming:mainfrom
barbatdev:fix/issue-159-unborn-review
Aug 14, 2026
Merged

Alan-TheGentleman merged 5 commits into
Gentleman-Programming:mainfrom
barbatdev:fix/issue-159-unborn-review

Conversation

@barbatdev

@barbatdev barbatdev commented Aug 12, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Treat an unborn repository's base as Git's object-format-aware empty tree.
  • Materialize staged and untracked content without mutating the contributor index or creating a phantom commit.
  • Carry an absent original HEAD safely through the first reviewed commit transaction.
  • Fail closed on uncertain HEAD state and support Git versions older than 2.42 without masking unrelated failures.

Closes #159

Review path

  1. Review lib/review-candidate-view.ts for unborn detection, private index materialization, orphan candidate worktrees, and the status-129 compatibility fallback.
  2. Review lib/git-commit-transaction.ts for absent-HEAD handling and fail-closed probe classification.
  3. Review the focused tests for SHA-1/SHA-256 empty trees, corruption and timeout failures, index preservation, cleanup, and native START routing.
  4. Confirm runtime/git-commit-transaction.mjs matches the TypeScript source.

Review Budget

585 changed lines (545 additions + 40 deletions).

size:exception is explicitly accepted for this PR. The additional scope is review hardening for the same issue and delivery unit: it addresses CodeRabbit findings, keeps behavior-first tests with the fixes, and avoids a cross-fork child workflow that would not receive upstream CI.

Verification

  • First reviewed commit runtime flow passed with pinned Gentle AI v2.2.3.
  • SHA-1 and SHA-256 unborn repository coverage passed.
  • git diff --check passed.
  • pnpm run check:transaction-runner passed.
  • node scripts/verify-package-files.mjs passed.
  • node --experimental-strip-types --test tests/git-commit-transaction.test.ts tests/review-candidate-view.test.ts passed (79/79).
  • Native 4R review completed and receipt was approved.
  • Native pre-commit, pre-push, and pre-PR gates returned allow.

The focused 79-test suite passes. The full local suite retains known environment-sensitive baseline failures unrelated to this candidate; upstream CI verify passes on the published head.

Review Note

The remaining actorBinding thread is a false positive: the referenced test scope contains exactly one declaration. The similarly named binding near the earlier line belongs to a different test, and the TypeScript test suite compiles and runs successfully.

Two unchanged controller-routing tests still expose the known macOS /var versus /private/var canonicalization baseline; the added unborn START test passes.

Scope

This PR only addresses ordinary review and the first reviewed commit in unborn repositories. It does not change review policy, consent behavior, or repository authority semantics outside that flow.

Summary by CodeRabbit

  • New Features

    • Added support for reviewing and committing changes in repositories without an existing HEAD.
    • Candidate views now include staged and untracked files in newly initialized repositories.
    • Added compatibility support for Git versions without native orphan worktree support.
  • Bug Fixes

    • Improved recovery when HEAD is missing or disappears during a commit.
    • Preserved validation, cancellation, and retry behavior for first-commit workflows.
    • Added safeguards for detached, broken, timed-out, or otherwise unresponsive HEAD states.

@barbatdev barbatdev added the type:bug Bug fix label Aug 12, 2026
@coderabbitai

coderabbitai Bot commented Aug 12, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4c9b0b52-b27f-4061-a868-a732603e118d

📥 Commits

Reviewing files that changed from the base of the PR and between 82f2a39 and efe339a.

📒 Files selected for processing (2)
  • lib/review-candidate-view.ts
  • tests/review-candidate-view.test.ts

📝 Walkthrough

Walkthrough

The change adds support for unborn Git repositories across commit transactions, candidate views, and native review startup. It uses optional HEAD values, Git’s format-aware empty tree, orphan worktrees, and fail-closed probing.

Changes

Unborn repository support

Layer / File(s) Summary
Nullable HEAD transaction lifecycle
lib/git-commit-transaction.ts, runtime/git-commit-transaction.mjs
Transaction records accept missing original_head values. Transaction creation, recovery, verification, cancellation, and abandonment handle optional HEAD values.
Unborn candidate base and materialization
lib/review-candidate-view.ts
Candidate scope uses tree comparison. Unborn repositories use the native empty tree, an empty index, and orphan worktrees with a compatibility fallback.
Repository and routing validation
tests/git-commit-transaction.test.ts, tests/review-candidate-view.test.ts, tests/review-controller-native-routing.test.ts
Tests cover first commits, validation denial, fail-closed probes, staged and empty repositories, worktree cleanup, and native START routing.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to efe33

The PR adds support for reviewing unborn repositories and reports passing focused validation; no actionable merge-blocking risk remains beyond normal checks and review.

Possibly related PRs

Suggested reviewers: alan-thegentleman

Sequence Diagram(s)

sequenceDiagram
  participant NativeSTART
  participant CandidateView
  participant GitRepository
  participant CandidateWorktree
  NativeSTART->>CandidateView: Resolve default HEAD candidate
  CandidateView->>GitRepository: Probe HEAD and resolve base tree
  GitRepository-->>CandidateView: Unborn HEAD and empty tree
  CandidateView->>CandidateWorktree: Materialize candidate index and worktree
  CandidateWorktree-->>NativeSTART: Workspace projection and target identity
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 38.46% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the primary change: adding review support for unborn repositories.
Linked Issues check ✅ Passed The changes satisfy issue #159 by supporting ordinary reviews, deterministic identity, candidate snapshots, and first commits in unborn repositories.
Out of Scope Changes check ✅ Passed The changes remain within the stated scope of unborn repository reviews, including compatibility, fail-closed probing, transactions, and native START routing.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 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 `@lib/git-commit-transaction.ts`:
- Around line 183-185: Update resolveHead in lib/git-commit-transaction.ts to
return undefined only for a valid unborn HEAD response and propagate timeout,
I/O, corruption, and other non-unborn Git failures; ensure createRecord cannot
persist those failed probes as original_head: undefined. Add transaction tests
covering probe timeout and corrupt HEAD behavior, then regenerate
runtime/git-commit-transaction.mjs at lines 184-186 so it preserves the same
change.

In `@lib/review-candidate-view.ts`:
- Around line 754-755: Update the worktree creation flow around the unborn
branch handling to avoid relying exclusively on git worktree add --orphan:
either declare Git 2.42.0 as the required minimum version or implement a
fallback using commands supported by older Git versions. Preserve the existing
orphan-worktree behavior and branch naming.

In `@tests/review-controller-native-routing.test.ts`:
- Line 1878: In the test block around the actorBinding setup, remove the
duplicate block-scoped actorBinding declarations and retain exactly one typed
const declaration. Ensure subsequent assertions continue using that single
binding.
🪄 Autofix

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: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 07584458-00bf-4450-b40c-10b7473e03f4

📥 Commits

Reviewing files that changed from the base of the PR and between 19b0ed7 and 429502d.

📒 Files selected for processing (6)
  • lib/git-commit-transaction.ts
  • lib/review-candidate-view.ts
  • runtime/git-commit-transaction.mjs
  • tests/git-commit-transaction.test.ts
  • tests/review-candidate-view.test.ts
  • tests/review-controller-native-routing.test.ts

Comment thread lib/git-commit-transaction.ts
Comment thread lib/review-candidate-view.ts Outdated
// No baseRef is sent to native START: the unborn default uses the
// workspace projection only, exactly as the provider expects.
assert.deepEqual(requests, [{ cwd, targetIdentity: `sha256:${"a".repeat(64)}`, projection: "workspace" }]);
const actorBinding = (started.details as { actor_binding: { workspace_root: string; candidate_root: string; candidate_tree: string; candidate_paths: readonly string[] } }).actor_binding;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Remove the duplicate actorBinding declarations.

Line 1878 declares the same block-scoped const five times. TypeScript rejects this file before the test can run. Keep one declaration.

🤖 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/review-controller-native-routing.test.ts` at line 1878, In the test
block around the actorBinding setup, remove the duplicate block-scoped
actorBinding declarations and retain exactly one typed const declaration. Ensure
subsequent assertions continue using that single binding.

@barbatdev barbatdev changed the title fix(review): support unborn repository reviews tracker: fix(review): support unborn repository reviews Aug 12, 2026
@barbatdev barbatdev changed the title tracker: fix(review): support unborn repository reviews fix(review): support unborn repository reviews Aug 13, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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 `@tests/git-commit-transaction.test.ts`:
- Around line 399-414: Strengthen the test around unbornInvocation and
resolveHead by asserting that the thrown error specifically represents the
configured probe timeout, rather than accepting any failure. Also validate that
the POSIX-only wrapper assumptions (/bin/sh, executable permissions, which, and
sleep duration) match the CI platform matrix and GIT_TIMEOUT_MS configuration,
adjusting the test setup or constraints as needed.

In `@tests/review-candidate-view.test.ts`:
- Around line 1436-1447: Update the test around CandidateViewRegistry.create to
record each Git argument vector received by the injected executor. After
asserting the propagated CandidateViewError, also verify that no invocation used
commit-tree or worktree add with --no-checkout and --detach, proving the
fallback was not executed.
🪄 Autofix

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: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d2898596-3d35-4e25-bf5a-3e9e1e9f2a31

📥 Commits

Reviewing files that changed from the base of the PR and between 429502d and 17d0edd.

📒 Files selected for processing (5)
  • lib/git-commit-transaction.ts
  • lib/review-candidate-view.ts
  • runtime/git-commit-transaction.mjs
  • tests/git-commit-transaction.test.ts
  • tests/review-candidate-view.test.ts

Comment thread tests/git-commit-transaction.test.ts
Comment thread tests/review-candidate-view.test.ts
@Alan-TheGentleman
Alan-TheGentleman merged commit 6546a5c into Gentleman-Programming:main Aug 14, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(review): support ordinary review in unborn repositories

2 participants