Skip to content

Remove the /version and /all-versions endpoints - #1530

Closed
joepio wants to merge 8 commits into
developfrom
claude/clever-carson-ubt5pe
Closed

joepio wants to merge 8 commits into
developfrom
claude/clever-carson-ubt5pe

Conversation

@joepio

@joepio joepio commented Sep 16, 2026 •

Copy link
Copy Markdown
Member

This PR is the /version + /all-versions removal, awaiting review. It is now the only thing in the diff.

The fork-bar fix that used to share this branch went out in batch #1947, which merged on 2026-10-01. develop carries it verbatim, so this branch's copy auto-merged to a no-op and ForkBar.tsx is no longer in the diff at all. Nothing was reverted or force-pushed to get there; the ordinary develop merge did it. Section 2 below is kept only because the changelog entry for that fix lives here (#1947 added none of its own).


1. Remove /version and /all-versions — the substance of this PR

Resource history is read from the Loro oplog client-side now. routes/History/useVersions.ts calls resource.getLoroHistory() directly and gets signers from /history-attribution — "Instant — no network requests needed, no progress bar," as its own comment puts it. These two endpoints were the older server-side path to the same data.

The only thing still reaching them was one "All versions" link at the bottom of the version scroller (VersionScroller.tsx), which led from a Loro-native history view to a paginated, unattributed rendering of what that view was already showing. Nothing reached /version at all, since the only source of its version ids was /all-versions.

Removed: server/src/plugins/versioning.rs and its two registrations in appstate.rs; history::encode_version_id / history::decode_version_id; the versionsURL helper, the allVersions path entries and the AllVersionsLink block (VersionScroller no longer needs a subject prop, so both call sites drop it); the /all-versions round-trip test.

Kept: lib/src/history.rs is already Loro-backed, and versions() / at_version() are live — the WASM and Flutter bridges read history through Resource::get_history and Resource::view_at.

Public API note, and the reason this waits for review: dropping those two pub fns is a breaking change to the published atomic_lib surface, on a beta version, for functions whose only caller was the deleted endpoint. Worth a second opinion on whether the HTTP endpoints should stay as a server-side API for consumers that do not want the full SDK — that is a product call, not a code one, and it is the one open question here.

planning/commit-retention-and-state-certificates.md has an unticked "Retire server/src/plugins/versioning.rs" checkbox that this change would tick. Left alone deliberately, since ticking it presumes the answer to that question.

One thing to check before this is batched: the Downstream job

AGENT_WORKFLOW.md notes that main.yml's Downstream job builds ontola/atomic-saas against this commit with cargo check --locked, and that a change breaking an API atomic-saas uses needs a same-named atomic-saas branch with an open PR.

This PR removes two pub fns from atomic_lib: encode_version_id and decode_version_id. No caller remains in this repository. I have not been able to confirm whether atomic-saas calls either one — that repo is outside this session's scope. If it does, Downstream will go red for a real reason attributable to this diff, and a paired claude/clever-carson-ubt5pe branch on atomic-saas would be needed. Worth one grep by someone who can see it. Both functions were pure version-id codec helpers for the deleted endpoint, so the chance is low, but it is not zero and it is not something I can rule out from here.

2. The fork-bar fix — shipped in #1947, no longer in this diff

Recorded here because the changelog entry for it is in this PR and nowhere else.

ForkBar renders above every resource view — that is its job, to say you are looking at a fork. For a non-fork it returns null, but React runs the hooks first, so useResource(original ?? '') fired on every resource page. An empty subject resolves against the page origin, and nothing serves a resource there: AppState::init deliberately creates no Drive at /, so handle_not_found answers 404 (lib/src/storelike.rs). Every page load spent a request on that and logged a console error. It asks for unknownSubject instead, which Store.getResourceLoading short-circuits to an in-memory placeholder without any fetch.

It also repaired the offline-persistence and offline-tables e2e specs, which were failing on develop too. The fetch happened online as well — it only failed those specs because they reload with the WebSocket disconnected, and over HTTP the miss becomes a logged 404 rather than a silent protocol answer. The specs assert that a test produces no unexpected browser console errors.

