test(screenshot): cover the screenshot pipeline#275
Open
isadeks wants to merge 4 commits into
Open
Conversation
4 tasks
isadeks
force-pushed
the
feat/97-screenshot-tests
branch
2 times, most recently
from
330874e to
21e8fa8
Compare
Closes #97. Adds 53 jest tests across the four screenshot files that landed with PR #241 + #273 with no existing coverage: - github-webhook-verify.test.ts (14): SHA256 sign/verify, sm cache TTL + forceRefresh, ResourceNotFound, transparent re-fetch on signature mismatch / null fresh. - github-webhook.test.ts (15): missing body/sig, ping ack, non-deploy events ignored, malformed JSON, state/environment filters, SCREENSHOT_TARGET_ENVIRONMENT override, missing fields, dedup hit, happy path, rollback-on-invoke-failure, non-condition DDB error. - linear-issue-lookup.test.ts (18): regex covers extract / multi / bounds / case-sensitivity, prefix-routing happy path, case-insensitive prefix match, fallback for legacy rows + post-prefix-miss, null token skip, fuzzy-match guard, GraphQL errors / non-2xx / network failure. - github-webhook-processor.test.ts (15): empty / malformed body, missing fields, token resolve failure, PR retry exhaustion, OPEN-only filter, happy path with CloudFront-host URL assertion, screenshot/S3/comment failure modes (each non-fatal where appropriate), Linear branch fires / falls back to body / skips on no-id / no-resolve / non-fatal post. - agentcore-browser.test.ts (6): StartBrowserSession failures, full CDP exchange (Target.getTargets -> attach -> enable -> navigate -> loadEventFired -> captureScreenshot) returning PNG bytes, Stop invoked in finally even on CDP error, Stop's own failure logged not thrown, 403 unexpected-response surfaced, navigate errorText raised. All tests use jest mocks for AWS SDK clients + an in-test FakeWebSocket for the CDP stream so they run hermetically without real AWS or network. Existing 286/286 handler tests still pass.
CI's @typescript-eslint/member-ordering rule requires static members to be declared before instance fields. Move FakeWebSocket's static last/onConstruct/reactions to the top of the class body. Behavior unchanged; 6/6 agentcore-browser tests still pass.
Three comment-spacing fixes flagged by CI's mutation guard: - github-webhook-processor.test.ts: collapse double-space-before-// - github-webhook.test.ts: same - linear-issue-lookup.test.ts: same No behavior change; 68/68 across the five new test files still pass.
isadeks
force-pushed
the
feat/97-screenshot-tests
branch
from
9f49b69 to
08233d1
Compare
isadeks
pushed a commit
to isadeks/sample-autonomous-cloud-coding-agents
that referenced
this pull request
Jun 10, 2026
…aws-samples#240) krokoko PR aws-samples#241 round-3 review: - Finding 1 (security): percent-encode `(`/`)` in the payload-derived `environment_url` before interpolating it into the PR/Linear comment markdown. A clean-hostname URL like `https://preview.vercel.app/x)](https://evil/a.png)` passes `isAllowedScreenshotUrl` yet breaks out of the `](…)` link and injects content into a comment posted under ABCA's token (reachable in fork-PR configs). New `encodeMarkdownUrl` helper + unit tests. - Finding 3 (cosmetic): screenshot-bucket.ts and the deploy guide still described the old `screenshots/<repo>/<sha>.png` layout; updated both to the actual `screenshots/<owner>_<repo>/<sha>-<deploymentId>-<16hex>.png`. Removed the unused `SCREENSHOT_KEY_PREFIX` export. Findings 2 (IPv6 unique-local) and 4 (validateDeploymentStatusPayload coverage) folded into aws-samples#286 and aws-samples#275 respectively per the reviewer's routing.
`](https://preview.vercel.app/x)](https://evil/a.png)`){kind=link}
Contributor
Author
|
Folding in krokoko's PR #241 round-3 finding 4 (test coverage), since this PR owns the screenshot-pipeline test gap:
While here, also cover the uncovered |
isadeks
pushed a commit
to isadeks/sample-autonomous-cloud-coding-agents
that referenced
this pull request
Jun 10, 2026
…us validator (aws-samples#240) krokoko PR aws-samples#241 round-3 findings 2 and 4, brought onto aws-samples#240 because the code they touch (screenshot-url.ts, the shared github-deployment-status validator) lives only on this branch — aws-samples#275 predates the aws-samples#240 refactor that extracted them, and aws-samples#286 is an issue with no code branch. - Finding 2 (security): isAllowedScreenshotUrl enumerated IPv6 ranges (::1, fe80:, ::ffff:) and missed unique-local fc00::/7 (e.g. [fc00::1]) and NAT64. Replace with a blanket reject of any IPv6 literal — a bracketed host or a `:` in the host — since preview URLs are always DNS names. Also documents that the WHATWG parser normalizes integer-form IPv4 (2130706433, 0x7f000001) to dotted-quad, so the existing regex catches those too. New table-driven tests for unique-local, NAT64, IPv4-mapped, and integer IPv4; screenshot-url.ts now at 100% branch coverage (closes the old ::ffff: gap). - Finding 4 (test gap): validateDeploymentStatusPayload had zero coverage. New github-deployment-status.test.ts — happy path, each missing/empty required field, absent nested objects, and the id:0 type-vs-truthiness edge. Validator at 100%.
Contributor
Author
|
Update: the No action needed on this PR for that item; flagging so the cross-reference doesn't go stale. |
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
1 participant
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.
Closes #97.
Summary
Adds 53 jest tests across the four screenshot files that landed with #241 + #273 and previously had no coverage. Hermetic — no real AWS, no network. All AWS SDK clients and the WSS browser stream are mocked.
github-webhook-verify.test.tsforceRefresh,ResourceNotFound→ null, transparent re-fetch on signature mismatch / null freshgithub-webhook.test.tsSCREENSHOT_TARGET_ENVIRONMENToverride, missing fields, dedup hit, happy path, rollback-on-invoke-failure, non-condition DDB errorlinear-issue-lookup.test.tsgithub-webhook-processor.test.tsagentcore-browser.test.tsStartBrowserSessionfailures, full CDP exchange returning PNG bytes (Target.getTargets→ attach → enable → navigate →loadEventFired→captureScreenshot), Stop invoked in finally even on CDP error, Stop's own failure logged not thrown, 403 unexpected-response surfaced, navigateerrorTextraisedExisting 286/286 handler tests still pass — 16 suites / 385 tests across the screenshot + linear + fanout + orchestrate + create-task + webhook surface area.
Test plan
yarn jest --runInBandon the five new files: 53/53 pass