Repository navigation
Improve webchat-html-tests skill for writing tests and shared grids - #5880
Merged
Merged
Conversation
- Document what `webchat2` serves (URL to folder), that source changes need a workspace build first, and why a re-created served folder returns 404s. - Document the theme variant redirect pattern (`*.fluent.html`) and which query parameters base tests read. - Add `--min-age` and `--dry-run` to `cleanup-grid-sessions.py`; without options it still deletes every session. Recommend `--min-age 400` on a shared grid so cleanup cannot kill another run's test.
Eugene (OEvgeny)
marked this pull request as ready for review
October 17, 2026 03:24
Eugene (OEvgeny)
requested review from
William Wong (compulim),
Chris Whitten (cwhitten) and
TJ Durnford (tdurnford)
as code owners
October 17, 2026 03:24
William Wong (compulim)
approved these changes
Oct 17, 2026
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Non-finite age input bypasses cleanup protection and can delete active sessions.
1 open issue
What changed in this PR
Improves HTML test guidance and adds age-filtered cleanup for shared Selenium grids.
Changes:
- Documents rebuild requirements, asset URLs, and theme variants.
- Adds
--min-ageand--dry-runcleanup options. - Updates troubleshooting and shared-grid guidance.
| File | Description |
|---|---|
.github/skills/webchat-html-tests/SKILL.md |
Expands test-writing and cleanup workflow. |
.github/skills/webchat-html-tests/scripts/cleanup-grid-sessions.py |
Adds age filtering and dry-run support. |
.github/skills/webchat-html-tests/references/failure-modes.md |
Documents build, asset, and session troubleshooting. |
.github/skills/webchat-html-tests/references/architecture.md |
Explains served paths, rebuilds, and theme variants. |
Review effort: Balanced
|
|
||
| arguments = parser.parse_args() | ||
|
|
||
| arguments.min_age is not None and arguments.min_age < 0 and parser.error('--min-age must not be negative') |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changelog Entry
Not required, development only.
Description
Agents writing and running HTML tests with the
webchat-html-testsskill lost time on four things the skill did not cover. They checked whetherdistwas stale, because nothing says that tests load built files rather thansrc. They guessed asset URLs:/assets/surface1.jpgreturns 404, because/assets/only serves__tests__/assets/. They had to find out how*.fluent.htmlvariants work. Andcleanup-grid-sessions.pydeletes every grid session, so on a grid shared by several runs it killed another run's busy session.This PR documents the first three in the skill references and makes the cleanup script safe for shared grids. Without options, the script still deletes every session.
Design
--min-age 400keeps any session that a running test could still be using.jest-serverreuses a grid session only while it is at most 270 seconds old, and a test and itsafterEacheach time out after 50 seconds, so a session older than 400 seconds is not in use by any html2 test. The grid status has no idle time, so the script uses session age. Chrome nodes still end sessions idle for 300 seconds (SE_NODE_SESSION_TIMEOUT).Specific Changes
SKILL.md: new "Build What You Changed" step, recommendcleanup-grid-sessions.py --min-age 400, name the actual cause of dist 404s, and add "writing tests" to when to use the skillreferences/architecture.mdwebchat2serves (fromdocker-compose-wsl2.ymlandserve-test.json)webchat2restartsreferences/failure-modes.md: rows for "old behavior after a source change" and "asset 404", plus a "Grid Sessions" sectionscripts/cleanup-grid-sessions.py--min-age SECONDS: delete only sessions started at least that long ago, and keep sessions with no start time--dry-run: list sessions with their age and delete nothingI have added tests and executed them locally
I have updated
CHANGELOG.mdI have updated documentation
Testing: the script was run against a fake grid status with young, old, and no-start sessions (all option combinations, plus invalid
--min-age). It was also run with--dry-runagainst a live grid, with one session it created and then deleted. Every documented URL was checked withcurlagainstwebchat2. The bind mount behavior was checked with a throwaway container.Review Checklist
z-index)package.jsonandpackage-lock.jsonreviewed