Follow-up from #3019 / #3021. cypress.config.ts sets retries: 3, so a spec that fails and then passes reports the job green and leaves no trace anywhere a human will look. #3023 is a live example: a genuine oauth_auth failure on windows-latest-3 was absorbed by a retry, and finding it took pulling and grepping 20 job logs by hand.
Retries are the right call for CI stability. The gap is that nothing reports them.
Option A — the plumbing is already here but switched off
cypress.config.ts:28 sets projectId: 'ij1tyk', and .github/workflows/e2e-tests.yaml already passes CYPRESS_RECORD_KEY: ${{ secrets.CYPRESS_RECORD_KEY }} into the test step — but pnpm test:e2e is cypress run with no --record flag, so nothing is ever recorded. Cypress Cloud has flake detection and PR reporting built in, so this may be a one-flag change.
Needs a maintainer decision rather than a contributor patch: it depends on an active Cloud plan and a valid secret. Two caveats if picked up — forked PRs will not have the secret, and cypress-split is a different sharding mechanism from Cloud's --parallel, so keep the current splitting and only record.
Option B — self-contained retry reporting (no external service)
Cypress exposes retries through the after:spec node event (docs):
on('after:spec', (spec, results) => {
const retried = results.tests.some((test) =>
test.attempts.some((attempt) => attempt.state === 'failed')
)
})
An attempt with state: 'failed' followed by a pass is a retry. Convenient starting point: cypress.config.ts:50 already registers an after:spec handler for killChainlit and currently discards both arguments.
Sketch:
- Each shard writes a small JSON of retried tests (spec, title, attempt count).
- Upload it as an artifact.
- An aggregating job merges the shards and renders a table into
$GITHUB_STEP_SUMMARY.
- Optionally post/update a PR comment only when the retry count is non-zero, so clean runs stay silent.
Two constraints worth knowing before starting:
.github/workflows/e2e-tests.yaml:11 and .github/workflows/ci.yaml:15 both set permissions: read-all. A PR comment needs pull-requests: write on the aggregating job.
- On forked PRs
GITHUB_TOKEN is read-only regardless, so make the comment best-effort and $GITHUB_STEP_SUMMARY the primary output. Please do not reach for pull_request_target to work around this — it runs untrusted code with write credentials.
Option B works on forks, has no vendor dependency, and produces a machine-readable per-shard record that could later be trended to catch a spec degrading before it starts failing outright. The two options are not exclusive.
Unrelated small item, same area
cypress/e2e/thread_resume/spec.cy.ts:48 asserts cy.get('[data-testid="read-only-banner"]').should('not.exist'), but that testid appears nowhere in frontend/ or libs/ — so the assertion can never fail and is checking nothing. Either the frontend is missing the attribute (and the assertion is right but inert) or the assertion is dead and should go. Someone needs to work out which; it is left alone in #3021 as out of scope.
Follow-up from #3019 / #3021.
cypress.config.tssetsretries: 3, so a spec that fails and then passes reports the job green and leaves no trace anywhere a human will look. #3023 is a live example: a genuineoauth_authfailure onwindows-latest-3was absorbed by a retry, and finding it took pulling and grepping 20 job logs by hand.Retries are the right call for CI stability. The gap is that nothing reports them.
Option A — the plumbing is already here but switched off
cypress.config.ts:28setsprojectId: 'ij1tyk', and.github/workflows/e2e-tests.yamlalready passesCYPRESS_RECORD_KEY: ${{ secrets.CYPRESS_RECORD_KEY }}into the test step — butpnpm test:e2eiscypress runwith no--recordflag, so nothing is ever recorded. Cypress Cloud has flake detection and PR reporting built in, so this may be a one-flag change.Needs a maintainer decision rather than a contributor patch: it depends on an active Cloud plan and a valid secret. Two caveats if picked up — forked PRs will not have the secret, and
cypress-splitis a different sharding mechanism from Cloud's--parallel, so keep the current splitting and only record.Option B — self-contained retry reporting (no external service)
Cypress exposes retries through the
after:specnode event (docs):An attempt with
state: 'failed'followed by a pass is a retry. Convenient starting point:cypress.config.ts:50already registers anafter:spechandler forkillChainlitand currently discards both arguments.Sketch:
$GITHUB_STEP_SUMMARY.Two constraints worth knowing before starting:
.github/workflows/e2e-tests.yaml:11and.github/workflows/ci.yaml:15both setpermissions: read-all. A PR comment needspull-requests: writeon the aggregating job.GITHUB_TOKENis read-only regardless, so make the comment best-effort and$GITHUB_STEP_SUMMARYthe primary output. Please do not reach forpull_request_targetto work around this — it runs untrusted code with write credentials.Option B works on forks, has no vendor dependency, and produces a machine-readable per-shard record that could later be trended to catch a spec degrading before it starts failing outright. The two options are not exclusive.
Unrelated small item, same area
cypress/e2e/thread_resume/spec.cy.ts:48assertscy.get('[data-testid="read-only-banner"]').should('not.exist'), but that testid appears nowhere infrontend/orlibs/— so the assertion can never fail and is checking nothing. Either the frontend is missing the attribute (and the assertion is right but inert) or the assertion is dead and should go. Someone needs to work out which; it is left alone in #3021 as out of scope.