Skip to content

feat: explorer stale source banner#1738

Open
megha-narayanan wants to merge 1 commit into
aws:feat/cdk-explorerfrom
megha-narayanan:feat/explorer-stale-source-banner
Open

feat: explorer stale source banner#1738
megha-narayanan wants to merge 1 commit into
aws:feat/cdk-explorerfrom
megha-narayanan:feat/explorer-stale-source-banner

Conversation

@megha-narayanan

Copy link
Copy Markdown
Contributor

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:

  1. Require synth-on-save in the web explorer. Keep the assembly always fresh by triggering a synth on every source change. Rejected: web synth-triggering was descoped, the explorer is a read-only viewer over cdk.out, and forcing a synth on every edit duplicates the LSP's job, adds a synth runner and lock coordination to the web process, and takes control of an expensive operation away from the user.
  2. Snapshot and serve (caching). On synth start, cache the source files and serve that snapshot so the markers always line up with the displayed code, showing a banner when disk differs from the snapshot. Rejected for this change: it needs a per-generation content store, an atomic swap of assembly and snapshot together, and a full source-tree read per synth, and it shows the user older code rather than their current edits. It removes wrong-line markers entirely, but that failure is rare and low-impact, so the cost is not justified here.
  3. Reference the synth finish. Read the source at synth finish and flag a file whose last-modified time is later than that. Simple, but it misses the in-flight case: an edit made while a synth is running lands before that synth finishes, so it would not be flagged even though the synth may not have picked it up.
  4. Reference the synth start (chosen). Record when synth write-lock activity first appears (the synth.lock marker the toolkit's RWLock creates), and treat a file modified after that point as stale. This catches the in-flight edit that option 3 misses, at the cost of over-flagging by at most one synth's duration, which self-heals on the next assembly. It needs no synth machinery and keeps the explorer read-only.

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".
Screenshot 2026-07-17 at 12 31 29 PM

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

  • This change contains a major version upgrade for a dependency and I confirm all breaking changes are addressed
    • Release notes for the new version:

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

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.
@github-actions github-actions Bot added the p2 label Jul 17, 2026
@aws-cdk-automation
aws-cdk-automation requested a review from a team July 17, 2026 16:44
@megha-narayanan
megha-narayanan changed the base branch from main to feat/cdk-explorer July 17, 2026 16:45
@megha-narayanan
megha-narayanan marked this pull request as ready for review July 17, 2026 16:46
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (feat/cdk-explorer@a46da51). Learn more about missing BASE report.

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           
Flag Coverage Δ
suite.unit 89.55% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants