Detect concurrent config edits before writing to disk - #2290
Conversation
Saving the config was a blind whole-file write, so two people editing the dashboard at once would silently overwrite each other - a browser tab left open for an hour could clobber changes saved minutes earlier, with no warning and no indication anything had been lost. The server now sends a SHA-256 of the config file as an X-Config-Hash header on every YAML response. The client captures that at page load and sends it back as `baseHash` when saving; if the file on disk no longer matches, the write is refused and the current contents are returned so the client can show what changed. A conflict opens a side-by-side merge view with per-hunk accept/reject and four exits: review later, discard mine and reload theirs, overwrite with mine (behind a confirmation), or save a hand-merged version. Requests without a baseHash - the REST API, older clients - write exactly as before, so nothing can be locked out of saving. Both sides of the diff are serialized through the same shared toSaveShape() helper, otherwise a hand-maintained conf.yml diffs against reordered, comment-stripped output and every hunk is noise rather than a real change. The change count comes from the merge view's own chunks, and every highlight is paired with a gutter marker so the diff still reads without colour vision. Also fixes request.js returning fetch's native Headers instance while documenting an axios-compatible API; response headers are now normalized to a lowercase-keyed plain object, since bracket access on Headers silently yields undefined.
✅ Deploy Preview for dashy-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Hmm, my worry with this, is that it doesn't seem quite right. And it adds a LOT of code to do something relatively simple. Realistically, it shouldn't ever be possible to reach this edge case. So I don't think we need to over-engineer it so much. |
lissy93
left a comment
lissy93
left a comment
There was a problem hiding this comment.
Right now, I'm maintaining Dashy alone. So I need to ensure that every line of code added is of good quality, and will be maintainable going forwards.
Unfortunately, this diff seems to be quite poor quality, likely AI slop. It will introduce a lot of issues for everyone, and be hard to maintain going forwards.
So I'm going to close this for now. If it is working for you, keep it going on your fork though :)
| import Footer from '@/components/PageStrcture/Footer.vue'; | ||
| import EditModeTopBanner from '@/components/InteractiveEditor/EditModeTopBanner.vue'; | ||
| import CriticalError from '@/components/PageStrcture/CriticalError.vue'; | ||
| import ConflictResolver from '@/components/Configuration/ConflictResolver.vue'; |
There was a problem hiding this comment.
Let's not import this here. Conflicts are a niche edge case, so eagerly loading the module is going to slow down everyone's dashboard with a component which won't be used 99% of the time.
| 'SET_CONFIG_HASH', | ||
| 'SET_ROOT_CONFIG_HASH', | ||
| 'SET_SAVE_CONFLICT', |
There was a problem hiding this comment.
I'd prefer to keep out of global store, unless absolutely essential to be needed globally always
| // The conflict can be cleared (e.g. dismiss()) before this deferred | ||
| // $nextTick callback runs, so guard against building a view for it | ||
| if (!this.$refs.mergeEl || !this.conflict) return; | ||
| // Bumps a reactive counter on every doc/chunk change so mergedText, | ||
| // isMergedValid and changeCount stay live as the user edits or accepts/ | ||
| // rejects hunks - the MergeView doc itself is markRaw'd and non-reactive | ||
| const onUpdate = EditorView.updateListener.of((update) => { | ||
| // Selection/viewport-only updates (e.g. cursor movement) carry a | ||
| // transaction too, so `transactions.length` alone doesn't filter them | ||
| // out - they must not trigger this, or a full js-yaml re-parse of the | ||
| // whole config fires on every click/arrow-key inside the merge pane. | ||
| // MergeView's own chunk-recompute broadcast to the *sibling* pane | ||
| // (after an edit on the other side) has docChanged: false but carries | ||
| // a StateEffect - that one IS real signal and must still get through, | ||
| // or changeCount stops updating once a hunk resolves. |
There was a problem hiding this comment.
No need for the essay.
Good code should be readable (without the need for a 10-line comment per 1-line of code).
| if (configHash) { | ||
| res.set('X-Config-Hash', configHash) | ||
| .set('Access-Control-Expose-Headers', 'X-Config-Hash'); | ||
| } |
There was a problem hiding this comment.
don't do this!
Need to protect config from unauthenticed visitors, tthis will break that
| try { | ||
| const parsed = yamlLoad(raw) || {}; | ||
| // If parsed is not a plain object (e.g. scalar or array), return raw unchanged | ||
| // to keep malformed files viewable rather than fabricating an object | ||
| if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) { | ||
| return raw; | ||
| } | ||
| const isSubPage = !!this.$store.state.currentConfigInfo.confId; | ||
| // Pull the four config-owned keys out to the front in the canonical | ||
| // order, defaulting the ones buildRootEffective/INITIALIZE_CONFIG | ||
| // always populate - but keep any other top-level key the file may | ||
| // have (`...rest`), so an unrecognised key never silently vanishes | ||
| // from the diff. | ||
| const { | ||
| appConfig, pageInfo, sections, pages, ...rest | ||
| } = parsed; |
There was a problem hiding this comment.
I think there's a reusable method for this, which will be better than re-creating
| /* Reshape 'theirs' (raw disk bytes) into the exact shape 'yours' actually | ||
| has, then run it through the same save-shape transform ConfigSaving.js | ||
| uses, so both sides of the diff are compared in identical serialization | ||
| style. 'yours' is never simply a load->dump round-trip of the file on | ||
| disk: | ||
| - root: INITIALIZE_CONFIG's buildRootEffective always emits keys in | ||
| {appConfig, pageInfo, sections, pages} order and always includes | ||
| `pages` (defaulting to []), regardless of the on-disk key order. | ||
| - sub-page: INITIALIZE_CONFIG builds 'own' as | ||
| stripRootOwnedFields({appConfig, pageInfo, sections}) - no `pages`, | ||
| no inherited `auth`. | ||
| Without matching that shape (not just the serializer), a hand-maintained | ||
| conf.yml still diffs against reordered/pages-added output and hunks show | ||
| up that are pure serialization noise, not real changes. toSaveShape is | ||
| the same helper ConfigSaving.js's writeConfigToDisk uses to build the | ||
| real 'yours', so this can never silently drift from it. | ||
| Falls back to the raw text if it fails to parse - a malformed file on | ||
| disk must still be viewable, not throw. */ |
There was a problem hiding this comment.
again, no in-code essay needed. Just write readable code instead.
| editMode: false, // While true, the user can drag and edit items + sections | ||
| modalOpen: false, // KB shortcut functionality will be disabled when modal is open | ||
| currentConfigInfo: {}, // For multi-page support, will store info about config file | ||
| configHash: null, // Load-time hash of the active config, for conflict detection |
There was a problem hiding this comment.
This is going to break everybody's config saving whenever something local changes normally
|
Odd, it’s working for us. I liked dash and so I brought it into 3 different projects, 2 work networks and a group hobby network lab. You mentioned edge case and what this is there to help is people making changes and overwriting other configs without reloading. In the first few days we had dozens of overwrites, smashing others work. I stood over a couple of instances and watched them refresh the page, make a change and it stomped on another previous update. We still like it but we ended up turning off gui save to disk and make people edit the file manually. I’m happy to rework the code but right now we’ll stick with ours updates. |
|
I think a simpler solution, would be to just fetch the fresh config when an edit session starts? That would be a line or 2. And it wouldn't need the heaviness of whole new config resolving components. |
|
Yes, but some type of basic lock so when it’s being edited others know. A
merge / diff feature adds more complexity though.
…On Sun, Aug 9, 2026 at 3:56 PM Alicia Sykes ***@***.***> wrote:
*lissy93* left a comment (lissy93/dashy#2290)
<#2290 (comment)>
I think a simpler solution, would be to just fetch the fresh config when
an edit session starts? That would be a line or 2. And it wouldn't need the
heaviness of whole new config resolving components.
—
Reply to this email directly, view it on GitHub
<#2290?email_source=notifications&email_token=AALWQ47LVJBAKPK2VQGLKDD5JDJODA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMRTGM2TKOBWGEYKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-5233558610>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AALWQ44RJZLCJZ55QU2LJSL5JDJODAVCNFSNUABFKJSXA33TNF2G64TZHMZTIMZQG44DANRQHNEXG43VMU5TKMJQGM3DEMRXGY22C5QC>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/AALWQ46KNDDJE26OOPBIGZD5JDJODA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMRTGM2TKOBWGEYKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG>
and Android
<https://github.com/notifications/mobile/android/AALWQ47RME2YIDUBUNN6Q5T5JDJODA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMRTGM2TKOBWGEYKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>.
Download it today!
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
2 participants
Saving the dashboard config is currently a blind whole-file write, so two people editing at once silently overwrite each other — a browser tab left open for an hour can clobber changes someone saved minutes ago, with no warning that anything was lost. This adds an optimistic concurrency check: the server sends a SHA-256 of the config file as an
X-Config-Hashheader on YAML responses, the client captures it at page load and sends it back asbaseHashwhen saving, and if the file on disk no longer matches, the write is refused and the current contents are returned so the client can show a side-by-side merge view of exactly what would have been overwritten — with per-hunk accept/reject and the choice to reload theirs, overwrite with yours behind a confirmation, or save a hand-merged version. Requests without abaseHash(the REST API, older clients) write exactly as before so nobody can be locked out of saving, and a save from an up-to-date page is completely unchanged: no extra round-trip, no polling, nothing new on screen.