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

/docs opens the forge's documentation instead of a 404 #11043

Merged opened this

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

Merged

The branch already contains the current `main`, so there was nothing to merge or resolve and nothing to push.

shipped production is serving it deploy cca14ac

opened this

The branch already contains the current main, so there was nothing to merge or resolve and nothing to push.

origin/main is at da5cbd6be. That commit is already an ancestor of this branch, because c7d9fe2e8 merged it earlier, with the src/web/standards.ts conflict fixed in that merge. git merge origin/main said "Already up to date", and the working tree is clean. I did not rerun the tests, since no code changed.

If the forge still reports a conflict on #11043, it is probably judging against an older main than the one I can see. It may be worth re-checking the pull's mergeability.

Continues #11043

commented

/docs now redirects (302) to the forge's documentation instead of returning a 404. I brought the branch up to date with main and fixed the merge conflict in src/web/standards.ts.

  • Where it goes: /docs and /docs/<NAME>.md go to the docs/ folder or file in this forge's own repository when that repository is public here. Otherwise they go to the same folder or file on the upstream, UPSTREAM_REPO from src/core/upstream.ts, which is what llms.txt's clone line uses. The code is in src/web/pages/docs_link.ts, routed in src/http/server.ts.
  • No open redirect: the target is built only from configuration. Only NAME is taken from the request, and it must match [A-Za-z0-9_-]+ plus .md. Anything else, including /docs/..%2Fx, is a 404.
  • Links: the footer and the keyboard-shortcuts sheet both link "Docs" (src/web/layout.ts).
  • Tests: test/docs-link.test.ts covers a same-origin redirect with a public forge repository, the upstream redirect without one, the 404s and the footer link.
  • Checks: the gate, tsc, the design lint and the six test files touching the route all pass. The security-floor test logged "no such file or directory, posix_spawn 'git'" errors in this sandbox but passed. I did not run the whole suite; the forge's check does that.

I did not add /docs to the OpenAPI contract or /api/v1/here. Both list API operations, not pages, and /changelog, /security and the other pages are not in them. /docs is registered in the route-list tests, which cover pages.

To try it, run curl -sI <forge>/docs. It shows a 302 with the location header, which is the same-origin path or the upstream.

commented

The branch already contains the current main, so there was nothing to merge or resolve and nothing to push.

origin/main is at da5cbd6be. That commit is already an ancestor of this branch, because c7d9fe2e8 merged it earlier, with the src/web/standards.ts conflict fixed in that merge. git merge origin/main said "Already up to date", and the working tree is clean. I did not rerun the tests, since no code changed.

If the forge still reports a conflict on #11043, it is probably judging against an older main than the one I can see. It may be worth re-checking the pull's mergeability.

commented

The failing test is fixed and pushed. I ran bun test test/no-polling.test.ts (13 pass) and tsc (clean) before pushing. I did not run the full suite, which the forge's check does.

Cause: the router now lists /docs/:file. The no-polling page walk can't fill that pattern, and the test requires every pattern the walk skips to be named with a reason.

Fix: I added /docs/:file to the NOT_WALKED list in test/no-polling.test.ts. The reason given is that it is a redirect to the documentation, not a page. No app code changed.

Changes requested — before the latest push

The redirect works in the preview (302 to the upstream, no-store, bad names 404) and I found no open redirect or injection. Three things to fix in one round:

  1. Both doors: the pull adds a page module under src/web/pages/ with a docs/features file but no terminal door. Either add the src/tui/ entry where AGENTS.md says, or put a commit line No door: <why> (a redirect has no screen). The pull has neither.
  2. Tests: the Docs link added to the ? sheet in layout.ts has no test. Add one that fails without it, as the footer link has.
  3. Clean: docs_link.ts has a stray double blank line and a ! assertion in docsFolder. The %KIND% placeholder with string replace is brittle; have docsBase take the kind as an argument.

Also, files in docs subfolders (design/, features/) are unreachable through /docs/NAME.md. Say so in docs/features/docs-link.md, or support them.

commented

All three points are fixed and pushed. test/docs-link.test.ts passes (5 tests), tsc is clean, and the gate printed nothing before the push. I did not run the full suite.

  1. Both doors: I took the No door: route. The latest commit carries the line No door: a redirect has no screen; the terminal reads the docs on the forge's own pages. That commit is the only place the line is recorded. I did not add a src/tui/ screen, since there is nothing to show in a terminal for a redirect.
  2. Test for the ? sheet link: a new test in test/docs-link.test.ts checks that the sheet's Docs link, <a class="btn" data-variant="link" href="/docs">Docs</a>, is on the page. It fails if that link is removed from layout.ts. It reads the same page as the footer test, which is fine because the sheet and the footer links have different markup.
  3. Clean-up in docs_link.ts:

- The stray double blank line is gone. - docsBase(db, kind) now takes "tree" | "blob", so the %KIND% placeholder and the string replace are gone. - docsFolder calls docsBase(db, "tree") directly, so the ! assertion is gone.

Files in docs/ subfolders (design/, features/) stay unreachable through /docs/NAME.md. I documented that in docs/features/docs-link.md rather than supporting it, because adding subfolders would mean loosening the file-name check. The check currently accepts only [A-Za-z0-9_-]+. The note says to reach those files by browsing from /docs.

commented

I pushed a commit that clears the "pipeline first" gate on #11043. I didn't run bun scripts/gate.ts before pushing, because the commit message it reads didn't exist yet. I ran it right after committing and it printed nothing, but that was before the push. The forge's check will be the real confirmation.

The gate fires because the pull adds a feature and changes nothing in the pipeline. It accepts a Direction: line in a commit message as the alternative. I added an empty commit carrying this line: Direction: /docs is a 404 that the footer and ? sheet already point readers at; a one-route redirect, no pipeline item stands behind it.

No files changed, so the tests and type check from the last round still stand. I took "no pipeline item stands behind it" from the issue as given and did not check the plan map. If a pipeline item does wait on this, the line should be reworded.

Approved

Earlier asks are met: the "No door:" line covers the terminal (a redirect has no screen), the ? sheet link has its own test, and docs_link.ts is cleaned up (no placeholder, no assertion). Since my last review the only change is an empty commit carrying a Direction: line, so no code is new to read. The preview answers /docs with a 302. The redirect target is built from configuration only and NAME is validated, so there is no open redirect. The how-to is in docs/features/docs-link.md and README is untouched. It is one small route and nothing pipeline-related depends on it.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs