Repository navigation
Adds an opt-in GitKraken sign-in for e2e workers, with signed-in MCP write-path e2e - #6030
nikolay-1986 wants to merge 5 commits into
Conversation
Signed-in specs opt in with the `signIn` worker option and the @signed-in tag: the worker asks the GitKraken site for a sign-in code with the test account's session cookie and hands GitLens the login deep link through the evaluator, since the normal sign-in can't run under the test host. Picks the backend from GK_E2E_ENV (production/staging/dev), passes a Cloudflare Access credential off production, retries only the code request, and keeps credentials, codes and account text out of its diagnostics. Without a cookie the specs skip.
🤖 Augment PR SummarySummary:
🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| } | ||
|
|
||
| if (vscodeOptions.signIn) { | ||
| await signInToGitKraken( |
There was a problem hiding this comment.
Could we account for the CLI authentication side effect at tests/e2e/baseTest.ts:600? SubscriptionService.loginCore() invokes gitlens.ai.mcp.authCLI, and the AI-enabled harness inherits the real home/config environment, so a production sign-in can replace the developer's machine-wide CLI login with the test account and leave its non-expiring token behind after worker cleanup.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Fixed in c5cda86: signed-in workers now get their own XDG_DATA_HOME (the GK CLI keeps its install and sessions there), so the gk auth login GitLens runs after sign-in lands in the worker's temp dir — deleted at teardown — instead of replacing the machine-wide CLI login. Verified on CI: the MCP specs that use a signed-in worker still pass with the isolated CLI data.
| await evaluate( | ||
| // The host's own scheme (`vscode-insiders`, a fork's) — a `vscode://` link would reach another editor, or none | ||
| async (vscode, query: string) => | ||
| vscode.env.openExternal(vscode.Uri.parse(`${vscode.env.uriScheme}://eamodio.gitlens/login?${query}`)), |
There was a problem hiding this comment.
Could we account for the extension's own logging at tests/e2e/fixtures/gkSignIn.ts:134? SubscriptionService.onLoginUriReceived() uses @info() and the logger serializes Uri with its full query, so this path writes the sign-in code and state into the GitLens output log despite the fixture's no-secret-logging guarantee.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Confirmed: GitLens' own @info() on the login handler logs the URI with its query. That's product code, outside this test-only PR, so it's noted for a separate fix (log the callback with code/state masked or hashed). For the harness: the code is single-use and exchanged right after, the log lives in the worker's temp profile (deleted at teardown), and the fixture's docs now say so instead of claiming nothing is logged (c5cda86).
| }); | ||
|
|
||
| test.skip(!canSignIn(), 'GK_E2E_ACCESS_TOKEN_COOKIE is not set'); | ||
| test.use({ trace: 'off', video: 'off', screenshot: 'off' }); |
There was a problem hiding this comment.
Could we account for Playwright's error-context.md attachments at tests/e2e/specs/signIn.test.ts:17? These three options do not disable failed-assertion ARIA snapshots in Playwright 1.63, so a failure on the Account webview can still copy the account name/e-mail into test artifacts despite the recording opt-out.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Confirmed: Playwright 1.63 writes error-context.md on any test error, and there is no option to turn it off. d0818be makes the fixture wait with locator.waitFor instead of expect matchers, so a failed sign-in no longer snapshots the Account page, and documents the limitation for assertions in specs. Artifacts from the CI job will be scanned and redacted before upload — that's part of the separate CI-wiring PR.
There was a problem hiding this comment.
🟡 Changes recommended
Extension logs and failure diagnostics can expose authentication values or account information.
4 open issues
What changed in this PR
Adds opt-in, unattended GitKraken sign-in for E2E workers, enabling future tests that need connected cloud integrations.
Changes:
- Adds backend-aware sign-in with guarded requests and retries.
- Adds signed-in account and integration checks.
- Documents setup, credentials, and token costs.
| File | Description |
|---|---|
| tests/e2e/specs/signIn.test.ts | Checks account sign-in and required integrations. |
| tests/e2e/fixtures/gkSignIn.ts | Implements unattended sign-in and diagnostics. |
| tests/e2e/baseTest.ts | Adds worker sign-in options and setup. |
| docs/testing.md | Documents opt-in execution and safeguards. |
| .claude/skills/add-test/SKILL.md | Adds guidance for signed-in specs. |
Review effort: Balanced
After signing in, GitLens logs the GK CLI in with the account's token. Signed-in workers now get their own XDG_DATA_HOME so that login lands in the worker's temp dir instead of replacing the machine-wide CLI login. Also documents that GitLens' own log records the single-use deep link code inside the temp profile, and that Playwright's error-context snapshots can still carry account text on a failure.
Waits with locator.waitFor instead of expect matchers in the sign-in fixture, so a failed sign-in doesn't put an ARIA snapshot of the Account page into Playwright's error-context, and reports a page answer to the code request in fixed trait labels only instead of the response's title and content type. Documents that error-context can't be turned off.
…explicit opt-in The harness strips GK_E2E_* from the environment it launches the editor with, so GitLens, git, the GK CLI and the MCP server never inherit the cookie or the Cloudflare credential; only the Playwright process reads them. GK_E2E_ENV no longer defaults to production: it must name staging or dev, and production also needs GK_E2E_ALLOW_PRODUCTION=1, since a production sign-in mints a token the backend never expires or revokes.
|
Follow-up from a design review of the CI rollout (eda5e8f):
Merged up into #6033 and #6039. Verified with the signed-in specs against staging in a sandbox fork (4 passed). |
#6033) * Adds signed-in e2e for the MCP Start Work and Start Review happy paths (#5106) Runs gitlens_start_review and gitlens_start_work on a GitHub test repository from a signed-in worker under a simulated Pro plan, and checks what they report against git. On failure, prints the GitLens log lines that explain the wizard's decision, with the CLI sign-in entries dropped and sign-in codes, tokens and e-mail addresses masked. Skips without the cookie or the E2E_GITHUB_* variables. * Checks Start Review tracks the PR's own branch, not just any origin branch
nikolay-1986
changed the title
Summary
Parts 1 and 2 of #5682 (#6033 was merged into this branch): lets an e2e worker sign in to a GitKraken test account with no person involved, and uses it for the integration-backed happy paths of the MCP Start Work / Start Review tools (#5106). Opt-in only — without a session cookie the signed-in specs skip, and the existing suite is unaffected.
Why this way
The normal sign-in can't run under the test host: VS Code refuses the "open external website" dialog in tests (
DialogService: refused to show dialog in tests). Instead the worker asks the GitKraken site for a sign-in code with the test account's session cookie (the code is in the first redirect of/login, no browser), and hands GitLens thelogindeep link through the harness evaluator. The account's cloud integrations come with the sign-in — no provider OAuth.Changes
Sign-in fixture (#5682)
tests/e2e/fixtures/gkSignIn.ts— the unattended sign-in.GK_E2E_ENV, which has no default. It must namestagingordev;productionalso needsGK_E2E_ALLOW_PRODUCTION=1. The setting picks each backend's session cookie name and GitLens'gitkraken.env.locator.waitFor, so a failed sign-in doesn't snapshot the Account page.tests/e2e/baseTest.ts:vscodeOptions.signIn(trueor{ requiredIntegrations });gitkraken.envinto the signed-in worker's settings;XDG_DATA_HOME, so GitLens' follow-upgk auth logindoesn't replace the machine-wide GK CLI login;GK_E2E_*from the editor's launch environment, so VS Code, GitLens, git, the GK CLI and the MCP server never inherit the credentials;canSignIn/requiredIntegrations.tests/e2e/specs/signIn.test.ts—@signed-in: the account page shows the signed-in state, and the required integrations are connected.Signed-in MCP write paths (#5106)
tests/e2e/specs/mcpGitlensWritePathSignedIn.test.ts—@signed-in. It runsgitlens_start_reviewandgitlens_start_workon a GitHub test repository under a simulated Pro plan, and checks what they report against git:E2E_GITHUB_REPO_URL/_ISSUE_URL/_PR_URLvariables, and fails instead underE2E_REQUIRE_SIGNIN.tests/e2e/specs/mcpGitlensWritePath.test.ts— its header now points to the signed-in spec.Docs
docs/testing.md— "Opt-in specs: signed-in workers": how to opt in, how to run locally, the variables (including the GitHub fixtures), and what a sign-in costs..claude/skills/add-test/SKILL.md— a pointer for signed-in specs.How it was tested
pnpm run checkclean.ubuntu-latest,bundle:e2e, same harness):signIn.test.ts×2 and both Add E2E test coverage for GitLens MCP server #5106 cases pass (4/4, most recently after the review fixes);GK_E2E_ACCESS_TOKEN_COOKIEthe signed-in specs skip, and no editor is launched for them.Notes
main, staging only, and artifacts scanned for secret values before upload. It stays off until enabled, and waits on a Cloudflare Access credential for CI.🤖 Generated with Claude Code