Skip to content

test(worker): a visitor with no account leaves a note, on a worker with accounts (FRU-224) - #281

Merged
Mheaus merged 3 commits into
mainfrom
mathieu/fru-224-worker-une-note-dun-visiteur-sans-compte
Oct 13, 2026
Merged

Mheaus merged 3 commits into
mainfrom
mathieu/fru-224-worker-une-note-dun-visiteur-sans-compte

Conversation

@sakuga-agent

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

Copy link
Copy Markdown
Contributor

Stack: #277 → this. The base is the branch of #277 (FRU-223): the E2E spec needs the workers of the suite to have accounts, which is what #277 gives them. For Mathieu, with #277: it states what is protected (SECURITY.md), and it raises one question below. I do not merge it.

Tests and documents only. app.ts does not change.

Scope

FRU-224 has two halves. This is the first: the guard on the path of a visitor with no account. The second half is not here, on purpose: the token signed by the site (a secret for each site of the console, or the end of that way of working) is a decision that is not taken. Nothing in this pull request adds, removes or prepares it; docs/modes.md says what holds today.

What it does

Mathieu, 2026-10-13: « on peut créer un compte quoiqu'il arrive aussi pour la partie admin, par contre pas forcément besoin de compte pour laisser des notes côté user ». The worker did it already. No test held the whole path on a worker whose sites come from the console: the tests that did ran on FRUITBACK_CLIENTS, which is going (FRU-184).

  • apps/worker/src/visitor.test.ts (7 cases, through handleRequest, on PostgreSQL (PGlite), better-auth on, the notes in the store of the database). An admin signs in and makes the site through the route of the console. A visitor then sends what the <script> of the site sends: the id of the site and the Origin of the page, with no Authorization and no Cookie.
  • e2e/visitor.spec.ts (3 specs, in Chromium, on the two workers of the suite). What a browser adds is CORS: a page of another origin gets no status and no body.
  • SECURITY.md, docs/modes.md, docs/decisions/worker.md.

One thing measured that the ticket says otherwise

A site for members takes a note that has no session. The ticket and FRU-184 say that such a site « asks for a paired session ». That is true of a read (401, identity-required). A write with no token is a claim on every site (attributionFor), and is kept: 201. Its sender cannot read it back; the members find it, in the worker or in the tracker of the site.

That is what main does, and has done since the visibility is read. I changed nothing: test:still takes a note sent with no session, which its sender cannot read back holds the behaviour with a warning that it is not a promise, and SECURITY.md now says it in plain words (« The visibility of a site decides who reads it, and not who writes on it »).

Question for Mathieu: should a site read by members refuse a note that comes with no session? Today a team has no setting for « notes from our members only ». It is one condition in attributionFor, and it would also close the team mode to a reviewer whose extension is not paired yet.

Proof

Done when (ticket) Test
A site everyone made by the console, a note with no Authorization, read with no token leaves a note and reads it again, with no session, no token and no cookie; in a browser: a visitor plants a pin and finds it again, and no request carried a credential (each request to /feedback is read: no authorization, no cookie)
Refused from another origin refuses a note and a read that name the site from the page of another site (two checks, both named: origin-not-allowed for an origin that is no site, origin-not-allowed-for-client for the site of another workspace); in a browser: a page of another origin reads no note of the site, and plants none, with its control (the page of the site reads its note with the same request) and the answer of the worker read from outside the browser (403, no CORS header)
The visibility reads no note: the worker asks for a session, and says so; in a browser: on a site that its members read, a visitor reads no note (the same page and the same site id on two workers: 401 and 200)
The name is a claim keeps the name the visitor typed, and never says the worker verified it (the visitor sends verified: true)
Fails when the check of the Origin is removed planted in resolveClient: 1 test of visitor.test.ts fails
Fails when the visibility is removed planted in clientOf (read: 'public' for every site): 2 tests fail

On this machine: visitor.test.ts 7 pass; the three specs pass against the workers of #277; the whole worker suite, 1568 tests, 0 fail, 109 skipped (the ones that need a server); typecheck, oxlint, oxfmt clean.

Not done, not verified

  • « The decision of point 3 is written in worker.md » (ticket): there is no decision to write. worker.md says that it is not taken.
  • The table « What each one needs » of docs/modes.md is not rewritten. It describes a worker of FRUITBACK_CLIENTS as well as a worker with accounts, and what it becomes depends on point 3 and on FRU-225. I added a section that says what the visibility of a site is in the words of that page, and that the host's token does not exist on a worker with accounts.
  • The words of the screen « Sites » (point 4 of the ticket: does it say that on everyone anybody who opens the page reads and writes notes?). That screen is being rebuilt from the new mock-up by other agents (FRU-243); I did not touch it. With the measure above, the sentence for members should not promise that only members write.
  • The E2E spec reads the notes of the first worker of the suite, which keeps them on the Linear in memory (feat: pnpm dev and the E2E suite run on a PostgreSQL in the process (FRU-223) #277 says why). The notes in PostgreSQL are held by visitor.test.ts.
  • The planted violations were played on the tests of the worker, not on the E2E spec.

🤖 Generated with Claude Code


Sakuga Review · Score de confiance : 4/5

Le changement ne contient que des tests, de la documentation et des décisions déjà documentées ; il peut être fusionné, sous réserve de vérifier les fixtures et le code worker non présents dans la PR.

Résumé

La PR ajoute des tests qui vérifient qu'un visiteur sans compte peut laisser et relire une note sur un site visible par tous, que l'origine est contrôlée, et qu'un site réservé aux membres refuse la lecture sans session (401). Elle ajoute aussi un test e2e (CORS, absence de cookie et d'Authorization) et met à jour SECURITY.md, docs/decisions/worker.md et docs/modes.md. Le diff ne montre pas le code des routes ni d'attributionFor ; je n'ai donc pu vérifier les tests que par leur cohérence interne avec la documentation.

Non relu
  • apps/worker: le code des routes /feedback et /console, resolveClient, clientOf, attributionFor et authorizeRead n'est pas dans le diff ; les assertions des tests ne sont vérifiables que contre la documentation.
  • e2e/pin.ts, e2e/cloud-worker.ts, e2e/worker-sessions.ts et dev-worker.fixture.ts : les fixtures utilisées par visitor.spec.ts ne figurent pas dans la PR ; je n'ai pas vérifié les sites 'playground' ni 'visitor-members'.
  • apps/worker/src/console-world.fixture.ts et postgres.fixture.ts : non fournis, leur comportement (signInTo, workspaceIn, rowsOf) est supposé.

Revues (8) · Dernier commit relu : test(worker): the control of the read of a site... · Niveau lite · anthropic/claude-haiku-5.5 · 0,003 $

@sakuga-review sakuga-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 · Peut être fusionnée: la PR ajoute des tests et de la documentation sur le parcours visiteur sans modifier le code de production; le seul point de vigilance est l'acceptation de notes sans session sur un site réservé aux membres, documentée et signalée comme question ouverte.

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

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

@sakuga-review sakuga-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 · La PR peut être fusionnée : je n'ai relevé aucun défaut nouveau introduit par ce changement. Le point d'attention est un choix de produit déjà documenté (une note anonyme est gardée sur un site réservé aux membres), qui existait avant ce changement.

Niveau lite · anthropic/claude-haiku-5.5 · 0,003 $ · 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

@sakuga-review review heavy

@sakuga-review sakuga-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 : 5/5 · ✅ Rien de bloquant · La PR peut être fusionnée : elle n'ajoute que des tests et de la documentation, sans aucun changement du code du worker.

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

@sakuga-review sakuga-review Bot added review: 5/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
@sakuga-agent

sakuga-agent Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

Reply to the heavy review of 444684f (5/5), on what it could not read: workspaceIn(world, token, name = 'Acme') takes a third argument, and callOn takes origin (string | null, the console's when absent) in console-world.fixture.ts; seeds.seed is text (postgres-schema.ts, FRU-218: the text of the seed as the widget sent it, not jsonb), so JSON.parse(String(...)) reads what was written. typecheck is green on this head, and the seven cases pass.

@Mheaus
Mheaus added this pull request to stack #284 October 13, 2026 14:49

@sakuga-review sakuga-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 · Fusion envisageable : la PR ne change aucun code de production de l'worker, seulement des tests, de la documentation et un spec e2e ; le seul point relevé est une assertion faible dans le spec e2e, non bloquant.

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

Comment thread e2e/visitor.spec.ts
@sakuga-review sakuga-review Bot added review: 4/5 Score de confiance de Sakuga Review (5 : prête à merger) and removed review: 5/5 Score de confiance de Sakuga Review (5 : prête à merger) labels Oct 13, 2026

@sakuga-review sakuga-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 · Le changement peut être fusionné : il ne touche que des tests, de la documentation et une spec e2e, sans code de production. Le seul point à corriger est un test qui ouvre une seconde base PGlite dans le même cas.

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.

@sakuga-agent

sakuga-agent Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

Reply to the lite review of 90ce4a2 (4/5): a second PGlite world in one case, right. The control of test:reads no note: the worker asks for a session, and says so now reads, in the same world, a site of the same workspace that everyone reads (made through the console), from its own origin: 200, then the site for members answers 401. Fixed in 8873aa7. With the visibility not read (clientOf), 2 tests of the file still fail.

@sakuga-review sakuga-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 en l'état : la PR ajoute des tests, de la documentation et une règle de visibilité sans modifier le code de production visible ; le point à surveiller est la note acceptée sur un site réservé aux membres, documentée comme choix délibéré.

Niveau lite · anthropic/claude-haiku-5.5 · 0,002 $ · 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

@sakuga-review review heavy

What changed since the heavy review of 444684f (5/5): the branch took the new head of #277 (a merge of main, with FRU-219), and two fixes raised by the lite reviews, in tests only: the Origin assertion of e2e/visitor.spec.ts has no fallback, and the control of one case of visitor.test.ts stays in one PGlite world.

@sakuga-review sakuga-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 : la PR n'ajoute que des tests et de la documentation, sans changer le code du worker ; aucun défaut bloquant n'a été trouvé.

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

Base automatically changed from mathieu/fru-223-dx-pnpm-dev-la-suite-e2e-et-la-demo-tournent-sur-postgresql to main October 13, 2026 17:46
Mheaus and others added 3 commits October 13, 2026 19:46
…th accounts (FRU-224)

Mathieu, 2026-10-13: an account is always possible for whoever administers, and is not needed to
leave a note. The code did it already, and no test held the whole path on a worker whose sites come
from the console: the ones that did ran on FRUITBACK_CLIENTS, which is going.

- `visitor.test.ts`: a site made through the console, a note with no session, no token and no
  cookie, read again, refused from another origin; a site for members answers 401 to a read.
- `e2e/visitor.spec.ts`: the same in a browser, where CORS stops a page of another origin.
- Measured, and written in SECURITY.md: a site for members still takes a note that has no session.
- The token signed by the site is not decided, and nothing here changes it.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…h no fallback (FRU-224)

Raised in review: the assertion fell back to the origin of the page when the header was absent, and
then compared the page with itself.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
… same world (FRU-224)

Raised in review: the case opened a second PGlite world, and a process holds one PGlite database at
a time. The control reads a site of the same workspace that everyone reads.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@Mheaus
Mheaus force-pushed the mathieu/fru-224-worker-une-note-dun-visiteur-sans-compte branch from 8873aa7 to 6e1886a Compare October 13, 2026 17:46

@sakuga-review sakuga-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 · Le changement ne contient que des tests, de la documentation et des décisions déjà documentées ; il peut être fusionné, sous réserve de vérifier les fixtures et le code worker non présents dans la PR.

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

@Mheaus
Mheaus merged commit 0739add into main Oct 13, 2026
13 checks passed
@Mheaus
Mheaus deleted the mathieu/fru-224-worker-une-note-dun-visiteur-sans-compte branch October 13, 2026 20:48
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