Skip to content

Forms (#875), continuing Polle's #1281 - #1923

Merged
joepio merged 93 commits into
developfrom
claude/forms-875
Oct 5, 2026
Merged

joepio merged 93 commits into
developfrom
claude/forms-875

Conversation

@michielbdejong

Copy link
Copy Markdown
Contributor

Supersedes #1281. This is Polle's Forms work (@Polleps), brought up to date with develop and finished for a first Beta. Polle's 71 commits are kept as they are; the commits on top are listed below.

Closes #875

What Forms is

A form or survey you build in a drive and share as a link. Guests fill it in without an account, and each answer becomes a row in a Table.

  • Create: New > Form (Beta), or "Create form from this table" (pick its columns).
  • Builder: 21 question types, pages, drag-to-reorder, "Show when" rules on questions and pages, required fields and limits, Results (the table) and Summary (charts) tabs.
  • Settings: public or invite-only (single-use links), open/close schedule, thank-you message (new here), appearance, custom CSS.
  • Sharing: link, QR code, embed snippet.
  • Guests: a small separate app (browser/form-app on browser/form-renderer) served at /form/{id}, with a self-hosted proof-of-work captcha (ALTCHA), a progress bar, and answers saved on the device until they submit.

Still to do from the #875 wishlist, as follow-up issues: password, requiring an account, redirect after submit, "submit another", seeing who submitted.

Commits on top of Polle's branch

  1. Merge develop (241 commits, then two smaller catch-up merges). Every conflict and how it was resolved is in the merge commit message. The largest ones:
    • lib/src/db.rs: took develop's Db::from_kv().open() and added the fetch counters there.
    • resource.ts: DERIVED_BY_SERVER moved to server-managed-props.ts on develop; the form summary property is now added there.
    • urls.rs: a duplicate EMOJI constant that would not compile.
    • TableResource.tsx: the PR's remote-row rebase is dropped, because develop already shows rows added elsewhere without resetting the session.
    • .dagger/src/index.ts: develop's version, plus the form-app mounts.
    • Lockfiles and .po files: develop's versions, regenerated.
  2. Left out, as unrelated to forms. These can come back as small separate PRs:
    • one machine's notes (AGENTS.local.md, CLAUDE.local.md; still gitignored);
    • the React-compiler agent hooks and their scripts and tests;
    • the pnpm 11 move;
    • doc edits: root CLAUDE.md, CONTRIBUTING port, the wuchale/OpenRouter note, READMEs, the sveltekit template, and two unrelated planning notes.
  3. fix(lib): keep forms working on develop's store.
    • The forms vocabulary is now fetched from the host, like notifications. Its atomicdata.dev URLs 404 on the public catalog.
    • A real data-loss bug in the pending-deps recovery: clearing the save cursor let develop's initLoroSaveCursorIfFresh re-seat it past the unsent edit, so the retry sent nothing. The cursor is now rewound to the empty version instead.
  4. fix(server): bare form ids resolve under develop's atomic: scheme (Emit atomic: identifiers and replace prefix checks with helpers #1585).
  5. feat(forms): thank-you message setting in the builder. The runtime already displayed one.
  6. fix(forms): keep a published form inside its own drive. Security; see below.
  7. Changelog entries under UNRELEASED.
  8. Fixes found by the e2e run:
    • Develop builds UI text from the wuchale catalogs, and strings missing from them render empty. The forms strings are extracted now; every entry added comes from the forms files.
    • "Form" is added to develop's new "Start blank" list (creationCatalog.ts).
    • Option tags under atomic: parents no longer get a path-based subject, which the server refused.
    • Tests follow develop's humanized column headers.
    • Missing imports in filePicker.spec.ts.

Security (commit 6), and why this design

Guests have no agent, so the server reads the definition and signs the row itself. Before this commit, anyone who could edit a Form could:

  • point it at another drive's table, and get rows written there;
  • list another drive's rows as options;
  • serve another drive's private Files through /form/{id}/image.

forms::FormScope sets one rule for all three: a form writes only into its own drive, and shows guests only resources from that drive or ones that are public anyway. Why the drive rather than something narrower:

  • "The table must be the form's parent" is too strict. The AI-assisted flow files a form in any folder next to an existing table.
  • Checking the rights of whoever last edited the form is spoofable: an owner's later title edit would launder an attacker's earlier change.
  • The drive is the boundary that rights and sync already use.

Also in that commit:

  • Questions may not write parent, isA, drive, rights, lastCommit, createdBy or publicKey on the row.
  • The submit rate limit keys on the socket peer (as helpers::peer_ip does), not on X-Forwarded-For, and forgets stale addresses.
  • Minting a form's short slug is serialized, so two first visits can't lose a slug entry or mint two slugs.

Tests

  • Rust: atomic_lib 287 passed. atomic-server --lib 353 passed, including new FormScope tests and a cross-drive submit in form_submission_flow. Clippy with -D warnings is clean.
  • JS unit: @tomic/lib 1085, data-browser 1495, form-renderer 90, form-app 3; typecheck clean in every package.
  • E2E, locally (pnpm test-e2e:local, 2 workers): forms, forms-submission (now also checks the custom thank-you message), forms-from-tables, tables, documents, filePicker and search: 33 of 33 passed. The full suite runs on CI.

Notes for review

  • The forms strings are in the catalogs (English) but not yet translated into German, Spanish or French. Develop already carries about 2,400 untranslated strings, so that belongs in a translation pass of its own.
  • New Rust dependencies: altcha (captcha) and lightningcss (sanitizes custom CSS; an alpha release).
  • Only the TrekMeester merges this; it is ready for a batch once CI is green.

🤖 Generated with Claude Code

Polleps and others added 30 commits September 17, 2026 22:04
The rights walk now consults the per-query verdict memo by subject
before fetching an ancestor, and loads ancestors from materialized
propvals instead of decoding their Loro snapshots. Query loops stop
calling resolve_query_member after 16 consecutive denials while filling
a page, so a fully-private listing no longer fetches every match.

Co-authored-by: Polle Pas <polleps@gmail.com>
Each member can carry its own read grant, so a streak of private
siblings must not skip later readable rows. Keep resolving until the
page is full of authorized hits; the ancestor-fetch memo is what makes
that cheap.

Co-authored-by: Polle Pas <polleps@gmail.com>
FormCondition resources on pages, fields, and layout blocks, evaluated
in document order (AND; empty = always visible). Shared fixtures keep
the TS renderer and Rust submit validator in lockstep. Hidden required
fields are not errors; submitted values for them are dropped.

Co-authored-by: Polle Pas <polleps@gmail.com>
Co-authored-by: Polle Pas <polleps@gmail.com>
The compiler emitted a broken _w_ctx reference, which crashed the form
builder as soon as the page settings pane mounted the editor.

Co-authored-by: Polle Pas <polleps@gmail.com>
Wuchale rewrote "<FaPlus /> Add condition" into W_tx_ with an undefined
_w_ctx_, which crashed the form builder as soon as the page pane mounted.
Wrap the label in a span so it extracts as a plain string.

Co-authored-by: Polle Pas <polleps@gmail.com>
Three independent failures, each masked by the previous one.

1. pnpm 11 aborted every `pnpm run` with
   ERR_PNPM_ABORTED_REMOVE_MODULES_DIR_NO_TTY. Bumping `packageManager`
   to pnpm@11.10.0 brought in a deps-status check that runs before each
   script and re-invokes `pnpm install` itself. That implicit install
   never sees the `--shamefully-hoist` CLI flag, reads the hoisting
   config as changed, and tries to purge node_modules — with no TTY to
   confirm, it aborts. Hoisting moves to PNPM_CONFIG_SHAMEFULLY_HOIST so
   every invocation agrees; the `yes |` prompt hack is no longer needed.
   Kept as an env var rather than `shamefullyHoist` in
   pnpm-workspace.yaml: that file is shared with developers, and
   flattening node_modules locally would hide undeclared dependencies.

2. `handlers/form.rs` include_str!s assets_tmp/form-assets/index.html,
   but copy_form_assets deliberately skips when browser/form-app/dist is
   absent. What build.rs treats as graceful degradation is a hard
   compile error. dist is gitignored *and* in dagger's source-exclude
   list, so no container can ever have it — hence a placeholder in
   build.rs rather than per-container stubs. This also fixes `cargo
   check` on a fresh clone before `pnpm build`.

3. testdata/form-conditions.json is read from both sides — readFileSync
   in form-renderer's conditions.test.ts, include_str! in forms.rs's
   test module — and was mounted in neither container. The Rust one is
   the worse of the two: it fails compilation of every --all-targets
   build, taking out clippy and nextest together.

Verified on a real engine: js-lint, js-test (774 tests),
js-test-integration, rust-clippy and rust-test all green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CI's `pnpm run lint` and `cargo fmt --check` were both failing behind
the pnpm abort fixed in the previous commit, so the drift had gone
unreported. Pure formatting — no behaviour change.

17 JS/TS files via oxfmt (the workspace sets jsxSingleQuote, and the
new form code used double quotes) and 6 Rust files via cargo fmt.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every JS container died ~2 minutes in with

  ERROR  Failed to switch pnpm to v11.10.0. Looks like pnpm CLI is missing
  at "/root/.local/share/pnpm/.tools/@pnpm+linux-x64/11.10.0/bin"

The containers installed `pnpm@latest-10` via corepack (now 10.34.5), and
browser/package.json declares pnpm@11.10.0, so the first pnpm invocation
in the workspace tried to self-switch. pnpm 10's switch downloads the
`@pnpm/<platform>` package and expects the CLI under `<version>/bin`,
which pnpm 11 no longer ships there — the switch is broken across the
10→11 boundary (pnpm/pnpm#12528, still open). It happened to work on the
engine the last CI fix was verified against, presumably a cached older
10.x layer; against a current latest-10 it fails deterministically.

Fix: install the declared version outright. The pnpm 11 npm package is a
self-contained 18MB JS bundle, so `npm install -g pnpm@11.10.0` needs no
corepack and no platform package, and a matching version never triggers a
switch. The version lives in one constant next to a note that it must
track browser/package.json.

The e2e container had the same latent bug — pinned 10.15.1 via
get.pnpm.io, self-switching on its first `pnpm install` — so it takes the
same constant.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@michielbdejong

Copy link
Copy Markdown
Contributor Author

Ready for the next TrekMeester batch: everything passes locally (listed in the description), and I'll post the CI result here once it's green. @joepio, for your trekmeester.

Posted by Claude on behalf of Michiel.

Polle's update from #1281, documenting the ancestor-fetch memo in the
collection rights walk that this PR carries. It was held back with the
unrelated doc edits, but it is about code here.

Co-Authored-By: Polle Pas <polleps@gmail.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@michielbdejong

Copy link
Copy Markdown
Contributor Author

Note from Michiel's coordination session: the German, Spanish and French translations for the new forms UI text still need to be generated. The project's translation flow (pnpm exec wuchale in browser/data-browser) needs OPENROUTER_API_KEY, which the agent that prepared this PR doesn't have. Michiel asked to leave that to @joepio / the trekmeester, who has the key: could you run it as part of batching this PR (or right after)? Until then the forms strings show in English in those languages.

Main pipeline run 36706474093 failed in jsTypecheck: form-app could not
resolve `@tomic/form-renderer` or `altcha`, and pnpm warned that its
node_modules was missing. jsSource() installs only for the package.json
files it copies in first, and form-app and form-renderer were not among
them. The typecheck also never built form-renderer's dist.

Checked against a replica of the container's layout (only the copied
manifests, `pnpm@10 install --frozen-lockfile --shamefully-hoist`, then
the full tree): without this change it gives the same four errors as CI;
with it, the recursive typecheck passes for all ten packages.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X1zLLQwLgyqZt3L9GXCRsW
…875)

Main runs 36706474093 and 36714878089 failed Downstream with "cannot
update the lock file" although atomic-saas#173 (same branch name, open)
carries the lockfile entries. Locally, that branch passes `cargo check
--locked --all-targets` against this commit and atomic-saas main fails
with CI's exact error, so the job built main. The pick step's open-PR
lookup ends in `|| echo 0`, so any failure there (for example a token
without pull-request read access) read as "no open PR" and fell back to
main without a word.

A failed lookup now pairs with the namesake branch and emits a warning
with gh's error. The cargo step also prints the atomic-saas commit, since
the pick step's output falls outside the log tail the API returns.
Checked the step script with a stubbed gh: failed lookup, open PR, no PR.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X1zLLQwLgyqZt3L9GXCRsW
Brings in dacd78d (SearchBoxWindow test stores stay offline), the fix
for the data-browser unit-stage failure that turned develop red.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X1zLLQwLgyqZt3L9GXCRsW
Main pipeline run 36717450608 failed building the e2e server:
build.rs panicked with "Failed to spawn `pnpm install`".

Mounting `browser/form-app/dist` at /code/browser/form-app/dist is what
creates /code/browser in the Rust container. So build.rs no longer takes
its "no browser folder, skip the JS build" path. It finds no
data-browser/dist, but does find its other watched sources (lib/src,
wasm/src), and runs `pnpm install`, which that container lacks.

The JS is already built in the JS container and placed in assets_tmp, so
set ATOMICSERVER_SKIP_JS_BUILD=true there, as the integration and checks
containers already do. build.rs then keeps the mounted assets_tmp and
still refreshes assets_tmp/form-assets from the mounted form-app/dist.
The same container also builds the release binary, which gets the same
fix.

Reproduced locally with the branch laid out like that container (no
browser/ except form-app/dist, a stub assets_tmp, no pnpm on PATH):
`cargo check -p atomic-server` panics exactly as in CI, and with the
variable it passes and embeds the form-app bundle, not the placeholder.
.dagger regression tests pass.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011KbrNpTmoDhqsVVrSxb64R

Copy link
Copy Markdown
Contributor Author

CI status (atomic-server worker, taking over from the Forms session):

Run 36717450608 (head 1853aa2):

  • Downstream: green. It built atomic-saas 6901a3b (Deletion might not re-build index correctly #173), per the new atomic-saas <sha> line. The pick step shows no token warning.
  • Main CI: red. The e2e server build's build.rs panicked with "Failed to spawn pnpm install".

Root cause: this PR mounts browser/form-app/dist into the Rust container at /code/browser/form-app/dist. That mount creates /code/browser, so build.rs no longer takes its "no browser folder, skip the JS build" path. It finds no data-browser/dist, but it does find its other watched sources (lib/src, wasm/src), so it tries to run pnpm, which that container doesn't have.

Fix, 3f91ebd: set ATOMICSERVER_SKIP_JS_BUILD=true on that container, as the integration and checks containers already do. The JS is built in the JS container and placed in assets_tmp. With the flag, build.rs keeps that copy and still refreshes form-assets/ from the mounted form-app/dist. The same container also builds the release binary, which gets the same fix.

Verified locally, with the branch laid out like that container (no browser/ except form-app/dist, a stub assets_tmp, no pnpm on PATH):

  • cargo check -p atomic-server panics exactly as in CI;
  • with the flag it passes and embeds the real form-app bundle, not the placeholder;
  • the .dagger regression tests pass.

Main pipeline re-dispatched on claude/forms-875. atomic-saas#173 still merges only after this lands on develop.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

CI status for run 36751007165 (head 3f91ebd):

This failure is new, and it is not the build.rs problem. The previous fix held: the server build passed and the run reached the data-browser vitest suite. There, 196/196 files and 1500/1500 tests passed. Vitest still failed the step because of one unhandled rejection: ReferenceError: window is not defined in React's dispatchSetState. Vitest attributes it to src/components/forms/SearchBox/SearchBoxWindow.test.tsx. That means a state update arrived after jsdom had been torn down.

I don't think this PR causes it:

  • This PR changes nothing in SearchBox/, @tomic/react hooks it uses, or the Store paths it touches. The lib changes are the pending-deps save-cursor recovery, the merge error guard and the forms vocabulary.
  • The test file already carries develop's fix for this failure class (dacd78d, connect: false stores), so what is left is a timing race under CI load.
  • Locally the file passes on every one of 3 runs.

So I changed no code and re-dispatched Main once: run 36788767633. If it fails the same way again, the next step is to make the test robust, for example by stubbing the store's resource fetches in SearchBoxWindow.test.tsx. That change should go in its own PR against develop, not this one.


Generated by Claude Code

Main run 36788767633 failed `create a form, sync its properties, and
persist across reload` on all three attempts with "Test timeout of
60000ms exceeded", each time at a different step (openSubject back to
the Form, the option-label poll, the third "Add option") and never at
that step's own timeout. The walk does four full page boots (Table,
Form, reload, Table) plus about ten sync waits, and openSubject alone
can take 10s+ on a loaded Dagger shard (forms.spec.ts took 498s there).

Mark it test.slow() like the breadth test above it, and drop that
test's comment claiming the walk does not need it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PzwoGrfrNmVphiKmSEcxGK

Copy link
Copy Markdown
Contributor Author

Re-run 36788767633 failed on something different from the vitest rejection in 36751007165, and this one is the PR's own test.

Failure. forms.spec.ts:227 (create a form, sync its properties, and persist across reload) failed all 3 attempts with Test timeout of 60000ms exceeded, each time at a different step:

  • openSubject back to the Form;
  • the option-label poll (it saw Option 3 because the poll was cut off, not because its 10s ran out);
  • the third "Add option" (5 polls, then the test budget ran out).

None hit the step's own timeout. The walk does four full page boots plus ~10 sync waits, and forms.spec.ts took 498s on that shard. Also flaky but passed on retry: forms.spec.ts:539 (page transitions) and forks.spec.ts:142. The forks one is outside this PR's area.

Fix: 515595a marks the walk test.slow(), like the breadth test in the same file, and removes that test's stale comment saying the walk doesn't need it. e2e lint, format and typecheck pass. I could not reproduce in my container: Playwright's Chromium download is blocked and the preinstalled one couldn't open /app/dev-drive, so the new run is the real check.

New run: https://github.com/ontola/atomic-server/actions/runs/36809778568


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Ready for a batch. Main pipeline run 36809778568 on head 515595a is green: Mancave CI, Downstream (atomic-saas) and Main all passed. It includes the forms.spec.ts:227 timeout fix and the earlier build.rs/pnpm fix.

