Repository navigation
Gate fixes: stale endpoint responses, stale project target, false magic-link success (PR #100 findings) - #102
Merged
sgardoll merged 9 commits intoSep 28, 2026
Conversation
…success - G3: a project fetch or connection check started for the previous endpoint discards its outcome once the endpoint generation advances, so a late response can never overwrite the current endpoint's status or project list. - D2: replacing the FlutterFlow key without choosing a (different) project clears the stored target so a deploy cannot silently use the old project. - G4: an HTTP-success magic-link response carrying any code other than the success code (or none) is surfaced as an error instead of 'Sent!'. - Regression tests for all three behaviours (unit + e2e).
The held check now resolves with an empty project list, so a stale response would flip the card to 'no-projects' — visibly different from staging's 'connected' — instead of repeating the same outcome.
sgardoll
marked this pull request as ready for review
September 28, 2026 07:02
Contributor
|
This PR does not match any of the 1 configured review trigger rule. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Devin finding 1 (app.js): a projects fetch or connection check now captures the key it was issued with plus a monotonic request sequence. An outcome that a newer request has superseded, or that was issued for a key since replaced or cleared, is discarded — so a late old-key response can no longer populate the new key's project list or connection status when the endpoint did not change. A preview fetch made with the key being saved stays valid. Devin finding 2 (src/authMagicLink.js): an explicit success:false or truthy error is never a success, with or without a code, and an empty or malformed body is not success-shaped. The legacy message-only response still counts. getMagicLinkResultMessage follows the same predicate, so a 200 that says it failed can no longer read as "Sent!". Regression tests: a stale-old-key fetch held across a key replacement (e2e), a success:false sign-in response (e2e), and unit tests for the failure, error-field and malformed-body classification.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Devin finding 1 (app.js): the dropdown fetch and the connection check shared
one supersession sequence, so a check starting while a fetch was pending
discarded the fetch's response and left the dropdown stuck on "Loading
projects..." forever. Each kind now keeps its own sequence — both still gated
by endpoint generation and the configured key — so a check cannot supersede a
pending fetch and vice versa; same-kind supersession and stale
key/endpoint rejection are unchanged. Regression test in
e2e/stu384-connection.spec.js holds a listProjects fetch, starts a validation,
releases the fetch, and asserts the dropdown populates and the card stays
connected (fails before the fix).
Devin finding 2 (src/accountAccess.js): createSendLinkController marked every
resolved response as "sent" and never consulted isMagicLinkSuccess, so a 200
{code:"SEND_FAILED"} or {success:false} rendered the sent confirmation with the
server's error message. The controller now classifies the response with the
shared magic-link predicate before emitting "sent" and routes unsuccessful
responses through its error state with the server's own message (message, else
error, else a generic fallback). Unit tests cover SEND_FAILED, success:false
and genuine success; an e2e assertion in stu387 covers the row rendering.
Pre-existing tests that used the fictional {code:"sent"} fixture now use
MAGIC_LINK_SUCCESS_CODE.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Devin finding 3 (app.js): a project chosen from the old key's dropdown before
the key was replaced was still persisted as the deploy target, so a later
deploy could target a project the new key never listed. Third instance of the
stale-identity class, so the approach changes instead of patching the
instance: a selection now carries an identity binding ({projectId, key,
endpointGeneration}) recorded only from a completed listProjects response, and
every read of a selection — saveApiKeys preservation, the dropdown value, the
deploy confirm modal and every deploy target read — goes through
isProjectSelectionConfirmed (or the modal list-identity gate). A key
replacement or endpoint change moves the binding out of match, so no
previously confirmed selection can be used until a completed fetch
re-confirms it or it is cleared.
Regression tests: selecting a project from the old key's list and replacing
the key now leaves no stored target and requires a fresh selection (fails
before the fix); a stored target the current key's completed list omits is
refused at deploy time (fails before the fix).
…ui-ux' into pr-102-fix Co-Authored-By: Stuart Gardoll <sgardoll@gmail.com> # Conflicts: # app.js
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…target Three follow-on bugs from the identity-binding gate (Devin Review on the merged head): - A previewed key's leftover dropdown choice could erase the stored target on save: the unconfirmed-selection path now clears the stored project only on a real key replacement; an unchanged configured key keeps its target and reloads its list instead. - An older dropdown fetch could overwrite a newer connection check's outcome: a shared issue/applied order counter across both request kinds now gates writes to ffConnectionState and stored-selection confirmation, while the newest fetch still fills its own dropdown. - A direct commit during the post-reload project refresh refused the saved target on an unpopulated binding: the confirmed-stored-target read now verifies the project against the current key's real list (same completed-response guard, superseded/failed verifies still refuse) and the deploy dialog's stored fallback uses the same read. Regression coverage: preview-then-clear-save preserves the stored target, a late fetch can't resurrect connected after a failed check, and a direct commit during a pending startup check still deploys. Co-Authored-By: Stuart Gardoll <sgardoll@gmail.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Follow-on fixes for the second Devin Review round on f151de4: - getConfirmedStoredProjectId() now refuses its own result when a newer request already applied an outcome (including one that stripped the target's confirmation), and re-checks the configured key + endpoint generation before returning a target — a stale verification can no longer revive a removed project. - A project-scoped key whose listProjects is denied (403) no longer loses its stored target: the denial cannot disprove it, so the deploy proceeds and the push decides reachability; endpoint/key guards still apply. - populateConfirmProjectSelect() now treats an endpoint-generation or stored-key move during its fetch as a stale identity and repopulates for the current one instead of filling the deploy dialog with undeployable options. Regression coverage: stale verify cannot revive a removed target, a scoped key keeps its stored target, and an old endpoint's response cannot fill the deploy dialog (all fail on the previous head). Co-Authored-By: Stuart Gardoll <sgardoll@gmail.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…tion ordering Third Devin Review round on cbe2f39: - executeCommit, executeBundleCommit and commitToFlutterFlow now capture key + endpoint as one deploy identity: the captured key is passed into getConfirmedStoredProjectId so the verification describes the same credential the push will use, and the identity is revalidated after every asynchronous read — a key save or endpoint switch landing mid-deploy can no longer pair the old key with a target confirmed for a different identity. - Ordering of shared request state is split by write domain: connection outcomes (the account status card) order separately from selection confirmations. populateConfirmProjectSelect claims only the confirmation order, so a dialog fetch can no longer claim the outcome a pending check needs to settle the card — the card no longer sits on 'Checking connection…' after both requests finish. - fetchProjects writes account status and claims the connection outcome only for the configured key: a preview fetch for a key that is never saved can no longer mark the saved account rejected, nor flash validating on its card. Preview validity still populates the editor dropdown independently. Regression coverage: a deploy-dialog fetch cannot strand the card on checking, a preview key's failure cannot mark the saved account rejected, and a mid-deploy key save cannot pair the old key with a new target (all fail on the previous head). Co-Authored-By: Stuart Gardoll <sgardoll@gmail.com>
Contributor
|
E2E-tested head
One transient
More: success path + connected card
|
sgardoll
merged commit Sep 28, 2026
f95a16d
into
milestone/stu-implement-updated-ui-ux
2 checks passed
sgardoll
pushed a commit
that referenced
this pull request
Sep 28, 2026
…ed-ui-ux The re-sync onto f95a16d pulls in #102, which changes app.js and src/. The committed bundle was stale against that source, so dist is rebuilt to keep the published assets and the source in agreement. New bundle: index-C31N1DOp.js -> index-LEYLuMwg.js. The CSS hash is unchanged and dist/index.html references only files present in dist/assets/.
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.
Summary
Fixes the stale-identity class of review findings on the milestone integration PR (#100): any async response or stored value that outlives the key/endpoint it was produced under must not be applied to the current one. Three mechanisms now cover every surface the reviewers flagged:
ffEndpointGeneration— monotonic counter bumped on every endpoint change;fetchProjects()andvalidateFlutterFlowConnection()capture it at start and discard their outcome (status writes, dropdown population, error rendering) if it moved on.ffProjectFetchSeq/ffConnectionCheckSeqviaisCurrentProjectRequest()) — a connection check starting while a dropdown fetch is pending no longer discards the fetch (which stranded the dropdown on "Loading projects..."), and a late response for a replaced key can never overwrite the new key's list or status.projectSelectionIdentity+isProjectSelectionConfirmed()) — a project selection is usable as a deploy target only while it names the project and the key + endpoint generation a completedlistProjectsresponse confirmed it under. Key replacement, endpoint change, or a completed list that omits the stored target all strip confirmation, so an old-key choice can never become the new key's deploy target. Applied atsaveApiKeys(), the editor dropdown, the deploy confirm modal (confirmProjectListIdentity), and every deploy target read.isMagicLinkSuccess(data)insrc/authMagicLink.js— success only for the explicit success code, an explicitsuccess: true, or a legacy message-only body;success: false, a truthyerror, any othercode(incl.PLUS_ALIAS_REJECTED), or a malformed/empty body is an error. Used by both the sign-in modal (handleMagicLinkRequest) and the account "Send new link" controller (createSendLinkController), so a failed send is never reported as "Sent!".Findings addressed
ffEndpointGenerationguards both consumersisMagicLinkSuccessin the sign-in handlersuccess:false/errorbodies claim sentcreateSendLinkControllerusesisMagicLinkSuccess, routes failures to its error stateffRequestOrderSeq/ffAppliedOutcomeOrder) across both request kinds gates all shared-state writes; dropdown options stay per-fetchgetConfirmedStoredProjectId()verifies an unconfirmed stored target against the current key's real list before refusing; the deploy dialog's stored fallback uses the same readlistProjectsdenial (403) can no longer disprove the stored target — the push decides reachability; endpoint/key guards still applypopulateConfirmProjectSelectguards on endpoint generation + stored key and repopulates for the current identitygetConfirmedStoredProjectId, and revalidate it after the last async readfetchProjectswrites status/claims outcomes only while its key matches the configured key at write time; previews populate their own dropdown onlyVerification on head
683f43d(includes merge ofmilestone/stu-implement-updated-ui-ux@5d040d7)CI=1 npm test— 360 pass / 0 failCI=1 npm run build— ✓ builtCI=1 CCC_TEST_PORT=3106 npx playwright test e2e/stu384-connection.spec.js e2e/stu386-magic-link-signin.spec.js e2e/stu387-account-access.spec.js— 41/41 pass, covering: stale endpoint fetch/check, check-during-fetch, stale old-key fetch, key replacement without/with an old-key selection, stored target omitted by the new list, previewed-key leftover choice on save, a late dropdown fetch after a newer failed check, a direct commit during a pending startup check, a stale verify reviving a removed target, a scoped key keeping its stored target, an old endpoint filling the deploy dialog, a dialog fetch stranding the card, a preview key rejecting the saved account, a mid-deploy key save pairing credentials, HTTP-200 error-code /success:falsesends, and the account resend failure pathscbe2f39(headed browser, mocked backends, recorded): all async-race fixes exercised live — see the testing comment on this PRf496402:npm run test:ui209 pass + 1 flaky (pre-existingstu385race, passes on retry),test:accessibility7/7,test:deploy-guard9/9,test:proxies30/30,scan_bundle_for_leaks.pycleanMerge note
Resolved the
saveApiKeys()conflict withmilestone/stu-implement-updated-ui-uxby keeping both guards: the base's stored-key check (previousKeyfrom storage, clears when the key changed with no selection) and this branch's confirmation gate — the base's early declaration ofselectedProjectIdwas kept so the merged flow compiles unchanged. Same-endpoint out-of-order responses keep last-write-wins within one request kind, which is the intended scope of the generation guard.Link to Devin session: https://app.devin.ai/sessions/f52b679964d54e84a7f2d8bf3b1abe37
Open in Devin Desktop: https://app.devin.ai/desktop/session/f52b679964d54e84a7f2d8bf3b1abe37?variant=devin
Requested by: @sgardoll