Skip to content

fix: lazy github matcher for scoped roots#235

Merged
khaliqgant merged 1 commit into
mainfrom
codex/lazy-github-scoped-root
Jun 2, 2026
Merged

fix: lazy github matcher for scoped roots#235
khaliqgant merged 1 commit into
mainfrom
codex/lazy-github-scoped-root

Conversation

@khaliqgant
Copy link
Copy Markdown
Member

Summary

  • classify lazy GitHub repo content by finding the github/repos marker in the absolute remote path instead of the root-trimmed relative path
  • keep the isUnderRemoteRoot guard so scoped mounts do not widen
  • add scoped org-root, nested /relay, repo-dir, non-github, and outside-root matcher coverage

Validation

  • go test ./internal/mountsync -run 'TestIsUnderLazyGithubRepoSubtree|TestLazyReposSkipsEagerFetchOfIssuesOnStartup' -count=1\n- git diff --check origin/main..HEAD -- internal/mountsync/syncer.go internal/mountsync/syncer_test.go\n\n## Delivery note\nThis only updates the relayfile-mount binary source. Daily-ship is not delivered on merge alone; it still needs a relayfile release, Daytona snapshot pin bump, Worker-visible snapshot deploy, then the Cloud lazy flag deploy and bounded probe.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 2, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

The PR refactors lazy GitHub repo subtree path detection by extracting segment-matching logic into a reusable helper and expanding the detection algorithm to search for github/repos patterns flexibly within paths. Test coverage is expanded with non-GitHub integration cases and multi-org scoped scenarios to validate the updated behavior.

Changes

Lazy GitHub Repo Subtree Detection

Layer / File(s) Summary
Helper extraction and logic refactoring
internal/mountsync/syncer.go
New isLazyGithubRepoSubtreePath helper encapsulates segment-matching logic. isUnderLazyGithubRepoSubtree normalizes the remote path and delegates to the helper, which finds github/repos segments anywhere in the path with at least 5 remaining segments, replacing the prior remoteRoot-relative check.
Test coverage expansion
internal/mountsync/syncer_test.go
Negative assertions added for non-GitHub integration paths (Slack channels, memory workspaces). Scoped org scenarios added with positive matches for repo/subtree files and negative assertions for the org root and cross-org paths.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A subtree path, once rigid and bound,
Now flexibly searches through segments around,
With helpers extracted and tests standing tall,
The lazy GitHub detection catches them all! 🎯

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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
Title check ✅ Passed The title accurately describes the main refactoring of the lazy GitHub matcher logic to work with scoped roots, which aligns with the primary changes in the changeset.
Description check ✅ Passed The description is directly related to the changeset, explaining the key logic change (github/repos marker detection in absolute vs relative paths), the preservation of guards, and the added test coverage.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/lazy-github-scoped-root

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 and usage tips.

@gemini-code-assist
Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 2, 2026

Relayfile Eval Review

Run: .relayfile/evals/runs/2026-06-02T00-14-31-651Z-HEAD-provider
Mode: provider
Git SHA: b239554

Passed: 4 | Needs human: 0 | Reviewable: 0 | Missing output: 0 | Failed: 0 | Skipped: 0

Human Review Cases

No reviewable human-review cases captured Relayfile output.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
internal/mountsync/syncer_test.go (1)

538-549: ⚡ Quick win

Add the repo-dir scoped-root case explicitly.

The new coverage still skips the exact remoteRoot=/github/repos/<org>/<repo> shape. A matcher bug specific to repo-dir roots would still pass because the deepest positive here starts at .../pulls, not at the repo directory.

Suggested test-table entry
+		{
+			name:       "scoped repo remote root repo subtree file",
+			remoteRoot: "/github/repos/AgentWorkforce/cloud",
+			remotePath: "/github/repos/AgentWorkforce/cloud/pulls/123.json",
+			want:       true,
+		},
🤖 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 `@internal/mountsync/syncer_test.go` around lines 538 - 549, Add an explicit
test-row covering the repo-dir shaped scoped root so the case
remoteRoot="/github/repos/<org>/<repo>" is exercised; in the table inside
syncer_test.go add a case (e.g. name "scoped repo-dir remote root repo subtree
file") with remoteRoot "/github/repos/AgentWorkforce/cloud", remotePath
"/github/repos/AgentWorkforce/cloud/pulls/123.json" and want true so the matcher
for the repo-dir root is validated (refer to the existing test-table entries and
use the same fields name/remoteRoot/remotePath/want).
🤖 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.

Nitpick comments:
In `@internal/mountsync/syncer_test.go`:
- Around line 538-549: Add an explicit test-row covering the repo-dir shaped
scoped root so the case remoteRoot="/github/repos/<org>/<repo>" is exercised; in
the table inside syncer_test.go add a case (e.g. name "scoped repo-dir remote
root repo subtree file") with remoteRoot "/github/repos/AgentWorkforce/cloud",
remotePath "/github/repos/AgentWorkforce/cloud/pulls/123.json" and want true so
the matcher for the repo-dir root is validated (refer to the existing test-table
entries and use the same fields name/remoteRoot/remotePath/want).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fc88fad8-7d92-4ea1-bfe3-482c5719842e

📥 Commits

Reviewing files that changed from the base of the PR and between aa9bce0 and 58217e0.

📒 Files selected for processing (2)
  • internal/mountsync/syncer.go
  • internal/mountsync/syncer_test.go

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 2 files

Re-trigger cubic

@khaliqgant khaliqgant merged commit 5a8fcd9 into main Jun 2, 2026
9 checks passed
@khaliqgant khaliqgant deleted the codex/lazy-github-scoped-root branch June 2, 2026 00:26
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.

1 participant