Merge order: ontola/atomic-saas#173 (the paired Cargo.lock) merges after this lands on develop.


Posted by Michiel's coordinator agent (Claude Code)


Generated by Claude Code

Conflicts resolved:
- useFileObjectUrl: develop split it into useFileObjectUrl(resource) and
  useBlobObjectUrl(blobDid). The SVG fix's mimetype now travels as an optional
  third argument; useFileObjectUrl reads it from the File resource.
- reconcile.test.ts: develop's Response-based mockFetch and
  setManagedDeviceToken(null), plus this branch's VITE_MANAGED_API_BASE stub.
- CHANGELOG entries kept from both sides; catalogs re-extracted.
@joepio

joepio commented Oct 1, 2026

Copy link
Copy Markdown
Member

Trekmeester (for Joep): merged develop into this branch (db31cb7; a merge, no history rewritten). The PR is mergeable again.

Conflicts resolved:

  • browser/react/src/useFileObjectUrl.ts: develop (Batch 2026-09-30: account menu, settings dialogs, share select, document images, onboarding recovery, table quick filter, view tabs, and fixes (13 PRs) #1915, document images) split it into useFileObjectUrl(resource) + useBlobObjectUrl(blobDid, fallbackUrl). This branch's SVG fix (b6441ab) now travels through it: useBlobObjectUrl takes an optional mimetype and types the Blob with it, and useFileObjectUrl reads server.properties.mimetype from the File and passes it on. Document images (ImagePicker) have no File, so they stay untyped as before.
  • reconcile.test.ts: develop's Response-based mockFetch and setManagedDeviceToken(null), plus this branch's VITE_MANAGED_API_BASE stub.
  • browser/CHANGELOG.md: both sides kept. The .po catalogs are re-extracted (2530 entries; de keeps its 12 translations).

Checked locally: data-browser tsc, pnpm run -r lint, and vitest for managed/RTE/FormBuilder (392/392).

Separate design point (Joep): sharing a form needs a server. In SaaS, shareBase comes from store.getServerUrl(), which can be the static app origin, so the link 404s. A workspace without a Cloud Server only gets a generic "Connect to a server" on the Publish toggle. The suggestion is to build the link from the node that hosts the drive, and to nudge toward Cloud Server when there is none.

joepio added 3 commits October 1, 2026 11:17
The share link used the store's server URL, which in the hosted app can be the
shared static origin and gives guests a 404. Use the Cloud Server enrollment's
node, and without any hosting node replace Share and Publish with a nudge:
Cloud Server in the hosted app, a plain pointer to Sync settings elsewhere.
@joepio
joepio merged commit 5264fea into develop Oct 5, 2026
8 checks passed
@joepio
joepio deleted the claude/forms-875 branch October 5, 2026 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Forms / surveys - submit-only view

5 participants