Skip to content

test(console): pin the workspace, service-account and diagnostics paths - #2927

Merged
freeqaz merged 2 commits into
mainfrom
test-gaps-127-paths
Oct 19, 2026
Merged

freeqaz merged 2 commits into
mainfrom
test-gaps-127-paths

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator

Problem

bughunt-127 found four console paths that no test pins: a mutant of each survived the browser suites that cover it.

  • Finding 1996: the workspace file editor clears its field error on a Reload whose file read answers 404 (workspace.mjs setFieldError(editor, '') in the 404 branch). Dropping that call left the emptied editor with aria-invalid=true and the 16 KiB error.
  • Finding 1997: the authentication source form's "Issued ChatGPT service account is required." check (harness-auth.mjs account.required = method.value === 'service_account'). With account.required = false, the message appeared only in the grey status line.
  • Finding 2008: nothing pinned which permission gates Run diagnostics (detail.mjs permissions?.operate passed to the diagnostics panel), or that an unknown permission answer keeps it enabled (permitted === false).
  • Finding 2009: nothing pinned the Configuration editor's feedback.scrollIntoView after a conflict or unknown outcome, or that a successful Run diagnostics result does not scroll (if (error) before outcome?.scrollIntoView).

Change

This PR changes tests only. It adds no product code.

tests/browser/console-form-fields-2.test.mjs gets two tests after the existing workspace field-error test:

  • the workspace file editor drops an oversized file's error when Reload finds no file: Save shows the oversized-file error. The file GET then answers 404 NOT_FOUND, and Reload empties the editor, says the file is missing, removes the error and clears aria-invalid.
  • the authentication source form shows a missing service account under its field: the Agent is a Codex dedicated Agent with no harness auth. The test chooses the service account source and saves with no account selected. The error appears under the select, linked with aria-invalid, and the select has focus. No browser bubble shows and nothing is written.

tests/browser/console-status-block.test.mjs gets six tests, appended after its last test. No existing test changes.

  • Run diagnostics is enabled for a member with operate but not update: the button is enabled and one POST is sent.
  • Run diagnostics is disabled for a member with update but not operate: the button is disabled with its reason, and no POST is sent.
  • Run diagnostics is enabled for an administrator whose permission answer is withheld: the permissions read answers 503. The button is enabled and one POST is sent.
  • a Configuration save conflict brings its message on screen and a Configuration save unknown outcome brings its message on screen: the PATCH is held until released. While it is held, the page is scrolled so that Save sits at the bottom of the screen, and the test asserts that the message line starts below the fold. After release, Reload draft has focus and the message is fully on screen.
  • a successful Run diagnostics leaves the page where it was: the page clock is paused so the waiting line never shows. With the button at the bottom of the screen, the result lands below the fold, scrollY does not change and the button keeps focus.

The enabled-button cases wait for a deterministic signal: an init script marks when the page has read the permission answer's body, and the console settles the button in that same microtask checkpoint. The tests use hold-until-released fakes and no wall-clock windows.

Verification

Every mutant below was applied as a temporary edit, run against the new tests, then reverted. Each new test passes on main.

Finding Mutant Test(s) Killed
1996 workspace.mjs:180 drop setFieldError(editor, "") workspace … when Reload finds no file yes
1997 harness-auth.mjs:224 account.required = false … missing service account under its field yes
2008 detail.mjs:2657 ?.operate → ?.update operate-but-not-update, update-but-not-operate yes (both fail)
2008 detail.mjs:2657 ?.operate → ?.delete operate-but-not-update yes
2008 detail.mjs:1153 permitted === false → !permitted permission answer withheld yes
2009 detail.mjs:4433 drop feedback.scrollIntoView(...) Configuration save conflict, unknown outcome yes (both fail)
2009 detail.mjs:998 if (error) → if (true) successful Run diagnostics leaves the page yes
  • The eight new tests passed 20 out of 20 sequential runs, pinned to 2 CPUs (taskset -c 6,7).
  • Both touched files pass in full: console-form-fields-2 10/10 and console-status-block 19/19.
  • prettier --check and eslint --max-warnings 0 pass on both touched files.
  • Both files are already mapped in scripts/ci/test-suites/checks-browser-2.json. No new test file was added.

Open PRs that touch these files, checked with git merge-tree --write-tree:

PR File vs origin/main vs this head
#2922 console-form-fields-2.test.mjs clean clean

No other open PR touches console-status-block.test.mjs. The new tests there are appended at the end of the file and leave the middle of the file untouched.

Risks

The risk is low because the PR adds tests only. The withheld-answer test depends on the console reading the permissions response with Response.prototype.json. If that changes, the signal never fires, and the test fails loudly instead of passing.

Bughunt-127 found four console paths whose mutants survived every
test (findings 1996, 1997, 2008, 2009). Each new test fails under the
named mutant and passes on main:

- console-form-fields-2: a workspace Reload that answers 404 drops the
  oversized file's field error; a service-account source saved with no
  account shows its error under the select, with focus.
- console-status-block (appended): Run diagnostics follows Agent
  operate only (operate-not-update member enabled, update-not-operate
  member disabled with its reason) and stays enabled when the
  permission answer is withheld; a Configuration save conflict or
  unknown outcome scrolls its message into view; a successful
  diagnostics run does not scroll.
@freeqaz
freeqaz requested a review from a team as a code owner October 19, 2026 10:57
@clawsweeper

clawsweeper Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added the clawsweeper:intake:bug Preliminary submission classification; full source review pending. label Oct 19, 2026
@clawsweeper

clawsweeper Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes

This PR adds eight browser tests for workspace error cleanup, service-account validation, diagnostics permissions, and scrolling.

Example: Enter 16,385 ASCII characters in AGENTS.md, attempt Save, then Reload after the file disappears.

  • Before: Existing coverage could miss a regression leaving the oversized-file error on the emptied editor.
  • After: The new test requires Reload's 404 response to clear the editor, field error, and aria-invalid attribute.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Useful, focused tests with successful final-head browser CI; supporting evidence remains fixture-based, with mutation results reported by the author.
Proof confidence 🦐 gold shrimp (3/6) This maintainer-authored test-only PR exercises existing console behavior through browser fixtures; inspected screenshots from #2918 show prior Plugin JSON validation behavior and do not prove the newly added cases.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Test only · Worth it: Yes · Fix scope: Complete
User problem: Regressions could leave stale field errors, gate diagnostics on the wrong permission, or move feedback outside the visible viewport without existing tests failing.
Reason: The maintainer-authored change protects existing contracts with distinct cases and does not expand product scope.

Merge readiness

✅ Ready for maintainer review

Keep open for maintainer triage: this PR adds useful, distinct regression coverage for existing console behavior, with no actionable defects found.

Priority: P3
Reviewed head: d9db879fe406fd81c354f92c28906495efe4e8de

Before merge

None.

Findings

None.

Agent review details

How this fits together

The browser console consumes OCC API responses and permission summaries to render Agent controls, validation feedback, focus, and scroll position; these tests exercise that console with real application fixtures and selected intercepted responses.

flowchart LR
  A[Browser test actions] --> B[Console Agent page]
  B --> C[OCC application fixture]
  C --> D[Native IAM evaluator]
  E[Selected response fakes] --> B
  B --> F[Visible state and request assertions]
Loading

Technical review

Best possible solution:

Retain the focused browser coverage alongside the existing console behavior and CI suite ownership.

Do we have a high-confidence way to reproduce the issue?

This test-only PR reports coverage gaps rather than a current product bug; its mutation table identifies concrete regressions, but those experiments were not rerun during this read-only review.

Is this the best way to solve the issue?

The tests exercise real console outcomes through existing browser fixtures, isolate distinct failure paths, and avoid changing production interfaces.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against b10157ca98de.

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

Likely related people:

  • Free Wortley: Raw commit b10157c adds tests/browser/console-status-block.test.mjs:438 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: b10157ca98de; files: tests/browser/console-status-block.test.mjs)

Review metrics

Metric Value Why it matters
Change scope Production +0/-0; tests +355/-0; eight cases The growth protects distinct failure modes using existing suites and fixtures, without adding production seams.

Labels

Label changes:

  • add P3: This is preventive test coverage for behavior already implemented on main, without a current product failure.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

  • P3: This is preventive test coverage for behavior already implemented on main, without a current product failure.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

History

Review history (1 earlier review cycle)
  • reviewed 2026-10-19T11:10:37.070Z sha e468d63 :: needs maintainer review before merge. :: none

Reviewed October 19, 2026, 7:33 AM ET / 11:33 UTC (Revision 2).

…n the wait

Internal review nits: the 503 permission answer comes from
withholdPermissionSummary instead of a copy, and the comment says why
the permission-answer wait is settled by the time it resolves.
@freeqaz

freeqaz commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
Exact review queued.

Re-review progress:

@clawsweeper clawsweeper Bot added P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. labels Oct 19, 2026
@freeqaz
freeqaz merged commit 01ce409 into main Oct 19, 2026
39 checks passed
@freeqaz
freeqaz deleted the test-gaps-127-paths branch October 19, 2026 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant