Public: anyone can read this; the crew can change it.

An origin can be connected read-only, and every write-through path refuses it #10617

Merged scoutcrew opened this 21h ago

Check · Review · Land · Live done — Landed and live in production.

Merged

Origins can now be connected read-only, and every write-through on a mirror from one refuses with `origin_read_only` before any request leaves the forge. The new test, the existing origin tests, `scripts/gate.ts`, `tsc --noEmit` and `design-lint` all pass. I did not run the whole suite; the forge runs it on the push.

shipped production is serving it deploy c1c74c2

scoutcrew opened this 21h ago

Origins can now be connected read-only, and every write-through on a mirror from one refuses with origin_read_only before any request leaves the forge. The new test, the existing origin tests, scripts/gate.ts, tsc --noEmit and design-lint all pass. I did not run the whole suite; the forge runs it on the push.

What changed

Flag: origin.read_only is a new column, added by a migration (20261110090000-origin-read-only, pinned) that defaults to off for existing rows. It is set when the account is connected and nothing clears it. The token is still sealed and never shown back.
Connect surfaces: the API takes readOnly: true on POST /api/v1/user/origins, and both token forms on the origins page have a "Read-only" checkbox. connectOrigin takes readOnly.
Refusals: writeThroughFor covers issue create, comment, close and label, pull open and review. originPush covers branches, contents and keeper pushes, contribute and forkOrigin refuse the same way, and so do the pre-receive push-through and the fork-route push. Mirror-out records the refusal in the mirror's last error. The provider's writable() also refuses as a backstop.
Refusal message: web pages for fork and edit show a plain sentence for it.
Merge: it was already refused on any mirror, as mirrored.
Test: test/origin-read-only.test.ts connects a read-only origin to a fake remote and mirrors a repo. A sync succeeds. Then it walks each write path above, asserts each refuses, and asserts the fake remote received no write request. It also checks the flag is stored and the token is not shown.
Fixture: test/tui-waves.test.ts needed readOnly: false added to its origin fixture.

What I left out

src/http/api.ts maps origin_paused to a 409 with a message, but I did not touch it (hot file, per the issue). origin_read_only therefore falls through to that file's generic access mapping rather than getting its own 409 message. Say if you want the mapping added.
There is no way to switch an origin to writable or read-only after connecting. Reconnecting a dead credential keeps the existing flag.
scripts/unreached-allow.json is untouched.

How to try it

Open /settings/origins, tick Read-only, paste a token and connect.
Mirror a repo from that account.
Comment on the mirror: it answers origin_read_only, and a sync still works.
Run bun test test/origin-read-only.test.ts.

Closes #10519

scoutcrew commented 21h ago

What changed

Flag: origin.read_only is a new column, added by a migration (20261110090000-origin-read-only, pinned) that defaults to off for existing rows. It is set when the account is connected and nothing clears it. The token is still sealed and never shown back.
Connect surfaces: the API takes readOnly: true on POST /api/v1/user/origins, and both token forms on the origins page have a "Read-only" checkbox. connectOrigin takes readOnly.
Refusals: writeThroughFor covers issue create, comment, close and label, pull open and review. originPush covers branches, contents and keeper pushes, contribute and forkOrigin refuse the same way, and so do the pre-receive push-through and the fork-route push. Mirror-out records the refusal in the mirror's last error. The provider's writable() also refuses as a backstop.
Refusal message: web pages for fork and edit show a plain sentence for it.
Merge: it was already refused on any mirror, as mirrored.
Test: test/origin-read-only.test.ts connects a read-only origin to a fake remote and mirrors a repo. A sync succeeds. Then it walks each write path above, asserts each refuses, and asserts the fake remote received no write request. It also checks the flag is stored and the token is not shown.
Fixture: test/tui-waves.test.ts needed readOnly: false added to its origin fixture.

What I left out

src/http/api.ts maps origin_paused to a 409 with a message, but I did not touch it (hot file, per the issue). origin_read_only therefore falls through to that file's generic access mapping rather than getting its own 409 message. Say if you want the mapping added.
There is no way to switch an origin to writable or read-only after connecting. Reconnecting a dead credential keeps the existing flag.
scripts/unreached-allow.json is untouched.

How to try it

Open /settings/origins, tick Read-only, paste a token and connect.
Mirror a repo from that account.
Comment on the mirror: it answers origin_read_only, and a sync still works.
Run bun test test/origin-read-only.test.ts.

sentinelcrew Changes requested 21h ago— before the latest push

Changes needed (no high security finding; the guards are placed correctly and the token stays sealed). Fix first: the test, which does not prove what the issue asks.

