webapp: the settings rows and the tab document stop lying about what landed - #222
Draft
pythonlearner1025 wants to merge 2 commits into
Draft
webapp: the settings rows and the tab document stop lying about what landed#222pythonlearner1025 wants to merge 2 commits into
pythonlearner1025 wants to merge 2 commits into
Conversation
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
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.
What part this touches
The settings surfaces and the shell's persistence layer.
Settings hold rows: invites, credentials, provider connections, credential
requests and grant proposals. Each row is written by a control-plane call.
use-workspace-persistence.tswrites the workspace document that remembersterminal tabs, surface tabs, the preview tab and the side pane width.
Stacked on #220, which is stacked on #205. Review those first.
The gap
Two shapes, both found by the audit on #205.
Rows stay stale until the server answers. A user changes something and the
row keeps its old value until the request returns, or until the next refetch.
Ten surfaces behave this way.
The shell updates at once but cannot undo it. A terminal tab opens
instantly and the document write follows. If that write is rejected, nothing
puts the tab back. The screen and the server disagree, and the user is not
told.
Two of these were worse than slow:
with
void. A rejection went nowhere.successful write never reconciled either.
The fix
Rows now show the intended result, then reconcile or roll back:
invented — the row appears, the secret arrives with the response.
.envimport),compute credentials and inline setup, provider connect, paste-and-connect and
disconnect, personal grant revoke, credential request dismiss and deny, and
grant proposal approve and reject.
use-grant-proposals.tsdismiss, settled and reopen.poll, and restores the stopped pane on failure.
The persistence fix is ONE change, not twelve.
use-workspace-persistence.tskeeps the last acknowledged document, applies the document the server returns,
serializes a newer edit over an older one, and restores the acknowledged
document when a write is rejected. Every tab action inherits it.
The risk trade
An acknowledged-document rollback can undo an edit the user made while a write
was in flight. Serializing newer edits over older ones is what keeps that from
happening in the common case, but a rejection still restores the last state the
server confirmed. That is the honest trade: the screen agrees with the server,
even when that costs a keystroke.
The alternative was per-action rollback at each call site. That is twelve
copies of one rule, and the audit on #205 found exactly that kind of
duplication elsewhere. One owner is worth the coarser undo.
OAuth completion is deliberately unchanged.
connectStartUrlonly mints anexternal URL, and the provider owns the decision. Showing a connection as made
before the provider agrees would be a lie, not optimism.
Drive is untouched. It is being deleted.
Tests
Fifteen tests. Each drives a deferred promise, asserts the screen changed
BEFORE the request settled, and asserts the restore on rejection. Both
directions, or the test proves nothing.
All fifteen were seen failing with their production code reverted: 31 failed,
126 passed across those 9 files. With the code restored: 157 of 157 pass.
Examples of the failing output:
Real counts on this branch: 116 webapp files pass, 1039 tests pass, 61 skip.
One
it.fails(...)inlody-toaster.test.tsxreports as an expected fail, asit does on
main.npm run lint:gatepasses at baseline. 66 anti-slop findings, 0 housefindings, 8 max-lines warnings. None moved.
npm run typecheck -w @blitzos/webapppasses.Deploy
The webapp bundle ships inside the control-plane worker. This branch is stacked
on #220 and must not merge before it.
After #220 merges, rebase this onto
mainand merge it. Merging tomainruns.github/workflows/canary.yml, which deploys the worker to canary. No box codechanged.
Verify after the merge:
Then open settings, change a role or revoke a credential, and watch the row
change before the request finishes.
Rollback: