Skip to content

feat(recorder): move login into a version-aware preamble, out of measured steps - #82

Merged
myselfsiddharth merged 3 commits into
mainfrom
track1/b2-login-preamble
Jul 28, 2026
Merged

feat(recorder): move login into a version-aware preamble, out of measured steps#82
myselfsiddharth merged 3 commits into
mainfrom
track1/b2-login-preamble

Conversation

@myselfsiddharth

Copy link
Copy Markdown
Contributor

Closes #60.

The live recorder counted login as gate-task steps — navigate → fill → fill → click → (skip)
were all recorded — so roughly five of six "gate task" steps were scaffolding, and
step-level validity was mostly measuring whether Grafana's login form had moved.

establishSession() (src/recorder/preamble.ts) now logs in
without touching TrajectoryRecorder, so no preamble action reaches trajectory.steps or a
step-validity denominator.

What I actually observed

All eight pinned versions booted and their login surface dumped, 2026-07-27. Login identity
churns once, at 10.4.19, in two places at the same time:

9.5.21, 10.0.13 10.4.19 → 13.0.3
username aria-label="Username input field", no <label>, no testid <label>Email or username</label> + data-testid, no aria-label
password aria-label="Password input field" + <label> <label> + data-testid
submit aria-label="Login button" wrapping <span>Log in</span> no aria-label, data-testid, text Log in
accessible name of submit Login button Log in

The old getByLabel("Email or username").or(getByLabel("Username")) was, as the issue says,
written without a running instance. The submit button is worse: aria-label wins over text
content for the accessible name, so on the two oldest versions
getByRole("button", { name: /log in/i }) matches zero elements.

So the preamble selects on what did not churninput[name="user"],
input[name="password"], and the single button[type="submit"], all present and unique on all
eight versions. Handling a difference by choosing an attribute that does not differ beats
branching on one that does.

Two more observed differences, both handled:

  • Landing URL gains &from=now-6h&to=now&timezone=browser at 11.5.2 → the preamble waits
    for "no longer on /login" and never asserts an exact post-login URL.
  • 13.0.3 opens a first-run Grafana Assistant dialog over the app on every boot (nothing
    persists the dismissal — no volume). Dismissed here. It is occlusion, not hiding, so
    Playwright's isVisible() reports elements underneath as visible and nothing downstream would
    notice.

No change-password interstitial on any of the eight — compose sets
GF_SECURITY_ADMIN_PASSWORD, so Grafana never forces the reset. The old conditional "Skip"
click was dead code and is not reproduced (no speculative branches). If that screen ever
appears, the preamble raises a named failure instead of guessing its way through.

Failure is named, never silent

The preamble ends by asking /api/user — 200 with the login on every version — and throws
LoginFailedError carrying the stage (open-login-page, fill-credentials, submit-login,
password-change-interstitial, verify-session). A DOM probe would be weaker: a page can look
logged in.

Live, wrong password on 11.0.0:

recorder: LOGIN FAILED (stage: verify-session) — session not established: GET /api/user
returned 401. Credentials rejected, or login did not complete.
No trajectory was written. This is scaffolding failing, not a measured step.
$ echo $?
3

Live verification — 8/8

npm run recorder -- --base-url … against each booted version:

9.5.21   exit=0  preamble: session established as admin (landed /?orgId=1)
                 steps=1 ["navigate:Open Dashboards list page"]  PREAMBLE STEPS LEAKED: none
10.0.13  exit=0  … same
10.4.19  exit=0  … same
11.0.0   exit=0  … same
11.5.2   exit=0  landed /?orgId=1&from=now-6h&to=now&timezone=browser   leaked: none
12.0.0   exit=0  … same
12.2.1   exit=0  … same
13.0.3   exit=0  … leaked: none

parameters is now {host, port} — the username/password slots are gone, because no
recorded step references those values any more. The values were never written either way.

Tests, and proof they bite

tests/unit/recorder-preamble.test.ts (8 tests) runs the preamble against fixtures of both
observed login shapes over a loopback fake-Grafana — no Docker. Demonstrated to fail on the
pre-#60 approaches, one at a time:

# username via getByLabel("Email or username")
× logs in against the aria-label login shape (observed on 9.5.21 / 10.0.13)
✓ logs in against the labelled login shape (observed on 10.4.19 → 13.0.3)

# submit via getByRole("button", { name: /log in/i })
× logs in against the aria-label login shape (observed on 9.5.21 / 10.0.13)
✓ logs in against the labelled login shape (observed on 10.4.19 → 13.0.3)

One thing I got wrong and fixed. My first fixtures used a plain
<button type="submit">Log in</button>, the unit tests passed, and the live run then failed on
9.5.21 and 10.0.13 with submit-login timeouts. The fixtures now reproduce the real markup
including aria-label="Login button" and the second "Show password" button, so that class of
bug fails in unit tests instead of on a container.

Scope

Non-goals honoured

No version-conditional logic inside measured steps; nothing added to src/runner/locators.ts;
no widened locator chains — every selector comes from something observed on a running instance.

Merge note

Test-merged against my four other open branches (#78, #79, #80, #81): all four merge clean.

Tests

npm run ci            # green — 53 unit (8 new), 1 integration, secret-scan clean
npm run test:canary   # 6 pass

🤖 Generated with Claude Code

…ured steps

The live recorder counted login as gate-task steps: navigate -> fill -> fill ->
click -> (skip) were all recorded, so roughly five of six "gate task" steps were
scaffolding and step-level validity was mostly measuring whether Grafana's login
form had moved.

`establishSession()` (src/recorder/preamble.ts) now logs in WITHOUT touching
TrajectoryRecorder, so no preamble action reaches trajectory.steps or a
step-validity denominator. Verified live on all eight pinned versions: every
emitted trajectory has exactly one measured step and zero login steps.

Login identity churns once, at 10.4.19, and in two places at the same time:

  9.5.21, 10.0.13   username aria-label "Username input field", no <label>
                    submit   aria-label "Login button" wrapping <span>Log in</span>
  10.4.19 -> 13.0.3 username <label> "Email or username" + data-testid
                    submit   no aria-label, data-testid, text "Log in"

The previous `getByLabel("Email or username").or(getByLabel("Username"))` was
written without a running instance. The submit button is worse: aria-label wins
over text content for the accessible name, so on the two oldest versions the
button is named "Login button" and getByRole("button", {name:/log in/i}) matches
ZERO elements.

So the preamble selects on what did NOT churn — input[name="user"],
input[name="password"], and the single button[type="submit"], all present and
unique on all eight. Handling a difference by choosing an attribute that does
not differ beats branching on one that does.

Also observed and handled: the landing URL gains time-range query parameters at
11.5.2 (so no exact post-login URL is asserted), and 13.0.3 opens a first-run
Grafana Assistant dialog over the app on every boot (dismissed here — it is
occlusion, which Playwright's isVisible() does not notice). No change-password
interstitial appears on any version, because compose sets
GF_SECURITY_ADMIN_PASSWORD; the old conditional "Skip" click was dead code and
is not reproduced. If that screen ever appears the preamble names it instead of
guessing.

The preamble ends by asking /api/user and throws LoginFailedError carrying the
failing stage; the CLI exits 3 and writes no trajectory. A DOM probe would be
weaker — a page can look logged in. Live check with a wrong password:

  recorder: LOGIN FAILED (stage: verify-session) — session not established:
  GET /api/user returned 401. Credentials rejected, or login did not complete.
  No trajectory was written. This is scaffolding failing, not a measured step.

tests/unit/recorder-preamble.test.ts runs the preamble against fixtures of BOTH
observed login shapes over a loopback server (no Docker). Proven to bite: a
label-based username selector fails the 9.5.21 shape, and a role-based submit
selector fails it too. The fixtures reproduce the real aria-labels — an earlier
kinder fixture let the submit-button bug through to a live run.

Two identifiers are named away from the credential noun because secret-scan
treats that noun followed by an assignment as an env-assignment hit. The scanner
was not weakened.

Scope: the --fixture path still records its login steps. That fixture is a
self-contained pipeline stand-in for the integration test, not the gate task;
stripping its login would shrink it to two steps. The live trajectory now has
one measured step, which is honest and far too thin to measure — the gap #59
and #24 exist to close.

Closes #60

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@myselfsiddharth
myselfsiddharth requested a review from a team as a code owner July 28, 2026 06:32
@github-actions
github-actions Bot requested a review from OM152002 July 28, 2026 06:32
@github-actions github-actions Bot added documentation Improvements or additions to documentation gate PRD section 9 gate measurement area: recorder Touches recorder size/XL > 600 changed lines — consider splitting labels Jul 28, 2026

@OM152002 OM152002 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The central claim holds — I verified it against live Grafana, not just the fixtures. But the 13.0.3 modal handling does not work, and I reproduced that 3/3.

Confirmed live on 9.5.21

getByRole("button", {name:/log in/i})   -> 0 match(es)
getByLabel("Email or username")         -> 0 match(es)
getByLabel("Username")                  -> 1 match(es)
input[name="user"]                      -> 1 match(es)
input[name="password"]                  -> 1 match(es)
button[type="submit"]                   -> 1 match(es)
submit: aria-label="Login button"  data-testid=null  text="Log in"

The old submit selector matched zero elements — the pre-#60 code could not have submitted the form on the two oldest pins. login-aria-label.html is faithful to the live DOM, aria-label included. Selecting on name= is the right call and the reasoning for it is correct.

One nuance the description overstates: getByLabel("Username") does match on 9.5.21, so the old .or() fallback did resolve the username field. It was the submit button alone that was unreachable. Worth fixing in the doc, since "written without a running instance" is the right diagnosis but the username selector was the half that happened to work.

Also — my own first probe on 13.0.3 showed input[name="user"] matching 0, which looked like it contradicted "stable across all eight". That was my artifact: the form renders async and I counted immediately. The preamble's waitFor({state:"visible"}) handles it, and establishSession() logs in cleanly on 13.0.3. The claim stands.

The 13.0.3 modal dismissal loses a race

establishSession() returns dismissed_first_run_modal: false, and the dialog then appears about a second later:

preamble returned: dismissed_first_run_modal=false  landed_url="/"

polling after the preamble finished:
  t+1s  dialogs=1  url="/?orgId=1&from=now-6h&to=now&timezone=browser"
  DIALOG APPEARED: "NEW\nGrafana Assistant is now available to OSS users…"

Three consecutive fresh boots, identical result. dismissFirstRunModal probes with isVisible({ timeout: 2_000 }) immediately after login lands, but the SPA finishes booting after that, so the probe returns false and the modal comes up behind it.

The consequence is exactly the one this PR set out to prevent: on 13.0.3 the dialog is covering the app when step 1 of the measured task runs, and as the module docstring itself says, isVisible() is not occlusion-aware — so nothing downstream notices, and the recording quietly captures a task performed under a modal.

The same early sampling makes landed_url wrong. It records /; a second later the URL is /?orgId=1&from=now-6h&to=now&timezone=browser — the exact string the PR's own table documents for 11.5.2 → 13.0.3. So the table is right and the field that would have caught it is measured too early to show it.

Fix is a settle point rather than more selectors: wait for a known app landmark (or poll for the dialog over a few seconds) before probing and before reading page.url(). The three-strategy closer chain looks fine — it just never runs.

Minor

A wrong password takes the full 45 s to produce its named error: waitForURL burns the entire budget before falling through to /api/user, which is the actual verdict. The comment says the fall-through gives a better message, and it does, but polling /api/user (or a much shorter URL wait) would give the same message in about a second. tests/unit/recorder-preamble.test.ts shows this — the wrong-password case is the slowest test in the file at ~8 s with a 5 s override.

The rest checks out

Tests pass, secret-scan clean, merges clean against main, #80 and #85. Keeping login out of trajectory.steps is plainly right, and verifying the session through /api/user rather than a DOM probe is the stronger choice. Recording that the change-password interstitial never appears — and refusing to write a speculative branch for it — is the correct call.

@myselfsiddharth
myselfsiddharth enabled auto-merge (squash) July 28, 2026 15:30
@myselfsiddharth
myselfsiddharth disabled auto-merge July 28, 2026 15:30
@myselfsiddharth
myselfsiddharth merged commit 3b6d4ae into main Jul 28, 2026
13 checks passed
@myselfsiddharth
myselfsiddharth deleted the track1/b2-login-preamble branch July 28, 2026 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: recorder Touches recorder documentation Improvements or additions to documentation gate PRD section 9 gate measurement size/XL > 600 changed lines — consider splitting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Recorder: move login into a version-aware preamble excluded from measured steps

2 participants