Sign up
View stargazersSign up

test(source-control): forge behaviour is tested in its own package#24000

merged1
scp/08-change-request-wording
scp/09-forge-tests
opened19d agobyJJulius Marminge
45 files changed
View on GitHubTwo-way sync enabledPR last imported 18d ago

Layer 9 of 9 in the source-control plugin boundary stack: forge knowledge (GitHub, GitCafe, GitLab, Forgejo, Bitbucket, Azure DevOps) moves into packages/source-control-<forge>, and shared, contracts, server and clients ask forge-neutral hooks in source-control-core instead.

Server tests reached into forge internals to test forge behaviour: template detection, stack rebase, Bitbucket's API, auth-probe parsing, remote claims, Forgejo checkout, and provider halves of pull request service cases. Those now live in their forge packages, on a TestSourceControlHost that can run real git (layerWithRealGit). Forge-neutral server logic (GitManager, PullRequestService, discovery, the provider registry, pull request watches) runs on fake providers and discovery specs from @t3tools/source-control-testing/fakeSourceControlDriver, and GitManager asserts the selectors it asks a host for instead of gh argv.

Forge packages now export only client/definition and server/driver; the built-in drivers testkit and the drivers' makeInstance exports are gone.

Done by Claude Opus 5.5 in Claude Code, running inside T3 Code.

🤖 Generated with Claude Code

Checks

23 successful · 14 skipped
All checks passed
1f2675f
Ggithub-actionsBot19d ago
Synced from GitHub

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire—5.0 KiB—6.8 KiB✅
CodexThread snapshot wire—3.8 KiB—4.9 KiB✅
CodexLive turn WebSocket wire—1.2 KiB—2.0 KiB✅
CodexLive turn WebSocket decoded—20.9 KiB—29.3 KiB✅
CodexLive turn messages—2—8✅
ClaudeTotal thread wire—5.0 KiB—6.8 KiB✅
ClaudeThread snapshot wire—3.8 KiB—4.9 KiB✅
ClaudeLive turn WebSocket wire—1.2 KiB—2.0 KiB✅
ClaudeLive turn WebSocket decoded—21.2 KiB—29.3 KiB✅
ClaudeLive turn messages—2—8✅

Baseline: unavailable · PR result: 1f2675f · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

MmacroscopeappBot18d ago
Synced from GitHub

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This PR primarily relocates forge-specific tests and adds a test-only real-git harness, with little apparent production runtime change. It also changes package exports and adds a file-level static-analysis diagnostic suppression, so human review is warranted.

Notes:

  • All code in this push has already been reviewed. Approvability was decided on eligibility alone.

No code changes detected at 1f2675f. Prior analysis still applies.

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

CcoderabbitaiBot18d ago
Synced from GitHub

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

External image Files ignored due to path filters: 1
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between bafa55842a0515ac26cc0412a45c12b95375fab9 and 1f2675f1aa143b2257add3ecb0fc674237f50fba.

External image Files ignored due to path filters: 1
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review.

External image Recent review info
External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between 6c003dbc6509116942414916301aaf651c85f3b4 and bafa55842a0515ac26cc0412a45c12b95375fab9.

External image Files ignored due to path filters: 1
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
External image Files selected for processing: 1
  • apps/server/src/git/GitManager.test.ts

Limit details: You’ve used all 10 included reviews currently available.

Walkthrough

This PR adds shared source-control test utilities, updates server and provider tests to use them, adds provider-specific test coverage, and narrows package and module exports.

Changes: Source-control test infrastructure and coverage

Layer / File(s)Summary
Shared test utilities and provider test migration
packages/source-control-testing/*, apps/server/src/assets/AssetAccess.test.ts, packages/source-control-*/src/server/*test.ts		Adds fake source-control providers and a test host for real processes and Git. Updates provider tests to use shared test interfaces and adds coverage for provider authentication, discovery, listing, checkout, credentials, file revisions, templates, and rate limits.
Provider export boundaries
packages/source-control-*/package.json, packages/source-control-*/src/server/driver.ts, packages/source-control-github/src/server/GitHubPullRequestProvider.ts		Removes provider server-module subpath exports and makes selected constructors and a GitHub error mapper module-local.
Pull-request service and watch coverage
apps/server/src/pullRequest/*test.ts, apps/server/src/orchestration-v2/pullRequestWatch*.test.ts, apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts		Updates service fixtures, adds coverage for merge-credit forwarding, credential scope, viewed files, rate limits, and edited remarks. Removes the GitCafe linking test and Forgejo watch test file.
Application discovery and registry coverage
apps/server/src/sourceControl/*		Reworks discovery tests around CLI and managed discovery specifications. Makes the registry constructor module-local and removes the built-in driver testkit.
GitManager provider-based tests
apps/server/src/git/GitManager.test.ts		Replaces GitHub CLI-oriented fixtures with typed provider scenarios and assertions on provider calls for pull-request lookup, creation, checkout, and worktree behavior.

Priority: External image Low

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

Change: Other

Suggested reviewers: maria-rcks, t3dotgg

Merge Risk: External image Minimal · up to bafa5

No actionable risk is established for this change; it is mergeable after normal checks.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check nameStatusExplanationResolution
Description checkWarningThe description clearly explains the architectural change and test relocation, but it omits the required Problem, Change, Scope and approval, and Verification sections. It also lacks focused verificat…Rewrite the description using the required headings. Add the problem and expected behavior, explain the change, link the triaged issue or maintainer approval, and document focused tests with observed results and limitations.
Passed Passed checks: 3 passed
Check nameStatusExplanation
Title checkPassedThe title is concise, specific, and accurately summarizes the main change: forge behavior tests now reside in the corresponding package.
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description clearly explains the architectural change and test relocation, but it omits the required Problem, Change, Scope and approval, and Verification sections. It also lacks focused verification results and approval or issue evidence.

Resolution

Rewrite the description using the required headings. Add the problem and expected behavior, explain the change, link the triaged issue or maintainer approval, and document focused tests with observed results and limitations.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Added 1 commit

  • test(source-control): forge behaviour is tested in its own package1f2675f