Skip to content

fix(server): bare repositories now group with projects of the same remote - #17340

Open
shrikantkalar023 wants to merge 1 commit into
pingdotgg:mainfrom
shrikantkalar023:fix/bare-repo-identity
Open

shrikantkalar023 wants to merge 1 commit into
pingdotgg:mainfrom
shrikantkalar023:fix/bare-repo-identity

Conversation

@shrikantkalar023

Copy link
Copy Markdown

Problem

Projects that share a git remote show as one project-picker entry across environments. A bare repository registered as a project (a git clone --bare dir, or the .bare + .git file layout) never joins that entry: each environment's copy shows separately.

RepositoryIdentityResolver finds the repository root with git rev-parse --show-toplevel. In a bare repository that exits 128 (fatal: this operation must be run in a work tree), so the project gets no repository identity and the client falls back to its per-environment path key.

Change

When --show-toplevel exits non-zero, the resolver runs one probe, git rev-parse --is-bare-repository --absolute-git-dir, and uses the git dir as the root only when git reports true. Remote lookup is unchanged; git remote -v works in a bare git dir.

  • A checkout's .git folder reports false and still gets no identity.
  • Root discovery for a checkout still runs one git command. A timeout or spawn failure skips the probe. A non-git folder adds one probe on each root-cache miss or explicit refresh (the negative TTL is one minute).
  • rootPath is the git dir for a bare repository; the contract comment on rootPath says so. No schema, client, or DB change.
  • Bare-repo projects still have no VCS status or worktree threads (GitVcsDriver.detectRepository requires a work tree). That is unchanged.

Scope and approval

No prior issue. I think this fits the small, focused bug-fix exception: projects are grouped by remote, bare repositories have remotes, and the resolver drops them only because it asks git for a work-tree-only value. The change touches root discovery, its tests, and the contract comment for rootPath.

Related:

Verification

  • vp test run apps/server/src/project/RepositoryIdentityResolver.test.ts apps/server/src/project/ProjectEnrichmentService.test.ts: 25/25 pass. New tests:
    • A plain bare repo and a .bare + .git file layout (resolved from the container and from .bare) get the same canonicalKey as a checkout of the same repo with an SSH remote. rootPath is the git dir.
    • A checkout's .git folder resolves to null.
    • A bare repo with no remote resolves to null.
    • A timed-out or failed-to-start --show-toplevel resolves to null without the probe.
    • Before the fix, both layout tests failed with expected null not to be null. Removing the "true" check fails the .git folder test; removing the timeout check fails the timeout test.
  • Resolver-to-grouping integration, using a temporary test that was not committed: real resolver results on temp repos feed buildProjectGroups from packages/client-runtime, with the bare project in one environment and the checkout in another.
    • Before: 2 groups. The bare project's key was env-laptop:<path>.
    • After: 1 group, keyed github.com/t3tools/t3code.
    • Same result in repository and repository_path modes, for a plain bare repo, a .bare container, and .bare itself.
  • vp lint and vp fmt --check on the changed files; vp run --filter t3 typecheck and vp run --filter @t3tools/contracts typecheck pass.
  • Not checked: Windows and macOS (Linux, git 2.43 only); git older than 2.25, where --show-toplevel exits 0 with empty output in a bare repo, so the result stays null as before; a live two-machine client session.

Claude Opus 5.5 via Claude Code in T3 Code; verification by GPT-6.1-Sol via Codex.

…mote

`git rev-parse --show-toplevel` exits 128 in a bare repository, so bare-repo
projects resolved no repository identity and never grouped with same-remote
projects in other environments. When it fails, probe `--is-bare-repository
--absolute-git-dir` and key bare repositories by their git dir.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Oct 8, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at f18d843

Macroscope's review found this PR approvable — This is a localized server bug fix that extends repository identity resolution to bare repositories while preserving existing worktree behavior. The added tests cover the new layouts and failure paths, and no schema, deployment, configuration, or static-analysis behavior is changed.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 8, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b2423421-9f89-482c-a2be-35b3342a49c9
📥 Commits

Reviewing files that changed from the base of the PR and between 48b71f0 and f18d843.

📒 Files selected for processing (3)
  • apps/server/src/project/RepositoryIdentityResolver.test.ts
  • apps/server/src/project/RepositoryIdentityResolver.ts
  • packages/contracts/src/environment.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Repository root resolution now recognizes bare repositories after an unsuccessful work-tree lookup. Tests cover bare repository layouts, failure cases, and checkout .git directories.

Changes

Repository identity resolution

Layer / File(s) Summary
Resolve work-tree and bare-repository roots
apps/server/src/project/RepositoryIdentityResolver.ts, packages/contracts/src/environment.ts
The resolver uses a shared Git invocation helper. It returns a nonempty work-tree path on success, and probes for a bare repository after an unsuccessful work-tree lookup. The contract comment describes rootPath for both repository types.
Test repository root resolution
apps/server/src/project/RepositoryIdentityResolver.test.ts
Tests cover retries, timeouts and process-start failures, bare clones and .bare layouts, checkout .git directories, and bare repositories without remotes.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to f18d8

Bare repositories now group with checkouts that share a remote. The change is narrow, guarded and covered by tests, so no merge-blocking risk remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to f18d8

The change extends existing repository grouping to bare repositories without changing the data format. Existing remote and hosting-account selection controls remain in place, and no introduced security vulnerability was demonstrated. Concurrent refresh behavior and external hosting-command behavior remain incompletely verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated exposure expansion is previously unrecognized bare-repository projects entering existing identity, grouping, and hosting-provider paths. Influence over their local repository configuration can influence remote-derived metadata. Newly available identities can also make those projects eligible for existing pull-request routing; this is broader downstream behavior than grouping alone.

Trust Boundaries and Controls

  • observed — Identity refinement requires a parseable remote, nonempty rootPath, and an eligible provider. Forgejo refinement matches the remote against configured login hosts, aliases, and URL paths rather than selecting an arbitrary default account. The fj credential-storage location is derived from server home and environment settings, not repository cwd; this credential-selection implementation predates the PR.
  • observed — Existing pull-request routing checks hostless references against the selected project’s repository. Explicitly hosted references may use another registered checkout on that host, while Azure routing requires matching canonical identity. This broader same-host routing is pre-existing; grouping is not itself a per-repository authorization barrier.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: grouping bare repositories with projects that use the same remote.
Description check Passed The description includes all required sections. It clearly explains the problem, implementation, scope justification, related work, focused verification, observed results, and untested environments.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@shrikantkalar023 shrikantkalar023 left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

self review done.

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

size:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant