Skip to content

fix(console): « Add a source » waits for the list of the sources, so its buttons do not move under a press (FRU-271) - #295

Merged
sakuga-agent[bot] merged 2 commits into
mainfrom
fru-sf-entree/fru-271-connectors-list-keeps-its-room
Oct 13, 2026
Merged

sakuga-agent[bot] merged 2 commits into
mainfrom
fru-sf-entree/fru-271-connectors-list-keeps-its-room

Conversation

@sakuga-agent

@sakuga-agent sakuga-agent Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Not critical: one screen of the console, no route, no safeguard. Part of FRU-271 (parent FRU-230): the Connectors screen. Members is the next pull request. Same defect as My account (#293, #294).

The defect, measured

« Add a source » is under the list « Connected », which says « Loading… » and then takes its rows. On main, with GET …/connectors held, « Connect REST API » goes down by the height of the list:

sources connected the button moves
1 45 px
2 110 px
3 175 px

A press that starts before the list and ends after it is lost. A second defect is in the same window: before the read, the screen does not know whether this worker has a Linear application, so « Connect Linear » opens the form of a key where the consent at Linear was meant.

The fix

« Add a source » shows once the list is read (answered in routes/connectors.tsx). After a read that failed, the screen has the alert and « Try again », and no source to add: it does not know how this worker connects Linear. The number of sources is not known before the read, so no room can be kept for them; the buttons come with the list, at the place where they stay. Most of the diff of connectors.tsx is the indentation of the section, now inside one condition: read it with whitespace hidden.

Proof

e2e/console-connectors.spec.ts, one new case, on stubs: with the read held, « Loading… » is there and « Add a source » lists nothing. Once released, the eight rows are there. The pointer goes down on « Connect REST API », the page comes to rest, the box of the button is the same, the pointer goes up, and the form of an address opens. It failed before the change (eight rows where none is expected). The case of a read that failed now also holds that nothing is offered until « Try again » succeeded, then eight rows and « Use an API key » (this worker has a Linear application); with the first condition planted back, it fails. Locally: console-connectors 12, console-source-panel 7, console-destinations 2, console-narrow 5, console-frame 17. The measures of the mockup are read with the list there, and that state is unchanged.

Not verified

Firefox, Safari, a touch screen, a screen reader. The press was made with a mouse in Chromium.

🤖 Generated with Claude Code


Sakuga Review · Score de confiance : 4/5

Merge possible : le changement est localisé, il masque « Add a source » tant que la liste n'est pas lue ; un seul point mineur (adding/problem non rendus après échec de lecture).

Constats

  1. P2 Formulaire d'ajout perdu si la relecture échoue apps/console/app/routes/connectors.tsx:1
    Une personne a ouvert le formulaire AddRest et tape une adresse. Si un rechargement (load) échoue alors que connectors était déjà défini, il reste défini : pas de souci. Mais au premier échec, connectors reste undefined et la section disparaît, ce qui est voulu ; le risque est faible. Correction : Aucune action requise ; éventuellement vérifier que connectors n'est jamais remis à undefined après une lecture réussie.
Résumé

La PR conditionne l'affichage de la section « Add a source » à connectors !== undefined, pour éviter le déplacement des boutons et l'ambiguïté OAuth/clé. Les tests e2e et la doc sont mis à jour. J'ai vérifié la logique de load, les chemins d'échec et le rendu ; rien de bloquant.

Revues (2) · Dernier commit relu : fix(console): after a read that failed, no sour... · Niveau balanced · anthropic/claude-sonnet-5.5 · 0,066 $

…its buttons do not move under a press (FRU-271)

The list « Connected » arrives above « Add a source », whose buttons went down by the height of the
list: 45 px for one source, 110 for two, 175 for three (measured). A press that started before and
ended after was lost. And before the read, « Connect Linear » did not know whether this worker has a
Linear application: it opened the form of a key where the consent at Linear was meant.

« Add a source » now shows with the first answer of the list, or with the alert of a read that
failed. The new case of console-connectors.spec.ts holds the read: nothing to press while it is
held, then a press on « Connect REST API » finds the button where it stays and opens its form. It
failed before the change.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

@kento-review kento-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Score de confiance : 4/5 · ⚠️ Rien de bloquant · Mergeable après un regard sur les deux constats mineurs : le changement attend bien la liste, mais l'affichage en cas d'échec de lecture garde un état par défaut, et le test ne reproduit pas la pression commencée avant l'arrivée de la liste.

Niveau lite · anthropic/claude-haiku-5.5 · 0,005 $ · Le résumé, le diagramme et la liste des constats sont à la fin de la description de la PR.

@kento-review kento-review Bot added the review: 4/5 Score de confiance de Sakuga Review (5 : prête à merger) label Oct 13, 2026
@sakuga-agent

sakuga-agent Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

@sakuga-review review balanced

@kento-review kento-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Score de confiance : 3/5 · ⚠️ Rien de bloquant · Fusion possible avec une réserve : la section « Add a source » peut disparaître après coup, ce qui détruit un formulaire en cours de saisie.

Niveau balanced · anthropic/claude-sonnet-5.5 · 0,068 $ · Le résumé, le diagramme et la liste des constats sont à la fin de la description de la PR.

@kento-review kento-review Bot added review: 3/5 Score de confiance de Sakuga Review (5 : prête à merger) and removed review: 4/5 Score de confiance de Sakuga Review (5 : prête à merger) labels Oct 13, 2026
…e list is read (FRU-271)

« Add a source » showed with the alert of a read that failed, when the screen does not know whether
this worker has a Linear application: « Connect Linear » then opened the form of a key where the
consent at Linear was meant. The section now waits for a list that was read. The case of a read
that failed holds it, with its control after « Try again ». Answers the reviews of 8f1a0ab.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@sakuga-agent

sakuga-agent Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

On the reviews of 8f1a0ab:

Balanced P1 and lite P2, a read that failed leaves « Connect Linear » on a default. Right, fixed in d441f3d. The section now waits for a list that was read (connectors !== undefined). After a read that failed there is the alert and « Try again », and no source to add. The case « with no source, and with a read that failed » holds it: no row to add while the read failed, then eight rows and « Use an API key » after « Try again ». With the old condition planted back, it fails. The comment says the same. The first part of the P1 (a form unmounted while somebody types) does not happen, as the finding itself concludes: once connectors is set it is never unset, so the section does not go away.

Lite P2, « the test does not press before the list ». The test takes the second way the finding proposes: with the read held, it asserts that the button is not drawn (toHaveCount(0) on the rows and on « Connect REST API »). That assertion is the one that fails on main. A press cannot start on a button that is not there; the press that follows shows that the button, once drawn, does not move while the page comes to rest.

@kento-review kento-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Score de confiance : 4/5 · ⚠️ Rien de bloquant · Mergeable : le changement ne retient « Ajouter une source » que lorsque la liste des connecteurs a été lue, ce qui est cohérent avec le cas de lecture en échec. Seul un test manque de précision, sans effet sur le comportement.

Niveau lite · anthropic/claude-haiku-5.5 · 0,004 $ · Le résumé, le diagramme et la liste des constats sont à la fin de la description de la PR.

@kento-review kento-review Bot added review: 4/5 Score de confiance de Sakuga Review (5 : prête à merger) and removed review: 3/5 Score de confiance de Sakuga Review (5 : prête à merger) labels Oct 13, 2026
@sakuga-agent

sakuga-agent Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

@sakuga-review review balanced

Since the balanced of 8f1a0ab, d441f3d answers its finding: « Add a source » waits for a list that was read, and a read that failed offers « Try again » and no source to add.

@kento-review kento-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Score de confiance : 4/5 · ⚠️ Rien de bloquant · Merge possible : le changement est localisé, il masque « Add a source » tant que la liste n'est pas lue ; un seul point mineur (adding/problem non rendus après échec de lecture).

Niveau balanced · anthropic/claude-sonnet-5.5 · 0,066 $ · Le résumé, le diagramme et la liste des constats sont à la fin de la description de la PR.

@sakuga-agent

sakuga-agent Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

On the two P2 of the reviews of d441f3d, both with no change:

Balanced, « the form to add a source is lost if a read made again fails ». It is not lost. connectors is set by a read that succeeded and nothing sets it back to undefined: a later read that fails keeps the list and the section, with the form somebody is typing in (load writes setConnectors only when listed.ok). The section is absent only before the first read that succeeded, when no form can be open. The finding says so itself, and asks for no action.

Lite, « the test does not cover the arrival of the list ». The arrival is covered by the assertion before the press. With the read held, the test asserts that no source to add is drawn and that « Connect REST API » is not there (toHaveCount(0), twice). That is what fails on main, where the eight rows were drawn above a list still to come. A press cannot start on a button that is not drawn, so the lost press of the defect has no place to start. The press that follows guards the other half: once drawn, the button does not move while the page comes to rest (networkidle, the late notes of each address included). A regression that draws the section before the list fails the first assertion; one that moves the button after it is drawn fails the second.

@sakuga-agent
sakuga-agent Bot merged commit ea4b9ec into main Oct 13, 2026
13 checks passed
@sakuga-agent
sakuga-agent Bot deleted the fru-sf-entree/fru-271-connectors-list-keeps-its-room branch October 13, 2026 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review: 4/5 Score de confiance de Sakuga Review (5 : prête à merger)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant