feat: explorer stale source banner - #1738
Conversation
The explorer reads source files live, but violation source lines are frozen at the last synth, so editing without re-synthing leaves markers on stale lines. Surface that with a per-file banner warning the open file has been modified since the last synth. Staleness is decided server-side against the synth-start time (the RWLock write-lock marker), pushed to the browser over SSE so it appears on save and clears after the next synth. toolkit-lib: hoist RWLock's 'synth.lock' literal to an exported SYNTH_LOCK_FILE constant (consumed by RWLock, re-exported via cdk-explorer's api-private) so the explorer detects synth activity without re-hardcoding the name.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feat/cdk-explorer #1738 +/- ##
====================================================
Coverage ? 89.55%
====================================================
Files ? 79
Lines ? 11836
Branches ? 1659
====================================================
Hits ? 10600
Misses ? 1207
Partials ? 29
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Rico answered your open question async, but putting this here for the public record - it is fine if the explorer doesn't have any synthing capabilities itself, our customers should be smart enough to figure out how to run synth themselves through command line or they will just get an agent to do it. We'd rather not add all the options needed to run synth with any custom synth set ups they may have. |
| if (timer) { | ||
| clearTimeout(timer); | ||
| } | ||
| timer = setTimeout(() => { |
There was a problem hiding this comment.
Does it make sense to put this timeout in a sleep function?
There was a problem hiding this comment.
put this in a debounce helper, lmk if it looks ok
…unce helper Address review feedback on the stale-source-banner PR: - Export SOURCE_WATCH_EXCLUDES from source-watcher and import it in the LSP server, replacing the duplicated inline exclude array so the ignore policy has a single owner. - Extract the source watcher's inline setTimeout into a debounce() helper with trigger()/cancel(), keeping error handling at the call site. It is a debounce (cancelable, reset on each trigger), not a sleep.
fc79c65
into
aws:feat/cdk-explorer
Adds a banner to the web explorer's source pane that warns when the open file has been edited since the last synth, so its violations and diagnostics may be stale.
The explorer reads source files live from disk, but violation source locations come from the cloud assembly and are frozen at the last synth. If you edit a file without re-synthing, the code shown updates while the violation markers stay where synth left them, so a marker can sit on the wrong line. The explorer never runs synths itself and has no direct signal that one happened, so it cannot keep the two in lockstep. This surfaces that gap honestly instead of showing misplaced markers silently.
Behavior
The source pane always shows the current file on disk.
When the open file's last-modified time is newer than the current assembly's synth, a warning shows: "This file has been modified since the last synth, so its violations and diagnostics may be stale. Re-run cdk synth to refresh."
The banner appears the moment you save. A source-tree watcher pushes an event over the existing SSE stream, so you do not have to re-open the file.
After a synth completes and the explorer reloads the assembly, the open file is re-fetched and the banner clears.
Options considered
I looked at four ways to handle staleness:
Design decisions

Reference is synth start, not synth finish, for the reason in option 4. When no write-lock activity is observed for a generation (the synth predated the server, or the lock was too brief to catch), it falls back to the manifest mtime, which degrades gracefully to option 3.
Staleness is decided server-side and returned as a stale flag on /api/file. The client only renders it; it cannot compute it, since it does not know synth timing.
The assembly watcher is the single owner of the reference. It already observes both signals it needs, the synth.lock start and the assembly generation change, so it advances the reference once per generation before waking browsers, and the read endpoints only read it. Keeping this in one place avoids a reference that lags a synth or gets overwritten by concurrent reads.
mtime, not a content hash. A save that does not change content gives a false stale flag. That is acceptable under the same best-effort bar: the worst case is a banner you did not need until the next synth.
The banner covers any open source file, not only files carrying violations, since a stale navigation anchor matters too. It is scoped to the source pane; templates are regenerated by synth, so they are never "source".
Open questions:
I know in my demo meeting we decided that the web shouldn't have synthing capabilities, but having a banner that says "run synth" without a button to directly run synth seems unintuitive. Happy to hear opinions on if we think this means we should add manual and / or auto synth back into the web (and how we should handle the queue of 1 then, just have it be in-process still?).
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license