Skip to content

ci: DH-23793: Automated agents to look at flaky E2E tests - #2782

Open
cryolins wants to merge 61 commits into
deephaven:mainfrom
cryolins:dh-23793-copilot-flake-auto-fix
Open

cryolins wants to merge 61 commits into
deephaven:mainfrom
cryolins:dh-23793-copilot-flake-auto-fix

Conversation

@cryolins

@cryolins cryolins commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Extensions to the e2e.yml pipeline to process flaky tests within runs and assign custom Copilot agents to look into fixes.

To setup:

  • Currently uses a personal access token with read and write access to actions, code, issues, and pull requests, and read access to metadata, to assign Copilot agents.
  • Store as COPILOT_ASSIGN_TOKEN as an actions repo secret
  • Could consider creating a github account specifically to handle providing the PAT. These will only be used in workflows, not by the agents.
    • If the token is not set or expired, issues should get created and the summary will show but no agents get assigned.

Changes:

  • Added agent and instruction files under .github/ for the new agent, with a new workflow Copilot can run to setup with. Model is default Sonnet 5.5.
  • Increased logs and failure trace collection to provide more information for the agent and the triaging process.
    • Agents are also told to add appropriate diagnostics to collect more info if they cannot find a fix.
  • Triage pipeline file at tests/ightly-triage.ts that deduplicates and classifies flakes, prints a summary to the actions summary, then opens up an issue and assigns an agent to it.
    • Deduplicates by stripping variable info such as line numbers and timestamps from the first relevant part of the stack trace and hashing the result into a "fingerprint" that is 1-to-1 with an issue unless its data/cache gets reset.
    • Classifies fingerprints/issues for human escalation/agent assignment by if it's a sensitive path or if previous agents attempted and could not fix.
    • Builds issue body using read stack traces and links related attachments
    • Assigns playwright-flake-investigator agents to the new issue if the token is set.
      • Max agents assigned per night at 8 just in case a wide failure doesn't trigger a large flow of agents. Can be adjusted.
    • Reassigns a new agent when an issue (not its PR) is closed, either as completed or not implemented. Or: unassigned open issue from before that also recurred this night.
  • e2e.yml also pushed back later in the night by 4 hours

Notes on possible changes:

  • Currently this is set up with Copilot, if we want to switch to another provider ex. Claude, files and file contents may have to be moved to migrate it
  • Additional failure collection could help if no helpful changes are being made by the agent, but can increase pipeline time.
    • ex. in playwright.config.ts , trace has been turned to retain-on-failure. Doing the same to video would provide some more info but as the comment notes it would slow down tests.
  • If agent behaviour is not fixing flakes in a manner that we consistently approve of, the agent instructions can be iterated on

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The exposed anonymous server endpoint and unresolved triage and reviewer-automation errors need correction before approval.

2 open issues
4 previously missed issues

In code that hasn't changed since last review

Medium severity Skip draft PRs when requesting reviewers

.github/​workflows/​copilot-pr-reviewers.yml:15

Copilot initially opens its PRs as drafts, so the opened event runs this job before review can be requested. GitHub rejects reviewer requests on draft PRs, making the new workflow fail on that initial event. Exclude drafts from the job condition; the existing ready_for_review subscription will request the team once the PR is ready.

Medium severity Use the supported Playwright trace viewer command

tests/​nightly-flake-triage.ts:745

The repository pins Playwright 1.56.1, whose CLI has show-trace but no trace subcommand. All three commands emitted here therefore fail with an unknown-command error instead of inspecting the failure artifacts. Use the supported trace viewer command and remove the claim that it works without a GUI, or provide a compatible headless inspection tool.

Medium severity Paginate timeline before selecting the latest merged PR

tests/​nightly-flake-triage.ts:855

The timeline endpoint caps per_page at 100, so this reads only the first page. Once a diagnostics-only PR reference falls on a later page, triage misses it and counts the recurrence as a failed fix, potentially escalating prematurely. Paginate the timeline before selecting the latest merged PR.

Medium severity Reconcile fresh issue states before recurrence accounting

tests/​nightly-flake-triage.ts:1387

With more than 50 recurring tracked fingerprints, issues omitted from the state refresh can retain a cached open state even when the later label lookup reports them as closed. This expression prefers the stale state, leaving those issues closed while recording them as open and preventing the intended reopen and reassignment. Reconcile fresh issue states before recurrence accounting and use the reconciled state when reopening.

1 resolved since last review

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Reproduction commands need the server-starting configuration, and timeline retrieval needs pagination to preserve correct recurrence accounting.

2 open issues
2 resolved since last review

Review effort: Balanced

Comment thread scripts/nightly-ci/githubApi.ts Resolved
Comment thread scripts/nightly-ci/issues.ts Resolved

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The PAT-backed automation requires a trusted repository-backed end-to-end run to validate agent assignment, recurring triggers, and team review requests before nightly enablement.

