Repository navigation
fix(amazonq): merge from main - #2895
Merged
Merged
Conversation
… throttle cooldown, reaped-beam drop-off (#2883) * fix(beam): cache parsed beam artifacts so discovery stops re-downloading them Beam discovery re-scans the same candidate artifacts on every sweep, and each candidate was downloaded and re-parsed every time. Measured in one 90-second window on a workspace with 32 jobs: 1980 CreateArtifactDownloadUrl completed 1230 "artifact is neither raw JSON nor a zip" parse failures 295 listBeamedRepos calls 217 CreateArtifactDownloadUrl AccessDeniedException That is ~22 download-URL creations per second, sustained. The AccessDenied errors were not confined to artifacts - ListMessages took the same error in the same window, so this starves unrelated calls, chat included. The existing bounds (BEAM_MAP_SCAN_MAX_CANDIDATES, BEAM_MAP_SCAN_BUDGET_MS) limit ONE scan and are correct, but they reset per call and so cannot limit the number of scans. Caching is what bounds the total. Artifacts are immutable - a re-upload mints a new artifactId - so a cached entry can never be stale. Keyed on artifactId, capped, and cleared wholesale on overflow rather than evicting a chosen victim; the only cost of a clear is downloading once more. A cached `null` means "these bytes are definitively not JSON", which is the case that dominated the waste (1230 of the failures). It also collapses that log line from once-per-sweep to once-per-artifact. Transient failures are deliberately NOT cached - a missing presigned URL, a network or throttle error, an AccessDenied, or a malformed zip that makes AdmZip throw. Caching one of those would permanently mark a readable artifact unreadable. Not changed here, deliberately: - No result-level cache for listBeamedRepos. Its result genuinely changes as lbvOpen flips, and making that staler risks the LBV-open timing that the Load gate depends on. With artifact downloads cached, a repeat call is a single ListArtifacts plus cache hits. - No name/extension prefilter on candidates. The code already documents why that regressed before: the beam-map is itself stored as a zip with a .zip path, so filtering on the extension skipped the beam-map entirely and fell back to zip-discovery with an empty stepId. - No cadence change on the IDE side. Measure the effect of this first. * fix(beam): correct the artifact cache - the cap was below the working set The previous commit's cache did nothing. Built and run, the flood was unchanged at ~1200 CreateArtifactDownloadUrl per minute, and the new "caching as not-JSON" line fired 14649 times in 25 minutes. Measured why, rather than guessing again: distinct artifacts touched 1043 previous cap 500 wholesale clears in 25 min 36 The cap sat below the working set, so the cache filled, cleared everything, refilled, and cleared again - discarding on every pass the entries that were about to be hit. Hit rate approximately zero. It paid the bookkeeping and delivered none of the benefit. Two changes: 1. Cap 500 -> 4000, derived from the 1043 measurement rather than chosen. Redundancy in that session was ~29x (about 30000 downloads for 1043 distinct artifacts), so a cache that actually holds the working set should remove most of the volume. 2. Evict ONE least-recently-used entry instead of clearing the map, and refresh an entry's position on a hit so the ordering is a true LRU (Map iterates in insertion order, so "oldest key first" is only correct if hits re-insert). The eviction strategy is the part that mattered. Clearing wholesale turns an under-sized cap from a partial loss into a total one: at the cap it discards every entry including the ones in active use. Single-victim eviction degrades gradually, so a workspace whose working set exceeds the cap still gets most of the benefit instead of none. The cap-reached notice is kept but latched to log once per process rather than once per eviction - that signal is what made the under-sized cap diagnosable, and per-eviction logging would itself become a flood. Still to verify by measurement, not assertion: that this actually reduces the download volume. The previous version looked correct and did nothing. * fix(beam): stop the artifact cache assuming artifacts are immutable The cache was keyed on artifactId with no expiry, justified by "a re-upload mints a new artifactId". That is asserted nowhere in the platform contract, and this file's own comments record the opposite: web-orc RE-WRITES the beam-map ("re-written on each beam; latest wins"), and the BeamArtifactId "thrash" that forced the beam-status fallback is the same observation. If a rewrite reuses the id, a permanent cache pins the FIRST beam-map for the whole LSP session — so a repo beamed later never appears in the panel until restart, and a re-beam leaves chat routed at a dead stepId. Split the policy by polarity instead, which needs no platform guarantee: negatives ("not JSON") -> cached permanently; a property of the bytes, and 1230 of the 1980 wasted fetches, so this is where the win actually is positives (parsed) -> 60s TTL; far longer than the ~13s sweep so the repeated fetches still collapse, while staleness is bounded to a minute The TTL also bounds RETENTION: parsed values are arbitrary size, so without one, customer artifact content stayed resident for the whole session. Also label the beam discovery logs with their job id. One workspace commonly holds several jobs containing the SAME repo names, so unlabelled per-repo lines read as one repo changing state when they are really N jobs interleaved — which produced a confident, wrong "lbvOpen is flapping" diagnosis that was really four jobs each reporting correctly. * fix(beam): stop skipPolling duplicating every message the user sends The skipPolling early return handed back the raw sendResult, whose sent-message id sits at `message.messageId`. The polling path below it returns `data: { sentMessage: sendResult.message }`, and the client reads `data.sentMessage.messageId` to add the id to its seen-set so its own 3s poll does not re-render the message it just sent locally. On the skipPolling path that read resolved to null, nothing was seeded into the seen-set, and the poll then treated the user's own message as newly arrived — so every beamed message appeared twice in the transcript. Return the same shape the polling path returns. Regression from the skipPolling change; only beamed sends set the flag, so only they were affected. * fix(beam): pick the live beam node, and back off when throttled Discovery matched a beamed repo by name and took the first hit. A repo beamed more than once has more than one node under that name, and the first is the oldest, so a re-beamed repo never came back - the IDE kept resolving to a step that had already finished. Matching now collects every node with that name and prefers one whose build-verification child is still live, falling back to the newest. A matched node's subtree is not searched again, so nested names cannot shadow their parent. The beam-map scan also had no reaction to being throttled: it would retry the same sweep on the next poll and keep the throttling alive. It now sits out a cooldown longer than the poll interval, so whole sweeps drop rather than being retried piecemeal. The cooldown empties the candidate list instead of returning early, because returning early would abandon beam-status discovery too and empty the tab for the whole cooldown. * fix: drop a reaped beam from the transferred list instead of showing it forever A beam whose sub-agent never booted gets reaped by the web orchestrator: the instance is stopped and the repo node marked terminal. The IDE kept showing it, because the only signal read was the Local Build Verification child - and a reaped beam has none, which is indistinguishable from a fresh transfer whose HITL has not been created yet. Read the repo node's own status to separate them. Terminal with no LBV child means the beam ended without ever opening LBV, so it is closed. Safe on re-beam: the orchestrator re-drives that node to IN_PROGRESS before reusing it, so a re-beamed repo reads live again. The user is otherwise left waiting on a repo that will never build - the re-beam notice goes to the job owner, not to whoever is in the IDE. * test: cover the reaped-beam gate in both directions The terminal-repo-node branch had no behavioural coverage. Deleting it or inverting it both left the suite green, so a passing run could not tell a working gate from an unreachable one. Four cases, mutation-verified: removing the gate fails two, over-gating fails the third, and the fourth pins that an LBV child still decides when one exists.
* fix(amazonq): stream large ATX artifact downloads to disk
downloadArtifactToPath buffered the whole artifact into memory via
got.get(responseType:'buffer') under timeout:{request:30000}. The
'request' timeout caps the ENTIRE request including body transfer, so a
large artifact (e.g. Transformation_Report.html) that takes >30s to
download aborts with a TimeoutError. That error was swallowed by the
catch and surfaced only as a generic download failure — matching the
customer report where CreateArtifactDownloadUrl succeeds but the report
never downloads (V2381716118).
Stream the presigned URL straight to disk with got.stream + pipeline,
and switch to inactivity-based timeouts (response 30s to headers, socket
60s idle) so a large-but-healthy download is no longer capped by wall
clock, and no longer double-buffered in memory. Add success/error
logging for diagnosability.
* fix(amazonq): log all downloadArtifactToPath failures with diagnostics
Every failure branch now logs clearly: the previously-silent missing-URL
branch logs an error, and the catch records the got error code, timeout
phase, HTTP status, bytes transferred before failure, and full stack.
* feat(chat-client): persist deprecation notice dismissal * fix(chat-client): replace agentic feature card * fix(chat-client): show deprecation notice once per instance * fix(chat-client): finalize deprecation notice * fix(chat-client): preserve deprecation notice on chat options * fix(chat-client): show deprecation notice until acknowledged * test(chat-client): document JSDOM Node global
dungdong-aws
approved these changes
Sep 29, 2026
chungjac
approved these changes
Sep 29, 2026
laileni-aws
merged commit Sep 29, 2026
9750b63
into
feature/mcp-security-enchance
149 of 177 checks passed
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
6 participants
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.
Problem
Solution
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.