fix(e2e): lock the shared GitHub session, and stop a lane silently booting legacy

[gustavolira]

Three defects found while triaging the overlay NFS migration.

Closes RHIDP-16457, RHIDP-16459. Partially addresses RHIDP-16458.

Re-ordered after checking which of these are live on main today rather than insurance. The session-file race is happening now; the NFS guard is a post-condition that nothing trips yet but that matters more as workspaces lose their legacy lane.

The GitHub session file has no lock, and github races it today — RHIDP-16459

const sessionFileName = `authState_${userid}.json`;   // relative

Relative resolves against process.cwd(), which the worker fixture sets to the workspace's e2e-tests directory — the same value for every project in it.

On main right now, the github workspace has two projects, github-actions and github-issues, and both specs call loginAsGithubUser. Workers default to 50% of cores, so they run concurrently against one unlocked file. Two outcomes: a reader lands mid-write and dies on truncated JSON, or both decide there is no session and both log in — with the same TOTP secret, inside the same 30-second window.

That second one is not speculation; this file already carries the retry for it:

"The two-factor code you entered has already been used"
"too many codes have been submitted"

Still one file per user, deliberately not per project. Scoping per project was the first attempt and would have made it worse: every lane logging in concurrently is exactly the TOTP collision. What was missing is a lock spanning read-or-login-and-write — two lanes that both find no session go on to log in anyway — plus an atomic temp-file write (removed even on failure) and an unreadable session falling through to a full login instead of throwing.

RHDH cookies from another lane were never the hazard: each lane's RHDH lives on its own namespace hostname.

A lane can boot legacy and pass — RHIDP-16457

_applySecrets merges defaults → auth → new-frontend-system → tests/config/rhdh-secrets.yaml. The workspace file is last, so a workspace setting APP_CONFIG_app_packageName or ENABLE_STANDARD_MODULE_FEDERATION overrides the NFS layer and the lane boots the legacy shell — passing, because the legacy suite re-runs and nothing errors.

No workspace does that today, so the throw is a post-condition rather than a live catch. What makes it worth having now is the shape the migration has taken: app-defaults, keycloak and, once #3325 and #3343 land, argocd and roadie have no non-app-next project left. When a legacy lane sat beside the NFS one, a silently-broken NFS lane still had a sibling proving the plugin works. With the lane replaced rather than added, a silent fallback means that workspace's entire coverage is fake and there is no control.

The same reason is why deploy() now logs which mechanism chose the shell:

[nfs] argocd-app-next: new frontend system ON, from the -app-next project name
[nfs] github: new frontend system ON, from configure({ useNewFrontendSystem: true })

Logged from deploy(), not configure() — the worker fixture calls configure() with no arguments for every project before any spec runs, so for github and homepage the first line would have read off, from nothing, and the first line is the one people grep.

The guard also rewrites an unquoted YAML true to "true": coercing only in the comparison green-lit a payload the API server rejects with cannot unmarshal bool into Go struct field.

SidebarTabs — RHIDP-16458, partially

The union hardcoded "Self-service", the legacy shell's scaffolder title; under app-next the page is "Create". Both are members now. Nothing references "Create" yet — this is enabling work for RHIDP-16462, where three workarounds are in flight. Deliberately only widening the type: picking one of them here would be a fourth.

Verification

yarn check   typecheck + eslint (0 errors) + prettier clean
yarn test    141 passed — five consecutive runs, no flake
docs build   clean

Mutation-tested; each turns exactly the matching tests red: guard never reporting a dropped marker (4), intent conflict ignoring the explicit-false guard (1), lock removed (1), temp-file cleanup removed (1), boolean not rewritten in place (1).

Left open

The guard covers the secrets layer only. config/new-frontend-system/dynamic-plugins.yaml also injects plugin-app-auth and plugin-app-integrations, and a workspace can disable either from its own dynamic-plugins.yaml, which merges last — degrading the lane the same silent way. Extending the check across a second merge with different semantics is more than this PR should carry.

Version

Takes 2.1.11. #151 merged and claimed 2.1.10, which is published — latest is 2.1.10.

🤖 Generated with Claude Code

[gustavolira]

Review pass found two of the three changes wrong in ways the tests did not reach. Both fixed.

The session fix would have caused the failure it prevents

Keying the file per project removed cross-lane sharing, which is what the ticket asked for — and would have made every lane of a workspace log in to the same GitHub account concurrently. logintoGithub derives its 2FA code from one shared TOTP secret, so lanes starting inside the same 30-second window submit the identical code and GitHub rejects the second.

That this happens is not speculation — this file already carries the retry for it:

"The two-factor code you entered has already been used"
"too many codes have been submitted"

bulk-import would have gone from one login to three simultaneous ones, with a single retry to clear it.

So the file is one per user again, and what was actually missing is added instead: a lock spanning read-or-login-and-write, not just the write — two lanes that both decide there is no session go on to log in anyway. RHDH cookies from another lane were never the hazard: each lane's RHDH lives on its own namespace hostname, so they are never sent anywhere they matter.

The [nfs] line said the wrong thing for the lanes it was written for

It logged from configure(). The worker fixture calls configure() with no arguments for every project before any spec runs, so for github and homepage — which opt in via configure({ useNewFrontendSystem: true }) in beforeAll — the output was:

[nfs] github: new frontend system off, from nothing — the default is the legacy shell
[nfs] github: new frontend system ON, from configure({ useNewFrontendSystem: true })

The first line is the one a reader greps. Reported from deploy() now — once, after configuration is final.

Two smaller ones

The guard accepted an unquoted YAML true by coercing in the comparison only, so it green-lit a payload the API server rejects with cannot unmarshal bool into Go struct field ... of type string — an opaque k8s failure in place of the guard's own message. It rewrites the value now, so the tolerance is real.

The temp file survived a failed storageState write, accumulating untracked authState*.tmp in a directory nothing gitignores them from.

Verification

yarn check   typecheck + eslint (0 errors) + prettier clean
yarn test    141 passed — five consecutive runs, no flake
docs build   clean

Mutation	Tests red
lock removed	1
temp-file cleanup removed	1
boolean not rewritten in place	1

The serialisation test now waits for the first holder rather than racing two callers from the same tick — the original raced the scheduler and flaked once in five runs.

Left open, deliberately

The guard covers the secrets layer only. config/new-frontend-system/dynamic-plugins.yaml also injects plugin-app-auth and plugin-app-integrations, and a workspace can disable either from its own dynamic-plugins.yaml, which merges last — degrading the lane in the same silent way. Worth a follow-up; extending the marker check across a second merge with different semantics is more than this PR should carry.