1 open issue
1 resolved since last review

Review effort: Balanced

@cryolins
cryolins marked this pull request as ready for review October 28, 2026 13:54
@cryolins
cryolins requested a review from mofojed October 28, 2026 13:54

@mofojed mofojed left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we can remove the auto-assign reviewer. Looking good though!


Use Playwright's locator/action auto-waiting.

Do not use page.waitForTimeout().

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder if we should just say "Prefer not to use page.waitForTimeou()". That, coupled with the "Prefer deterministic application state" item below, should make it pretty clear.

@cryolins cryolins Oct 28, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The original idea was that especially for fixing tests to try to not make it pass just by waiting for things to clear, perhaps using more deterministic waits instead, or else to add diagnostics instead. Keeping it strict might be better to prevent the agent from falling back to it at all. If we do want to be a bit more lenient I'm thinking to make it specify proper documentation for why discouraged functions were used


## When you cannot find the root cause

Do not guess, and do not quarantine. Make the _next_ occurrence diagnosable

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I like this, this is interesting. We haven't really played around with adding more logging to the tests themselves.

Comment on lines +20 to +21
# Needs pull requests write and org members read; GITHUB_TOKEN cannot read org teams.
GH_TOKEN: ${{ secrets.REVIEW_REQUEST_TOKEN }}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If we're only creating a token so we can read the org team, I think we can just skip that. If we really want to automate assigning it, then we can add the tests folder to CODEOWNERS with web-reviewers. One less token to manage.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay I've just removed auto-assigning reviewers for now

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder if this would be appropriate to use a decider for: https://strandsagents.com/blog/introducing-strands-decider/
We can see how this script holds up for now, a decider might have a bit more flexibility (but might be harder to configer? Haven't really played much with them yet)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yea I haven't looked into decision models as much yet, but it looks interesting.

export const COPILOT_CUSTOM_AGENT =
process.env.COPILOT_CUSTOM_AGENT ?? 'playwright-flake-investigator';
export const COPILOT_BASE_BRANCH = process.env.COPILOT_BASE_BRANCH ?? 'main';
export const COPILOT_MODEL = process.env.COPILOT_MODEL ?? 'claude-sonnet-5.5';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Did you try any other models, like opus? Just curious.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I haven't really used Opus for this one. I had it with auto for a while, then I switched to Sonnet since I thought these may fall into the category of smaller tasks, and to switch to Claude models. The performance is better than auto of course, and I've left it in as the default. I could play around with manually assigning Opus on my fork though.

Comment thread .github/workflows/e2e.yml
Comment on lines +317 to +319
# Assigning Copilot needs a user-to-server token; GITHUB_TOKEN is rejected.
# Unset this secret to keep triage label-only.
COPILOT_ASSIGN_TOKEN: ${{ secrets.COPILOT_ASSIGN_TOKEN }}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure we need this - Copilot should automatically get assigned as a reviewer in our repo already.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh this token is for assigning the agent assignee to work on the task. That looked to require a token for.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copilot AI balanced review requested due to automatic review settings October 28, 2026 20:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Human assessment is needed because the triage logic can mark fixes as held without executing tracked tests and omit prior diagnostic investigation links from subsequent handoffs.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@cryolins
cryolins requested a review from mofojed October 28, 2026 20:31
Copilot AI balanced review requested due to automatic review settings October 29, 2026 19:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Human assessment is needed because the new triage state machine lacks automated test coverage despite controlling issue reopening, failed-fix budgets, and agent launches.

No open issues 🎉

Review effort: Balanced

jobs:
# Must be named exactly this -- Copilot won't pick up the job otherwise.
copilot-setup-steps:
runs-on: ubuntu-latest

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ubuntu-latest now uses v26.04 of Ubuntu (recently migrated), which is causing issues: https://github.com/deephaven/web-client-ui/actions/runs/40941146661/job/123684540884?pr=2782

BEWARE: your OS is not officially supported by Playwright; installing dependencies for ubuntu26.04-x64 as a fallback.
Cannot install dependencies for ubuntu26.04-x64 with Playwright 1.56.1!
Failed to install browsers
Error: ERROR: Playwright does not support chromium on ubuntu26.04-x64

Just pin it to ubuntu-24.04 for now like we do elsewhere.

@cryolins cryolins Nov 3, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah good point. I did see all the testing actually still runs on 22.04. I think we should have that instead unless the others are going to 24.04 as well right?

Copilot AI balanced review requested due to automatic review settings November 3, 2026 17:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Human assessment is needed because stackless timeouts from unrelated tests can share a fingerprint, merging separate investigations and their failed-fix budgets.

No open issues 🎉

Review effort: Balanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants