[cslong]

Important

Draft on purpose, so this cannot merge out of order. Two blockers, both tracked below:

1. #2969 must merge first. It rewrites the same four applyChanges loops and backupConflictedFile. After it lands, rebase this PR and delete escapesRoot and the beam-only containment block in favour of its general rule.
2. The Visual Studio end-to-end run has not happened. Two cases: a repo whose .sln is not at the repo root, and a two-repo batch. The observed scope payload and diff paths go in this description once they are known.

Background

In Beam to IDE, a customer moves a repo from an AWS Transform web transformation into Visual Studio, and the language server shows the job's plan there. Once the local build check passes, the customer can ask in chat for unit tests. The transform agent then generates them and adds a "Merge Tests" step whose diff adds the test project to the solution (transform agent changes, already merged).

Problem

On a beam job the tests are generated but never reach the customer's solution, and nothing says so. The chat narrates success, both plan steps go green, no warning appears, and DiffApplyFailed is never set. Only the file system disagrees.

Two things cause it, and both are structural:

1. The apply pass is skipped. getTransformationPlan returns early for isBeam, on purpose: the beamed source is already the transformed output, so the prior job's completed-step diffs are baked in and their checkpoint folders are not in the bundle. Applying them logs "Checkpoint folder not available for step" and sets DiffApplyFailed on every poll, stalling the LBV loop.
2. The step is at the wrong depth. The agent's own comment states the IDE contract: the LSP applies a step's artifacts based only on "the sub-step being at depth 2 under a top-level step", a CUSTOMER_OUTPUT diff tagged with its stepId, and the step marked SUCCEEDED. The beam orchestrator inserts "(Beamed) Batch" and a card per repo, so the Merge Tests sub-step sits at depth 4. Even with the skip removed, findCompletedSteps' depth === 2 would never match.

Everything the agent side needs already happens — it uploads the tagged diff and marks the step SUCCEEDED regardless of beam. Only the LSP's selection is wrong.

Solution

Reuse the mechanism the normal IDE flow already uses, rather than reimplement it.

In the beam branch of getTransformationPlan: find the loaded repo's beam card, take the SUCCEEDED "Merge Tests" steps under it in plan order, and hand each to the existing downloadCompletedStepArtifact. That already downloads, retries, applies and records the step in checkpoints-applied.json exactly as it does for an IDE-started job, so already-applied steps are skipped and this is safe on every poll.

Picking the solution

One beam card in the plan is unambiguous and needs no scope at all — the common case, since "beam <repo>" beams one at a time.

With several cards (a multi-repo batch) the IDE's scope says which repo is loaded, and it accepts either the card's own StepId or any step beneath it, since either identifies exactly one card. The feature therefore does not depend on which of the two the toolkit sends, a contract that is still unverified end to end.

If the scope cannot identify exactly one card, nothing is applied and an error is logged. The wrong repo's tests landing in this solution is worse than no tests.

With one card and no scope there is nothing to disambiguate, so the card is taken. With one card and a scope that names none of its steps, nothing is applied: that is the IDE saying the open solution belongs to a different repo, and a shared .sln name would be enough for the root check to pass and the wrong repo's project to land. It fails closed, the same as the multi-card branch on the same mismatch.

The customer is told about it only when some card has a SUCCEEDED Merge Tests step that is not yet applied. Most multi-repo batches never ask for unit tests, and telling those customers that tests "could not be added — please report this" is a false alarm that costs a support ticket. The log line is unconditional either way.

Only steps under a beam card apply. A web transform's own unit test run sits under the repo's transform card and its tests are already in the beamed zip, so it never applies again even when the scope covers that card.

Local edits are not preserved — they are backed up

A beam job never passed through startTransform, so it has no source-files.json. writeSourceFilesManifest has exactly one caller, on the IDE start path, gated on interactive mode. Without that manifest, getModifiedFilesSinceCheckpoint returns [] and applyChanges has nothing to protect against, silently.

Rather than rebuild that protection, the beam apply copies every file it would overwrite into conflict-backups first. These rules make that safe to rely on:

• the customer is told, in the job's worklog — which the IDE renders in its activity feed — which files were updated and where their previous version is. The LSP output log is not somewhere a Visual Studio customer looks;
• a backup that cannot be taken keeps the file and leaves the step unapplied, so the existing retry and give-up handle it. Recording it applied would retire the step with the diff only half landed, and the solution would never get the test project;
• success from applyChanges means the diff landed. A kept file or a failed write makes it false, so a caller that checks success alone can never record a half-landed diff as applied. Gated on beamApply: the IDE flow has always logged a per-file error and carried on, and its tests pin that;
• a write that fails does the same. applyChanges catches every per-file copy, unlink and mkdir error, logs it and returns success — right for a large IDE transform diff, wrong for a Merge Tests diff that is one test project plus the .sln, where one file locked by Visual Studio or a virus scanner leaves the solution naming a .csproj that is not on disk. It now reports writeFailed, beam-only, and the step is retried rather than recorded as applied;
• and a failed add or removal stops the .sln landing at all. The updates and moves are skipped, so the solution is never left naming the test project that just failed to write. The retry lands the whole diff; until then the customer's solution still builds. The non-beam apply still finishes the diff, and a test pins that;
• a backup is reported to the customer only once the write it protected has actually succeeded, so a failed write never claims the file was updated;
• an existing backup is never replaced — the first copy is the customer's own version, later copies get a numbered suffix. This sits behind a preserveExisting flag that only the beam apply passes, so other callers keep one backup per path, replaced in place, exactly as before;
• a file whose bytes already match is not backed up.

Known consequence, accepted deliberately. The agent's .sln is the server's whole solution, so overwriting it restores a project the customer removed locally. It is backed up and reported.

This is the behaviour customers already have. The released IDE unit test generation applies the same way — whole after/ files, the .sln replaced outright — so the beam path is not introducing a new risk, it is matching one customers are already told about. The only difference is the edit watermark: the IDE flow has source-files.json and protects edits made after a previous apply, and a beam job has no manifest, so it protects none. Backing up every overwritten file is what covers that gap, and on a first apply the two paths are identical anyway, because the IDE flow has no watermark yet either.

The alternative is a structural .sln merge, which is a much larger change for a rarer case; the proper fix is a three-way merge against the beamed original, below.

Failures

A step that has not applied stops the loop rather than letting a later one land first: applyChanges copies whole after/ files, so an earlier diff landing after a later one would overwrite the later one's .sln and drop its test project from the solution.

listArtifactsForStep swallowed every error, throttling included, and returned null — which reads as "not up yet" and spends the arrival bound, retiring a healthy step while continuing to feed the throttle. It notes the throttle now, so the cooldown below actually engages. (createArtifactDownloadUrl already did.)

The beam apply also honours the throttle cooldown the rest of the beam code already does (isBeamThrottleCooldownActive, 90s). While it is active the loop holds off and touches no state: otherwise every poll would cost a ListArtifacts and up to three presigned-URL attempts for the same pending step — feeding the very throttle the cooldown exists to relieve — and would spend the arrival bound below, permanently retiring a step whose artifact is fine.

A step still unapplied after 5 attempts is given up on, so it cannot re-list artifacts and retry its download on every poll against a service that already throttles us.

Only an attempt that got the diff onto disk counts against that bound. downloadDiffArtifact reports one of five outcomes, and downloadCompletedStepArtifact adds no-artifact:

Outcome	Meaning	Counts against
applied	the diff landed and the step is recorded	—
apply-failed	it was on disk and writing it did not fully work	apply bound (5)
apply-refused	refused before writing anything, on a verdict that can change	refusal bound (30)
rejected	refused before writing anything, settled by the diff's own paths	nothing — given up at once
transfer-failed	it never arrived: throttled URL, dropped download, bad archive	arrival bound (30)
no-artifact	the listing has nothing for the step yet	arrival bound (30)

Counted in attempts, not elapsed time, so a laptop sleep cannot spend either budget.

A diff that never arrives — no artifact in the listing, a throttled presigned URL, a dropped download, a bad archive — gets its own, far looser bound: 30 polls, in a separate waits counter. Most of those causes clear up on their own, which is why it must not share the apply bound. But it still has to be bounded, because a never-uploaded artifact and a corrupt archive never clear up: without it, that step re-lists and re-downloads on every poll for the rest of the job against a throttling service, every later run stays blocked behind it, and the customer is told nothing — the silent failure this PR exists to remove. Tripping it writes one worklog line saying the tests could not be downloaded.

