Built a small private link shortener: a form turns a long http/https… #1

Merged

cmoriarty merged 3 commits from osf/66-shortcut-a-link-shortener-combined into main 2026-10-16 16:48:38 -04:00

Conversation 0 Commits 3 Files changed 64 +4297 -1

cmoriarty commented 2026-10-16 13:08:00 -04:00

Owner

Copy link

What was done

• Built a small private link shortener: a form turns a long http/https URL (up to 2,000 characters) into a 6-character code, a visit to /<code> redirects (302) to the stored URL, and an unknown or differently-cased code shows a 404 page.
• A value that is not a URL, is not http/https, or is over 2,000 characters is refused with a message saying why, and the rejected value is kept in the form.
• Each visit to a short link atomically increments its click count, and a Links page lists every stored link newest first with its code, URL, and click count.
• Links and counts live in a Postgres table, created on start if missing, behind a plain Node HTTP server (no web framework) that boots with a start script in package.json.
• Added unit tests (vitest) over storage, validation, page rendering, and the full HTTP behaviour, plus browser tests (playwright, chromium) over the shorten, follow, and Links-page flows.
• Satisfied the CI surface: CHANGELOG.md gained an entry naming this change for the release-notes job, and the unit and browser test lanes were wired into the existing workflow.
• Braid ran the unit and browser test lanes itself after the implementation and after each fix round, and a judge opened the running app in a browser as a person would.
• Not done: the judge found the Links page unreadable once a long URL is stored — a 2,000-character URL is neither wrapped nor truncated, so the table stretches to roughly 17,000px wide and a person in a normal window cannot read any link's click count.

How to review

1. Get the branch: git fetch origin && git checkout osf/66-shortcut-a-link-shortener-combined, then git diff origin/main --stat to see it add the server, pages, storage, tests, and config.
2. Set DATABASE_URL to a reachable Postgres before any test or the app (export DATABASE_URL=postgres://app:app@127.0.0.1:5432/app); scripts/services.mjs can boot an embedded one if you have none.
3. Run the unit tests: npm install then npm test — expect all to pass.
4. Run the browser tests: npx playwright install chromium once, then npm run test:e2e — expect all to pass (it starts the server itself).
5. See it working: npm start (listens on PORT, default 3000), open /, shorten a long URL, follow the /<code> link, and view /links — expect the 6-character code with a working Copy button, a 302 to the exact stored URL, a 404 for a bogus code, and the newest-first list; to see the known gap, shorten a 2,000-character URL and open /links to find the URL column overflowing the window.
6. Read first: src/server.ts for the routes and src/db.ts for storage and the click count.

Code review

Reviewed the full Shortcut link-shortener change (src/, unit and e2e tests, configs, CHANGELOG/README, CI surface) against its proposal, delta specs, design and tasks. Found and fixed three defects: a strict-mode tsc failure in test/link-storage.test.ts (broken address() destructuring, fixing it restored the task 2.3/3.2 typecheck), a nondeterministic race where concurrent CREATE TABLE IF NOT EXISTS on a fresh database failed npm test (reproduced 150/150 with 8 concurrent pre-warmed sessions; fixed by re-running the no-op statement on the race's catalog-conflict codes, then 0/150 and the full suite passed), and a never-sorted expected column list in test/db.test.ts that always fails once the DDL runs. Left three minor notes unfixed as documented: the spec's wording for a bare non-URL message vs the tasks-pinned 'not a valid URL', the uncapped POST body in readBody (design explicitly excludes scale engineering), and the proposal's file list omitting files its own tasks create. All 58 unit tests pass and npx tsc --noEmit is clean.

Left for you

• minor, src/validate.ts:14: The link-shortening spec scenario 'Value that is not a URL is refused' (e.g. bare example.com) asks for a message stating the URL 'must be http or https', but the implementation rejects with 'not a valid URL' — the mapping design decision 7 and task 2.1 explicitly pin, and which unit (test/validate.test.ts:11), HTTP (test/server.test.ts:115) and e2e (e2e/shorten.spec.ts:101) tests assert. The requirement's core (refused, inline reason, no short link) is met; reconciling the wording would contradict the tasks, so left as is (also flagged as a note by the prior review round).
• minor, src/server.ts:11: readBody buffers the entire POST body before validation, so a multi-megabyte body is held in memory even though the url field itself is capped at 2,000 characters. The design's Non-Goals explicitly exclude availability/scale engineering for a one-team private app, so left as is.
• minor, openspec/changes/build-shortcut-a-small-link-shortener-fo-d2hmzy/proposal.md: The proposal's What Changes/Impact file list omits files its own tasks create: CHANGELOG.md (task 3.3), the start script (task 1.1), tsconfig.json and vitest.config.ts (task 1.2). Documentation-only gap; the change's docs are out of scope for this code review, so left as is (also flagged as a note by the prior review round).

Fixed before the commit

• blocking, test/link-storage.test.ts:18: const { port } = s.address() fails npx tsc --noEmit (TS2339: address() is string | AddressInfo | null in strict mode), so the commit broke the typecheck that tasks 2.3 and 3.2 require to pass. Fixed with the same narrowing pattern already used by listen() in src/server.ts:94-95.
• blocking, src/db.ts:31: Concurrent createTables calls on a fresh database (parallel vitest workers, CI's fresh postgres container, overlapping app starts) race in the system catalogs: the loser gets duplicate key value violates unique constraint pg_type_typname_nsp_index (23505) or relation/type already exists (42P07/42P04), making npm test fail nondeterministically (observed on 2 of 4 runs; reproduced 150/150 with 8 concurrent pre-warmed sessions before the fix). Fixed: on those codes the IF NOT EXISTS statement is re-run once (safe: the conflicting creator has committed, so the re-run is a no-op); other errors are re-thrown. After the fix the repro is 0/150 and the full 58-test suite passes.
• major, test/db.test.ts:46: The expected column list ['code', 'clicks', 'created_at', 'id', 'url'] is not in sorted order ('clicks' < 'code'), while the received list is .sort()ed, so this assertion can never pass; it was previously masked by the createTables race failing the same test earlier. Fixed the literal to the actual sort order ['clicks', 'code', 'created_at', 'id', 'url']; the column set asserted is unchanged.

Tests

All 31 scenarios of build-shortcut-a-small-link-shortener-fo-d2hmzy have a tagged test.

The test lint found, in the run's tests on the last check:

Recorded, not decided

• e2e/follow.spec.ts:1 B7: no relative import: this file asserts on things it constructed itself, which is the measured 'asserted on my own mock' failure
• e2e/links.spec.ts:1 B7: no relative import: this file asserts on things it constructed itself, which is the measured 'asserted on my own mock' failure
• e2e/shorten.spec.ts:1 B7: no relative import: this file asserts on things it constructed itself, which is the measured 'asserted on my own mock' failure
• test/click-tracking.test.ts:1 B7: nothing imported here looks like the module under test ('click-tracking')
• test/clicks.test.ts:1 B7: nothing imported here looks like the module under test ('clicks')
• test/link-storage.test.ts:1 B7: nothing imported here looks like the module under test ('link-storage')
• test/list.test.ts:1 B7: nothing imported here looks like the module under test ('list')
• test/shorten.test.ts:1 B7: nothing imported here looks like the module under test ('shorten')

Braid run

• Workflow: Braid's OpenSpec flow, change build-shortcut-a-small-link-shortener-fo-d2hmzy (42 steps, 2 retried)
• Model: vLLM · qwen3.8-27b
• Time: 6 h 2 m from start to this pull request, 3 h 32 m of it in the model
• Tokens: 17.5M input, 108k output, 471k reasoning, over 351 model calls
• Pre-flight estimate: fits, p50 119k / p80 184k tokens against a 394k-token step allowance
• CI: passed its 6 jobs on a31512e208
• Run: run_01M52JT93Q9X1VH3GD05D2HMZY

Opened by Braid.

## What was done - Built a small private link shortener: a form turns a long http/https URL (up to 2,000 characters) into a 6-character code, a visit to `/<code>` redirects (302) to the stored URL, and an unknown or differently-cased code shows a 404 page. - A value that is not a URL, is not http/https, or is over 2,000 characters is refused with a message saying why, and the rejected value is kept in the form. - Each visit to a short link atomically increments its click count, and a Links page lists every stored link newest first with its code, URL, and click count. - Links and counts live in a Postgres table, created on start if missing, behind a plain Node HTTP server (no web framework) that boots with a `start` script in `package.json`. - Added unit tests (`vitest`) over storage, validation, page rendering, and the full HTTP behaviour, plus browser tests (`playwright`, chromium) over the shorten, follow, and Links-page flows. - Satisfied the CI surface: `CHANGELOG.md` gained an entry naming this change for the release-notes job, and the unit and browser test lanes were wired into the existing workflow. - Braid ran the unit and browser test lanes itself after the implementation and after each fix round, and a judge opened the running app in a browser as a person would. - **Not done:** the judge found the Links page unreadable once a long URL is stored — a 2,000-character URL is neither wrapped nor truncated, so the table stretches to roughly 17,000px wide and a person in a normal window cannot read any link's click count. ## How to review 1. Get the branch: `git fetch origin && git checkout osf/66-shortcut-a-link-shortener-combined`, then `git diff origin/main --stat` to see it add the server, pages, storage, tests, and config. 2. Set `DATABASE_URL` to a reachable Postgres before any test or the app (`export DATABASE_URL=postgres://app:app@127.0.0.1:5432/app`); `scripts/services.mjs` can boot an embedded one if you have none. 3. Run the unit tests: `npm install` then `npm test` — expect all to pass. 4. Run the browser tests: `npx playwright install chromium` once, then `npm run test:e2e` — expect all to pass (it starts the server itself). 5. See it working: `npm start` (listens on `PORT`, default 3000), open `/`, shorten a long URL, follow the `/<code>` link, and view `/links` — expect the 6-character code with a working Copy button, a 302 to the exact stored URL, a 404 for a bogus code, and the newest-first list; to see the known gap, shorten a 2,000-character URL and open `/links` to find the URL column overflowing the window. 6. Read first: `src/server.ts` for the routes and `src/db.ts` for storage and the click count. ## Code review Reviewed the full Shortcut link-shortener change (src/, unit and e2e tests, configs, CHANGELOG/README, CI surface) against its proposal, delta specs, design and tasks. Found and fixed three defects: a strict-mode tsc failure in test/link-storage.test.ts (broken `address()` destructuring, fixing it restored the task 2.3/3.2 typecheck), a nondeterministic race where concurrent `CREATE TABLE IF NOT EXISTS` on a fresh database failed `npm test` (reproduced 150/150 with 8 concurrent pre-warmed sessions; fixed by re-running the no-op statement on the race's catalog-conflict codes, then 0/150 and the full suite passed), and a never-sorted expected column list in test/db.test.ts that always fails once the DDL runs. Left three minor notes unfixed as documented: the spec's wording for a bare non-URL message vs the tasks-pinned 'not a valid URL', the uncapped POST body in readBody (design explicitly excludes scale engineering), and the proposal's file list omitting files its own tasks create. All 58 unit tests pass and `npx tsc --noEmit` is clean. **Left for you** - minor, `src/validate.ts:14`: The link-shortening spec scenario 'Value that is not a URL is refused' (e.g. bare `example.com`) asks for a message stating the URL 'must be http or https', but the implementation rejects with 'not a valid URL' — the mapping design decision 7 and task 2.1 explicitly pin, and which unit (test/validate.test.ts:11), HTTP (test/server.test.ts:115) and e2e (e2e/shorten.spec.ts:101) tests assert. The requirement's core (refused, inline reason, no short link) is met; reconciling the wording would contradict the tasks, so left as is (also flagged as a note by the prior review round). - minor, `src/server.ts:11`: readBody buffers the entire POST body before validation, so a multi-megabyte body is held in memory even though the url field itself is capped at 2,000 characters. The design's Non-Goals explicitly exclude availability/scale engineering for a one-team private app, so left as is. - minor, `openspec/changes/build-shortcut-a-small-link-shortener-fo-d2hmzy/proposal.md`: The proposal's What Changes/Impact file list omits files its own tasks create: CHANGELOG.md (task 3.3), the `start` script (task 1.1), tsconfig.json and vitest.config.ts (task 1.2). Documentation-only gap; the change's docs are out of scope for this code review, so left as is (also flagged as a note by the prior review round). **Fixed before the commit** - blocking, `test/link-storage.test.ts:18`: `const { port } = s.address()` fails `npx tsc --noEmit` (TS2339: `address()` is `string | AddressInfo | null` in strict mode), so the commit broke the typecheck that tasks 2.3 and 3.2 require to pass. Fixed with the same narrowing pattern already used by listen() in src/server.ts:94-95. - blocking, `src/db.ts:31`: Concurrent `createTables` calls on a fresh database (parallel vitest workers, CI's fresh postgres container, overlapping app starts) race in the system catalogs: the loser gets `duplicate key value violates unique constraint pg_type_typname_nsp_index` (23505) or `relation/type already exists` (42P07/42P04), making `npm test` fail nondeterministically (observed on 2 of 4 runs; reproduced 150/150 with 8 concurrent pre-warmed sessions before the fix). Fixed: on those codes the IF NOT EXISTS statement is re-run once (safe: the conflicting creator has committed, so the re-run is a no-op); other errors are re-thrown. After the fix the repro is 0/150 and the full 58-test suite passes. - major, `test/db.test.ts:46`: The expected column list `['code', 'clicks', 'created_at', 'id', 'url']` is not in sorted order (`'clicks' < 'code'`), while the received list is `.sort()`ed, so this assertion can never pass; it was previously masked by the createTables race failing the same test earlier. Fixed the literal to the actual sort order `['clicks', 'code', 'created_at', 'id', 'url']`; the column set asserted is unchanged. ## Tests All 31 scenarios of `build-shortcut-a-small-link-shortener-fo-d2hmzy` have a tagged test. The test lint found, in the run's tests on the last check: **Recorded, not decided** - `e2e/follow.spec.ts:1` B7: no relative import: this file asserts on things it constructed itself, which is the measured 'asserted on my own mock' failure - `e2e/links.spec.ts:1` B7: no relative import: this file asserts on things it constructed itself, which is the measured 'asserted on my own mock' failure - `e2e/shorten.spec.ts:1` B7: no relative import: this file asserts on things it constructed itself, which is the measured 'asserted on my own mock' failure - `test/click-tracking.test.ts:1` B7: nothing imported here looks like the module under test ('click-tracking') - `test/clicks.test.ts:1` B7: nothing imported here looks like the module under test ('clicks') - `test/link-storage.test.ts:1` B7: nothing imported here looks like the module under test ('link-storage') - `test/list.test.ts:1` B7: nothing imported here looks like the module under test ('list') - `test/shorten.test.ts:1` B7: nothing imported here looks like the module under test ('shorten') ## Braid run - **Workflow:** Braid's OpenSpec flow, change `build-shortcut-a-small-link-shortener-fo-d2hmzy` (42 steps, 2 retried) - **Model:** vLLM · qwen3.8-27b - **Time:** 6 h 2 m from start to this pull request, 3 h 32 m of it in the model - **Tokens:** 17.5M input, 108k output, 471k reasoning, over 351 model calls - **Pre-flight estimate:** fits, p50 119k / p80 184k tokens against a 394k-token step allowance - **CI:** passed its 6 jobs on `a31512e208` - **Run:** [run_01M52JT93Q9X1VH3GD05D2HMZY](http://openspecflow.internal.underthere.xyz/run/run_01M52JT93Q9X1VH3GD05D2HMZY) _Opened by Braid._

cmoriarty added 2 commits 2026-10-16 13:08:00 -04:00

propose: the change, its specs, its tasks and its estimate 4a53856244

check: the code of fix-loop round 2, for the project's CI aa602ec78a

cmoriarty force-pushed osf/66-shortcut-a-link-shortener-combined from aa602ec78a to a31512e208 2026-10-16 13:14:13 -04:00 Compare

cmoriarty changed title from Build "Shortcut", a small link shortener for one team, as a new… to Built a small private link shortener: a form turns a long http/https… 2026-10-16 16:48:33 -04:00

cmoriarty force-pushed osf/66-shortcut-a-link-shortener-combined from a31512e208 to 77b7f1e0a3 2026-10-16 16:48:34 -04:00 Compare

cmoriarty merged commit dcb9632df8 into main 2026-10-16 16:48:38 -04:00

cmoriarty deleted branch osf/66-shortcut-a-link-shortener-combined 2026-10-16 16:48:38 -04:00

cmoriarty referenced this pull request from a commit 2026-10-16 16:48:39 -04:00

Merge pull request 'Built a small private link shortener: a form turns a long http/https…' (#1) from osf/66-shortcut-a-link-shortener-combined into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

cmoriarty/braid-combined-test!1

No description provided.