docs(skills): cap web-verify frames at 2000px before reading them by CrysisDeu · Pull Request #2137 · kirodotdev/KiroCrew

CrysisDeu · 2026-08-07T23:26:43Z

What

One block in the web-verify skill: keep any frame you read back under 2000px on
both edges, and downscale a file that is already oversized before reading it.

Why

An image past 2000px on either edge does not fail the turn that read it — it
wedges the session. The provider rejects the entire request once a
conversation carries many images, kiro-cli replays the full history every turn,
and the offending block sits at a fixed history index that nothing can evict.
Reading a smaller copy just appends another image while the original stays put.

Observed twice in one working session, at two different indices, with two request
ids:

messages.12.content.1.image.source.base64.data: At least one of the image
dimensions exceed max allowed size for many-image requests: 2000 pixels
(request_id: 91b58be4-9156-4071-837a-894b42ecb423)

messages.26.content.1.image.source.base64.data: … 2000 pixels
(request_id: fc072362-566d-4e3b-b403-73559194cf80)

This is easy to hit while doing exactly what the skill asks for: a 1500x900
viewport at deviceScaleFactor: 2 is 3000x1800, and that is the default in
several harnesses under website/scripts/.

Scope

Mitigation only, on the one surface we own. Two things deliberately NOT in here:

The read-side fix. fs_read's Image mode should downscale before inlining,
the way acp/prompt_blocks.py already does for every image entering a prompt
(2000px longest edge, 5 MiB encoded cap, shrink loop, 256px floor). The bytes
never cross our process, so it cannot be fixed here — filed upstream as
kiro-team/kiro-cli#3936 with that file cited as a reference implementation.
The proxy's height hole. mcp_playwright_proxy._encode_frame caps width
at 1920 and leaves height unbounded, so browser_take_screenshot on a long page
saves a frame taller than the cap. That one reaches real users rather than just
agents and wants its own change.

Verification

The downscale command in the block was run against a real oversized capture:
(3000, 1800) -> (2000, 1200).
It resolves the interpreter via command -v kirocrew on purpose: bare python3
frequently has no Pillow (it does not on this dev desk), while KiroCrew's venv
always does — prompt_blocks.py depends on it.
scripts/docs_lint.py: 187 markdown files scanned, all checks passed.

github-actions · 2026-08-07T23:28:12Z

Opus 5 Review — ✅ no blocking findings

Reviewed 96ed647bd6cf55b14e41a2051df79a520595fb44 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 96ed647

Verdict parsed from the review's SHA-scoped output markers for commit 96ed647bd6cf55b14e41a2051df79a520595fb44.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 96ed647bd6cf55b14e41a2051df79a520595fb44: <one-sentence reason>

github-actions · 2026-08-07T23:28:32Z

Design Review (Fable 5) — ✅ PASS

Advisory design-level review of 96ed647bd6cf55b14e41a2051df79a520595fb44 — updated in place on each push; does not block merge.

Design-Verdict: PASS

Real session-wedging failure, mitigated on the only surface this repo owns, with the root cause correctly pushed upstream (kiro-team/kiro-cli#3936) rather than papered over here.

The scope call is sound: the cap literal is pinned to prompt_blocks.MAX_IMAGE_EDGE_PX by test so the two enforcement points can't drift silently, the script lives in builtin_skills/ (the only path that reaches installed users, per the repo's own rule), and both deliberately-excluded fixes (fs_read downscaling, the proxy's unbounded height) are named with owners instead of silently dropped.

[DESIGN-REVIEWED] 96ed647

github-actions · 2026-08-07T23:29:47Z

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 96ed647bd6cf55b14e41a2051df79a520595fb44 and found no blocking issues.

This comment is updated in place on each push.

Review details

FINDING -- src/kiro_crew/builtin_skills/web-verify/scripts/downscale_image.py:82 -- Desktop installs provide bin/python3.12, so "python" is not found and Pillow-less system Python leaves the oversized image unchanged -> Fix: also accept the python3.12 sibling.
[GPT-REVIEWED] 96ed647

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 96ed647bd6cf55b14e41a2051df79a520595fb44: <one-sentence reason>

CrysisDeu · 2026-08-08T00:02:16Z

Dispositions for the two BLOCKING findings on 8810a03fb453cfe7b5da69c82731ce3fbbbc2ed1:

SKILL.md:46 — downscale command fails on native Windows: fixed. Both this
and the earlier readlink -f finding on eaa7c255 were holes in the same
mechanism — a shell one-liner doing path math. Rather than add a second
PowerShell variant to keep in sync, the logic moved into
src/kiro_crew/builtin_skills/web-verify/scripts/downscale_image.py, which the
skill now invokes. The interpreter lookup is one platform-agnostic rule (the
kirocrew launcher's sibling, resolved through its symlink); bin/ and
Scripts/ differ only in the executable suffix, which the script handles via
os.name. Pinned by test_bundled_python_uses_the_exe_suffix_on_windows and
test_bundled_python_is_the_launcher_s_sibling.
SKILL.md:47 — apostrophes in screenshot paths break resizing: fixed. Paths
are argv now, not interpolated into Python source, so quoting is the shell's
concern. Pinned by test_path_containing_an_apostrophe_is_handled, which runs
the real entrypoint against O'Brien dir/it's a shot.png.

Local gates on 1c6b5cc9: 10/10 new tests pass, isort + flake8 + mypy clean on
both new files, brand gate clean on added lines, docs_lint.py all checks passed.

Migrates the Schedule page's hand-rolled table to the shared shadcn `ui/table` primitives and its resizable detail side panel to `ui/dialog`, then fixes what that exposed: the page could not fit its own data. ## Table The 10-column table was 39 repetitions of one 12-class `<th>` string (already drifted into 10 variants) and 134 copies of a per-cell border declaration. Borders now belong to the row and padding to the component. Removing the boilerplate surfaced a latent bug: folder header rows spanned `colSpan={11}` against a 10-column table, tolerated by browsers but broken by any column change. One `SCHEDULE_COLUMNS` constant now feeds every full-width row. Typography follows UPSTREAM shadcn rather than the old table's look: 14px/font-medium/sentence-case headers instead of 10px uppercase, 8px padding instead of 10/12. Measured on the same fixture, that is a 47px row against the old 111px -- the previous look bought its narrow width by folding cells (a long agent/model line wrapped to four lines and dragged every row in that table to 111px). `table-fixed` makes the column widths a contract instead of a hint. Upstream's `whitespace-nowrap` alone overflowed the container by 346px at a 1500px window and pushed the entire Actions column outside the scroll area -- content-dependent, so it appeared and disappeared with the data. Fixed layout makes horizontal overflow structurally impossible: over-long values truncate with a tooltip instead of shoving their neighbours. Verified 0 overflow at 1500 / 1280 / 1100px, including a fixture with a deliberately long job name and agent name. ## Row actions Six controls per row (Strict, Run/Cancel, View/Continue, Pause/Resume, Move, Delete) did not fit a 210px cell and were the bulk of the width problem. The row keeps Run/Cancel and Delete; the rest move into an overflow menu (`CronRowActions`), with folder moves as a submenu. Delete deliberately stays in the row: it is an arm-then-Confirm state machine and a menu that closes on select cannot hold the armed state. ## Detail view The side panel becomes a centered dialog, following the same migration the Crews page already made. Focus trap, Escape and overlay dismissal now come from Radix instead of a hand-rolled backdrop, and three more hand-rolled `role="dialog"` blocks on this page (new folder, bulk delete, per-job delete confirm) go with it, taking their `eslint-disable jsx-a11y` comments along. Job selection is split from dialog visibility on purpose. `selected` also drives the calendar highlight and the Executions job filter, and a modal cannot sit beside the thing it filters the way a side panel could -- so dismissing the dialog no longer clears them. Dropped with the panel: drag-to-resize (a modal has a fixed width) and viewing the form next to the list. ## Chrome The page had one `<Card>` whose title was the word "Jobs", under a header already reading "Schedule / Manage recurring cron jobs and scheduled tasks". Card and title are gone, matching AgentsPage and KiroCrewAgentsPage which carry no card at all. The header strip mixed four intents with no separation, which made the view switcher read as three more buttons; view switching is navigation and moves to `PageHeader` actions, while the toolbar keeps filter, batch actions, New folder and a split Add Job button whose second half opens the template gallery (one intent, two starting points). Create sits above the view switch so it survives Calendar and Executions, which the card header used to do implicitly. The permanently-on accent banner restating the subtitle is gone; the affordance it carried (you can just ask in chat) is now a link in the subtitle, which is why `PageHeader.subtitle` widens from `string` to `ReactNode`. ## Tests Four assertions encoded the old row layout and were rewritten against the new contracts -- notably "move sits before delete" becomes "the row action cell holds exactly Run, Delete and the overflow menu", which is the width budget this change is defending. Three tests reached the Templates button through page chrome that is now a split-button menu, and three more reached *behind* an open modal, which Radix correctly marks aria-hidden. Two new tests cover row-click-opens-dialog and selection-survives-dismissal. `playwright/cron.spec.ts` drives pause/resume through the overflow menu. Nine i18n keys left dead by the removed chrome are deleted across 13 catalogs; the dead-key ratchet holds at its baseline of 22. Screenshots are captured at 1x and stay under 2000px per edge, so they can be read back without wedging a conversation (see #2137).

…s it An image whose width or height exceeds 2000px wedges the session permanently, not just the turn that read it: the provider rejects the whole request once a conversation carries many images, kiro-cli replays the full history every turn, and the offending block sits at a fixed history index that nothing can evict. Observed twice in one session at two different indices (messages.12, messages.26) with two request ids. A 1500x900 viewport at deviceScaleFactor 2 is 3000x1800, which is the default in several harnesses under website/scripts -- so this is easy to hit by accident while doing exactly what the skill asks for. The guidance ships as a SCRIPT rather than the shell one-liner two earlier revisions carried, because that one-liner had a portability hole per platform and each round of review found the next one: `readlink -f` is GNU-only and exits 127 on macOS; native Windows has no `python3` and puts the venv interpreter in `Scripts/`; and a path like `/tmp/O'Brien/shot.png` broke an inlined `p='...'` literal with a SyntaxError, silently leaving the frame oversized. Paths now arrive as argv, so quoting is the shell's problem, and the interpreter lookup is one platform-agnostic rule: the `kirocrew` launcher's SIBLING, resolved through its symlink -- `bin/` on POSIX and `Scripts/` on Windows differ only in the executable suffix. If the invoking Python has no Pillow the script re-execs itself under Kiro Crew's venv, guarded against looping. Rewrites only files actually over the cap, so a repeat run is idempotent and never re-encodes a frame that was already safe. 10 tests pin the three holes that produced this shape (apostrophe in the path, the Windows suffix, idempotence), the re-exec loop guard, and that MAX_EDGE_PX still equals prompt_blocks.MAX_IMAGE_EDGE_PX -- one number with two enforcement points drifts silently otherwise. The read-side fix belongs upstream and is filed as kiro-team/kiro-cli#3936; this is the mitigation we own.

CrysisDeu · 2026-08-08T01:45:44Z

Dispositions for f4f0b296a4d07b680f50e757e06538a87de226c9 → 96ed647b:

BLOCKING downscale_image.py:126 — decompression-bomb screenshots abort the
helper: fixed. DecompressionBombError is not an OSError, so it escaped
main and abandoned every remaining path — the opposite of the script's job.
Two changes: the decode ceiling is raised to an explicit MAX_DECODE_PIXELS = 512_000_000 (Pillow's stock ~89M rejects a legitimate fullPage capture of a
long page — the finding's own 3000x60000 example is 180M, i.e. the exact input
this script exists for), and the error is now caught so a file beyond even that
fails its own path and the run continues. The ceiling is restored in finally,
because it is a module-level global in Pillow and leaving it raised would
silently relax the guard for every later decode in the process.
Pinned by test_decompression_bomb_does_not_abort_the_remaining_paths, which
squeezes the ceiling instead of committing a 180M-pixel fixture and asserts both
paths were reported (not just the first), the file was left byte-identical, and
the ceiling came back.
FINDING downscale_image.py:104 — function-local import: fixed. Moved to a
module-level optional import behind a TYPE_CHECKING guard, so a Pillow-less
interpreter still imports the module (which the re-exec path depends on) while
mypy keeps the real type. test_pillow_is_imported_at_module_level asserts the
shrink body carries no import.

Also fixed this round, root-caused from the CI log rather than guessed at (the two
previous attempts were wrong):

Backend Tests (Windows) (4). tempfile.TemporaryDirectory() on the Windows
runner returns the 8.3 SHORT path (C:\Users\RUNNER~1\...) while the code under
test runs os.path.realpath, which returns the LONG path
(C:\Users\runneradmin\...). The assertion compared those as raw strings, so it
could only ever fail on Windows. Both platform tests now compare resolved paths.
A real Windows defect in the helper itself, found while chasing the above:
img.save(path) inside with Image.open(path) is refused on Windows because
Pillow holds the source open. The rewrite now goes to a sibling temp file and
os.replace() after the with block — which also makes it atomic, so an
interrupted save cannot truncate a valid image.
test_failed_save_leaves_the_original_intact pins that.

Local gates on 96ed647b: 14/14 tests pass, isort + flake8 + mypy clean on both
files, brand gate clean on added lines, docs_lint.py all checks passed.

Migrates the Schedule page's hand-rolled table to the shared shadcn `ui/table` primitives and its resizable detail side panel to `ui/dialog`, then fixes what that exposed: the page could not fit its own data. The 10-column table was 39 repetitions of one 12-class `<th>` string (already drifted into 10 variants) and 134 copies of a per-cell border declaration. Borders now belong to the row and padding to the component. Removing the boilerplate surfaced a latent bug: folder header rows spanned `colSpan={11}` against a 10-column table, tolerated by browsers but broken by any column change. One `SCHEDULE_COLUMNS` constant now feeds every full-width row. Typography follows UPSTREAM shadcn rather than the old table's look: 14px/font-medium/sentence-case headers instead of 10px uppercase, 8px padding instead of 10/12. Measured on the same fixture, that is a 47px row against the old 111px -- the previous look bought its narrow width by folding cells (a long agent/model line wrapped to four lines and dragged every row in that table to 111px). `table-fixed` makes the column widths a contract instead of a hint. Upstream's `whitespace-nowrap` alone overflowed the container by 346px at a 1500px window and pushed the entire Actions column outside the scroll area -- content-dependent, so it appeared and disappeared with the data. Fixed layout makes horizontal overflow structurally impossible: over-long values truncate with a tooltip instead of shoving their neighbours. Verified 0 overflow at 1500 / 1280 / 1100px, including a fixture with a deliberately long job name and agent name. Six controls per row (Strict, Run/Cancel, View/Continue, Pause/Resume, Move, Delete) did not fit a 210px cell and were the bulk of the width problem. The row keeps Run/Cancel and Delete; the rest move into an overflow menu (`CronRowActions`), with folder moves as a submenu. Delete deliberately stays in the row: it is an arm-then-Confirm state machine and a menu that closes on select cannot hold the armed state. The side panel becomes a centered dialog, following the same migration the Crews page already made. Focus trap, Escape and overlay dismissal now come from Radix instead of a hand-rolled backdrop, and three more hand-rolled `role="dialog"` blocks on this page (new folder, bulk delete, per-job delete confirm) go with it, taking their `eslint-disable jsx-a11y` comments along. Job selection is split from dialog visibility on purpose. `selected` also drives the calendar highlight and the Executions job filter, and a modal cannot sit beside the thing it filters the way a side panel could -- so dismissing the dialog no longer clears them. Dropped with the panel: drag-to-resize (a modal has a fixed width) and viewing the form next to the list. The page had one `<Card>` whose title was the word "Jobs", under a header already reading "Schedule / Manage recurring cron jobs and scheduled tasks". Card and title are gone, matching AgentsPage and KiroCrewAgentsPage which carry no card at all. The header strip mixed four intents with no separation, which made the view switcher read as three more buttons; view switching is navigation and moves to `PageHeader` actions, while the toolbar keeps filter, batch actions, New folder and a split Add Job button whose second half opens the template gallery (one intent, two starting points). Create sits above the view switch so it survives Calendar and Executions, which the card header used to do implicitly. The permanently-on accent banner restating the subtitle is gone; the affordance it carried (you can just ask in chat) is now a link in the subtitle, which is why `PageHeader.subtitle` widens from `string` to `ReactNode`. Four assertions encoded the old row layout and were rewritten against the new contracts -- notably "move sits before delete" becomes "the row action cell holds exactly Run, Delete and the overflow menu", which is the width budget this change is defending. Three tests reached the Templates button through page chrome that is now a split-button menu, and three more reached *behind* an open modal, which Radix correctly marks aria-hidden. Two new tests cover row-click-opens-dialog and selection-survives-dismissal. `playwright/cron.spec.ts` drives pause/resume through the overflow menu. Nine i18n keys left dead by the removed chrome are deleted across 13 catalogs; the dead-key ratchet holds at its baseline of 22. Screenshots are captured at 1x and stay under 2000px per edge, so they can be read back without wedging a conversation (see #2137).

Migrates the Schedule page's hand-rolled table to the shared shadcn `ui/table` primitives and its resizable detail side panel to `ui/dialog`, then fixes what that exposed: the page could not fit its own data. The 10-column table was 39 repetitions of one 12-class `<th>` string (already drifted into 10 variants) and 134 copies of a per-cell border declaration. Borders now belong to the row and padding to the component. Removing the boilerplate surfaced a latent bug: folder header rows spanned `colSpan={11}` against a 10-column table, tolerated by browsers but broken by any column change. One `SCHEDULE_COLUMNS` constant now feeds every full-width row. Typography follows UPSTREAM shadcn rather than the old table's look: 14px/font-medium/sentence-case headers instead of 10px uppercase, 8px padding instead of 10/12. Measured on the same fixture, that is a 47px row against the old 111px -- the previous look bought its narrow width by folding cells (a long agent/model line wrapped to four lines and dragged every row in that table to 111px). The Actions column is 176px, not the 164px first written. Under `table-fixed` a declared width is a contract, so a cell needing more spills OUT of the table rather than widening it -- the three inline controls measure 176px and the 12px shortfall was absorbed by the shadcn wrapper's `overflow-x-auto`, i.e. a horizontal scrollbar this table must never need. Measured at 1500 / 1280 / 1100px: document scroll 0, wrapper scroll 0, and no element past the wrapper's right edge. `table-fixed` makes the column widths a contract instead of a hint. Upstream's `whitespace-nowrap` alone overflowed the container by 346px at a 1500px window and pushed the entire Actions column outside the scroll area -- content-dependent, so it appeared and disappeared with the data. Fixed layout makes horizontal overflow structurally impossible: over-long values truncate with a tooltip instead of shoving their neighbours. Verified 0 overflow at 1500 / 1280 / 1100px, including a fixture with a deliberately long job name and agent name. Six controls per row (Strict, Run/Cancel, View/Continue, Pause/Resume, Move, Delete) did not fit a 210px cell and were the bulk of the width problem. The row keeps Run/Cancel and Delete; the rest move into an overflow menu (`CronRowActions`), with folder moves as a submenu. Delete deliberately stays in the row: it is an arm-then-Confirm state machine and a menu that closes on select cannot hold the armed state. The side panel becomes a centered dialog, following the same migration the Crews page already made. Focus trap, Escape and overlay dismissal now come from Radix instead of a hand-rolled backdrop, and three more hand-rolled `role="dialog"` blocks on this page (new folder, bulk delete, per-job delete confirm) go with it, taking their `eslint-disable jsx-a11y` comments along. Job selection is split from dialog visibility on purpose. `selected` also drives the calendar highlight and the Executions job filter, and a modal cannot sit beside the thing it filters the way a side panel could -- so dismissing the dialog no longer clears them. Dropped with the panel: drag-to-resize (a modal has a fixed width) and viewing the form next to the list. The page's `<Card>` stays and frames the table, matching the repo's page-layout pattern (HooksPage). Its title does not: it was the word "Jobs", directly under a header already reading "Schedule / Manage recurring cron jobs and scheduled tasks", between which a third label restated both. The header strip mixed four intents with no separation, which made the view switcher read as three more buttons; view switching is navigation and moves to `PageHeader` actions, while the toolbar keeps filter, batch actions, New folder and a split Add Job button whose second half opens the template gallery (one intent, two starting points). Create sits above the view switch so it survives Calendar and Executions, which the card header used to do implicitly. The permanently-on accent banner restating the subtitle is gone; the affordance it carried (you can just ask in chat) is now a link in the subtitle, which is why `PageHeader.subtitle` widens from `string` to `ReactNode`. Four assertions encoded the old row layout and were rewritten against the new contracts -- notably "move sits before delete" becomes "the row action cell holds exactly Run, Delete and the overflow menu", which is the width budget this change is defending. Three tests reached the Templates button through page chrome that is now a split-button menu, and three more reached *behind* an open modal, which Radix correctly marks aria-hidden. Two new tests cover row-click-opens-dialog and selection-survives-dismissal. `playwright/cron.spec.ts` drives pause/resume through the overflow menu. Nine i18n keys left dead by the removed chrome are deleted across 13 catalogs; the dead-key ratchet holds at its baseline of 22. Screenshots are captured at 1x and stay under 2000px per edge, so they can be read back without wedging a conversation (see #2137).

GFM's autolink-literal extension ends a bare `https://…` run only at ASCII whitespace or `<`. CJK prose puts punctuation directly after a URL with no space, so the punctuation goes into the href: （https://github.com/kirodotdev/KiroCrew/pull/2137，`96ed647b`）：`readiness` renders as a single link whose href is `…/pull/2137%EF%BC%8C%6096ed647b%60%EF%BC%89%EF%BC%9A%60readiness`. The broken href is the smaller half of the damage. The run also eats the OPENING backtick of the code span that follows, which shifts every later backtick pairing in the paragraph by one: prose renders as inline code and real code renders with literal backticks. One missing space corrupts the rest of the message. Fixed at the source level, before micromark tokenizes — re-splitting the link node on the mdast cannot restore the code-span pairing, because that pairing is decided while the whole paragraph is tokenized. `fixCjkAutolinkBoundaries` closes the URL head as an angle autolink `<url>`, which has an explicit end and renders identically. Guards: - Only rewrites runs GFM would actually have autolinked (host must contain a dot; neither of the last two labels may contain `_`), so it never invents a link the author did not write. - Drops the trailing punctuation GFM trims but an angle autolink would keep (`?!.,:*_~` and an unbalanced `)`). - Offset-preserving mask keeps fenced and indented code blocks, inline-code spans, existing links and images, angle autolinks, raw HTML tags, and link reference definitions untouched. - CJK ideographs and fullwidth alphanumerics are NOT terminators, so `https://zh.wikipedia.org/wiki/中文` still links in full. Scope: `http(s)://` runs only. Scheme-less `www.` literals have the same flaw but cannot be closed with `<…>`, which requires a scheme.

Migrates the Schedule page's hand-rolled table to the shared shadcn `ui/table` primitives and its resizable detail side panel to `ui/dialog`, then fixes what that exposed: the page could not fit its own data. The 10-column table was 39 repetitions of one 12-class `<th>` string (already drifted into 10 variants) and 134 copies of a per-cell border declaration. Borders now belong to the row and padding to the component. Removing the boilerplate surfaced a latent bug: folder header rows spanned `colSpan={11}` against a 10-column table, tolerated by browsers but broken by any column change. One `SCHEDULE_COLUMNS` constant now feeds every full-width row. Typography follows UPSTREAM shadcn rather than the old table's look: 14px/font-medium/sentence-case headers instead of 10px uppercase, 8px padding instead of 10/12. Measured on the same fixture, that is a 47px row against the old 111px -- the previous look bought its narrow width by folding cells (a long agent/model line wrapped to four lines and dragged every row in that table to 111px). The Actions column is 176px, not the 164px first written. Under `table-fixed` a declared width is a contract, so a cell needing more spills OUT of the table rather than widening it -- the three inline controls measure 176px and the 12px shortfall was absorbed by the shadcn wrapper's `overflow-x-auto`, i.e. a horizontal scrollbar this table must never need. Measured at 1500 / 1280 / 1100px: document scroll 0, wrapper scroll 0, and no element past the wrapper's right edge. `table-fixed` makes the column widths a contract instead of a hint. Upstream's `whitespace-nowrap` alone overflowed the container by 346px at a 1500px window and pushed the entire Actions column outside the scroll area -- content-dependent, so it appeared and disappeared with the data. Fixed layout makes horizontal overflow structurally impossible: over-long values truncate with a tooltip instead of shoving their neighbours. Verified 0 overflow at 1500 / 1280 / 1100px, including a fixture with a deliberately long job name and agent name. Six controls per row (Strict, Run/Cancel, View/Continue, Pause/Resume, Move, Delete) did not fit a 210px cell and were the bulk of the width problem. The row keeps Run/Cancel and Delete; the rest move into an overflow menu (`CronRowActions`), with folder moves as a submenu. Delete deliberately stays in the row: it is an arm-then-Confirm state machine and a menu that closes on select cannot hold the armed state. The side panel becomes a centered dialog, following the same migration the Crews page already made. Focus trap, Escape and overlay dismissal now come from Radix instead of a hand-rolled backdrop, and three more hand-rolled `role="dialog"` blocks on this page (new folder, bulk delete, per-job delete confirm) go with it, taking their `eslint-disable jsx-a11y` comments along. Job selection is split from dialog visibility on purpose. `selected` also drives the calendar highlight and the Executions job filter, and a modal cannot sit beside the thing it filters the way a side panel could -- so dismissing the dialog no longer clears them. Dropped with the panel: drag-to-resize (a modal has a fixed width) and viewing the form next to the list. The page's `<Card>` stays and frames the table, matching the repo's page-layout pattern (HooksPage). Its title does not: it was the word "Jobs", directly under a header already reading "Schedule / Manage recurring cron jobs and scheduled tasks", between which a third label restated both. The header strip mixed four intents with no separation, which made the view switcher read as three more buttons; view switching is navigation and moves to `PageHeader` actions, while the toolbar keeps filter, batch actions, New folder and a split Add Job button whose second half opens the template gallery (one intent, two starting points). Create sits above the view switch so it survives Calendar and Executions, which the card header used to do implicitly. The permanently-on accent banner restating the subtitle is gone; the affordance it carried (you can just ask in chat) is now a link in the subtitle, which is why `PageHeader.subtitle` widens from `string` to `ReactNode`. Four assertions encoded the old row layout and were rewritten against the new contracts -- notably "move sits before delete" becomes "the row action cell holds exactly Run, Delete and the overflow menu", which is the width budget this change is defending. Three tests reached the Templates button through page chrome that is now a split-button menu, and three more reached *behind* an open modal, which Radix correctly marks aria-hidden. Two new tests cover row-click-opens-dialog and selection-survives-dismissal. `playwright/cron.spec.ts` drives pause/resume through the overflow menu. Nine i18n keys left dead by the removed chrome are deleted across 13 catalogs; the dead-key ratchet holds at its baseline of 22. Screenshots are captured at 1x and stay under 2000px per edge, so they can be read back without wedging a conversation (see #2137).

GFM's autolink-literal extension ends a bare `https://…` run only at ASCII whitespace or `<`. CJK prose puts punctuation directly after a URL with no space, so the punctuation goes into the href: （https://github.com/kirodotdev/KiroCrew/pull/2137，`96ed647b`）：`readiness` renders as a single link whose href is `…/pull/2137%EF%BC%8C%6096ed647b%60%EF%BC%89%EF%BC%9A%60readiness`. The broken href is the smaller half of the damage. The run also eats the OPENING backtick of the code span that follows, which shifts every later backtick pairing in the paragraph by one: prose renders as inline code and real code renders with literal backticks. One missing space corrupts the rest of the message. Fixed at the source level, before micromark tokenizes — re-splitting the link node on the mdast cannot restore the code-span pairing, because that pairing is decided while the whole paragraph is tokenized. `fixCjkAutolinkBoundaries` closes the URL head as an angle autolink `<url>`, which has an explicit end and renders identically. Guards: - Only rewrites runs GFM would actually have autolinked (host must contain a dot; neither of the last two labels may contain `_`), so it never invents a link the author did not write. - Drops the trailing punctuation GFM trims but an angle autolink would keep (`?!.,:*_~` and an unbalanced `)`). - Offset-preserving mask keeps fenced and indented code blocks, inline-code spans, existing links and images, angle autolinks, raw HTML tags, and link reference definitions untouched. - CJK ideographs and fullwidth alphanumerics are NOT terminators, so `https://zh.wikipedia.org/wiki/中文` still links in full. Scope: `http(s)://` runs only. Scheme-less `www.` literals have the same flaw but cannot be closed with `<…>`, which requires a scheme.

Reported from the dashboard chat transcript: **#2137 — review-ready**（https://github.com/kirodotdev/KiroCrew/pull/2137，`96ed647b`）：`readiness: passed` GFM's autolink-literal extension ends a bare `https://…` run only at ASCII whitespace or `<`. CJK prose writes punctuation directly after a URL with no space, so `，`96ed647b`）：`readiness` all lands inside the href. The broken href is the smaller half of the damage. The run also eats the OPENING backtick of the code span that follows, which shifts every later backtick pairing in the paragraph by one: prose renders as inline code and real code renders with literal backticks. One missing space corrupts the rest of the message. Fixed at the SOURCE level, before micromark tokenizes — re-splitting the link node on the mdast fixes the href but cannot restore the code-span pairing, because that pairing is decided while the whole paragraph is tokenized. The URL head is re-emitted as an angle autolink `<url>`, which has an explicit end and renders identically. Which regions are off-limits is read off remark's OWN parse (same plugin set as the render pipeline), so code, existing links, raw HTML and math are excluded by construction rather than by a hand-rolled scanner. The cut is EVIDENCE-BASED, because CJK punctuation reaches real URLs raw: 1. A CJK closing bracket that closes an opener left unclosed in the PROSE before the URL — GFM's own paren-balancing rule generalised. `（https://x.com/a）` and `（见 https://x.com/a）` cut; `https://x.com/苹果（公司）` does not, and neither does `https://x.com/search?q=foo）` (nothing opened a bracket). 2. A SEPARATOR-class CJK mark immediately followed by a BACKTICK — the destructive case, and the backtick is the one character RFC 3986 excludes so it cannot be inside a raw-written URL. Sentence-enders (`。．！？…｡`) are excluded from rule 2. Real page titles end in them and reach the URL raw — `…/wiki/モーニング娘。`, `…/wiki/魔法先生ネギま！` — so cutting there would point the anchor at the wrong article. Separators like `，`、`、`、`；`、`：` do not end titles, so they stay eligible, which is what keeps the reported case fixed. A mixed run cuts at the separator and leaves the sentence-ender inside the URL: `…/a。，`c`` gives `<…/a。>，`c``. Accepted trade, locked in by tests rather than left implicit: a genuine prose `。` directly before a code span keeps today's behaviour, and so does `…/pull/1，然后回来` (indistinguishable from `…/wiki/我，机器人`). Not adopted: `@streamdown/cjk`'s autolink handling, measured at 1.0.3. It splits at the FIRST character in a fixed 20-mark set with no evidence test, which breaks `…/wiki/苹果（公司）`, `…/wiki/モーニング娘。` and `…/wiki/我，机器人`; it also runs after tokenization so it cannot repair the code-span pairing, and it mis-splits explicit `<…>` autolinks. 34 tests. Full suite on this base: 877 files, 11829 tests, zero failures.

Reported from the dashboard chat transcript: **#2137 — review-ready**（https://github.com/kirodotdev/KiroCrew/pull/2137，`96ed647b`）：`readiness: passed` GFM's autolink-literal extension ends a bare `https://…` run only at ASCII whitespace or `<`. CJK prose writes punctuation directly after a URL with no space, so `，`96ed647b`）：`readiness` all lands inside the href. The broken href is the smaller half of the damage. The run also eats the OPENING backtick of the code span that follows, which shifts every later backtick pairing in the paragraph by one: prose renders as inline code and real code renders with literal backticks. One missing space corrupts the rest of the message. Fixed at the SOURCE level, before micromark tokenizes — re-splitting the link node on the mdast fixes the href but cannot restore the code-span pairing, because that pairing is decided while the whole paragraph is tokenized. The URL head is re-emitted as an angle autolink `<url>`, which has an explicit end and renders identically. Which regions are off-limits is read off remark's OWN parse (same plugin set as the render pipeline), so code, existing links, raw HTML and math are excluded by construction rather than by a hand-rolled scanner. The cut is EVIDENCE-BASED, because CJK punctuation reaches real URLs raw: 1. A CJK closing bracket that closes an opener left unclosed in the PROSE before the URL — GFM's own paren-balancing rule generalised. `（https://x.com/a）` and `（见 https://x.com/a）` cut; `https://x.com/苹果（公司）` does not, and neither does `https://x.com/search?q=foo）` (nothing opened a bracket). 2. A SEPARATOR-class CJK mark immediately followed by a BACKTICK — the destructive case, and the backtick is the one character RFC 3986 excludes so it cannot be inside a raw-written URL. Sentence-enders (`。．！？…｡`) are excluded from rule 2. Real page titles end in them and reach the URL raw — `…/wiki/モーニング娘。`, `…/wiki/魔法先生ネギま！` — so cutting there would point the anchor at the wrong article. Separators like `，`、`、`、`；`、`：` do not end titles, so they stay eligible, which is what keeps the reported case fixed. A mixed run cuts at the separator and leaves the sentence-ender inside the URL: `…/a。，`c`` gives `<…/a。>，`c``. Accepted trade, locked in by tests rather than left implicit: a genuine prose `。` directly before a code span keeps today's behaviour, and so does `…/pull/1，然后回来` (indistinguishable from `…/wiki/我，机器人`). Not adopted: `@streamdown/cjk`'s autolink handling, measured at 1.0.3. It splits at the FIRST character in a fixed 20-mark set with no evidence test, which breaks `…/wiki/苹果（公司）`, `…/wiki/モーニング娘。` and `…/wiki/我，机器人`; it also runs after tokenization so it cannot repair the code-span pairing, and it mis-splits explicit `<…>` autolinks. 34 tests. Full suite on this base: 877 files, 11829 tests, zero failures. Visual evidence under `temp-screenshots/cjk-autolink-boundary/`, captured by `website/scripts/capture-cjk-autolink.mjs`: the REAL built SPA behind the shared fixture server, once from this branch and once from a dist built with origin/main's `MarkdownRenderer.tsx`. The frame carries the reported line, the bracket rule, and a real article URL containing `（公司）` that must survive untouched in both builds.

Migrates the Schedule page's hand-rolled table to the shared shadcn `ui/table` primitives and its resizable detail side panel to `ui/dialog`, then fixes what that exposed: the page could not fit its own data. The 10-column table was 39 repetitions of one 12-class `<th>` string (already drifted into 10 variants) and 134 copies of a per-cell border declaration. Borders now belong to the row and padding to the component. Removing the boilerplate surfaced a latent bug: folder header rows spanned `colSpan={11}` against a 10-column table, tolerated by browsers but broken by any column change. One `SCHEDULE_COLUMNS` constant now feeds every full-width row. Typography follows UPSTREAM shadcn rather than the old table's look: 14px/font-medium/sentence-case headers instead of 10px uppercase, 8px padding instead of 10/12. Measured on the same fixture, that is a 47px row against the old 111px -- the previous look bought its narrow width by folding cells (a long agent/model line wrapped to four lines and dragged every row in that table to 111px). The Actions column is 176px, not the 164px first written. Under `table-fixed` a declared width is a contract, so a cell needing more spills OUT of the table rather than widening it -- the three inline controls measure 176px and the 12px shortfall was absorbed by the shadcn wrapper's `overflow-x-auto`, i.e. a horizontal scrollbar this table must never need. Measured at 1500 / 1280 / 1100px: document scroll 0, wrapper scroll 0, and no element past the wrapper's right edge. `table-fixed` makes the column widths a contract instead of a hint. Upstream's `whitespace-nowrap` alone overflowed the container by 346px at a 1500px window and pushed the entire Actions column outside the scroll area -- content-dependent, so it appeared and disappeared with the data. Fixed layout makes horizontal overflow structurally impossible: over-long values truncate with a tooltip instead of shoving their neighbours. Verified 0 overflow at 1500 / 1280 / 1100px, including a fixture with a deliberately long job name and agent name. Six controls per row (Strict, Run/Cancel, View/Continue, Pause/Resume, Move, Delete) did not fit a 210px cell and were the bulk of the width problem. The row keeps Run/Cancel and Delete; the rest move into an overflow menu (`CronRowActions`), with folder moves as a submenu. Delete deliberately stays in the row: it is an arm-then-Confirm state machine and a menu that closes on select cannot hold the armed state. The side panel becomes a centered dialog, following the same migration the Crews page already made. Focus trap, Escape and overlay dismissal now come from Radix instead of a hand-rolled backdrop, and three more hand-rolled `role="dialog"` blocks on this page (new folder, bulk delete, per-job delete confirm) go with it, taking their `eslint-disable jsx-a11y` comments along. Job selection is split from dialog visibility on purpose. `selected` also drives the calendar highlight and the Executions job filter, and a modal cannot sit beside the thing it filters the way a side panel could -- so dismissing the dialog no longer clears them. Dropped with the panel: drag-to-resize (a modal has a fixed width) and viewing the form next to the list. The page's `<Card>` stays and frames the table, matching the repo's page-layout pattern (HooksPage). Its title does not: it was the word "Jobs", directly under a header already reading "Schedule / Manage recurring cron jobs and scheduled tasks", between which a third label restated both. The header strip mixed four intents with no separation, which made the view switcher read as three more buttons; view switching is navigation and moves to `PageHeader` actions, while the toolbar keeps filter, batch actions, New folder and a split Add Job button whose second half opens the template gallery (one intent, two starting points). Create sits above the view switch so it survives Calendar and Executions, which the card header used to do implicitly. The permanently-on accent banner restating the subtitle is gone; the affordance it carried (you can just ask in chat) is now a link in the subtitle, which is why `PageHeader.subtitle` widens from `string` to `ReactNode`. Four assertions encoded the old row layout and were rewritten against the new contracts -- notably "move sits before delete" becomes "the row action cell holds exactly Run, Delete and the overflow menu", which is the width budget this change is defending. Three tests reached the Templates button through page chrome that is now a split-button menu, and three more reached *behind* an open modal, which Radix correctly marks aria-hidden. Two new tests cover row-click-opens-dialog and selection-survives-dismissal. `playwright/cron.spec.ts` drives pause/resume through the overflow menu. Nine i18n keys left dead by the removed chrome are deleted across 13 catalogs; the dead-key ratchet holds at its baseline of 22. Screenshots are captured at 1x and stay under 2000px per edge, so they can be read back without wedging a conversation (see kirodotdev#2137).

Reported from the dashboard chat transcript: **kirodotdev#2137 — review-ready**（https://github.com/kirodotdev/KiroCrew/pull/2137，`96ed647b`）：`readiness: passed` GFM's autolink-literal extension ends a bare `https://…` run only at ASCII whitespace or `<`. CJK prose writes punctuation directly after a URL with no space, so `，`96ed647b`）：`readiness` all lands inside the href. The broken href is the smaller half of the damage. The run also eats the OPENING backtick of the code span that follows, which shifts every later backtick pairing in the paragraph by one: prose renders as inline code and real code renders with literal backticks. One missing space corrupts the rest of the message. Fixed at the SOURCE level, before micromark tokenizes — re-splitting the link node on the mdast fixes the href but cannot restore the code-span pairing, because that pairing is decided while the whole paragraph is tokenized. The URL head is re-emitted as an angle autolink `<url>`, which has an explicit end and renders identically. Which regions are off-limits is read off remark's OWN parse (same plugin set as the render pipeline), so code, existing links, raw HTML and math are excluded by construction rather than by a hand-rolled scanner. The cut is EVIDENCE-BASED, because CJK punctuation reaches real URLs raw: 1. A CJK closing bracket that closes an opener left unclosed in the PROSE before the URL — GFM's own paren-balancing rule generalised. `（https://x.com/a）` and `（见 https://x.com/a）` cut; `https://x.com/苹果（公司）` does not, and neither does `https://x.com/search?q=foo）` (nothing opened a bracket). 2. A SEPARATOR-class CJK mark immediately followed by a BACKTICK — the destructive case, and the backtick is the one character RFC 3986 excludes so it cannot be inside a raw-written URL. Sentence-enders (`。．！？…｡`) are excluded from rule 2. Real page titles end in them and reach the URL raw — `…/wiki/モーニング娘。`, `…/wiki/魔法先生ネギま！` — so cutting there would point the anchor at the wrong article. Separators like `，`、`、`、`；`、`：` do not end titles, so they stay eligible, which is what keeps the reported case fixed. A mixed run cuts at the separator and leaves the sentence-ender inside the URL: `…/a。，`c`` gives `<…/a。>，`c``. Accepted trade, locked in by tests rather than left implicit: a genuine prose `。` directly before a code span keeps today's behaviour, and so does `…kirodotdev/pull/1，然后回来` (indistinguishable from `…/wiki/我，机器人`). Not adopted: `@streamdown/cjk`'s autolink handling, measured at 1.0.3. It splits at the FIRST character in a fixed 20-mark set with no evidence test, which breaks `…/wiki/苹果（公司）`, `…/wiki/モーニング娘。` and `…/wiki/我，机器人`; it also runs after tokenization so it cannot repair the code-span pairing, and it mis-splits explicit `<…>` autolinks. 34 tests. Full suite on this base: 877 files, 11829 tests, zero failures. Visual evidence under `temp-screenshots/cjk-autolink-boundary/`, captured by `website/scripts/capture-cjk-autolink.mjs`: the REAL built SPA behind the shared fixture server, once from this branch and once from a dist built with origin/main's `MarkdownRenderer.tsx`. The frame carries the reported line, the bracket rule, and a real article URL containing `（公司）` that must survive untouched in both builds.

CrysisDeu requested a review from a team as a code owner August 7, 2026 23:26

github-actions Bot added the readiness: checking Automated validation is still running label Aug 7, 2026

github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 7, 2026

CrysisDeu mentioned this pull request Aug 7, 2026

feat(schedule): shadcn table + dialog, and a layout that fits one screen #2139

Merged

CrysisDeu force-pushed the docs/web-verify-image-cap branch from eaa7c25 to 8810a03 Compare August 7, 2026 23:43

CrysisDeu force-pushed the docs/web-verify-image-cap branch from 8810a03 to 1c6b5cc Compare August 8, 2026 00:01

github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 8, 2026

github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 8, 2026

CrysisDeu force-pushed the docs/web-verify-image-cap branch from 1c6b5cc to ac60d14 Compare August 8, 2026 00:15

CrysisDeu force-pushed the docs/web-verify-image-cap branch from ac60d14 to f4f0b29 Compare August 8, 2026 01:26

CrysisDeu force-pushed the docs/web-verify-image-cap branch from f4f0b29 to 96ed647 Compare August 8, 2026 01:45

github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 8, 2026

github-actions Bot added readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running labels Aug 8, 2026

CrysisDeu merged commit b7947f1 into main Aug 8, 2026
48 checks passed

CrysisDeu deleted the docs/web-verify-image-cap branch August 8, 2026 03:29

github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Aug 8, 2026

Navigation Menu

Uh oh!

docs(skills): cap web-verify frames at 2000px before reading them - #2137

docs(skills): cap web-verify frames at 2000px before reading them#2137
CrysisDeu merged 1 commit into
mainfrom
docs/web-verify-image-cap

CrysisDeu commented Aug 7, 2026

Uh oh!

github-actions Bot commented Aug 7, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 7, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 7, 2026 •

edited

Loading

Uh oh!

CrysisDeu commented Aug 8, 2026

Uh oh!

CrysisDeu commented Aug 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

CrysisDeu commented Aug 7, 2026

What

Why

Scope

Verification

Uh oh!

github-actions Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Opus 5 Review — ✅ no blocking findings

Uh oh!

github-actions Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Design Review (Fable 5) — ✅ PASS

Uh oh!

github-actions Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

GPT 5.6 Review — ✅ no blocking findings

Uh oh!

CrysisDeu commented Aug 8, 2026

Uh oh!

CrysisDeu commented Aug 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

github-actions Bot commented Aug 7, 2026 •

edited

Loading

github-actions Bot commented Aug 7, 2026 •

edited

Loading

github-actions Bot commented Aug 7, 2026 •

edited

Loading