Checklist

  • Add changelog entry linking to issue, describe API changes — root CHANGELOG.md (server + atomic_lib API) and browser/CHANGELOG.md (both the removed link and the fork-bar fix)
  • Add or update tests if needed — removed the round-trip test covering the deleted endpoints; the fork-bar fix was covered by the two existing e2e specs it repaired
  • Update docs if needed — docs/src/interoperability/vc-old.md pointed at /all-versions for finding who signed a version; /history-attribution answers exactly that, so it points there now

CI status

This branch has no CI run, and it needs one dispatched. main.yml runs on pushes to develop, v* tags and workflow_dispatch only, since #1661 (2026-09-23). The previous head c5825ff predates that commit and still carried a bare on: push:, which is why it got runs. Confirmed for the current head: zero Actions runs, and the combined status is pending with no statuses. Only GitGuardian, a third-party App check unaffected by the change, has reported, and it passed.

AGENT_WORKFLOW.md gives the remedy — gh workflow run main.yml --ref <branch>. I attempted that dispatch and this session's permissions blocked it as a shared-resource change, so it still needs doing, either by someone with the access or by granting it here. An earlier version of this section said dispatching was a maintainer's call rather than something to trigger from a PR; that was my misreading of main.yml's batch-oriented comment, and AGENT_WORKFLOW.md is explicit that dispatching the branch is the normal way to test it.

For the record, on the last head that did get CI (c5825ff), one check was red and it was not this branch's code: chatroom @smoke, an e2e spec this diff touches nothing near (8 of 9 pass on the shard). It is a develop bug in browser/lib's collection layer, hit independently by #1568 on a different branch at a different point in the same spec. Both halves of the mechanism are still present on develop, so it would recur.

Two earlier failures are resolved: the convertFileToDocument vitest timeout was red on develop itself and fixed there in 042f1ea; the atomic-saas Downstream job was a flake, green four times over on the same unchanged saas commit. That flake is separate from the API risk noted above.

Full diagnosis is in the status comment below.

A note on the develop merge in this branch

a498ee6 merges develop at 264b543. It resolved a genuine conflict: the PR had become un-mergeable and both sides had appended to UNRELEASED in browser/CHANGELOG.md. Both sets of entries are kept, and that file was the only conflict.

Flagging it because AGENT_WORKFLOW.md says the trekmeester rebases stale PRs itself and asks contributors not to merge develop in just to keep a PR fresh (#1869). This merge was conflict-driven rather than freshness-driven, but if you would rather this branch carried no develop merges at all, say so and I will leave future conflicts to you. There is no conflict against the current develop (723b401).

Verification

Since CI has not run on this head, the local run is the only evidence. All of it against the merged tree at a498ee6:

  • cargo test -p atomic_lib --features db-redb --lib — 660 passed, 0 failed
  • cargo test -p atomic-server --lib — 271 passed, 1 failed; the one failure, plugins::triggers::tests::a_hard_restart_delivers_a_saved_arrival_once, passes on its own and is a timing-sensitive test in a develop module this diff does not touch, failing under full-suite CPU contention on this machine
  • cargo fmt --all --check — exit 0
  • cargo check clean for atomic_lib and atomic-server (with ATOMICSERVER_SKIP_JS_BUILD=true; build.rs otherwise shells out to pnpm run build)
  • The pre-commit hook's own cargo clippy --workspace --exclude atomic-server-tauri --no-deps --all-targets --no-default-features --features light -- -D warnings — exit 0

Corroborating the wasm32-wasip2 point AGENT_WORKFLOW.md makes about the Dagger containers: the same gap hits a local run. 54 of the atomic-server tests fail with no useful message unless that target is installed, because build.rs cannot build the server-side plugin runtime and says so only in a build warning. rustup target add wasm32-wasip2 fixes all 54.

No JS-side check has been run against this merge: the browser toolchain is not runnable in this environment (pnpm install 403s on the devonian tarball through the egress proxy). Earlier in this PR's history, on a working toolchain, pnpm test-e2e:local offline-persistence passed, both offline specs passed where they failed on develop, and data-browser typecheck and lint were clean.

The commits on this branch used --no-verify, which AGENT_WORKFLOW.md now forbids outright. The hook cannot run in this environment: it snapshots the tree with git checkout-index, so the gitignored dist and assets_tmp are absent and server/build.rs attempts a JS/WASM build needing blocked network access. I ran the hook's clippy invocation directly against the working tree, plus cargo fmt --all --check, which the hook does not do. Flagging it rather than leaving it implicit.

🤖 Generated with Claude Code

https://claude.ai/code/session_01YUL3EPFVmaDAok34JPev3F

joepio commented Sep 17, 2026 •

Copy link
Copy Markdown
Member Author

CI status log for this branch. Kept as one comment rather than a thread; updated in place.

Where it stands on a498ee6

a498ee6 merges develop at 264b543. It resolves a real merge conflict: both
sides had appended entries to UNRELEASED in browser/CHANGELOG.md. Both sets
are kept and that file was the only conflict.

This head has no CI run, and that is the current policy, not a failure. The
merge brought in #1661 (2026-09-23), which set main.yml to on: push: branches: [develop]: "Feature-branch pushes do not trigger CI. Dispatch full
validation explicitly on a temporary branch containing the exact PR heads to
test together." The previous head c5825ff predates that commit and still
carried on: push: for every branch, which is why it got a run. Only
GitGuardian (a third-party App check, unaffected) has reported on a498ee6, and
it passed.

So the required Main status is simply absent here, and pushing again cannot
produce one.

Correction to what this comment said yesterday. It claimed dispatching a run
was a maintainer's call rather than something to trigger from a PR. That was my
misreading of main.yml's batch-oriented comment. AGENT_WORKFLOW.md, which
AGENTS.md now points to, is explicit about the per-PR case: "PR branches get
no CI by themselves... To test a PR branch, dispatch it: gh workflow run main.yml --ref <branch>." Dispatching is the normal route, not an escalation.

I attempted that dispatch and this session's permissions blocked it as a
shared-resource change, so it still needs doing, either by someone with the
access or by granting it here. I have not tried to route around the block.

Also worth a grep by someone who can see atomic-saas: this PR removes
encode_version_id and decode_version_id from atomic_lib's public surface,
and main.yml's Downstream job runs cargo check --locked on atomic-saas
against this commit. If atomic-saas calls either, that job goes red for a real
reason attributable to this diff and a same-named atomic-saas branch with an
open PR would be needed. Both were codec helpers for the deleted endpoint, so
the chance is low, but I cannot rule it out from here.

The table below is therefore the last reported CI state, from c5825ff, and no
longer describes the current head.

Two things changed with this merge, neither of them to this branch's own code:

Check (on c5825ff) Result
data-browser test (incl. convertFileToDocument) ✅ resolved - fix ported from develop
Downstream (atomic-saas) ✅ green (22:59Z)
check, GitGuardian, Pick runner ✅
Main (Mancave) / CI ❌ one spec: chatroom @smoke (8 passed, 1 failed)

One red check remained, and it is not this PR's code.


Resolved: convertFileToDocument — fix ported from develop

develop fixed it in 042f1ea (#1573): test(data-browser): give the Markdown schema suite its own timeout, raising the budget to 60s for a suite whose lazily-imported collaborative Markdown schema exceeded vitest's 5s default whenever the runner was oversubscribed. Rather than wait for it, I merged origin/develop (3598fa9) into this branch as c5825ff.

It worked: data-browser test passed on c5825ff and the run reached the e2e stage for the first time since the timeout appeared. This had been red on the base branch (develop @ 5cca8a1), on this PR and on #1573; it is now fixed at the source.

Resolved: Downstream (atomic-saas) — was a flake

Green again on c5825ff. Four results now on the same unchanged atomic-saas main (3814a23): green here, green on the earlier re-run (19:42Z), green on develop (17:38Z), green on #1573 (14:52Z) — against a single failure.

Why it flaked. The test asserts five audit lines and captured three. The three that survived are outcome="rate_limited", "unauthorized" and "failed" — all warn!; the two missing are both outcome="ok", the only two from recovery_audit_ok's info! (src/main.rs:2217). It installs its capture subscriber with thread-local tracing::subscriber::set_default while tracing caches callsite interest process-wide across 446 tests, so whichever thread reaches that callsite first decides whether it is recorded.

It will flake again until atomic-saas uses one capture subscriber for the whole test binary (a OnceLock fan-out whose writer is swapped per test) or marks the log-reading tests serial. Say the word and I will open that PR.


Still red: chatroom @smoke — a develop bug in browser/lib

Unchanged on c5825ff: both the first attempt and Playwright's retry fail at e2e.spec.ts:423, the invited agent's context, with the same snapshot — write access on the chatroom and an empty message list.

  421 |       // Redirect may land outside the chatroom; open the same /app/show?subject=… URL as the owner.
  422 |       await page2.goto(chatRoomHref);
> 423 |       await expect(page2.locator(`text=${teststring}`)).toBeVisible({

(The # Test source window in error-context.md shows lines 323–359 and reads as if the failure were at line 328. That is a truncation artifact of the CI log dump — the authoritative marker is the > line in the shard stdout, which says 423 on every attempt.)

What c5825ff rules out

The merge brought 042f1ea, which reworks exactly this area: destroyed-subject tombstones move from per-instance caches onto the store, Collection and useChildren read Store.isDestroyed instead of their own sets, and applyResourceChange no longer treats any matching resource event as a restore. The failure is byte-identical after it. The tombstone/resurrection path is not the cause.

What is established

Collection.fetchPage consults the local WASM DB and returns on a hit, before the server is asked:

if (hasClientDb && (await this.fetchPageFromLocalDb(page)) === 'ok') return;

finishLocalDbPage returns 'ok' for a zero-member result whenever hasCompletedDriveSyncFor(drive) is true, writing setEmptyPage(page) and treating it as authoritative. Any zero from the local side becomes a trusted empty, and the server /query holding the message is never issued.

Two earlier conclusions of mine, corrected along the way:

  1. The wrong-drive fallback is not the mechanism. const drive = this.params.drive ?? this.store.getDrive(); resolves to the invited agent's own drive rather than the chatroom's, which explains this branch's line-423 failure — but not PR Don't delete a local database just because this page load can't open it #1568's at line 328, where the owner's current drive is the chatroom's. One route to a trusted empty, not the cause.
  2. The Rust query index is ruled out, by test. Don't delete a local database just because this page load can't open it #1568 reproduced the exact query shape against Db (property=parent, filters=[isA=Message], sort_by=createdAt, drive-scoped, watched while empty, then two members added — one with createdAt, one without): sorted=2 unsorted=2, check_query_index reports indexed: 2, expected: 2, missing: [], stale: [], and is_a_encodings_all_match_the_class_constraint passes. A server-side reproduction is uninformative anyway — the server is never asked on this path.

Still open

The optimistic append's matcher. applyResourceChange (browser/lib/src/collection.ts:575) gates membership on constraintMatches → valueMatches, which bottoms out at actual === query. It handles isA as a real array or as a scalar equal to the class URL, and has no handling for the JSON-array-string encoding ('["…/Message"]') that 859af20 taught the Rust side to accept — the strictest of the three matchers, and one 042f1ea did not touch. That is a hypothesis, not a result: the workspace here has no vitest binary (pnpm install 403s on the devonian tarball through the egress proxy), so I cannot test it. The other candidate gates with identical symptoms are if (resource.new) return 'unchanged' and _assemblingPage.

On the develop baseline

Earlier I wrote that no develop baseline for this spec could exist until the convertFileToDocument timeout was fixed. That is now out of date: develop @ 59b3c008 reached e2e and ran it for 46 minutes. Its failures include several plugins.spec.ts specs timing out on the New plugin heading — which #1579 has since fixed on develop. Whether chatroom @smoke is also among that run's failures I have not established, having read only part of a 61 KB log; develop has since moved to 71d36bf carrying the plugin fix, and its next run should give a much cleaner comparison.

That comparison is confirmatory rather than decisive, though: 2ec8784/c5825ff are a head that passed a full 21-minute suite plus develop merges with no branch code changed in between, and two independent branches (#1530 and #1568) fail this spec in two different places.

Proposed patch

Part 1 stands whichever gate turns out to be the trigger:

  1. Stop treating a zero-member local result as authoritative unless the completed sync is for the drive the queried parent lives in: read https://atomicdata.dev/properties/drive off the resource named by this.params.value rather than falling back to store.getDrive(), and return 'no-db' when that drive is unknown or unsynced so the server /query answers. Regression test belongs in browser/lib/src/collection-empty-trust.test.ts.
  2. Whatever keeps a just-written message out of the optimistic append needs its own fix; (1) turns a wrong answer into a slower correct one, which is the safe direction.

Not pushed here. None of it is in this PR's diff — 14 files: the two changelogs, the /version + /all-versions removal, and the one-line ForkBar fix, nothing touching chat, collections, the query index, rights or sync. It belongs in its own PR against develop. Say the word and I will open it.


Earlier failures on this branch, for the record

Head Failure Verdict
5b10dcb, b1cedf4 spawn EIO in docker/login-action@v3 Runner could not start docker. Cleared on its own.
4bd6321 none — full 21-minute suite passed ✅ Run 35278746431
2723420 ×2 start engine: context deadline exceeded Dagger engine would not start. develop's own changelog documents the same wedging.
675f696 convertFileToDocument timeout Fixed at source; see above.
2ec8784 ×2 chatroom @smoke, Downstream Both carried forward above.

Verified locally

cargo test -p atomic-server --lib 84 passed · cargo test -p atomic_lib --features db-redb --lib 543 passed · cargo check clean for both crates after every develop merge (with ATOMICSERVER_SKIP_JS_BUILD=true, since build.rs otherwise shells out to pnpm run build) · pre-commit clippy invocation exit 0.

The browser toolchain is no longer runnable in this environment, so the e2e/typecheck rows from earlier in this PR's history predate that and cannot currently be re-run. The chatroom analysis above is a code reading, not a bisect.

develop was merged in five times as it advanced (ec22e34, b0c604c, 6d55826, 3598fa9, 264b543); only the UNRELEASED changelog sections ever conflicted, and both sides' entries were kept each time.


Generated by Claude Code

@joepio joepio changed the title Remove the /version and /all-versions endpoints Remove the /version and /all-versions endpoints; stop the fork bar fetching the server root Sep 17, 2026
Resource history is read from the Loro oplog client-side now. The History
route (`routes/History/`) calls `resource.getLoroHistory()` directly and
gets signers from `/history-attribution`; no network round trip, no
paging. These two endpoints were the older server-side path to the same
data, and the only thing left reaching them was one "All versions" link
at the bottom of the version scroller, which led from that view to a
worse rendering of what it was already showing.

`lib/src/history.rs` itself stays -- it is already Loro-backed, and
`versions()` / `at_version()` are what the WASM and Flutter bridges read
through `Resource::get_history` and `Resource::view_at`. Only
`encode_version_id` / `decode_version_id` go: they existed to carry an
opaque VersionID through an HTTP query parameter, which nothing does now.
That is a breaking change to the published `atomic_lib` surface, on a
beta version, for two functions whose only caller was the deleted
endpoint.

The archived verifiable-credentials doc pointed at `/all-versions` for
finding who signed a version; `/history-attribution` answers exactly that
question, so it now points there.

Verified: `cargo test -p atomic-server --lib` (84 passed),
`cargo test -p atomic_lib --features db-redb --lib` (543 passed),
data-browser typecheck + lint, and the pre-commit hook's own
`cargo clippy --workspace --exclude atomic-server-tauri --no-deps
--all-targets --no-default-features --features light -- -D warnings`
(exit 0). The hook itself could not run here: it snapshots via
`git checkout-index`, so the gitignored `dist` / `assets_tmp` are absent
and `server/build.rs` attempts a network JS build.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YUL3EPFVmaDAok34JPev3F
`ForkBar` renders above every resource view -- that is its job, to say you
are looking at a fork rather than the original. For anything that is not a
fork it returns null, but React runs the hooks first, so
`useResource(original ?? '')` fired on every resource page. An empty
subject resolves against the page origin, and nothing serves a resource
there: `AppState::init` deliberately creates no Drive at `/`, so
`handle_not_found` answers 404 (lib/src/storelike.rs). Every page load
spent a request on that and logged a console error.

It now asks for `unknownSubject`, which `Store.getResourceLoading`
short-circuits to an in-memory placeholder without any fetch. Only the
non-fork path changes, and that path renders nothing; a real fork has
`originalSubject` set and behaves as before.

This is what failed the `offline-persistence` and `offline-tables` e2e
specs. Both assert that a test produces no unexpected browser console
errors, and both reload with the WebSocket disconnected -- which is the
only reason the miss was visible: over WS it is a protocol answer with
nothing logged, over HTTP it is a 404 the browser reports. The fetch was
happening online too, just silently.

Verified by reproducing the CI failure locally and re-running against it:
`pnpm test-e2e:local offline-persistence` and then both specs together,
2 passed. Typecheck, lint and format-check clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YUL3EPFVmaDAok34JPev3F
@joepio
joepio force-pushed the claude/clever-carson-ubt5pe branch from b1cedf4 to 4bd6321 Compare September 17, 2026 21:48
…on-ubt5pe

# Conflicts:
#	CHANGELOG.md
#	browser/CHANGELOG.md
joepio added a commit that referenced this pull request Sep 30, 2026
Split out of #1530, which also removes the /version endpoints and waits for
review. This part is a safe fix on its own: useResource('') resolved against
the page origin, so every page fetched / and got a 404.
@joepio

joepio commented Sep 30, 2026

Copy link
Copy Markdown
Member Author

Trekmeester: the ForkBar fix (useResource(original ?? unknownSubject)) is split into batch #1947, since it's safe on its own and stops a 404 on every page. Removing /version and /all-versions plus the encode/decode_version_id public API is hard to undo, so it waits for review.

joepio added a commit that referenced this pull request Oct 1, 2026
… e2e traces, Android menus, demo tooling, docs (#1947)

Includes #1884, #1930, #1940, #1888, #1933, #1882, #1834, #1631, #1932, #1945 and the ForkBar part of #1530.
develop carries the fork-bar fix now (#1947), so this branch's copy of it
auto-merged to a no-op and the diff is just the endpoint removal.

browser/CHANGELOG.md was the only conflict: both sides appended to
UNRELEASED. Kept both, develop's table-row-comments entry and this
branch's two. develop has no fork-bar entry of its own, so the one here
is still the only record of that fix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YUL3EPFVmaDAok34JPev3F
@joepio joepio changed the title Remove the /version and /all-versions endpoints; stop the fork bar fetching the server root Remove the /version and /all-versions endpoints Oct 3, 2026
joepio added a commit that referenced this pull request Oct 5, 2026
… chat, search, CI) (#2060)

Combines #1530, #1638, #1663, #1835, #1869, #1896, #1898, #1911, #1912, #1913, #1914, #1918, #1919, #1929, #1931, #1949, #1958, #1959, #1961, #1963, #1968, #1975, #1981, #1985, #1988, #2038, #2041, #2047 and #2050, tested in one full Mancave run on the exact head (run 4999, green).

joepio commented Oct 5, 2026

Copy link
Copy Markdown
Member Author

Landed on develop in the combined batch #2060 (95707c0), tested together in one full run. Closing as included.


Generated by Claude Code

@joepio joepio closed this Oct 5, 2026
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.

2 participants