Giving up stops the whole card, not just that step. The agent's .sln is the server's whole solution, so a later run's .sln already names the given-up run's test project — and that project's files never landed. Applying it would leave Visual Studio with a solution pointing at a .csproj that is not on disk, which dotnet build fails on outright. The give-up message says the tests could not be finished, that some files may already have been updated — applyChanges writes file by file, so a diff that failed part way through has already landed in part — where conflict-backups is, and that later test runs in this job will not be added either. Later polls stay quiet rather than repeating it.

Checking the diff belongs to this tree

Every path in the agent's metadata.json is relative, and on a beam the solution root comes from the toolkit rather than from an upload by this LSP, so nothing else says the two agree. filesUpdated and filesRemoved name files that must already be there, so if none of them exists under the solution root, the diff was computed against a different tree. The beam apply refuses before writing anything, rather than scattering a test project and a .sln over unrelated directories. Beam-only, and skipped when the diff only adds files, where there is nothing to check.

It also refuses a diff whose entries reach outside the solution — .. or an absolute path. A Merge Tests diff is a test project and the .sln, so it has no business outside, and such an entry would be written outside the solution and backed up outside it too, since backupConflictedFile derives the backup path from the same relative path. #2969 adds containment to the whole apply path for every caller; this is only the beam slice, so the gap is not open while that one is in review.

The containment refusal is settled — it comes from the diff's own paths, so a retry cannot change the answer. It returns rejected, which skips the three download retries and the five-poll bound entirely, and the customer is told at once, with the real reason, that nothing in the solution was changed.

The root refusal deliberately is not settled, because it reads the customer's working tree. A Merge Tests diff usually updates only the .sln, so a .sln they have momentarily renamed, moved or deleted looks exactly like a diff computed against another root. It gets its own counter and the generous 30-poll bound, not the 5-attempt apply bound: five polls is about two minutes, which a branch switch easily outlasts, and the give-up is permanent.

The card-wide stop still applies when a refusal never clears. Nothing was written by this step — but the next run's .sln names this run's test project, which never landed, so applying it would leave Visual Studio pointing at a .csproj that is not on disk.

These are guards, not a substitute for the end-to-end run below.

That count and the give-up decision live in beam-apply-state.json, beside checkpoints-applied.json. They have to be on disk for two reasons. getTransformInfo calls clearJobCache on every poll of a COMPLETED job, immediately before fetching the plan, so an in-memory count would never reach its bound. And a given-up step is not in checkpoints-applied.json, so without a record a later session would retry it and could apply it after a step that comes later in the plan — overwriting that step's .sln and dropping its test project, the exact hazard the in-loop stop exists to prevent.

Every outcome the customer could care about reaches the worklog: tests added, files overwritten, a step given up on, and a scope that cannot identify which beamed repo this solution is. Deliberately not _currentDiffContext, which becomes DiffApplyFailed and would stall the local-build loop.

A finished "Generate Unit Tests" run with no step named "Merge Tests" under it logs a [BEAM] line and reaches the worklog, so a step rename shows up instead of silently matching nothing. Matched on the name alone, and checked for every run on the card: requiring SUCCEEDED as well would call a FAILED merge step — what a run that generated no tests leaves behind — a rename, and only looking when the card yields no steps at all would miss a rename landing mid-job after an earlier run matched, which is the case it exists for. Recorded in the apply state so the customer is told once, not on every poll.

Overlaps with #2969

#2969 ("keep transform checkpoint changes inside the solution folder") touches the same four applyChanges loops this PR hooks the backup into, and changes backupConflictedFile. It should merge first and this PR rebase on top of it — its containment rule is the general case, and this PR's beam guard is a subset that can then be dropped. Whoever merges second hand-merges applyChanges.

One premise was worth re-checking against it: this PR relies on a beam job having no source-files.json. #2969 makes writeSourceFilesManifest unconditional rather than interactive-mode-only, but it is still called from startTransform alone, which a beam job never reaches. The premise holds.

Scope of change

The shared apply path gains one optional boolean, beamApply (default false), and backupConflictedFile gains preserveExisting (default false). Both default to the historical behaviour, so every non-beam path behaves exactly as before, and tests pin the non-beam replace-in-place backup, its unchanged handling of paths outside the solution, and the absence of write-failure reporting.

beamApply selects four things, which is why it is named for the caller rather than for one effect: the backups, the wrong-root refusal, the containment refusal, and write-failure reporting. They are listed where it is declared. It should become an options object as soon as a second caller wants some of them and not others; today that would be four parameters no live call site sets.

Considered and not done

• Writing source-files.json ourselves would let the beam reuse the IDE's protection, but that mechanism needs a full enumeration of the source tree and compares every file against a single lastAppliedTimestamp. It is weaker than backing everything up, it puts a synchronous filesystem walk on the thread that serves completions, and updateWorkspace also reads that file, so writing one would change what gets uploaded back.
• A structural .sln merge that transplants only the agent's new Project stanzas. Correct, and the right end state, but it is most of the complexity for the rarest case.
• A three-way merge against the beamed original. downloadBeamArtifact leaves beam-artifact.zip next to the extracted repo and never deletes it, so the original bytes are already on disk. This is the intended follow-up; it needs the end-to-end run to confirm where the zip sits relative to the solution root and how its entry paths map.

Release

Only the language server changes. The Visual Studio toolkit already shows the "Generate Unit Tests" step under the beam card and keeps polling after the build check, and it downloads the newest compatible 1.x language server at runtime, so this reaches customers with a language server release alone (no toolkit release).

Testing

New unit tests in atxTransformHandler.test.ts:

• the beamed card's finished Merge Tests step applies, with backups enabled; the web transform's own Merge Tests never does; nothing applies without a solution path, with no beam card, or for an already-applied step;
• card and step names match normalized (case and surrounding whitespace); a renamed step is logged;
• one card needs no scope; with two cards the scope picks the repo, and a scope naming a step under the card works too; a scope matching no card or more than one applies nothing and logs;
• two runs on the card apply once each in plan order; a step that did not apply stops the loop so a later one cannot land first; a step that keeps not applying is retried and then given up on once, after the bound; a step whose artifact is missing, and one whose download failed, are each retried without spending the bound at all;
• downloadDiffArtifact reports transfer-failed when the download URL never comes and when the download throws, and apply-failed when applyChanges fails on every retry; the step HITL caller treats a half-applied diff as failed even though it returns a path, and an applied one as applied;
• a diff that reaches outside the solution with .., in a move, or as an absolute path, writes nothing at all — not even the entries that were inside — while the non-beam apply still decides that for itself;
• a diff refused for reaching outside is marked rejected, is not downloaded again, is given up on at the first poll without spending either bound, and the customer is told the real reason and that nothing changed; a wrong-root refusal is apply-refused instead — retryable, counted against the apply bound, and its give-up says nothing in the solution was changed rather than pointing at backups that do not exist;
• a throttled listing starts the cooldown, and a plain error does not; one isThrottleError is the only definition of a throttle, shared with createArtifactDownloadUrl;
• a refusal is retried past the apply bound, applies normally if it clears up, and gives up card-wide only after the refusal bound; applyChanges reports success: false whenever the diff did not land; a failed removal holds the .sln back and the log says so;
• one beam card with a scope naming none of its steps applies nothing and logs why, while one card with an agreeing scope, or none, still applies;
• a failed add leaves the .sln and the moves untouched, with the log line saying why, while the non-beam apply still completes the diff;
• nothing is listed or downloaded while the throttle cooldown is active, no state is touched, and the apply resumes once it passes;
• a diff whose updated files are nowhere under the solution root is refused and writes nothing, not even its added files; it applies normally once one of them is there; a diff that only adds files is left alone;
• a FAILED Merge Tests step is not reported as a rename; a rename on a later run is still spotted after an earlier run matched; the customer is told about it once, not per poll;
• a diff that never arrives is given up on after the looser bound, with one worklog line, and the two bounds are not the same number; a failed write is reported so the step is retried, while the non-beam apply still logs and carries on; a backup is not announced when the write it protected then failed;
• an unidentifiable repo with no unit test run anywhere writes no worklog entry, and neither does one whose runs have all been applied, while one with a run still waiting does;
• nothing after a given-up step is applied, including a later run whose .sln names the missing test project; a give-up is remembered by a fresh handler, so neither it nor anything after it is ever applied in a later session;
• the customer is told in the worklog when tests are added, when a step is given up on — including that later runs are blocked — and when the beamed repo cannot be identified;
• with the real applyChanges on a temp solution: the customer's file is copied to conflict-backups before being overwritten, before being deleted, and before a move overwrites it, with the backup-failure path covered for each; the backed-up paths are returned for the caller to report; identical bytes are not backed up; a second beam run does not replace the first backup, while a non-beam conflict still replaces in place; a failed backup leaves the step out of checkpoints-applied.json; the apply state round-trips to disk; and the non-beam apply takes no backups and behaves exactly as before.

All netTransform tests pass (626); tsc and prettier clean, no ESLint errors. Twenty-four mutations each fail exactly their own test: putting refusals back on the apply bound, success always true, a log that blames only adds, a silent throttled listing, a single card ignoring a contradicting scope, one give-up wording for both kinds of failure, calling a root mismatch settled, letting the .sln land after a failed add, ignoring the throttle cooldown, letting a refused diff fall into the retry loop, letting it spend the poll bound, dropping the containment check, deciding the HITL result by path rather than outcome, turning the plan-order stop into a continue, letting a given-up step fall through to the next run, counting a missing artifact or a failed download against the apply bound, never giving up on a diff that never arrives, dropping the solution-root check, putting the status back into the rename check, ignoring a failed write, announcing a backup before the write, warning about an unidentifiable repo unconditionally, and letting the checkpoint save run on a failed backup.

End to end in Visual Studio (beam a repo, ask for unit tests after the build check, see them added to the solution) is still to run. Two cases are worth running deliberately: a repo whose .sln is not at the repo root, which is what the solution-root check above is for, and a two-repo batch, which is what exercises the scope. The observed scope payload and diff paths will go in this description once they are known.

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 61.60815% with 339 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...anguage-server/netTransform/atxTransformHandler.ts	61.60%	337 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

[cslong]

🟡 A later Merge Tests step still applies after an earlier one is given up, and its .sln names a project that is not on disk.

The in-loop stop and the persisted skip list stop a given-up step from landing out of order. But after step A goes into skipped, the loop continues, so a later run B on the same card still applies. applyChanges copies whole after/ files, and the agent's .sln is the server's whole solution, which already holds run A's test-project entry. Run A's project files never landed locally.

if (state.skipped.includes(step.StepId)) {
    continue
}

Why it matters: Run A is given up → run B overwrites the local .sln with the server's copy, which references A's test .csproj → that project path does not exist locally → Visual Studio shows the project as unavailable and dotnet build on the solution fails with 'project file not found'. The customer is left with a broken solution and only a 'could not add tests' worklog line.

Suggestion: Once any earlier step on the card is in skipped, stop applying later steps too: keep the break semantics for skipped steps, and tell the customer that later test runs are blocked. Or, before applying B, strip Project stanzas whose paths do not exist locally from B's after/.sln. Add a test with a skipped A and a succeeded B.

[code-review-sage]

[cslong]

🟡 Five failed polls retire a step for good, on disk and across restarts, with no way to retry.

Every poll that leaves the step unapplied counts against the bound. That includes a listArtifactsForStep result that comes back empty because of throttling, which the description says this service does, and a network blip. After 5 such polls the step is written to beam-apply-state.json and is never tried again, even after a Visual Studio restart. Nothing resets it short of deleting the file by hand. The customer message says the tests are 'still available from the job's artifacts', but the IDE gives them no way to get there.

if (polls >= this._beamGiveUpPolls) {
    state.skipped.push(step.StepId)
    this.writeBeamApplyState(solutionRootPath, jobId, state)

Why it matters: About a minute of throttling or an offline period right after the step succeeds → 5 counted polls → the step is permanently skipped → the customer never gets the tests they asked for, and has no recovery path except generating a new run.

Suggestion: Count only polls where the artifact was actually found and the apply failed. Do not count an empty or errored listing, or count it against a much larger bound. Also let a new session (or a clearly signalled retry) clear skipped, or expire entries after a time window. Make the give-up message tell the customer what they can do next.

[code-review-sage]

[cslong]

Code Review Sage Report 2026-11-05 235944Z-v1.html