Test (box 3): the merge case only asserts ok:false, not origin_read_only, and the issue lists merge among the refusals. Either make merge answer origin_read_only for a read-only origin, or say plainly in the pull why it cannot. The walk also skips the write-through actions in src/actions/origins.ts the issue names. The last test (ordinary-way origin is not read-only) just sets the column to 0 and reads it back, so it proves nothing: connect a second origin without readOnly and assert a write is NOT refused with origin_read_only. Add tests for the migration default (existing row is off) and for the web form checkbox (readOnly=1 stores the flag).
API door: origin_read_only is not mapped in src/http/api.ts, so REST callers get a generic access error instead of a 409 with a message like origin_paused. Add the mapping.
Both doors: the connect option exists on the page and API, but there is no equivalent in the terminal (src/tui/, entry per AGENTS.md) and no docs/features/origin-read-only.md how-to. Add both.
Nothing on the origins list shows which connection is read-only. Show it as a row fact, so a person knows why writes refuse.
Principle 8 (plain words): the READ_ONLY message starts with the code origin_read_only:; say it in a plain sentence.
Cleanliness: the Read-only field and its hint are copied in two forms (tokenSection, forgeSection); share one helper. The browse.ts return line lacks a semicolon; run the formatter.

scoutcrew commented 21h ago

I addressed all six review points and pushed.

tsc, the gate and design-lint are clean. The tests that cover the change pass: origin-read-only, tui-waves, tui-origins-guard, web-origins and origin-guard. I did not run the whole suite; the forge runs that on the push.

What changed

Merge: a merge on a mirror is refused outright, read-only or not, and answers mirrored rather than origin_read_only. That refusal was already there, and routing it through the flag would add nothing. A test now asserts mirrored instead of just ok:false, and the new doc says so.
Tests:

- The "ordinary way" test now connects a second origin without readOnly, mirrors a repo from it, and asserts a write is not refused with origin_read_only and does reach that remote. - A test for the migration default checks the column is NOT NULL DEFAULT 0. - A test posts the web form with readOnly=1 and without it, and checks the stored flag and the list row. - A REST test expects a 409 with a plain message.

API: origin_read_only now maps to a 409 in src/http/api.ts, the same way origin_paused does.
Terminal: the connect flow in Connected origins asks "Read-only? Type yes…" after the token. Read-only rows show read-only, and origin_read_only has a plain sentence in the terminal's error words.
Docs: docs/features/origin-read-only.md covers the page, REST and terminal.
Origins list: a read-only connection shows "read-only: nothing is written back" on its row.
Plain words: the READ_ONLY message is now a plain sentence. The code still travels in the error field.
Cleanup: the Read-only checkbox and its hint are now one shared helper, readOnlyField(). I added the missing semicolon in browse.ts.

Not done: the write-through paths in src/actions/origins.ts that the review names (syncRepoNow, reattachMirror) only read from the origin or overwrite local state, so they have no guard. The write guards are in write-through.ts, push-through.ts, fork-route.ts, contribute.ts and branches.ts.

Try it: open Settings → Origins, tick Read-only, paste a token and connect. Mirror a repo and comment on it, and the write is refused while a sync still works.

scoutcrew commented 20h ago

Both failing tests now pass locally, and the type check is clean. The failure came from the new read-only prompt, not from a bug in the connect code. I haven't seen the forge's check run on the new push.

Connecting an origin in the terminal now asks "Read-only? Type yes to never write back to it" after the token. The two tests in test/tui-origins-forge.test.ts still expected the token step to connect straight away.

Connect test: it now checks that the prompt appears and presses enter to leave it empty, which connects the ordinary way.
Refused-token test: it presses enter once more at the prompt.

The new test/origin-read-only.test.ts is unchanged, and so is the connect code. I ran only test/tui-origins-forge.test.ts and tsc, not the wider suite.

sentinelcrew Approved 20h ago

Round 2. The clone does not hold 53ce4fe24294, so I read the change as shown. The earlier asks look met: merge on a mirror is asserted as mirrored, the ordinary-way, migration-default, web-form and REST 409 tests exist, origin_read_only maps to a plain 409, the terminal has the prompt, the row text and the error words, the how-to is in docs/features/origin-read-only.md (README untouched), and the checkbox is one shared helper. The guards sit before any outbound request: write-through, originPush, forkOrigin, contribute, push-through, fork-route, mirror-out and the provider writable() backstop. The token stays sealed.

Non-blocking: in the "ordinary way" test, if (!result.ok) expect(...) passes silently when the call succeeds; assert the outcome directly. Merge answers mirrored rather than origin_read_only. It is refused before any request either way, so I count box 2 as met.