docs/maps/open-questions.md
Rama main · réplica de lectura del repositorio original.
# Open Questions & Suspected Defects
Read this before trusting any single claim in the other maps as gospel.
Most of this was found by **reading the source**, not by running the app
against a live database; each item states what was verified and what was not.
The distinction matters: a static read can prove that no code queries a column,
but it cannot prove what a user sees. B8 through B10 are the exception: they
were found by running the app against a real database in a real browser, and
are marked as such.
This is a findings list, not a changelog. Items that have since been fixed are
marked `(FIXED)` in their heading and kept, because the reasoning that found
them is worth more than the closure.
---
## A. Dead code paths, verified by exhaustive grep
These are the highest-confidence items. Each was checked by grepping the whole
`cloudcodex/` tree (excluding `node_modules`) for every reference.
### A1. `logs.read_access` and `logs.write_access` are write-only
- **Written by:** the PR-session log insert (`routes/github.js:1698`), and
only as an empty `JSON_ARRAY()`. Until 2026-08-09 the PR-session open also
`JSON_ARRAY_APPEND`ed the caller's id here, which is what made that feature
admin-only (B1); the grant now lands on a per-PR archive instead.
- **Read by:** nothing. `checkLogReadAccess` / `checkLogWriteAccess`
(`routes/helpers/shared.js:76-104`) join `logs` to `archives` and apply the
access fragment against the **archive** alias.
- **Verified:** grep for `read_access` across `routes/`, `services/`,
`middleware/` returns only `archives`-scoped reads plus the github.js writes.
- **Not verified:** runtime behaviour.
Same story for `versions.read_access` (`init.sql:372`): declared, never read,
never written.
### A2. `github_embed_refs` has no writer
`GET /api/logs/by-github-ref` (`github.js:1956-1977`) reads the table.
`migrations/p1_github_embeds.sql` and `init.sql:309-323` create it. There is no
`INSERT INTO github_embed_refs` anywhere in the repo.
**Consequence:** the "which documents reference this file / issue / PR"
back-link always returns an empty list. The endpoint's test
(`tests/routes/github.test.js:1818`) passes because `c2_query` is mocked, so the
suite cannot catch this class of gap.
**Open question:** was the writer planned for a later phase, or lost? The
embed nodes (`src/extensions/GitHubCodeEmbed.jsx`,
`GitHubIssueEmbed.jsx`) are the obvious place to record a ref on save.
### A3. `squad_permissions` was enforced by nothing (REMOVED)
Read and written only by `GET`/`PUT /api/squads/:id/permissions`.
`requirePermission` consults the global `permissions` table and then
`squad_members.can_create_*`, never `squad_permissions`. A workspace owner
could toggle squad-level create permissions through the API, the value
persisted, and no behaviour changed.
**Removed 2026-08-09** rather than enforced (Kyle's call): `squad_members`
already carries per-member `can_create_log` / `can_create_archive` flags that
*are* enforced, so this was a second, redundant answer to the same question,
in the repo's most trap-laden subsystem.
Gone: the table (`migrations/drop_squad_permissions.sql` plus the `CREATE` in
`init.sql`, with the `DROP` line kept so `reset-db` still cleans older
databases), both routes, the six tests that covered them, and the two
`util.jsx` wrappers, `fetchSquadPermissions` / `updateSquadPermissions`, which
turned out to have no callers anywhere in `src/`.
## B. Suspected functional defects
Higher-value, lower-certainty. Each needs a runtime check to confirm.
### B1. PR-as-document sessions were admin-only (FIXED)
**Confirmed at runtime**, 2026-08-09, by replicating `getOrCreatePrSession`'s
exact inserts and grant: `logs.read_access` came back `[19]` and that user
still got **403** on `GET` and `POST /api/logs/:logId/comments`, while an admin
got 200. The reading below was correct in every particular.
**A second defect, found while fixing it and not recorded here before:**
`GET .../pulls/:number/session` made **no GitHub call at all**. It took
`owner`, `repo` and `number` from the URL and created a session. That was
harmless only because the grant did nothing; making the grant real without
fixing this would have let any authenticated user with any GitHub account open
a session on a private repo's PR and read the Codex-side discussion. The route
now fetches the PR through the caller's token first.
**Fixed** by option (a) of the two below, per Kyle's call on 2026-08-09: one
hidden archive **per PR** (`__c2_github_pr_sessions__:owner/repo#N`) with the
grant appended to that archive's `read_access`/`write_access`. Per-PR rather
than one shared archive because the archive is the ACL boundary, so a shared
one would have made any session grant access to every other PR's session.
Sessions created earlier are moved into their per-PR archive on next open.
Verified live: the granted user now reads (200) and comments (201), two users
holding different PR sessions each get 403 on the other's, and the admin
control still passes. **A1 is unchanged: `logs.read_access` is still dead**,
and this fix deliberately routes around it rather than reviving it.
Original finding below.
### B1 (original reading). PR-as-document sessions are probably admin-only
`getOrCreatePrSession` granted the caller access by
appending their id to the virtual log's `read_access`/`write_access`. Per **A1**
those columns are inert.
The parent archive was a single `__c2_github_pr_sessions__`, created with
`squad_id NULL`, `created_by NULL`, and every ACL column empty. Walk the seven clauses of `readAccessWhere` against
that row for a non-admin user:
| Clause | Result |
|---|---|
| 1. `is_admin` | false |
| 2. `JSON_CONTAINS(p.read_access, uid)` | archive ACL is `JSON_ARRAY()`, false |
| 3. `p.created_by = uid` | `created_by` is NULL, false |
| 4. workspace owner via squad | `squad_id` is NULL, subquery empty, false |
| 5. squad member | `squad_id` NULL, false |
| 6. `read_access_squads` | empty array, false |
| 7. `read_access_workspace` | FALSE |
So `checkLogReadAccess` should return `undefined`, and every
`/api/logs/:logId/comments` call on a PR-session log should 403 for non-admins,
as should the `/collab` WebSocket (`collab.js:424-428`).
**Verified:** the clause-by-clause reading above, and that the comment routes
gate on `checkLogReadAccess` (`comments.js:33`, `:98`, `:122`, `:329`, `:374`,
`:436`, `:461`).
**Not verified:** actual runtime behaviour with a real database and a non-admin
user.
**If confirmed**, the minimal fixes are either (a) grant on the archive rather
than the log, or (b) teach the log-level checks to also honour
`logs.read_access`, which would resolve **A1** at the same time.
### B2. `html_content TEXT` capped documents at 64 KiB (FIXED)
`logs.html_content` was `TEXT`, i.e. 65,535 bytes, against an application
ceiling of 2 MiB (`documents.js:29`, `collab.js:56`).
**Confirmed at runtime**, 2026-08-09. `sql_mode` on the shipped image (MySQL
8.4.8) does include `STRICT_TRANS_TABLES`, so this is an error, not truncation.
Saving through the real `POST /api/save-document`:
| HTML size | Before |
|---|---|
| 40 KiB | 200, stored |
| 70 KiB | **500** "An internal server error occurred", nothing stored |
| 300 KiB | **500**, nothing stored |
The user's edit is simply lost, behind a generic 500 that names nothing.
**Two columns the original note missed**, both found by looking rather than
reasoning:
- `plain_content` is `STORED GENERATED` from `html_content`, and it was `TEXT`
too. Stripping tags from prose barely shrinks it (40,993 bytes of HTML gave
40,986 of text), so widening `html_content` alone would have moved the same
failure one column sideways.
- `versions.html_content` was `TEXT`, and publish copies the document into it,
so a widened `logs` would have pushed the same 500 from save to publish.
**Fixed** in `migrations/widen_log_content.sql` plus `init.sql` (both, per the
dual-tracking rule in [data-model.md](data-model.md)): all three columns are now `MEDIUMTEXT`. Verified
live after applying it: 40, 70 and 300 KiB all save 200, a 300 KiB publish
returns 200 with `versions.html_content` holding all 307,213 bytes, and the
`ft_logs_search` FULLTEXT index survived the `MODIFY` and still matches.
### B4. GitHub link CRUD did not check document access (FIXED)
`GET`, `PUT` and `DELETE /api/github/link/:logId` validated the id and acted.
They inherit `requireAuth` and `requireGitHub` from the router, but called
neither `loadLinkAndLog` nor any `check*Access` helper.
**Confirmed at runtime**, 2026-08-09, against a real database with two users
sharing no squad and a document the attacker provably could not read (`GET
/api/document` returned 404, and the gated `/status` route returned 404 for the
same log in the same run):
| Route | Before | Effect |
|---|---|---|
| `GET /github/link/:logId` | **200** | returned the private repo, branch and path the document is bound to |
| `PUT /github/link/:logId` | **200** | repointed the binding to an attacker-chosen repo and path; `linked_by` stayed the victim's id, so the audit trail still named them |
| `DELETE /github/link/:logId` | **200** | row gone |
**This map previously said "content cannot be exfiltrated this way". That was
wrong.** `/push` reads `repo_owner`, `repo_name`, `file_path` and `branch`
straight off the `github_links` row and takes only a commit message from the
caller. So an unauthorised `PUT` redirects the *victim's own next push*, which
commits their document content into the attacker's repo using the victim's
token. The four sync routes gating correctly is not sufficient when an
unguarded route decides where they point.
Deleting the row is not merely annoying either: it discards `base_sha`, the
merge base every later conflict check depends on.
**Fixed** by gating `GET` on `checkLogReadAccess` and `PUT`/`DELETE` on
`checkLogWriteAccess` (write, not read, precisely because `PUT` chooses a push
target). All three now 403, verified live on the same fixtures and covered by
three tests that were each confirmed to fail against the unfixed route.
### B5. GitHub team sync silently truncates above 100 members
Both team-sync routes fetched team members with a single `per_page=100`
call and no pagination loop.
**FIXED 2026-08-09.** Both call sites now go through `fetchTeamMemberLogins`,
which follows pagination to a 20-page (2,000 member) cap and reports whether
the listing is `complete`. When it is not, the removal pass is skipped
entirely and `members_complete: false` is returned, because a login on an
unfetched page is indistinguishable from one that left the team. Covered by
two tests: one proves page 2 is read, one proves a truncated listing deletes
nobody. Original finding below.
**Consequence:** a team with more than 100 members yields a partial `ghLogins`
set. The preview under-reports, and the sync's removal pass
then deleted every current member whose login fell outside
the first page.
**Not verified:** no team of that size has been tested.
**Suggested fix:** paginate, or refuse to run the removal pass when the response
is a full page.
### B6. `title` was the only WS message not gated on write access (FIXED)
The `{type:'title'}` handler in `collab.js` had no `canWrite` check, unlike
`cursor`, `save`, `publish` and `comment`.
**Confirmed at runtime**, 2026-08-09, with a squad member holding
`can_read = TRUE, can_write = FALSE`. The server itself sent
`sync { canWrite: false }`, correctly ignored a `save` in the same session, and
then honoured the rename: the title became "PWNED BY A READ-ONLY USER",
`updated_by` was set to the read-only user, and one `log.rename` row landed in
`activity_log`.
**This was a live hole, not defence-in-depth.** The earlier note wondered
whether the client exposes the affordance; that question does not matter, since
the message is reachable from any WebSocket client. And the activity row is the
sharper end of it: `logActivity` auto-enrols the actor as a watcher and emails
every other watcher, so a read-only user could rename a document *and* send
mail about it to everyone watching.
**Fixed** by adding `canWrite` to the branch condition. Re-verified on the same
fixtures: title unchanged, `updated_by` still NULL, zero `log.rename` rows. The
collab suite had no read-only coverage at all, which is why this survived; it
now has a `readOnlyClient` helper and a test that fails against the unfixed
service.
### B7. Squad-invite emails bypass notification preferences
`DEFAULT_EMAIL_PREFS` includes `email_squad_invite: true`
(`services/notifications.js:33`), but `services/email-templates.js:63` defines
builders for only five types, not including `squad_invite`. So
`deliverEmail` bails at `services/notifications.js:170` and the funnel sends nothing.
The email is sent anyway, by a direct `sendEmail` call in
`routes/squads.js:399`, which does not consult
`users.notification_prefs`.
**Consequence:** turning off "email me about squad invites" in the preferences
UI has no effect.
**FIXED 2026-08-09**, minimally: the direct send now calls `getPrefs()` and
honours `email_squad_invite`. The invitation itself, and the inbox
notification, are unaffected.
The larger option was to give `squad_invite` a builder in
`email-templates.js` and let the funnel own the send, which is what the rest
of the notification types do. It was not taken here: the route deliberately
sends its own mail and passes `emailData: null` so the funnel does not send a
second copy, and rerouting it would change the send's timing and content for
a defect whose whole substance was "the toggle does nothing". Worth revisiting
if a second such bypass appears.
### B8. `server.js` hardcoded port 3000 and reported success on a failed bind (FIXED)
`server.js` baked the port in and logged
`CloudCodex API Server is running on http://localhost:3000` from
`ViteExpress.listen`'s callback, on the assumption that reaching the callback
meant the bind had succeeded.
**Verified at runtime:** on a machine where another project already owned port
3000, the app never listened, yet still printed the success line, and requests
to `localhost:3000` were answered by the *other* application with no
indication anything was wrong.
**The root cause is Express 5, and it generalises well beyond this file.**
`express/lib/application.js` aliases the listen callback onto the socket's
error event:
```js
if (typeof args[args.length - 1] === 'function') {
var done = args[args.length - 1] = once(args[args.length - 1])
server.once('error', done)
}
```
So `app.listen(port, cb)` runs `cb` on a **failed** bind as well as a
successful one, with `server.listening === false`. Express 4 had no such
aliasing, so this arrived with the framework upgrade, not with `vite-express`.
The reusable rule: **in Express 5, listen's callback is not evidence of a
bind; `server.listening` is.**
**Fixed:** `PORT` is honoured (documented in `.env.example`, blank means the
3000 default, an out-of-range or non-numeric value exits rather than silently
falling back), the success line is guarded on `server.listening` and reports
the port actually bound (so `PORT=0` reads correctly), and an `'error'`
handler names the port and exits non-zero. Both paths were verified against
the running app: a busy port prints `Port N is already in use` with no success
line and exits 1, and a free custom port serves normally.
**Why the callback and not the `'listening'` event**, which is the other
honest signal: `vite-express` registers its own `'listening'` listener first
and injects the Vite middleware asynchronously, so `'listening'` fires roughly
**twelve seconds before the dev server can serve a page** (measured: 200 at
t=11.6s, `Cannot GET /` before that). Announcing there would trade a lie about
binding for a lie about readiness. The callback runs after injection.
**Docker:** both production compose files publish
`"${APP_BIND:-127.0.0.1}:${PORT:-3000}:${PORT:-3000}"` (the host address was
added by GHSA-9fmx-frrf-xxmq, see `build-test-and-ops.md` section 4), so
setting `PORT` moves the published mapping with it. Note that a port
conflict on a Docker host is a *host*-side collision that `PORT` alone cannot
resolve; it surfaces as Compose's own "port is already allocated".
**Still true, and worth knowing:** `APP_URL` is independent of `PORT`.
Changing the port without updating `APP_URL` produces invitation and
password-reset links pointing at the old one.
### B9. `make reset-db` and `docker compose down -v` do not reset the dev database
`docker-compose.yaml`'s `database` service mounts `./db-data/:/var/lib/mysql/`,
a **bind mount**, not a named Docker volume. `docker compose down -v` only
removes volumes Compose manages; a bind mount is host-owned and untouched, so
`-v` silently does nothing here. `make reset-db` (`Makefile`) does not touch
the volume at all either, it pipes `init.sql` then `seed.sql` through the
MySQL client against the already-running container, which drops and
recreates the tables it lists but never revisits the data directory or
anything `init.sql` doesn't already know about.
**Verified at runtime**: `db-data/` content survives both `make reset-db`
and `docker compose down -v` up. Getting a genuinely fresh database (the
`docker-entrypoint-initdb.d` path documented in
[data-model.md](data-model.md)'s Trap 1) needs `db-data/` emptied by hand,
and its contents are owned by uid 999 (the MySQL image's internal user), so a
plain host-side `rm -rf db-data/*` needs either `sudo` or a throwaway
container run as that uid to remove it.
**Consequence:** every "start from a clean database" instruction anywhere in
this repo that says `docker compose down -v` is wrong about what it does.
### B10. The squad `<select>` in the admin invite modal is unstyled
`AdminPage.jsx`'s invite-user modal renders a `<select>` for the optional
squad picker next to a text `<input>` for the email address. The input picks
up the app's dark theme; the `<select>` renders with default browser chrome
(white background, black text), visibly inconsistent right next to it.
**Verified by looking at the running app.** Cosmetic only, no functional
impact, but jarring enough to be worth a styled `<select>` (or a themed
listbox component) rather than the browser default.
### B11. Navigating away from the editor blanks the whole app (FIXED)
**Reproduced twice, in two independent browser sessions, against the built
production image.** Open any document, click `✏️ Edit`, then click any sidebar
nav link. React throws
```
NotFoundError: Failed to execute 'removeChild' on 'Node':
The node to be removed is not a child of this node.
```
and the DOM collapses from ~275 nodes to ~25. The page renders **completely
blank** and stays blank until a manual reload; the route changes in the URL
bar but nothing mounts.
Confirmed it is the editor unmounting, not any particular destination: one
session went editor to `/admin`, the other editor to `/`, with byte-identical
stack traces. Both were in edit mode with a live collab session open.
The shape is the familiar ProseMirror-and-React teardown race, where
ProseMirror removes DOM that React still believes it owns, so React's own
removal fails. It is fatal rather than merely noisy because **there is no
ErrorBoundary anywhere in `cloudcodex/src/`**, so one throw during unmount
takes down the entire root. That missing boundary is the same gap flagged for
the null-owner crash in the workspace `owner_id` review.
**Root cause.** `RichTextEditor` in `Editor.jsx` portalled the cursor and
comment-highlight overlays into `.tiptap`'s **`parentElement`**, the node
`EditorContent` renders and whose children ProseMirror manages. An effect even
reached in and set `position: relative` on it. React and ProseMirror therefore
both mutated the same parent, and on teardown ProseMirror emptied it before
React removed the portal children, so React's `removeChild` found a node that
was no longer its child.
**Both halves are fixed.**
1. **No more portals.** The overlays now render as ordinary siblings of
`EditorContent` inside `div.tiptap-overlay-host`, an element React owns
outright, which carries the `position: relative` in CSS rather than through a
DOM mutation. `RichTextCursors` and `RichTextHighlights` take an optional
`containerRef` so their coordinates are measured against that same host,
keeping the geometry identical (they previously measured against
`editor.view.dom.parentElement`, which was also the portal target).
2. **An `ErrorBoundary` now wraps the app** (`src/components/ErrorBoundary.jsx`,
mounted in `App.jsx`). It was the absence of any boundary that promoted an
unmount race into a total outage, and it would have done the same for the
null-owner admin crash. Covered by `tests/src/components/ErrorBoundary.test.jsx`.
**Verified against the built production image**, not the dev server: opened a
document, clicked Edit, clicked a sidebar link. Before, the DOM collapsed from
~275 nodes to ~25 with the `removeChild` exception; after, it navigates with
**274 nodes and zero console errors**, and the destination page renders. The
editor's own layout and the comment highlight positions are unchanged, checked
by comparing captures either side of the change.
Tested one destination (editor to `/`) after the fix. The original bug
reproduced identically to two different destinations and the cause is
destination-independent, so this was not re-run for every route.
### B12. Production CORS rejected the app's own browser (FIXED)
`app.js` used `cors({ origin: fn })`, whose callback receives only the origin
string and never the request, so it could not recognise a same-origin call.
Browsers send an `Origin` header on same-origin POST/PUT/DELETE, so with
`NODE_ENV=production` (forced by both `npm run start` and the Docker image) and
`CORS_ORIGIN` unset (what `.env.example` ships), every write request from the
app's own UI was rejected and answered **500**. Login was impossible through
the documented self-hosting path.
It survived because `curl` sends no `Origin` header, so every manual API check
passed, and because `tests/app.test.js` had **no CORS test at all** despite its
file header claiming to verify "CORS scoping". The suite also runs as
`NODE_ENV=test`, which never reaches the rejecting branch.
Fixed by switching to the request-taking form of `cors()` and comparing the
`Origin` host against `req.headers.host`, plus `APP_URL`'s host for installs
behind a proxy that does not rewrite `Host` (nginx's default `proxy_pass` does
not). See `request-lifecycle.md` for the resolution order and for why this must
not use `req.hostname`. Regression cover is the `CORS` describe block in
`tests/app.test.js`; the two same-origin cases were confirmed to fail against
the old implementation before the fix was kept, both by the author and
independently during adversarial review.
Still true and not addressed: a genuinely disallowed cross-origin request
produces a **500 with an HTML body** rather than a 403, because the rejection
throws before any router and `app.js` mounts no global error handler.
### B14. Committing a file through the browser falsified every linked doc's merge base (FIXED)
Found by adversarial review on 2026-08-09, not by the original survey.
`PUT /api/github/contents/*` (the CommitPanel path) ends with a
fire-and-forget `UPDATE github_links` matching on
`(repo_owner, repo_name, file_path, branch)` alone. It set **both** `file_sha`
and `base_sha` to the new commit's sha and `sync_status = 'clean'`.
**Consequence:** document 42 is linked to `acme/docs/a.md@main` holding content
A. Anyone with push rights on that repo commits content B to that path through
the browser. Document 42, untouched, is now recorded as clean against a remote
it has never seen, and since `classifySync` derives `remoteChanged` from
`remoteSha !== baseSha`, it can no longer *ever* report `remote_ahead` for it.
The owner sees a clean banner and their next push silently overwrites B with A:
no conflict, no 409, no trace.
**Fixed** by recording only what actually happened: advance `file_sha` (the
last observed remote blob), leave `base_sha` (the merge base) alone, and set
`sync_status = 'remote_ahead'`. The row is still updated for *every* linked
document regardless of who committed, which is correct, since "the remote file
moved" is true for all of them; it was the false `clean` that caused loss. The
export flow's own explicit `PUT /api/github/link` still settles that one
document back to clean, because there the document and the commit really do
match.
### B13. Document titles were not reachable by keyboard anywhere (FIXED)
On the browse grid, the archives page and the editor's page tree, a document's
title was a click-only element. The accessibility tree for those pages exposed
the surrounding controls (favourite `☆`, comment count, delete `×`, "+ New
Log") but **no actionable node for the document itself**, so the only way to
open a document from any list was a mouse click.
Found while driving the app over CDP for the README screenshots: navigating to
a document required going through the first-run welcome modal, whose "Start
here" link is a real `<a>`, because no list view offered one.
**The open question resolved the worse way.** The rows were plain
`<div onClick>` / `<span onClick>` with no `tabIndex`, no `role` and no key
handler, so the titles were unreachable outright rather than merely unnamed.
**Fixed 2026-08-09.** The title is now a real element in each view, and the row
or card keeps its own `onClick` so the mouse affordance is unchanged:
| View | Was | Now |
|---|---|---|
| Browse grid, `ExploreBrowser.jsx` `ExploreCard` | `<h3>` inside a click-only card | `<h3>` wrapping `<Link className="explore-card__title-link">` |
| Archives page, `ArchiveBrowser.jsx` `LogTreeItem` | `<span className="log-tree-title" onClick={navigate}>` | `<Link className="log-tree-title">` |
| Editor page tree, `PageTree.jsx` `TreeItem` | `<span className="page-tree-label">` | `<button className="page-tree-label">` calling `onSelect` |
| Topbar search dropdown, `SearchBox.jsx` (removed in W6-CDX-40; the palette, `SearchPalette.jsx`, lists its results as links) | `<div onMouseDown>` | `<Link className="search-dropdown-item">` |
| `SearchResultItem.jsx` (no callers) | click-only `<div>` | `<button className="result-title-btn">` |
**The topbar dropdown was the worst of the five and was nearly missed.** It is
mounted on every authenticated page (`Std_Layout.jsx`), and it was not merely
unnamed: `onMouseDown` meant even a synthetic click would not have fired it,
and the input's `onBlur={() => setTimeout(() => setShowDropdown(false), 200)}`
tore the results down as soon as focus left the input, so they were unreachable
by construction. The `onMouseDown` was not sloppiness, it was the workaround for
that timeout.
Fixing it therefore took more than swapping the element. Dismissal moved to the
container, closing only when focus leaves the whole search box
(`relatedTarget` containment), plus Escape-to-close returning focus to the
input. **And the dropdown container takes `onMouseDown={e => e.preventDefault()}`:**
Chromium focuses a link on mousedown but **Firefox and Safari do not**, so
without it the input would blur with a null `relatedTarget`, unmount the
results before mouseup, and the click would never land, reintroducing exactly
the bug the original `onMouseDown` was avoiding, on two browsers, invisibly to
a jsdom suite. Suppressing focus on mousedown does not suppress the click, so
the anchor still navigates; keyboard users never fire mousedown at all.
The two link cases and the page-tree button `stopPropagation` so the row's own
handler cannot fire a second, redundant navigation. A `<button>` is used in the
page tree rather than a link because `ArchiveView` selects with
`navigate(..., { replace: true })` on purpose, so the tree is an in-page
selector and not a history step; the tradeoff is that a tree page cannot be
opened in a new tab. Appearance is held constant by CSS resets on the four
classes, and focus rings come from the pre-existing global `:focus-visible`
rule (`index.css`), not from anything new.
**Verified against a running app, not just the suite.** Logged in over CDP, the
accessibility tree now lists one named `link`/`button` per document in all four
live views, and pressing Enter on it navigated: browse grid `/` →
`/archives/29/doc/113` (hit-tested to `a.explore-card__title-link`), page tree
`doc/113` → `doc/102` (`button.page-tree-label`), archives page `/archives` →
`/archives/22/doc/65` (`a.log-tree-title`), and the topbar dropdown `/` →
`/archives/27/doc/111` (`a.search-dropdown-item`) after two Tabs from the
input, with the dropdown staying open across the input's blur. The dropdown's
mouse path was re-checked in the same session and still navigates
(`doc/111` → `doc/95`), confirming the mousedown `preventDefault` does not
swallow the click. No console errors on any step. Tab order within a card is
title, then that card's `☆`.
**`SearchResultItem` has no callers.** Grep across `src/` finds the component
defined and default-exported, and rendered nowhere; search results reach the
user through `ExploreCard` and the topbar dropdown, not through this file. It
was fixed for consistency, but it is dead code and a deletion candidate in the
same shape as A3. Not deleted here because removing a component is a scope call.
**Two things about the dropdown remain unverified, and an attempt on
2026-08-10 established that neither is answerable on this workstation.** Both
concern `preventDefault` on the results container's `mousedown`.
- **Scrollbar thumb dragging** inside `.search-dropdown`
(`max-height: 320px; overflow-y: auto`). Chromium dispatches a gutter press
to the content, so preventing it could freeze the thumb. **A headless
Chromium probe reported exactly that, and the report was worthless:** a
control dragging the scrollbar of a plain scrollable `div` carrying no
handler at all failed identically, so the method was measuring nothing, not
the app. This headless build renders a 2 px overlay scrollbar with no
draggable thumb, and `--disable-features=OverlayScrollbar` did not change it.
**Settle it by hand in a headed browser with 6+ results**, not with CDP.
Worst case is degraded, not broken: wheel and keyboard scrolling cannot be
affected by this, and both were confirmed working (wheel scrolled 0 → 200
with the handler in place).
- **Touch.** WebKit stops dispatching the rest of its synthesized mouse
sequence once an earlier synthesized event is default-prevented. If that
includes `mousedown`, tapping a result would never fire `click`, a
regression from the old `onMouseDown` on iPad and touch laptops. The topbar
search is hidden below 768 px (`index.css`), so phones are out of scope.
**Playwright ships a real WebKit build and would answer this**, but
installing it needs ~25 system libraries (GTK 4, GStreamer, flite) that this
box lacks, i.e. a root package install. Either do that, or tap a result on
an iPad in landscape.
What the probe *did* confirm, with the handler in place: the dropdown stays
open across a press inside it, focus stays on `input.search-input`, and
clicking a result still navigates.
**Both halves of this item were found by adversarial review, not by the author.**
The first pass fixed the dead component and missed the live one, then wrote
"every list view" into three docs; by this repo's own rule that stale claim was
itself the defect. The second finding was a false green: `PageTree`'s row
`onClick` lost its only test the moment the title became a button, because the
test selected with `getByText`, which then resolved to the new button that
stops propagation. Deleting the row handler left all 346 frontend tests green.
The test now clicks `.page-tree-row` and fails when that handler is removed.
The 17 tests added with this fix were mutation-checked, not assumed: reverting
the source changes fails all of them.
### B15. Glyph-only controls announced as their glyph, not their purpose (FIXED)
Found while writing the B13 tests, and **distinct from B13**: these controls
were focusable and always had been, so they were reachable. What they lacked
was a name.
A control whose entire content is a glyph takes that glyph as its accessible
name, because content wins over the `title` attribute in the accname
computation. A `title` only becomes the name when the element has no content at
all. So these are announced as "☆ button", "+ button", "× button":
- `ExploreBrowser.jsx` favourite `☆` (has `title`, still names as `☆`)
- `PageTree.jsx` expand/collapse `▾`/`▸`, row-add `+`, header `←`, `+`, `◂`
- `ArchiveBrowser.jsx` `LogTreeItem` add `+`, comments `💬`, delete `×`, export `⤓`
Confirmed live over CDP: the archives page lists 88 actionable nodes, of which
the per-document controls appear as `button "⤓"`, `button "+"`, `button "💬"`,
`button "×"` repeated once per row, giving a screen-reader user no way to tell
which document any of them belongs to.
**Fixed 2026-08-10.** `aria-label` on all twelve, naming the **document** where
one is involved, because "Delete" repeated once per row is barely better than
"×":
| Control | Now announces |
|---|---|
| `PageTree` toggle | `Collapse <page>` / `Expand <page>`, plus `aria-expanded` |
| `PageTree` row `+` | `Add a subpage under <page>` |
| `PageTree` header `←`, `+`, `◂` | `Back to archives`, `New page`, `Collapse page tree` |
| `ExploreCard` `☆` | `Add <doc> to favorites` / `Remove <doc> from favorites`, plus `aria-pressed` |
| `LogTreeItem` `+`, `💬`, `×`, `⤓` | `Add a sublog under <doc>`, `Manage comments on <doc>`, `Delete <doc>`, `Export <doc>` |
Two things the fix needed beyond adding attributes. `ExportMenu` renders
`btnLabel` as its content, so a caller passing a bare `⤓` had no way to supply
a name; it now takes an optional `ariaLabel`. And the browse sort `<select>`
had **no accessible name at all** (iris reported it as
`combobox (no accessible name)`), now `Sort documents`.
**`VersionHistory`'s row was the worst case and is fixed in the same pass.** It
was a `div` with `role="button"` and `tabIndex={0}` but **no key handler**, so
it was focusable and not activatable, and the Restore `<button>` was nested
*inside* it, so the row's accessible name swallowed that button's label. The
info block is now a real `<button>` carrying `aria-expanded`, and Restore is
its sibling rather than its child.
Two traps this fix walked into, both caught by review:
- **`aria-label` overrides content**, so naming the comments button hid the
visible open-comment badge from screen readers entirely, having previously
announced it (badly) as part of `💬3`. The count is back in the label.
- **A toggle keeps one name.** The favourite button first flipped its name
*and* carried `aria-pressed`; the two channels then contradict, so "pressed"
reads as qualifying "Remove". It is now a stable `Favorite <doc>` with the
state on `aria-pressed` alone.
The `VersionHistory` row keeps its own `onClick` as a mouse convenience, so the
row padding and the gap before Restore stay clickable and the row-wide hover
highlight is not advertising a dead target. That is the same pattern B13
established for every other list view.
Mutation-checked per attribute, not per file: neutralising `PageTree`'s five
`aria-label`s fails 7 tests; reverting the `VersionHistory` info block to a
`div` fails 3; removing the row `onClick`, the comment count, or the sort
label fails 1 each.
### B16. A global create flag meant "may create anywhere" (FIXED)
**The semantic bug, stated once:** `permissions.create_squad`,
`create_archive` and `create_log` mean *this account may create this kind of
thing*. They have never meant *this account may create it in any workspace*,
but until 2026-09-08 four surfaces read them the second way, and
`createDefaultPermissions` (`shared.js`) hands every new account all three.
The workspace is the tenant boundary, and nothing in the codebase could ask
"is this user inside workspace W?", so there was no check to make.
Four holes, in descending severity:
- **(a) Squad creation.** `POST /api/workspaces/:workspaceId/squads` checked
that the workspace row existed and that the caller held `create_squad`, then
called `addSquadOwnerMember`. Squad ownership is a live term in clause 5 of
both access fragments, so **any authenticated account could give itself
owner-level footing inside any workspace** and read and write everything
under it.
- **(b) `requirePermission` returned early on the global flag**
(`permissions.js`), so the squad-context branch below it never ran for a
normal user. `POST /api/archives` takes `squad_id` from the body and that
middleware is its only gate, so any account could plant an archive inside any
squad in any workspace.
- **(c) The archive ACL grantee was never resolved.**
`POST /api/archives/:id/access` authorised the caller with `isArchiveOwner`
and then wrote the supplied `userId` or `squadId` straight into the ACL JSON,
validated as a well-formed id and nothing else.
- **(d) `GET /api/users/search` was unscoped.** `name LIKE ? OR email LIKE ?`
over the whole `users` table for any authenticated caller, returning id, name,
email and avatar. Every account on the install, with its email address, was
enumerable by every other account across every workspace.
**Fixed 2026-09-08.** `isWorkspaceMember` / `isSquadWorkspaceMember` in
`ownership.js` supply the missing predicate and all four routes now use it. The
7-param fragments were not touched. Full write-up, including why the
archive-derived squad is deliberately *not* validated in the middleware, in 3e
of [access-control.md](access-control.md).
**Known cost 1, accepted rather than special-cased.** The body-squad check runs
above the global flag, so a squad with `workspace_id` NULL no longer reaches
the `squad_members` fallback: a member holding `can_create_archive` on an
**orphaned** squad now gets `403` where they used to get `201`. An orphaned
squad has no tenant, so "is this user inside its tenant?" is unanswerable and
failing closed is the right answer. All four `INSERT INTO squads` sites set
`workspace_id`, so only legacy or hand-edited rows are affected.
**Two follow-on corrections, same day.** The first cut of (c) guarded the
grantee check with `if (owning?.workspace_id)`, which skipped it not only for an
archive with no squad but for an archive whose squad had a NULL `workspace_id`,
so that one case still failed **open** while the docs said it failed closed. The
guard is now `if (owning)` and an orphaned squad is refused, matching the rule
everywhere else. The first cut of (d) scoped on shared membership alone, which
over-blocked: an account with no `squad_members` row anywhere, which is how
every SSO and squad-less-invitation account begins, matched nothing but itself
and so could never be picked in the squad invite modal. It now also returns the
owners of the caller's workspaces and, only to a caller who can actually invite,
accounts with no squad membership. See 3e in
[access-control.md](access-control.md).
**Known cost 2: an empty mention picker for a grantee with no workspace
footing.** `/api/users/search` is bounded by `my_workspaces`, which is empty for
an account that owns no workspace and belongs to no squad. Such an account can
still hold read or write on an archive whose `squad_id` is NULL, because the ACL
boundary check in `POST /api/archives/:id/access` deliberately skips an archive
with no squad (there is no tenant to test the grantee against), and squadless
archives are easy to make: `ArchiveBrowser.jsx` passes `squadId={squadFilter}`
to `NewArchiveModal`, so creating one from `/archives` with no squad filter
selected produces an archive with `squad_id` NULL.
Concretely: an admin grants three contractors write access on a squadless
archive. Each opens a log there, types `@`, and the picker returns only
themselves, because none of them shares a workspace with anyone and none can
invite. Before this branch it returned every account on the install. They can
still read, write and comment; they just cannot address a mention to anyone.
**Not fixed, and widening the query is not the fix.** The natural widening,
"also return users who share an archive ACL with the caller", would hand a
single cross-tenant grant back the enumeration this boundary exists to stop, and
the archives it would key on are the exact rows (c) was written to prevent. The
honest options are to give the archive a squad, or to put the contractors in a
squad, either of which restores the picker through the ordinary path. Recorded
here so the empty picker reads as a known trade-off rather than a new defect.
**The fix is prospective, and there is no cleanup migration on purpose.** Rows
created through (a), (b) and (c) before the fix still resolve, because each is
an ordinary row the fragments read correctly. No query can separate one of them
from an install that used these routes exactly as they behaved, so deleting
automatically would destroy legitimate data. `docs/security.md` ships three
read-only enumeration queries instead; the disposition is the operator's.
Those three queries were run, not just written: against MySQL 8 with `init.sql`
loaded and a synthetic fixture holding one planted squad, a second planted squad
alibiing the first, a planted archive, a cross-tenant user grant and a
cross-tenant squad grant, alongside legitimate rows for a workspace owner, a
genuine member, an admin and a second workspace. Each query reported exactly its
attack rows and none of the legitimate ones. The first query is the one that
needs the care: the naive membership test returns **nothing** on that fixture,
because planting a squad enrols the planter as a member of the very workspace
under examination.
**A second silencing shape was found on re-run and fixed the same day.**
Excluding the planter's own squads is not enough: one `squad_members` row from
any squad they did not create clears every plant they made in that workspace, so
the planter simply being onboarded properly afterwards, the most likely thing to
happen next, hid both planted squads and, through the ordering of the three
queries, the planted archive too. Membership is now read as of the row's
creation (`sm.joined_at <= s.created_at` in query 1, and against `p.created_at`
in query 2's extra clauses). Query 3 still under-reports a grantee who holds any
membership in the workspace, because an ACL grant carries no timestamp to test
against; that is stated in `docs/security.md` with a `JSON_CONTAINS` follow-up
for reading the grants held by whoever query 1 named, so an empty query 3 is not
read as proof of none.
## C. Design tensions, not defects
### C1. `canWrite` is evaluated once per collab connection
`collab.js:430`, at session setup. Revoking write access does not take effect
until the user reconnects. Deliberate (re-checking per message would be a query
per keystroke), but worth stating.
### C2. One session row per user (RESOLVED)
`generateSessionToken` used to reuse the user's existing row, so signing in on
a second device returned the first device's token and `POST /api/logout`
signed out everywhere; the row also held the raw token. It was also why
`POST /api/update-account` deletes the caller's session along with every other
after an email or password change: the caller's token was every holder's, so
sparing it spared a stolen one, and the next sign-in with the new credentials
was handed that fresh token too.
**Resolved** by W6-CDX-2 (branch `track/w6-cdx-2-hashed-sessions`): every
sign-in inserts its own row, stored as `hashSessionToken(token)`, a SHA-256
digest, with the minting flow in `sessions.auth_provider`, and a daily prune
reaps expired rows. Logout signs out one device; update-account's delete by
`user_id` still signs out every other device, and a later sign-in gets a row
of its own. The mechanism is in [request-lifecycle.md](request-lifecycle.md)
("Session tokens") and [data-model.md](data-model.md) section 4.
### C3. Rotating `GITHUB_CLIENT_SECRET` invalidates every stored token
The AES key derives from it via scrypt (`getTokenEncryptionKey` in `oauth.js`). There is no key version
and no re-encryption path; every user must re-link. Fine for a self-hosted
product, worth documenting in the ops runbook.
### C4. Watch rows outlive their resources
`watches` has a FK on `user_id` only (`init.sql:449`); `resource_id` is
polymorphic and unconstrained. Deleting a document orphans its watches. Harmless
(`routes/helpers/activity.js:190-194` bails when the log is gone) but unbounded.
### C5. Watcher fan-out is a sequential await loop
`routes/helpers/activity.js:207-221`, one `createNotification` per watcher, inside a
fire-and-forget promise. Fine at current scale; a heavily-watched archive would
make it slow.
### C6. The `conflict` sync status is unreachable
`github_links.sync_status` is `ENUM('clean','remote_ahead','local_ahead',
'diverged','conflict')` (`init.sql:350`) but `classifySync`
(`github.js:1085-1091`) returns only the first four. Conflicts are expressed as
a 409 response instead. Either the enum value is vestigial or a state was
planned and never wired.
### C7. A Google sign-in could attach a second Google account to one user (FIXED)
Found 2026-09-25 while moving the Google ladder into `services/identity.js`
(W6-CDX-4), by reading the source; not reproduced against Google. The ladder
looks the identity up by `provider_user_id`, and on a miss links by email with
no check that the matched user already has a **different** Google subject.
`oauth_accounts` was unique on `(provider, provider_user_id)` only (`init.sql`,
`uq_provider_user`), so the insert succeeds and the user ends up with two
Google rows. The address has to be Google-verified to get that far, so the
realistic case is a recycled Workspace address or a deleted and recreated
Google account, which is exactly what the OIDC ladder refuses as
`identity_conflict` (spec Decision 3). The seam moved the Google branch without
changing it, because W6-CDX-4 is a pure refactor with zero test edits; whether
Google should also refuse is a behaviour change for its own session.
**Fixed** in the same PR (#56), after its refactor commits, on a gate approval
of 2026-09-25. `resolveGoogleIdentity` in `services/identity.js` now asks,
after the email match and before the link insert, whether that user already
holds a Google row (`SELECT id FROM oauth_accounts WHERE provider = 'google'
AND user_id = ?`). The subject lookup has just missed, so any row it finds is
another subject: the answer is `identity_conflict` and nothing is written. The
callback redirects to `/?oauth_error=identity_conflict`, which `Std_Layout.jsx`
shows as "This email is already linked to a different Google account. Ask your
administrator to relink it." (it used to fall through to "Google sign-in
failed. Please try again.", which invites a retry that cannot succeed).
Relinking is by hand: the account's owner unlinks Google from the account menu
(`POST /api/oauth/google/unlink`, which needs a password set), or an operator
deletes the old row. The check runs only on the link-by-email path, after the
`email_conflict` refusal, so a linked subject, an unknown person and the
auto-create path issue the same queries as before. Covered call by call in
`tests/services/identity.test.js` (both policies, no INSERT) and through the
route in `tests/routes/oauth-google-seam.test.js` (the redirect, no session),
each confirmed red against the unfixed seam.
**The race is closed by a key, for both providers** (same PR, gate approvals
of 2026-09-25). The check above is a SELECT before an INSERT, so two different
Google subjects signing in for one user at the same instant could both pass it,
and the GitHub link callback has the same shape (it updates the caller's
GitHub row, or inserts one when there is none). `oauth_accounts` now carries
`UNIQUE KEY uq_oauth_user_provider (user_id, provider)` (`init.sql`, and
`migrations/2026-09-25-oauth-one-link-per-provider.sql` for existing
installs), so the INSERT that lands second fails with `ER_DUP_ENTRY`, and
both paths answer that error, when it names this key
(`isSecondLinkForProvider` in `services/identity.js`), with a named refusal
instead of a 500: `resolveGoogleIdentity` with the same `identity_conflict`,
the GitHub callback with `/account?github_error=link_conflict`, which the
account page's Linked Accounts panel explains (it now shows a status line for
every `github_error` code and for `github_linked=1`; before, it ignored them
all). A duplicate on the subject key `uq_provider_user`, which is the same
account racing itself, still throws as before on both. Proven against MySQL
8.4 in `tests/integration/oauth-one-link-per-provider.test.js`, which holds
each race open with a lock on the user's own row until both attempts wait at
their INSERT's foreign-key check (at any isolation level): two
Google sign-ins through `resolveIdentity` must end with one link and one
`identity_conflict`, and two GitHub links through the real initiation and
callback routes with one `github_linked=1` and one `link_conflict`. Each was
red without the key (both linked) and red without its catch (a thrown
duplicate, and a 500 for GitHub).
**Found on the way, and fixed with it:** the GitHub callback's relink path
(the caller already has a GitHub row) UPDATEd that row onto whichever account
came back from GitHub without asking who held it, so relinking to an account
another user holds ran into `uq_provider_user` and a 500. It now looks the
holder up first and refuses as `already_linked_other`, as the first-link path
always did; the live test above proves it through the real routes.
Two limits stand. **The fix does not undo a double link made before it**, but
it no longer lets one pass silently: the migration refuses, deleting nothing,
while any user holds two links to one provider, naming the query that finds
them, `SELECT user_id, provider FROM oauth_accounts GROUP BY user_id, provider
HAVING COUNT(*) > 1`. The operator resolves each pair by hand (the CHANGELOG's
Migration section says how) and re-runs it. **It is still not reproduced
against Google itself.**
### C8. `index.css` redeclares four names the design package owns
The vendored `core.css` (W6-CDX-21) says a consumer never redeclares a name it
defines. `index.css`'s `:root` still declares `--brand-blue`, `--radius-sm`,
`--radius-md` and `--radius-lg`, and because it loads after `core.css` on the
same selector, Codex's values win for the whole page (the radii differ: 4, 8
and 12 px against 6, 10 and 14). `applyPrefsToDOM` also writes `--brand-blue`
on the root element. Nothing is wrong on screen today; the tension is that the
suite UI plan's legacy-alias bridge (W6-CDX-22, its Task 2.1) keeps
`--brand-blue` declared, as `var(--cx-accent)`, which is still a redeclaration
of a primitive. Whether the bridge renames it or the package's rule gets a
recorded exception is W6-CDX-22's call. `tests/design/codex-css.test.js` pins
the overlap to exactly these four, so it can shrink but not grow.
**Resolved in W6-CDX-22:** the bridge keeps the redeclaration as a recorded
exception (`--brand-blue` is `var(--cx-accent)`, and the three radii pointed at
Codex's own `--cx-radius-*`). The names stayed because the legacy names were
the bridge's contract. W6-CDX-38 then retired the radii and the two hover names,
so the bridge is 22 names and only `--brand-blue` still overlaps core.
### C9. The pool parses DATETIME as Node local time, so every DATETIME read is right only while Node runs in UTC
Found 2026-10-11 in W6-CDX-15 review. `mysql_connect.js` builds the pool from
`connectionOptions` (`mysql_connect.js:53-58`) and sets no `timezone`, so mysql2
parses each DATETIME as a local-time `Date` in the Node process. Every write of
these columns is UTC by convention (`UTC_TIMESTAMP(3)` in `services/webhook-worker.js`
and `routes/webhooks.js`). A
DATETIME read in a process whose zone is not UTC is therefore off by that zone's
offset. The image sets no `TZ` (`cloudcodex/Dockerfile`, `node:20-slim` base), so Node
runs in the container's default zone, UTC unless the host overrides it. Today's
deployments are correct for that reason, not by design.
The webhook console avoids the driver's parse: its UTC columns are read as text
(`utcText` in `routes/webhooks.js`, `DATE_FORMAT`) and its TIMESTAMP columns as
epoch seconds (`UNIX_TIMESTAMP`), and `tests/integration/webhooks-console-time.test.js`
proves a stored UTC instant reads back unchanged under `TZ=Europe/London`. Nothing
else does. Setting `timezone: 'Z'` on the pool would fix the reads but reinterpret
every DATETIME in the codebase at once, so it is a deliberate, separate change, not
a side effect of a webhook session. Until then, a deployment that sets `TZ` for any
reason breaks those reads.
### C9b. `users.email` is unique under an accent-insensitive collation
`init.sql:52` declares `email VARCHAR(255) NOT NULL UNIQUE` under the database default,
`utf8mb4_0900_ai_ci`, which treats `é` and `e` (and case) as the same character. W6-CDX-5 made the
OIDC identity lookups compare the lowercased address in binary (`services/identity.js:127`, `:151`),
so a verified address that differs only by an accent no longer matches someone else's account. The
UNIQUE key itself is still accent-insensitive repo-wide: two real mailboxes that differ only by an
accent cannot both hold a Codex account, and the second sign-up is refused as a duplicate. Changing
it needs a collation migration on `users.email` across existing installs (and a check that no
current rows collide under the new collation), so it is recorded here rather than done in an
identity PR. Found by W6-CDX-5's review, 2026-10-11.
## D. Stale claims in the root `CLAUDE.md`
Corrected in this pass, listed here so the drift pattern is visible:
- **Subsystems missing entirely** from the "Application areas" section:
notifications, activity log, watches, mentions, GitHub bidirectional sync,
live code embeds, PR-as-document, squad-to-GitHub-Team sync. Eight of the 25
tables and roughly a third of the routes were undocumented.
- **`tests/helpers.js` has no `ADMIN_USER`**, contrary to the testing section.
It exports `TEST_USER`, `TEST_USER_2`, `mockAuthenticated`,
`mockUnauthenticated`, `resetMocks`.
- **`tests/setup.js` mocks `fs/promises` only partially**, `mkdir` and `unlink`.
The doc's "filesystem globally" reads as more complete than it is.
- **`useGitHubStatus` is `.jsx`, not `.js`.**
- **The comment "no external job queue"** is accurate in spirit, but
`server.js` runs three in-process daily prunes (`pruneOldActivity`,
`pruneExpiredSessions` and `pruneWebhooks`) and a two-second delivery tick
(`startWebhookWorker`), which are scheduled jobs by another name.
## E. Things not investigated
Honest gaps in this pass, so nobody mistakes silence for a clean bill:
- `src/pages/` beyond routing and the editor's extension stack. `AdminPage.jsx`
(842 lines), `WorkspacesPage.jsx` (673), `ArchiveBrowser.jsx` (1159) and
`ExploreBrowser.jsx` (430) were not read line by line.
- `src/index.css`, 8844 lines, not read.
- `seed.sql`, 67 KB of sample data, not audited.
- `routes/avatars.js`, `routes/doc-images.js`, `routes/favorites.js` were read
only at the route-signature level.
- The `docs/api/*.md` contracts were not diffed against the actual handlers.
That is the obvious next pass and would likely surface more drift.
- No runtime verification of anything **in this pass**. No app was started, no
browser opened, no query run against a live database. The test suite was run
(57 files, 1128 tests, green) but it mocks the database, so it validates
handler logic and not schema interaction. A later pass (the first-run
experience work) did run the app against a real database and a real browser
and produced B8 through B10 above; the rest of this document is still
static-read-only.
## F. Open questions from W6-CDX-8 (OIDC-only mode), not defects yet
- **F1. (Closed: the machine credential was removed in CDX-SA-3, 0.15.0.)** `SERVICE_TOKEN` and the machine
JWT variables no longer exist, so an OIDC-only instance has no machine path to refuse.
- **F2. GitHub linking is open on an OIDC-only instance.** It links an account already signed in, so it is not a
sign-in, but a OIDC-only operator may expect it off. Not changed.
- **F3. Migration order on instances that already ran `2026-10-14-session-rotation.sql`.** `2026-10-12-user-deactivation.sql`
sorts before it. The runner's handling of an out-of-order pending file on such an instance is not verified here.
- **F4. The socket sweep's latency is one minute, by design.** A socket authenticated before a deactivation stays
open for up to 60 s. Closing sooner would need a push from the deactivation path, which does not exist yet.
## G. Open questions from W6-CDX-26 (the suite front door): G1 and G2 closed by CDX-SA-2, G3 stands
- **G1. (Closed: the suite front door was removed in CDX-SA-2.)** The live run against an OIDC issuer is not recorded here. The plan's Task 6.3 wants a signed-out
deep link to `/archives/:a/doc/:l` to land on the document after the bounce, with the sending application signed in, and
with no issuer session to land on the issuer's login first. That needs an OIDC issuer and a second
application on the same domain. Checked here only by the unit and route tests and the captures.
- **G2. (Closed with G1.)** The silent-return path is not exercised end to end. On a live issuer session, the bounce should return
the person silently to the document. The tests pin the URL and the loop guard, not the issuer's answer.
- **G3. The bare root (resolved 2026-10-11, still in force for an OIDC-only instance).** Kyle chose that a signed-out bare `/` goes straight to the issuer.
Codex now does this where OIDC is the only sign-in (`decideSignedOut`, `src/hooks/useSignedOutTarget.js`),
and keeps the screen for a local-sign-in instance. A sibling deployment's bounce flag is not changed by this branch.
## H. Carried from the retired suite UI record (CDX-SA-2), not defects
The record `suite-ui-alignment-record.md` was retired by CDX-SA-2, along with the shared preference cookie it
described. Its live items are here; its closed decisions are not carried, because the code no longer has them.
- **H1. The colour gamut is open.** The contract's colour steps are not all inside sRGB. Measured by converting each
`oklch(L C H)` to linear sRGB, 19 of 64 hue-theme-step combinations fall outside it (dark wash for 232; the dark text,
fill and wash for 200 and 175; the light wash for 355, 275 and 50), and 29 of 80 once the dark avatar step counts.
The browser and the vendored gate both clip, and every pair still passes on the clipped colour. The upstream product's
count for the same three hues (17) is not reconciled, since its script is not in this repository. A change to a step
is a versioned change to the table in `tests/design/theme.json`.
- **H2. Decisions taken without sign-off that still concern Codex's own interface.** Each was recorded as "decided
pending Kyle" and stands until Kyle confirms or reverses it: the sixth binding `--cx-accent-fill-hover` (the primary
button's hover on a filled accent is the dark hover step in both themes); `--cx-accent-dark`, derived per hue so the
white avatar label follows the pick; the light `--cx-on-accent` is the table's `oklch(0.16 0.03 240)`, and the
presence label constant `INK_LUMINANCE` is pinned to it; the light hover washes of the deferred families raised to a
0.025 floor, and the light black code washes moved to a 0.06 to 0.15 ink wash; the PageHeader on GitHub settings
(Codex has no GitHub settings page; the connection lives on the GitHub page).
- **H3. Not yet run.** A theme chosen in Codex has not been checked on a shared parent domain.
That check is void now: the preference is per host, so there is nothing to share.
## S. Standalone review, 2026-10-14 (CDX-SA-6)
- **S1. Entra sends no standard `email_verified` (IS8, open).** Microsoft's optional-claims reference lists no
`email_verified`; `xms_edov` is a different signal, and `services/oidc.js` does not read it. A sign-in from an
Entra tenant is refused `email_not_verified`. Options for Kyle: document Entra as unsupported until a mapping
exists; read `xms_edov` as the verification source for issuers that omit `email_verified` (a security decision,
since it changes who may link an account); or add a documented opt-out (not recommended). Not built.
- **S2. The legacy-cookie premise depends on the OIDC TTL (see `docs/stability.md`).** Sessions expire at
`created_at` plus their lifetime and are not extended by use. A long `OIDC_SESSION_TTL_HOURS` keeps a pre-0.13.0
OIDC row alive past 1.0.0's date, so the switch's removal needs a TTL check.
- **S3. Vendored NOTICE names the private sibling product.** `cloudcodex/vendor/cloud-city-design/NOTICE` names a
product in its trademark line. The vendor copy is pinned by checksum and is not edited here; fix upstream.
- **S4. Applied migration comments are immutable.** `migrations/2026-10-11-user-identities.sql` and
`migrations/2026-10-15-invitation-grants-admin.sql` name providers or a sibling product in comments. Migration
files are checksummed, so the comments stay as they are.