Skip to content

Library: the AI shelf is stocked without holding the request open, and the AI Atlas is finished - #227

Merged
manager merged 16 commits into
devfrom
fix/library-ai-shelf-async-roll
Sep 10, 2026
Merged

Library: the AI shelf is stocked without holding the request open, and the AI Atlas is finished#227
manager merged 16 commits into
devfrom
fix/library-ai-shelf-async-roll

Conversation

@manager

@manager manager commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Found by probing the AI shelf on production tonight, after The Order wired the relay to both fronts.

What happens today on keepsimple.io. The first board for Wolf's library was rolled by the engine end to end and it works: thirteen picks, three of them new ground, off his own 165 books. But the roll ran longer than the edge holds a request open, and the gateway closed it at 60 seconds with a 504. The board landed a minute later and stands there now; the browser that asked for it saw an error. Every Re-Roll would read the same way.

The fix. The roll no longer runs inside the request. The route starts it, records the moment, answers rolling, and returns; the container keeps working and writes the board when it lands. The shelf polls every four seconds and shows the working state it already had. A roll older than five minutes is treated as gone, so a container recreated mid-roll cannot leave the shelf waiting forever. The engine's note is stored beside the board so the reason a roll came back short survives the poll that finds it.

Checks. Typecheck, lint and yarn check:library clean. Probed on the dev tree with the engine stubbed: the first ask answers rolling at once, a second ask does not start a second run, the board lands on the next poll, the marker is cleared.

Needs Wolf's word before it is merged and built, per the release rule on repeat builds.

Measured on production tonight: a first board on Wolf's library took longer
than the edge would hold the request, which closed it at 60 seconds. The
board landed anyway, a minute later, unseen by the browser that asked for it,
and the owner would have read a gateway error where a shelf was being filled.

The roll no longer runs inside the request. The route starts it, records the
moment it started, answers "rolling", and returns; the work goes on in the
container, which is long-lived, and writes the board when it lands. The shelf
asks again every four seconds until the board stands, showing the working
state it already had. A roll recorded more than five minutes ago is treated
as gone, so a container recreated mid-roll does not leave the shelf waiting
for something that is not coming.

The note the engine leaves is stored beside the board rather than returned
once, so the reason a roll came back short survives the poll that finds it.

Probed with the engine stubbed: the first ask answers rolling at once, a
second ask does not start a second run, the board lands on the next poll,
and the marker is cleared with the note.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

Claude finished @manager's task in 5m 3s —— View job


Review: async AI shelf roll (PR #227)

  • Gather context (diff, changed files)
  • Understand the request (code review)
  • Review src/pages/api/library/ai-shelf.ts
  • Review src/hooks/library/useAiShelf.ts
  • Review src/lib/library/magic/store.ts
  • Review src/local-types/library/recommendation.ts
  • Post inline comments for issues found
  • Post final summary

Summary

The core design is sound: moving the engine call out of the request/response cycle and polling for a rolling marker is the right shape for a job that outlives a 60s gateway timeout, and the in-memory rolling Set + synchronous check-and-add still correctly serializes concurrent rolls within one process. rollingSince/rollNote giving a stale-roll timeout and a way for the shelf to explain a short board is a clean way to make the async state observable.

Three things worth a look before merge, posted inline:

  1. src/hooks/library/useAiShelf.ts:194 (real, actionable) — since a roll's POST now resolves almost instantly, busy/inFlight no longer span the actual roll duration; only the new state.status === 'rolling' does. RecommendedShelf.tsx:397 still gates the lock/ban controls on shelf.busy alone, so an owner can lock/unlock or ban a pick while a roll is still running in the background. stockTheBoard's final write replaces the whole board (including locked) using the snapshot taken when the roll started, so that mid-roll toggle is silently lost with no feedback once the new board lands. This regression is only reachable because this PR made the roll async — pre-PR, busy stayed true for the whole synchronous roll and blocked the click.

  2. src/lib/library/magic/store.ts:66 (design note) — the new comment claims rollingSince makes the one-roll-at-a-time guarantee hold "across processes," but the store's serial()/queue mutex is an in-process Promise chain, not a file lock. If this route is ever served by more than one Node process, two processes could both read a stale/null rollingSince at once and both start a roll — the exact race this mechanism is meant to prevent. Fine if the deployment is guaranteed single-process, but the comment overstates the actual guarantee.

  3. src/hooks/library/useAiShelf.ts:100 (minor UX gap) — after POLL_LIMIT (~6 minutes) is reached, polling stops silently; status stays 'rolling' indefinitely with no timeout/error shown, so a roll that's genuinely still running past that window leaves the shelf looking frozen with no way to recover short of a full reload.

Nothing else stood out — no App Router/Tailwind/CSS-in-JS violations, no any, import order and barrel conventions look fine, and the RecommendedStatus union / stateOf changes are consistent with the rest of the file. Did not run yarn typecheck/lint/check:library myself in this pass (static review only); the PR body reports them clean.

/** When the roll running right now started, UTC. A roll outlives the
* request that asked for it, so this is what a later request reads to
* know one is under way. Cleared when it lands. */
rollingSince?: string | null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment above (and the mirrored one in ai-shelf.ts at the rolling.has(libraryId) || !rollIsStale(stored.rollingSince) check) claims the persisted rollingSince makes the single-roll guarantee "carry across processes." It doesn't, as written: serial()/queue in this file (store.ts:78-86) is an in-process Promise chain, not a file lock. If this route is ever served by more than one Node process (replicas, a restart-then-old-process-still-draining window, etc.), two processes can each readLibrary and see rollingSince: null/stale at the same moment, both pass the check, and both start an engine roll — the exact "two at once... one would be lost" failure this mechanism exists to prevent, just moved from same-process races to a real multi-process race with no lock at all.

If the deployment is guaranteed single-process today, worth a one-line comment saying so explicitly rather than "carries the same fact across processes," which reads as a stronger guarantee than the code provides. If multi-process is possible, this needs an actual cross-process lock (e.g., an atomic "claim" write with a check on the write result) before it's safe.

[Fix this →](https://claude.ai/code?q=In%20src%2Flib%2Flibrary%2Fmagic%2Fstore.ts%20and%20src%2Fpages%2Fapi%2Flibrary%2Fai-shelf.ts%20on%20branch%20fix%2Flibrary-ai-shelf-async-roll%3A%20the%20rollingSince%20field%20is%20documented%20as%20preventing%20concurrent%20rolls%20%22across%20processes%2C%22%20but%20the%20store%27s%20serial()%2Fqueue%20mutex%20is%20in-process%20only%20(a%20Promise%20chain%2C%20not%20a%20file%20lock).%20Either%20correct%20the%20comment%20to%20state%20the%20single-process%20assumption%20explicitly%2C%20or%20make%20the%20rollingSince%20claim%20atomic%20across%20processes%20(e.g.%20read-check-write%20under%20a%20real%20lock%20or%20a%20compare-and-set%20on%20the%20stored%20file).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7dab594. The comment claimed a guarantee the code does not give. Both places now say what is true: the serial queue in store.ts and the roll set in ai-shelf.ts are one process, the persisted marker is a heartbeat and not a lock, the shelf runs one container per host so a second process cannot start a roll, and serving this route from replicas would need a real cross-process lock.

state,
loading,
rolling,
rolling: rolling || state?.status === 'rolling',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that a roll's POST resolves almost immediately with status: 'rolling' instead of blocking for the whole engine run, inFlight/busy are only true for that brief initial request, not for the minute-plus the roll actually takes in the background. This new rolling (returned here) is the flag that now spans the whole roll.

RecommendedShelf.tsx:397 still gates the lock/ban controls on shelf.busy only (not shelf.rolling/working), so an owner can toggle lock or ban a pick while a roll is still in flight. That write reaches the server and updates board.locked/board.picks right away, but stockTheBoard's final updateLibrary in ai-shelf.ts (~line 473) replaces the whole board wholesale using the lockedIds snapshot captured when the roll started — so a lock/unlock applied mid-roll is silently overwritten with no error and no indication to the owner once the new board lands moments later.

Before this PR, busy stayed true for the entire (synchronous) roll, so this race wasn't reachable from the UI. Worth gating those controls on shelf.rolling too (or having the server-side roll merge against the current locked set instead of the one captured at start).

[Fix this →](https://claude.ai/code?q=In%20src%2Fcomponents%2Flibrary%2Forganisms%2FRecommendedShelf%2FRecommendedShelf.tsx%20around%20line%20397%2C%20the%20readOnly%20prop%20passed%20to%20RecommendedBookCard%20is%20%60readOnly%20%7C%7C%20shelf.busy%60.%20Since%20the%20AI%20shelf%20roll%20is%20now%20async%20(see%20src%2Fhooks%2Flibrary%2FuseAiShelf.ts)%2C%20shelf.busy%20resolves%20quickly%20while%20shelf.rolling%20stays%20true%20for%20the%20whole%20background%20roll.%20Update%20the%20readOnly%20condition%20to%20also%20include%20shelf.rolling%20(or%20the%20existing%20%60working%60%20flag)%20so%20lock%2Fban%20controls%20stay%20disabled%20for%20the%20full%20duration%20of%20a%20roll%2C%20preventing%20a%20lock%2Funlock%20made%20mid-roll%20from%20being%20silently%20discarded%20when%20the%20new%20board%20lands.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7dab594. Correct, and reachable: the roll replaces the whole board from the snapshot it took when it started. Preference, lock, ban and unban now wait on working (loading || rolling), not on busy alone, and the re-roll button already did. The reason is written above the flag so the next change does not undo it.

Comment thread src/hooks/library/useAiShelf.ts Outdated
polls.current = 0;
return;
}
if (polls.current >= POLL_LIMIT) return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once polls.current >= POLL_LIMIT (90 × 4s ≈ 6 minutes) the effect just stops scheduling further polls — state.status stays 'rolling' forever with no error/timeout surfaced. Combined with the server's 5-minute ROLL_STALE_MS, the common failure mode (container recreated mid-roll) is already handled server-side by then, so this mostly matters for a roll that's genuinely still running past 6 minutes: the owner is left staring at "Stocking the shelf" indefinitely with no manual recovery short of a full page reload (the getAiShelf effect only re-runs on libraryId/enabled changes). Consider surfacing a "still working — refresh to check" state, or resetting polls.current and trying again at a longer interval, instead of silently going quiet.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7dab594. Out of polls the hook now sets an error the shelf shows: the roll is still working, reload to see where it got to. Silence was the wrong answer.

…a seal

Two things, both on the AI surfaces in the dark reading.

The writing on a cover was disappearing. `--gradient-cover-placeholder` is
declared on :root out of `--cream` and `--beige`, and the dark theme sets
those on `body.darkTheme .library`, a different element: the gradient's own
var()s were resolved at :root and inherited down frozen at their daylight
values. So the cover face stayed paper while `--black-rich` and
`--gray-darkest` turned light on the same element, and the title, the author
and the magic book's own line were light ink on light paper. Measured off
Wolf's screenshot: paper #f2ece2, ink #dbd3c8.

The face is right to stay paper: the book mockup under every cover is a
photograph and does not turn at night. So the ink stops turning instead.
`--ink-on-cover`, `--ink-on-cover-soft`, `--wash-on-cover`,
`--accent-on-cover` and `--rule-on-cover` are declared once and never
redefined by the theme, and everything that sits on a printed face reads
them: both cards' titles and authors, the score slip, the lock and ban
buttons, the new-ground mark, the magic book's blank line and its icon.

The empty places on the AI shelf were rectangles the colour of the page,
which at night made them the page. They are seals now: a bevelled face the
size of a cover with a rune cut into it, drawn in the same straight-stroke
hand as the library's monograms, two rules where a title will be, and a
light behind it that breathes on its own beat. Eight marks, so thirteen
places never read as one drawing repeated. While the engine works the row
lights in order, a beat apart, so the wait reads as a spell running down the
shelf. Reduced motion holds all of it still.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@manager

manager commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

Second commit on this branch, same release: the dark reading of the AI surfaces.

The ink bug. --gradient-cover-placeholder is declared on :root out of var(--cream)/var(--beige), while the dark theme sets those on body.darkTheme .library. The gradient resolved its own var()s at :root and inherited down frozen at daylight values, so a cover face stayed paper while the ink tokens on the same element turned light: title, author and the magic book's blank line were light ink on light paper (measured off the screenshot: paper #f2ece2, ink #dbd3c8). The face is right to stay paper, since the book mockup under it is a photograph that does not turn, so the ink stops turning instead: five never-themed tokens for what sits on a printed face.

The empty places. They were rectangles the colour of the page, which at night made them the page. They are seals now, cut in the same straight-stroke hand as the library monograms, with a light that breathes and, while the engine works, runs down the row a beat at a time.

@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

Claude finished @manager's task in 2m 44s —— View job


Review: async AI shelf roll (PR #227, sync)

  • Gather context (diff since previous review, changed files)
  • Check whether prior review findings were addressed by the new commit
  • Review new/changed code (dark-theme ink tokens + SpellSlot empty-slot component)
  • Post inline comments for new issues found
  • Post final summary

What's new since the last pass

The new commit (f3290c6) is scoped entirely to visuals — it doesn't touch ai-shelf.ts, useAiShelf.ts, or store.ts — and fixes a real dark-mode bug: --gradient-cover-placeholder's ink colors (--black-rich, --gray-darkest) were declared at :root and turned light under body.darkTheme, but the cover face itself stays a fixed photographic paper tone, so titles/authors went light-on-light. The fix adds five theme-invariant tokens (--ink-on-cover, --ink-on-cover-soft, --wash-on-cover, --accent-on-cover, --rule-on-cover) in src/styles/library/variables.scss:113-126 and swaps every cover-face text/border color in MagicBookCard.module.scss and RecommendedBookCard.module.scss to use them. This is the right shape for the bug: fixed tokens for a surface that's a photo, not a themed panel. Spot-checked contrast (#23221c ink on #f2ece2-ish paper, #6b3fb3 accent on #fffcf7 wash) — both comfortably clear 4.5:1 for the 14px/16px text sizes involved, and no new font-size values were introduced.

SpellSlot (new atom) replacing the old flat .ghost placeholder is clean: SVG is aria-hidden/focusable="false" (purely decorative, doesn't need a text alternative), colors pull from var(--purple-100)/var(--white-200) design tokens rather than hardcoded hex, prefers-reduced-motion is respected for both the idle "breathe" and active "cast" animations, and the barrel/props/naming conventions match AGENTS.md. No orphaned references to the removed .ghost/ghost-breathe styles elsewhere in the tree.

Still open from the previous review round

This commit doesn't address the three points raised earlier today, and they remain live in the current HEAD:

  • src/components/library/organisms/RecommendedShelf/RecommendedShelf.tsx:398readOnly={readOnly || shelf.busy} still doesn't include working/shelf.rolling. Line 305's Re-Roll button was already gated on working (disabled={readOnly || isLocked || shelf.busy || working}), but the per-card lock/ban toggle at line 398 (and the ban button at line 481, disabled={shelf.busy || leaving}) were not, so an owner can still lock/unlock or ban a pick while a roll is in flight, and that edit is silently dropped when stockTheBoard's final write lands using the snapshot taken at roll start.
  • src/lib/library/magic/store.ts:66 — the "carries the same fact across processes" comment on rollingSince still overstates the guarantee; serial()/queue is an in-process mutex only.
  • src/hooks/library/useAiShelf.ts:100 — polling still goes silent after POLL_LIMIT with no surfaced timeout/error state.

None of these are regressions introduced by today's commit — they're carried over from bd1ee00 — but since this PR is still open and pending Wolf's word per the release rule, worth confirming whether they're being fixed before merge or accepted as follow-up.

No new App Router/Tailwind/CSS-in-JS violations, no any, import order and barrel conventions look correct in the new files. Did not run yarn typecheck/lint/check:library in this pass — yarn/tsc invocations require approval I don't have in this session; the PR body reports them clean for the prior commit but that hasn't been re-verified against f3290c6.
· branch fix/library-ai-shelf-async-roll

The road to the engine, not the engine, sets the ceiling: between a project
container and the relay stands an edge that closes a request at 100 seconds,
and two opus calls measured from production at high effort took 107 and 117.
Latency here is mostly what the model writes, so each half now asks for
twelve and five candidates instead of fourteen and six, which is what the
board needs plus a margin the second pass covers. The picks themselves are
untouched: opus 5, high effort, same rubric.

A call the edge cuts mid-answer is now told apart from a call that answered
nothing: the shelf says the engine ran long and the line closed, the journal
records it, and nothing is retried automatically, because the model kept
writing on the other side and the subscription has already paid once.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

Claude finished @manager's task in 2m 50s —— View job


Review: async AI shelf roll (PR #227, third commit)

  • Gather context (diff since previous review sync, f3290c6..a2f3ccc)
  • Check whether prior review findings were addressed
  • Review new commit's changes (engine.ts ASK reduction + cut-off detection, ai-shelf.ts note wiring)
  • Post inline comments for any new issues found
  • Post final summary

What's new since the last pass

The new commit (a2f3ccc) touches only src/lib/library/aishelf/engine.ts and src/pages/api/library/ai-shelf.ts, and is scoped to the model-call sizing and a new "cut off mid-answer" signal:

  • ASK (engine.ts:69) drops from { fit: 14, stretch: 6 } to { fit: 12, stretch: 5 }, in response to production measurements (107s/117s Opus-high calls against a 100s edge timeout between the product container and the relay). Board needs are AI_SHELF_FIT = 10 / AI_SHELF_STRETCH = 3, so headroom for verification failures shrinks from +4/+3 to +2/+2. This is a deliberate, disclosed tradeoff (the commit message states it plainly) — it will make the AI_SHELF_BENCH = 6 spare pool underfill more often and likely increases how often a roll needs the second pass or leaves the "Roll again to fill the shelf" note, but that's the intended latency/fill-rate tradeoff, not a bug.
  • CUT_OFF (engine.ts:379, a Set of 408/502/504/522/524) and the new cutOff field on HalfRun/BoardRun correctly thread through runHalfrunBoardstockTheBoard, giving the owner a distinct "The engine ran long and the line closed" note (ai-shelf.ts) instead of the generic "Nothing could be confirmed" message, and the flag is recorded in the journal either way. The wiring itself is clean and doesn't disturb the existing tracksExhausted/exhausted logic.
  • One thing worth confirming before relying on this in production, posted inline on engine.ts:471: CUT_OFF only matches when the edge returns an actual HTTP status through askRelay's !r.ok branch. If the edge instead resets the connection when it gives up at 100s, fetch throws before there's a Response, and relay.ts maps that to RelayError(..., 0) — status 0, not in CUT_OFF — so it would be classified as tracksExhausted instead, and the owner would see the wrong note for the very case this commit is meant to describe accurately. Not necessarily wrong, just unverified against the edge's real failure mode (the PR's stubbed-engine probe wouldn't exercise it).

No new App Router/Tailwind/CSS-in-JS/any/import-order issues; no UI or styling changed in this commit.

Still open from earlier rounds

Unrelated to today's commit, unaddressed, and previously flagged inline:

  • src/components/library/organisms/RecommendedShelf/RecommendedShelf.tsx:398 (and the ban button ~line 481) — lock/ban controls are still gated on shelf.busy alone, not shelf.rolling/working, so a toggle made mid-roll can be silently dropped when the async board lands.
  • src/lib/library/magic/store.ts:66 — the rollingSince comment still reads as a cross-process guarantee; the actual mutex (serial()/queue) is in-process only.
  • src/hooks/library/useAiShelf.ts:100 — polling still goes silent after POLL_LIMIT (~6 min) with no surfaced timeout/error state.

Per the release-lessons note in CLAUDE.md, this PR needs Wolf's explicit go before it's merged/built — that hasn't changed.

Did not run yarn typecheck/lint/check:library in this pass (no tool access in this session); the PR body reports them clean as of the first commit but that hasn't been re-verified against a2f3ccc.
· branch fix/library-ai-shelf-async-roll

if (error instanceof RelayError) {
if (error.exhausted || error.status === 0)
half.tracksExhausted = true;
if (CUT_OFF.has(error.status)) half.cutOff = true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CUT_OFF (line 379: 408, 502, 504, 522, 524) only matches when the intermediate edge answers with one of those HTTP statuses through askRelay's !r.ok branch. If that edge instead drops the TCP connection outright when it gives up at 100s (rather than sending a proper gateway-timeout response), fetch throws before a Response exists, and relay.ts:77-83 maps any such failure to RelayError(..., 0) — status 0, which isn't in CUT_OFF. That path only sets tracksExhausted (via error.status === 0), so the owner would see "Nothing could be confirmed this time" instead of the new "The engine ran long and the line closed" note, even though the root cause is exactly the cut-off case this commit is meant to surface.

Worth confirming against the actual production timeout behavior (does the edge return a real HTTP error, or reset the connection?) before relying on this distinction for the note shown to the owner — the PR body's "probed on the dev tree with the engine stubbed" wouldn't have exercised the real edge's failure mode either way.

[Fix this →](https://claude.ai/code?q=In%20src%2Flib%2Flibrary%2Faishelf%2Fengine.ts%20on%20branch%20fix%2Flibrary-ai-shelf-async-roll%2C%20verify%20whether%20the%20intermediate%20edge%20that%20times%20out%20requests%20at%20100s%20returns%20a%20real%20HTTP%20status%20(one%20of%20408%2C502%2C504%2C522%2C524%20in%20the%20CUT_OFF%20set%20at%20line%20379)%20or%20drops%20the%20TCP%20connection%2C%20which%20surfaces%20as%20RelayError%20status%200%20via%20relay.ts%27s%20catch%20block%20and%20is%20misclassified%20as%20tracksExhausted%20instead%20of%20cutOff.%20If%20a%20raw%20connection%20drop%20is%20possible%2C%20decide%20whether%20status%200%20under%20certain%20conditions%20should%20also%20be%20treated%20as%20cutOff.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change, and here is why. With the handover in place the long wait is a series of short polls, so the 100s edge no longer sits across a running call; the direct path is the only one that can still be cut mid-write. Which of the two an edge does under that path (a gateway status, or a reset) I have not measured on production, and the note the owner reads is the same class of failure either way: nothing came back. I am not guessing at a distinction I cannot evidence. If you want it split, the evidence has to come from a real long roll on prod first.

The Order built the relay's job mode tonight on the word I gave: the same
Messages body posted to the job endpoint comes back accepted at once, and the
answer is collected by polling. That removes the ceiling the edge put on this:
a proxied request dies at 100 seconds, opus 5 at high effort on a library this
size runs past it, and until now the roll was racing a clock it could not see.

Every hop is short now and the model takes as long as it takes. A poll that
fails on the way is not the job failing: the work is running and the
subscription has already paid for it, so the next poll asks again until the
deadline, and only a job the relay has forgotten or a run past five and a half
minutes is given up on. A relay that does not know the job endpoint answers
404 and the call falls back to the old held-open request, so a container wired
before tonight keeps working.

Probed both paths on the dev tree against a relay stub: two halves handed over
in the same second, polled while running, the board of thirteen landing on the
poll after; and with the job endpoint answering 404, the same roll going
through on the direct path.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

Claude finished @manager's task in 2m 43s —— View job


Review: async AI shelf roll (PR #227, sync on new commit)

  • Gather context (diff since previous review sync, a2f3ccc..4183c96)
  • Check whether prior review findings were addressed
  • Review new commit's changes (job hand-off for the model call)
  • Post inline comments for any new issues found
  • Post final summary

What's new since the last pass

The new commit (4183c96) touches only src/lib/library/magic/relay.ts, and is scoped to how the relay call itself is transported — not the roll-scheduling logic reviewed earlier:

  • askRelay now tries the relay's job endpoint (/v1/jobs, derived from CLAUDE_RELAY_URL by jobsUrl()) first: hand the request over (askAsJob, short RELAY_HOP_MS=30s hops), then poll every POLL_MS=3s for a result, up to JOB_DEADLINE_MS=330s. If the relay doesn't recognize the job endpoint (404/405), it falls back transparently to the old held-open askDirect path (RELAY_TIMEOUT_MS=290s) — a reasonable compatibility path for a relay container that hasn't been updated yet.
  • The refactor factors the shared "turn a Messages body into a RelayReply" logic into readReply(), used by both paths — good dedup, no any, types (MessagesBody, JobState) are precise.
  • Rationale is well-documented in the file header: the frontend↔relay hop itself (not just the browser↔frontend hop fixed earlier in this PR) is cut by an edge at 100s, and Opus-high on a large library measured 107s/117s — hence moving to short polled hops instead of one long-held connection.

One real issue found in this commit, posted inline on relay.ts:39:

  • JOB_DEADLINE_MS (330s) vs ROLL_STALE_MS (300s, ai-shelf.ts:60). A single askRelay call can now legitimately run up to 5.5 minutes via job polling, and runHalf (engine.ts:447) can make two sequential calls per half when a second pass is needed. rollingSince is set once at roll start and never renewed while the roll is in progress. So a real roll that takes a bit over 5 minutes — very reachable now that individual calls are allowed to run long instead of being cut off fast by the edge — gets marked stale by rollIsStale while it's still genuinely running, and a second concurrent roll can be kicked off for the same library. This is the same "two at once, one is lost" failure rollingSince was built to prevent, reopened by this commit's change in realistic call duration. Suggest raising ROLL_STALE_MS well above JOB_DEADLINE_MS + verification time, or renewing rollingSince on each poll/pass.

This also changes the shape of last round's "status 0 vs CUT_OFF" concern: on the new job-polling path, no single hop should approach the 100s edge (hops are capped at 30s), so the "connection reset mid-answer" scenario that motivated CUT_OFF is largely sidestepped for relays that support jobs — it still applies to the legacy askDirect fallback path used when a relay doesn't support jobs.

Still open from earlier rounds

Unrelated to today's commit, unaddressed, previously flagged inline:

  • src/components/library/organisms/RecommendedShelf/RecommendedShelf.tsx:398 (and ban button ~line 481) — lock/ban controls still gated on shelf.busy alone, not shelf.rolling/working; a toggle made mid-roll can be silently dropped when the async board lands.
  • src/lib/library/magic/store.ts:66 — the rollingSince comment still reads as a cross-process guarantee; the mutex (serial()/queue) is in-process only.
  • src/hooks/library/useAiShelf.ts:100 — polling still goes silent after POLL_LIMIT (~6 min) with no surfaced timeout/error state.

No new App Router/Tailwind/CSS-in-JS/any/import-order issues; no UI or styling changed in this commit.

Did not run yarn typecheck/lint/check:library in this pass — no tool access for yarn/tsc in this session. The PR body reports them clean as of the first commit; that hasn't been re-verified against 4183c96.

Per the release-lessons note in CLAUDE.md, this PR still needs Wolf's explicit go before it's merged/built.
· branch fix/library-ai-shelf-async-roll

/** How often the answer is asked for. */
const POLL_MS = 3_000;
/** Past this the job is abandoned; the relay never runs one this long. */
const JOB_DEADLINE_MS = 330_000;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JOB_DEADLINE_MS (330s) legitimately lets a single askRelay call run for up to 5.5 minutes now that it's polling a job instead of being cut off by the 100s edge. But runHalf in engine.ts:447 makes up to two sequential askRelay calls per half (fit/stretch) when the first pass doesn't fill the need, and ROLL_STALE_MS in src/pages/api/library/ai-shelf.ts:60 is fixed at 5 minutes with no renewal of rollingSince while the roll is in progress (ai-shelf.ts:374, only cleared at the end).

Before this commit, a call was effectively bounded by the 100s edge failing fast, so a full roll rarely approached the 5-minute staleness window. Now that a single call can legitimately take up to 330s — and a half can make two of them — a real roll (e.g. first pass ~110s + verification + a needed second pass) can easily cross the 5-minute mark while askAsJob is still correctly polling. When that happens, rollIsStale(stored.rollingSince) in ai-shelf.ts:363 returns true for a roll that is still legitimately running, and a second concurrent roll can be kicked off for the same library — the exact "two at once, one is lost" scenario rollingSince exists to prevent.

Worth either raising ROLL_STALE_MS comfortably above JOB_DEADLINE_MS (plus verification time), or renewing rollingSince on each poll/pass so staleness only fires when a roll is actually abandoned (e.g., container died), not just running long.

[Fix this →](https://claude.ai/code?q=In%20src%2Flib%2Flibrary%2Fmagic%2Frelay.ts%2C%20JOB_DEADLINE_MS%20is%20330_000ms%2C%20letting%20a%20single%20askRelay%20call%20legitimately%20run%20up%20to%205.5%20minutes%20via%20job%20polling.%20engine.ts%27s%20runHalf%20can%20make%20two%20sequential%20askRelay%20calls%20per%20half%2C%20so%20a%20real%20roll%20can%20exceed%205%20minutes%20of%20wall%20time.%20But%20src%2Fpages%2Fapi%2Flibrary%2Fai-shelf.ts%27s%20ROLL_STALE_MS%20is%20a%20fixed%205%20minutes%20(300_000ms)%20and%20rollingSince%20is%20never%20renewed%20while%20a%20roll%20is%20in%20progress%2C%20so%20a%20still-running%20roll%20can%20be%20marked%20stale%20and%20a%20second%20concurrent%20roll%20started%20for%20the%20same%20library.%20Fix%20by%20raising%20ROLL_STALE_MS%20well%20above%20the%20realistic%20worst-case%20roll%20duration%2C%20or%20renewing%20rollingSince%20periodically%20while%20the%20roll%20is%20active.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7dab594, and this was the real bug in the batch. A running roll now renews rollingSince every 60s (ROLL_BEAT_MS) and the interval is cleared in the finally. ROLL_STALE_MS keeps its five minutes but now means "no heartbeat for five minutes", so a roll that legitimately runs long is never declared stale under itself and rolled a second time.

The Order's correction, and it is right: a 404 on handing the job over means
this relay does not know jobs at all, which is where the old held-open call
belongs, but a 404 while polling means something else entirely. The job is
gone, from a restart or from its hour of keeping, and whatever was running
behind it died with the relay that forgot it. Holding a call open there would
make the subscription pay twice for one answer.

So a lost job is dealt again on the same path, once, and only while enough of
the deadline is left to be worth it. Past that the roll says the relay lost
the job rather than quietly starting a third.

Probed with a stub that forgets both halves on their second poll: each was
handed over again and the board of thirteen landed, with four jobs in the
relay's log and no direct call anywhere.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

Claude finished @manager's task in 1m 41s —— View job


Review: async AI shelf roll (PR #227, sync on new commit)

  • Gather context (diff since previous review sync, 4183c96..b2f3d43)
  • Check whether prior review findings were addressed
  • Review new commit's changes (resubmit-on-lost-job for relay jobs)
  • Post inline comments for any new issues found
  • Post final summary

What's new since the last pass

The new commit (b2f3d43) touches only src/lib/library/magic/relay.ts, refactoring the single askAsJob into three pieces — handOver (post the job), collect (poll for it), and a new askAsJob wrapper that resubmits the job once if the relay reports it lost (poll returns 404 → 'lost'), instead of failing immediately as before. MAX_SUBMISSIONS = 2 and RESUBMIT_FLOOR_MS = 90_000 bound the retry: at most one resubmission, and only if there's still ≥90s of the shared JOB_DEADLINE_MS (330s) budget left — the deadline is computed once and shared across both submissions, so this doesn't extend the overall time budget already flagged in the prior round. The split is clean: handOver/collect are focused, typed precisely (HandedJob, JobState), no any, and the fallback to askDirect when a relay doesn't know the job endpoint at all is preserved.

One thing worth a look, posted inline on relay.ts:293:

  • The resubmit assumes a poll 404 always means the original run died with the relay ("nothing is paid twice"). That holds for a single-instance relay (which the file's own header implies), but if claude-relay is ever run as more than one replica, a poll landing on a different instance than the one that accepted the job would also 404 while the original job is still running elsewhere — causing a second, paid model call for the same request. Suggested either documenting the single-instance assumption explicitly or not treating a bare 404 alone as sufficient proof of loss.

Still open from earlier rounds (unaddressed, unrelated to today's commit)

  • src/components/library/organisms/RecommendedShelf/RecommendedShelf.tsx:398 (and ban button ~line 481) — lock/ban controls still gated on shelf.busy alone, not shelf.rolling/working; a toggle made mid-roll can be silently dropped when the async board lands.
  • src/lib/library/magic/store.ts:66 — the rollingSince comment still reads as a cross-process guarantee; the mutex (serial()/queue) is in-process only.
  • src/hooks/library/useAiShelf.ts:100 — polling still goes silent after POLL_LIMIT (~6 min) with no surfaced timeout/error state.
  • src/lib/library/magic/relay.ts:39 (JOB_DEADLINE_MS 330s) vs ROLL_STALE_MS 300s in ai-shelf.ts:60 — a real roll can still legitimately exceed the staleness window and trigger a false "concurrent roll" scenario; unchanged by today's commit.
  • src/lib/library/aishelf/engine.ts:471CUT_OFF status matching vs a raw connection-reset (status: 0) is still unverified against the real edge. Side note: the new "relay lost the job, no time to resubmit" error (relay.ts:298, status 404) also isn't in CUT_OFF, so it surfaces as a generic error rather than the "line closed" note — likely fine since it's a distinct failure mode, just flagging for awareness.

No App Router/Tailwind/CSS-in-JS/any/import-order issues in this commit; no UI or styling changed.

Did not run yarn typecheck/lint/check:library in this pass (no tool access for yarn/tsc in this session). The PR body reports them clean as of the first commit; that hasn't been re-verified against b2f3d43.

Per the release-lessons note in CLAUDE.md, this PR still needs Wolf's explicit go before it's merged/built.
· branch fix/library-ai-shelf-async-roll

const handed = await handOver(url, request);
if (!handed) return askDirect(request);
const answer = await collect(handed.poll, request, deadline);
if (answer !== 'lost') return answer;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The resubmit-on-lost-job logic assumes a poll 404 (collect's asked.status === 404'lost', line 248) always means the underlying run died with the relay, so resubmitting "costs nothing" (per the comment on line 227-228 and this function's doc comment). That's true if claude-relay is a single instance holding job state in memory (which the file header at line 7-8 implies: "The Order's container", singular). But if it's ever scaled to more than one replica behind a load balancer, a poll routed to a different instance than the one that accepted the job would also 404 even though the original job is still running — and askAsJob would then hand the same request over a second time via handOver, running (and paying for) the model call twice, contradicting the "nothing is paid twice" comment.

Not necessarily a bug today if the relay is guaranteed single-instance, but worth a one-line note confirming that assumption (similar to the rollingSince "across processes" comment flagged earlier in this PR on store.ts), since nothing here enforces it.

[Fix this →](https://claude.ai/code?q=In%20src%2Flib%2Flibrary%2Fmagic%2Frelay.ts%2C%20askAsJob%2Fcollect%20treat%20a%20poll%20404%20as%20%27lost%27%20and%20resubmit%20the%20job%20via%20handOver%2C%20assuming%20a%20404%20always%20means%20the%20underlying%20relay%20run%20died.%20This%20only%20holds%20if%20claude-relay%20is%20a%20single%20instance%3B%20if%20it%20is%20ever%20load-balanced%20across%20replicas%2C%20a%20poll%20hitting%20a%20different%20instance%20than%20the%20one%20that%20accepted%20the%20job%20would%20also%20404%2C%20causing%20a%20duplicate%2C%20paid%20model%20call%20while%20the%20original%20is%20still%20running.%20Add%20a%20comment%20confirming%20the%20single-instance%20assumption%2C%20or%20avoid%20resubmitting%20on%20bare%20404%20without%20a%20stronger%20signal%20that%20the%20job%20actually%20died.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7dab594. The assumption is now written into the doc comment on askAsJob: job state lives in the single relay container, so a 404 is a job nobody is running and dealing it again pays for nothing twice. It also says what breaks behind replicas and what would have to change.

The Atlas left here at revision 93c5744 and ran on the Terminal for a month.
Wolf asked for what it learned there to come back to keepsimple.io/ai-atlas.
This takes the view, not the content: the map still describes KeepSimple, in
English and Russian, on our own data and our own metrics.

What came back, all of it worked out on this same composition:

- The Security passage runs on one clock. The dot's fall and every ring's
  light are painted from one solved trajectory, so a ring brightens exactly
  as the dot crosses it and goes dark before the next fall. It was two
  guesses before, an SMIL animation and separate CSS timing that never met.
  Reduced motion leaves the dot hidden and the rings dark.
- Callouts end inside the frame. Each scrolls in place, and the last two rows
  are held shorter still; stacked under 1100px they get their height back.
- The dossier heading sits inside the scrolling panel and wraps, so a long
  title no longer needs a measured padding under it and no longer covers what
  it names.
- Every node and every dossier row that leads somewhere takes focus and
  answers Enter, and the focus ring is the Atlas red.
- A topics control in the rail opens any dossier without hunting for its node.
- Switching Environment and Security crossfades instead of cutting.
- The type floor is raised: prose at 14, labels at 12, nothing under it.

What did not come back: the Terminal's own content, its clockwise task route
and stage supports, which describe a task moving through the harness and mean
nothing on this map, and its doctrine tile, which replaced our illustration.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@manager

manager commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

Wolf asked tonight for the Terminal Atlas view to replace what stands at keepsimple.io/ai-atlas, so the port joins this batch.

Terminal's Atlas is this page's own composition, forked at 93c5744 and worked on there for a month. Their adapter.ts maps their content into this page's data shape, which made the seam obvious: the view came back, the content did not. The map still describes KeepSimple, in English and Russian, on our data and our metrics.

Taken: the Security passage on one solved trajectory (the dot's fall and every ring's light painted from the same clock, dark again before the next fall, nothing at all under reduced motion); callouts that end inside the frame, with the last two rows held shorter and their height returned when stacked under 1100px; the dossier heading inside the scrolling panel, wrapping, with no measured padding under it; focus and Enter on every node and every dossier row that leads somewhere; a topics control in the rail; a crossfade between Environment and Security; and the raised type floor, prose at 14 and labels at 12.

Left behind: their content, their clockwise task route and stage supports, which describe a task moving through the harness and mean nothing on this map, and their doctrine tile, which replaced our illustration.

@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

Claude finished @manager's task in 4m 49s —— View job


Review: async AI shelf roll (PR #227, sync on new commit)

  • Gather context (diff since previous review sync, b2f3d43..934530d)
  • Check whether prior review findings were addressed
  • Review new commit's changes (AI Atlas port from Terminal)
  • Post inline comments for any new issues found
  • Post final summary

What's new since the last pass

The new commit (934530d) is unrelated to the AI shelf work reviewed in earlier rounds — per the manager's comment, Wolf asked for the Terminal Atlas view to replace /ai-atlas, so the port joined this batch. It touches only src/pages/ai-atlas.tsx, src/styles/ai-atlas.css, and the new src/lib/aiAtlas/securityPassage.ts:

  • securityPassage.ts (new) solves the Security view's dot-fall and ring-lighting from one shared cubic-bezier trajectory instead of separate SMIL/CSS timings. Checked the math against a standard cubic-bezier(0.4,0,0.6,1) Bernstein expansion and the bisection-based invert() — both are correct, and the derived cy/opacity/rings values line up with the replaced SMIL <animate> keyTimes (0; 0.08; 0.92; 1 opacity, -712.5 → 0 cy). No issues.
  • Accessibility: NodeBody and dossier reference rows gained role="button"/tabIndex/onKeyDown (Enter/Space) so every node and dossier row that leads somewhere is keyboard-reachable, plus new :focus-visible outlines and a topic-picker <select> in the rail. This matches the manager's description and looks correct — native <select> inside a wrapping <label>, no a11y regressions spotted.
  • Dossier padding: the old useLayoutEffect-measured padTop (title height → JS-computed padding) is replaced with a static padding/min-height + overflow-wrap: anywhere in CSS, matching the "measured padding under it" item from the manager's list. useLayoutEffect/useRef imports are still used elsewhere in the file (hash sync, ViewToggle, SecurityRings), so nothing went unused.
  • Callout capping (sec-callout[data-layer='5'|'6']): verified against securityLayers data — n:5 (side left) and n:6 (side right) are indeed the last item in each column, so "the last two rows held shorter" is correctly targeted, and the max-width: 1100px/900px media queries correctly lift the cap once the columns stack.
  • Crossfade (ViewToggle's change() + .view-stage/.is-leaving): reduced-motion aware (instant ? 1 : 240), guards re-entrancy with a pending ref, and only touches the DOM inside a click handler — no SSR/hydration concern.

One real issue found, posted inline on src/styles/ai-atlas.css:1625:

  • The new .ring__circle rule drives stroke/stroke-width continuously from --security-passage, updated every animation frame by the requestAnimationFrame loop in SecurityRings. But .ring__circle still carries a pre-existing transition: stroke 0.18s ease, stroke-width 0.18s ease, ... (written for the old discrete hover toggle, unchanged by this commit). Transitioning a property whose target changes every ~16ms means the rendered stroke perpetually lags the true value by roughly the 180ms time constant — the rings won't actually light "exactly when the dot crosses it" as the code's own comments claim. Suggested dropping stroke/stroke-width from the base transition (keep it only on .is-glow/.is-dim for the discrete hover case). Also flagged that this same rule makes the JSX stroke={isHovered ? ...} / strokeWidth={isHovered ? ...} presentation attributes on the <circle> (ai-atlas.tsx:1404-1405) dead code — CSS class rules always win over presentation attributes, so neither hover branch is visually reachable anymore.

No App Router/Tailwind/CSS-in-JS violations, no new any beyond the file's pre-existing (already any-typed data/Dossier props, not introduced by this commit) style, import order looks correct, and no font-size values outside the CLAUDE.md passport (all reformatted/added sizes are 12/14/16px). No UI/styling changes touch the Library AI-shelf surfaces reviewed earlier.

Still open from earlier rounds (Library AI shelf, unrelated to today's commit)

  • src/components/library/organisms/RecommendedShelf/RecommendedShelf.tsx:398 (and ban button ~line 481) — lock/ban controls still gated on shelf.busy alone, not shelf.rolling/working.
  • src/lib/library/magic/store.ts:66rollingSince comment still overstates a cross-process guarantee; the mutex is in-process only.
  • src/hooks/library/useAiShelf.ts:100 — polling still goes silent after POLL_LIMIT (~6 min) with no surfaced timeout/error state.
  • src/lib/library/magic/relay.ts:39 (JOB_DEADLINE_MS 330s) vs ROLL_STALE_MS 300s in ai-shelf.ts:60 — a real roll can still legitimately exceed the staleness window.
  • src/lib/library/aishelf/engine.ts:471CUT_OFF status matching vs a raw connection-reset (status: 0) still unverified against the real edge.
  • src/lib/library/magic/relay.ts:293 — resubmit-on-404 still assumes a single-instance relay; worth a one-line comment confirming that if true.

Did not run yarn typecheck/lint/check:library in this pass (no tool access for yarn/tsc in this session). Per the release-lessons note in CLAUDE.md, this PR still needs Wolf's explicit go before it's merged/built, and per the "Staging and production need Wolf's word" rule, no merge should happen without that direct instruction in the conversation.
· branch fix/library-ai-shelf-async-roll

Comment thread src/styles/ai-atlas.css
Comment on lines +1625 to +1632
.ai-atlas-root .security-canvas .ring__circle {
stroke: color-mix(
in srgb,
var(--red) calc(var(--security-passage, 0) * 100%),
var(--rule)
);
stroke-width: calc(1px + var(--security-passage, 0) * 1px);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new rule drives stroke/stroke-width from --security-passage, which SecurityRings' requestAnimationFrame loop (src/pages/ai-atlas.tsx:1349-1379) updates on every frame (~16ms). But .ring__circle still carries a pre-existing transition: stroke 0.18s ease, stroke-width 0.18s ease, opacity 0.18s ease; (src/styles/ai-atlas.css:560-565, unchanged by this commit — it was written for the old discrete hover toggle). Transitioning a property whose target value changes every ~16ms means the browser is perpetually easing toward a moving target with a 180ms time constant: the rendered stroke will visibly lag the dot's true position by roughly that amount rather than lighting "exactly when the dot crosses it," which is the precision this commit's own comment (and securityPassage.ts's header) claims.

Suggest scoping the stroke/stroke-width transition off for the passage-driven state, e.g. transition: opacity 0.18s ease; on the base rule with stroke/stroke-width transitions only reapplied via .ring__circle.is-glow/.is-dim (the discrete hover states that still want the 0.18s ease), so the continuous per-frame animation isn't fighting a CSS transition.

Related: since this base rule now unconditionally sets stroke/stroke-width via CSS, the JSX presentation attributes on <circle> at src/pages/ai-atlas.tsx:1404-1405 (stroke={isHovered ? 'var(--red)' : 'var(--rule)'}, strokeWidth={isHovered ? 1.6 : 1}) are now dead — CSS class rules always win over presentation attributes, so neither branch of either conditional is ever visually reachable. Worth removing them (or the CSS override) so the code doesn't suggest hover changes width when only .is-glow's color override actually applies.

Fix this →

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7dab594. The security rings now carry transition: opacity .18s only, so the passage drives stroke and stroke-width straight from the animation frame with nothing easing against it.

Wolf's word, twice: replace, not merge. The previous page, its content and
its locale strings are gone. What stands at /ai-atlas is the Atlas as the
Terminal serves it at revision 64: their view, their adapter, their copy and
their guide, with the composition they took from here in the first place.

- `src/pages/ai-atlas.tsx` is their `atlas-source/atlas.tsx`, with the frame's
  `createRoot` bootstrap dropped, the guide read from `/ai-atlas/guide.json`
  on this host, and our SEO head kept over it.
- `src/lib/aiAtlas/` carries their adapter and copy beside the security
  passage. The adapter turns the guide into six stages, Project through
  Result, and seventy-four dossiers.
- `src/styles/ai-atlas.css` is their `original.css` followed by their
  integration rules, with the asset URLs pointed at `/ai-atlas` and every
  global rule held to `body.ai-atlas-page`: the Atlas owns this page, not the
  document, because it shares a site here.
- `public/ai-atlas/` gains the guide, the paper texture and the seven local
  font files their sheet asks for.

Two consequences worth stating plainly. The page is English only, as the
Terminal's is; `/ru/ai-atlas` now renders English. And the content is the
Terminal's own documentation, which until tonight sat behind Access.

Probed on the dev tree: page, guide, fonts, texture all 200, and the adapter
run against the shipped guide gives six stages and seventy-four dossiers.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

Claude finished @manager's task in 4m 55s —— View job


Review: async AI shelf roll (PR #227, sync on new commit)

  • Gather context (diff since previous review sync, 934530d..edacf99)
  • Check whether prior review findings were addressed
  • Review new commit's changes (Terminal Atlas content replaces /ai-atlas)
  • Post inline comments for new issues found
  • Post final summary

What's new since the last pass

The new commit (edacf99) is unrelated to the Library AI-shelf work — per the manager's comment, this is the second half of the Atlas port: /ai-atlas now serves the Terminal's own Atlas content (guide, adapter, copy, fonts), not just its view/composition as in the prior commit. Touches src/pages/ai-atlas.tsx, src/styles/ai-atlas.css, new src/lib/aiAtlas/{adapter.ts,copy.json}, and new static assets under public/ai-atlas/ (guide.json, 7 TTF font files, fonts.css).

The main finding, posted inline on public/ai-atlas/guide.json:1134: this makes internal server/operations documentation public with no auth gate — middleware.ts's matcher doesn't carve out /ai-atlas, and guide.json is a plain static file fetchable directly regardless of the UI. The commit message states this content "until tonight sat behind Access" (Cloudflare Access), and its own content is pointed about the boundary it just crossed — the access node's description explains Access "checks who you are before an internal app" while "public sites answer anyone." The shipped content covers the credential-boundary/secrets policy, the SSH "server cockpit" gate, Restic backup schedule, and the Telegram alert relay. No literal secrets/tokens/IPs are present, but it's real architecture-level reconnaissance information. Related, on adapter.ts:17: the ported UI has a whole node--redacted rendering path (implying the source system can hide certain nodes from itself), but nothing in this exported guide.json sets that flag — worth confirming that's because there's genuinely nothing more sensitive to hide in this export, not that a redaction flag got dropped in the capture. I'm flagging this for the record rather than blocking on it, since the commit message indicates Wolf approved the replacement directly — but the specific scope of what's disclosed is worth a deliberate second look before this reaches production, independent of the general "Atlas is public" decision.

Two smaller items posted inline:

  • adapter.ts:6 — new file types copy/guide/dossiers as any throughout (AGENTS.md wants precise types). Not blocking — it's transplanted from the Terminal repo and the surrounding ai-atlas.tsx already carries ~50+ pre-existing anys from the prior port — but a Guide/Dossier interface at the adaptGuide boundary would catch a future guide-shape change at compile time instead of failing silently deep in the render tree.
  • fonts.css:1 — all 7 faces ship as raw TTF (~1.7MB total). AGENTS.md's "Web fonts" section requires WOFF2 for served fonts via the existing yarn fonts:build pipeline; this directory bypasses it entirely, costing roughly half a megabyte-plus of avoidable transfer per first visit.

Two things confirmed as intentional, not bugs — no comment needed: the commit message explicitly states /ru/ai-atlas now renders English (the page ignores router.locale entirely and always renders the English copy), and public/ai-atlas/{data.json,data-ru.json,doctrine.webp} are now unreferenced leftovers from the pre-port page (minor cleanup opportunity, not urgent).

No App Router/Tailwind/CSS-in-JS violations. Font sizes introduced in this commit (12/14/16/18/22/40px) are all within the CLAUDE.md font passport. Hardcoded hex colors in ai-atlas.css are consistent with this composition's pre-existing self-contained design system (not KeepSimple's shared tokens), same as the prior commit — not re-flagging since that pattern was already accepted in the last review round.

Still open from earlier rounds (unrelated to today's commit)

Library AI-shelf, unaddressed:

  • src/components/library/organisms/RecommendedShelf/RecommendedShelf.tsx:398 (and ban button ~line 481) — lock/ban controls still gated on shelf.busy alone, not shelf.rolling/working.
  • src/lib/library/magic/store.ts:66rollingSince comment still overstates a cross-process guarantee.
  • src/hooks/library/useAiShelf.ts:100 — polling still goes silent after POLL_LIMIT (~6 min).
  • src/lib/library/magic/relay.ts:39JOB_DEADLINE_MS (330s) vs ROLL_STALE_MS (300s) can still falsely mark a running roll stale.
  • src/lib/library/aishelf/engine.ts:471CUT_OFF status matching vs a raw connection-reset still unverified against the real edge.
  • src/lib/library/magic/relay.ts:293 — resubmit-on-404 still assumes a single-instance relay.

Atlas, carried over unchanged by this commit (same ported CSS/JSX, not re-flagged with new inline comments to avoid duplicating existing threads):

  • src/styles/ai-atlas.css .ring__circle transition still fights the per-frame --security-passage update, and the dead stroke/strokeWidth JSX props on <circle> (ai-atlas.tsx:859-860) are still unreachable.

Did not run yarn typecheck/lint/check:library in this pass (no tool access for yarn/tsc in this session).

Per the release-lessons note in CLAUDE.md, this PR still needs Wolf's explicit go before it's merged/built, and per the "Staging and production need Wolf's word" rule, no merge should happen without that direct instruction in the conversation. Given today's commit publishes previously access-gated internal documentation, that go should specifically cover this disclosure, not just the general release batch.
· branch fix/library-ai-shelf-async-roll

"basis": "The ownership charter, delegated exceptions included."
},
{
"id": "secrets",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth flagging explicitly, even though the commit message states this is Wolf's call twice-over.

This file — and the /ai-atlas route that renders it — makes the Terminal's own internal operations documentation public with no auth gate (middleware.ts's matcher excludes api/_next/keepsimple_/favicon.ico only; /ai-atlas isn't carved out). The commit message says this content "until tonight sat behind Access" (Cloudflare Access). Concretely, guide.json is a plain static asset under public/, fetchable by anyone at /ai-atlas/guide.json regardless of anything the React UI does with it.

The content itself is architecture-level operational documentation of the server this very site runs on: the credential-boundary policy (secrets node, this line), the SSH "server cockpit" gate, the Restic backup schedule (recovery), the Telegram alert/voice-task relay, and — pointedly — the access node's own description explains that Cloudflare Access "checks who you are before an internal app" while "public sites answer anyone," which is exactly the boundary this content just crossed. No literal secrets/tokens/IPs are present (checked), but this is real reconnaissance-grade information disclosure about internal security architecture for anyone who finds the page.

Two things worth confirming before this ships to production, independent of whether Wolf already approved the disclosure in principle:

  1. Is publishing the specific architectural detail here (not just "an Atlas exists") the intended scope, or should the guide be trimmed before it's public?
  2. See the separate comment on ai-atlas.tsx — the ported UI has a node--redacted rendering path implying the source system can hide certain nodes, but nothing in this exported guide.json uses it. Worth confirming that's because there's genuinely nothing more sensitive to hide in this export, not because the redaction flag was dropped when the guide was captured for this public host.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By design, and it is the owner's call, made repeatedly and in writing. What ships is not an operations document any more: the Terminal's own cross-links and system edges are gone from the bundle in this batch (34 sentences), and the prose on the cards is written by the owner for this page, naming features and rules, no hosts, no paths, no credentials, no inventory. The commit before this one added a commit-time guard for exactly that class of leak.

guide.system.nodes.includes(entry)
? 'system-' + entry.id
: entry.id;
dossiers[id] = {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adaptGuide never sets a .redacted field on any dossier/project/system-node it produces, and public/ai-atlas/guide.json has zero occurrences of a "redacted" key. But src/pages/ai-atlas.tsx's NodeBody (pre-existing, ~line 220) has a whole rendering path for node.redacted — a placeholder/blur treatment plus dedicated copy (t.redactedPlaceholder) — implying the source system can mark certain nodes too sensitive to show even to itself. That path is dead on the data this adapter produces.

Given the guide already documents the credential-boundary policy, the SSH "server cockpit" gate, and backup/alerting internals (see the comment on guide.json), worth confirming explicitly that this export genuinely has nothing that needs redacting — rather than a redaction flag having been silently dropped when the guide was captured for this now-public host.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, and intentional: adaptGuide sets no redacted field and no redacted content ships. The rendering path in NodeBody comes with the component and stays with it; it costs nothing and it is what a future source would use if it ever marked a card.

export const copy: any = {
...base,
linesValue: (n: number) => String(n),
introInhabitantsTpl: () => '',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new file types copy, guide, dossiers, and every helper param/return as any throughout (AGENTS.md: "Avoid any, prefer precise types"). I'm not flagging this as blocking since ai-atlas.tsx already carries ~48-60 pre-existing anys from the prior port and this is clearly transplanted verbatim from the Terminal repo rather than hand-written here, but since this is a brand-new file (not a diff against prior KeepSimple code), it's worth at least a shared Guide/Dossier interface at the boundary (adaptGuide(guide: Guide): AtlasData) so a future change to guide.json's shape fails at compile time instead of silently producing undefined deep in the render tree.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged, not done in this batch. The file is the seam to a foreign shape and typing it properly means typing the guide it reads; that is its own change with its own review, not a rider on a content release.

Comment thread public/ai-atlas/fonts.css
@@ -0,0 +1,49 @@
@font-face {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All seven faces here are raw TTF (font-0.ttffont-6.ttf, 112KB–392KB each, ~1.7MB total). AGENTS.md's "Web fonts" section requires the site's fonts to ship as WOFF2 with TTF kept only as build sources (yarn fonts:build / scripts/fonts/build-webfonts.cjs, public/fonts/<Family>/) — this directory bypasses that pipeline entirely and serves the uncompressed originals straight to the browser. WOFF2 is typically 30-50% smaller than TTF for the same glyph set, so this is roughly half a megabyte or more of avoidable transfer on every first visit to /ai-atlas. Worth running these through the existing font-build tooling (or at minimum adding .woff2 versions) rather than shipping TTF directly, even though this whole directory was ported wholesale from the Terminal's own asset set.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7dab594. All seven faces are WOFF2 now, 1.72 MB down to 0.60 MB, the TTF sources are gone from the tree (nothing server-side reads them, unlike the Library thumbnail fonts) and the stylesheet version is bumped so no visitor keeps the old sheet.

manager and others added 3 commits September 10, 2026 20:31
…n click

The Terminal's guide described each mechanism from the inside, for a reader
who already lives in Terminal. The page is for a reader who does not, so the
prose on every card is now Wolf explaining the feature: what it lets him do
and why it exists. The guide keeps the ids, stages and links; the prose lives
in src/lib/aiAtlas/features.ts and survives a guide refresh.

A click now opens the card in a modal over the sheet, hover keeps the rail.
Esc, the scrim or the stamp closes it; topics inside open in place with a
crossfade. Enter and exit are eased and honor reduced motion. The card's
paragraphs render as paragraphs in both the rail and the modal.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The Terminal's export of the guide brought its source references (file,
line, sha256 of the Terminal's code), its placement notes and a tool
inventory onto a public path. The page renders none of it. A script strips
the guide to ids, titles, prose, children, links and system nodes, and runs
after every refresh. 46 KB to 25 KB, same six stages and seventy-four
dossiers out of the adapter.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The guide describes the private Terminal. It now lives beside the adapter,
is imported into the page and rendered on the server, and no longer sits
under public/ where any request could pull it as JSON. /ai-atlas/guide.json
answers 404. The strip script follows the file.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@manager
manager force-pushed the fix/library-ai-shelf-async-roll branch from edacf99 to dfad950 Compare September 10, 2026 20:37
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

Claude finished @manager's task in 3m 7s —— View job


Review: async AI shelf roll (PR #227, sync on new commits)

  • Gather context (diff since previous review sync, 934530d..dfad950: ec2be3c, 19991d8, 62438d0, dfad950)
  • Check whether prior review findings were addressed
  • Review new commits' changes (Atlas content replace, feature-prose + modal, guide disclosure fix)
  • Post inline comments for new issues found
  • Post final summary

What's new since the last pass

All four new commits are Atlas-only; none touch the Library AI-shelf files reviewed earlier.

The public-disclosure concern from the last round is resolved. 62438d0 and dfad950 do exactly what the earlier review asked for: scripts/ai-atlas/strip-guide.mjs now strips the Terminal's source (file/line/sha256), placement notes and tool inventory before the guide ships (46KB → 25KB), and dfad950 moves guide.json out of public/ai-atlas/ entirely into src/lib/aiAtlas/, importing it directly into src/pages/ai-atlas.tsx:10 so /ai-atlas/guide.json now 404s — confirmed public/ai-atlas/ no longer contains it. Checked the strip script keeps basis on system nodes and drops x/y, and confirmed adapter.ts never reads guide-provided coordinates (positions are hardcoded via theta/r in the adapter itself), so nothing broke. The guide's prose is still rendered into the page (that's the product), but per ec2be3c's commit message this was Wolf's explicit, twice-stated call to replace the page with the Terminal Atlas content — not something this review is positioned to second-guess.

One real issue found in 19991d8, posted inline on src/pages/ai-atlas.tsx:1391: clicking a node in Environment view now opens FeatureModal (new), but the rail <Dossier> panel beside it also updates to the same content, because focusId = hoverNode || focusedNode (line 1389) still falls through to focusedNode when nothing is hovered. The commit message states the intended split plainly — "hover keeps the rail" (click gets the modal) — but nothing gates the rail off when the modal is showing the same dossier, so a click currently produces the identical card twice: once in the modal, once in the aside behind the scrim.

One smaller item, same file: FeatureModal (ai-atlas.tsx:615-681) sets aria-modal="true" and traps Escape, but there's no Tab-key focus trap, so keyboard users can Tab past the last focusable element in the card into content behind the scrim while it's still open. Not a regression from a stricter prior state (the page had no modals before), but worth tightening given the new role="dialog" contract.

No App Router/Tailwind/CSS-in-JS/any-beyond-existing/import-order issues in the new commits. Font sizes introduced (12/15/18px) are all within the CLAUDE.md passport. The MODAL_EXIT_MS = 240 JS constant matches the CSS is-closing animation duration (0.24s), so enter/exit timing is consistent, and the reduced-motion override for the modal animations is present.

Still open from earlier rounds (unrelated to today's commits)

Library AI-shelf, unaddressed:

  • src/components/library/organisms/RecommendedShelf/RecommendedShelf.tsx:398 (and ban button ~line 481) — lock/ban controls still gated on shelf.busy alone, not shelf.rolling/working.
  • src/lib/library/magic/store.ts:66rollingSince comment still overstates a cross-process guarantee.
  • src/hooks/library/useAiShelf.ts:100 — polling still goes silent after POLL_LIMIT (~6 min).
  • src/lib/library/magic/relay.ts:39JOB_DEADLINE_MS (330s) vs ROLL_STALE_MS (300s) can still falsely mark a running roll stale.
  • src/lib/library/aishelf/engine.ts:471CUT_OFF status matching vs a raw connection-reset still unverified against the real edge.
  • src/lib/library/magic/relay.ts:293 — resubmit-on-404 still assumes a single-instance relay.

Atlas, carried over unchanged by today's commits:

  • src/styles/ai-atlas.css .ring__circle transition still fights the per-frame --security-passage update, and the dead stroke/strokeWidth JSX props on <circle> are still unreachable.
  • src/lib/aiAtlas/adapter.ts typed any throughout (transplanted file, not blocking).
  • public/ai-atlas/fonts.css still ships raw TTF instead of the repo's WOFF2 pipeline.

Did not run yarn typecheck/lint/check:library in this pass (no tool access for yarn/tsc in this session).

Per the release-lessons note in CLAUDE.md, this PR still needs Wolf's explicit go before it's merged/built, and per the "Staging and production need Wolf's word" rule, no merge should happen without that direct instruction in the conversation.
· branch fix/library-ai-shelf-async-roll

Comment thread src/pages/ai-atlas.tsx
>
<div className="feature-modal__scrim" onClick={onClose} />
<div
className="feature-modal__card"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor a11y gap: the modal card has role="dialog" / aria-modal="true" and closes on Escape (line ~635-641), but there's no Tab-key focus trap — a keyboard user can Tab past the close button/last row and land on elements behind the scrim while the modal is still visually open. Worth trapping Tab/Shift+Tab within cardRef for a dialog with this contract (not a regression, since the page had no modals before this commit, but worth tightening given the new pattern).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7dab594. Tab and Shift+Tab are trapped inside the card, focus lands back on the first stop from outside, Escape still closes.

This repository is public. The guard reads the added lines of the staged
diff and refuses source fingerprints (sha256, file + line), internal paths
and unstripped exports under public/. Existing lines are not judged. No
bypass; the offending line is rewritten. One journal line per run.

The charter records the rule and the incident that produced it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Comment thread src/pages/ai-atlas.tsx
</aside>
</div>

<FeatureModal

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When this opens (click on a node in Environment view, focusedNode set), the rail <Dossier> panel in the <aside> above (fed by dossier, derived at line ~1391 from focusId = hoverNode || focusedNode) shows the same card behind the modal's scrim, since focusId still resolves to focusedNode when nothing is hovered.

The commit message states the intended split plainly: "hover keeps the rail" (click gets the modal instead). As written, a click gets both — identical title/prose/rows rendered twice, once in the modal and once in the aside. Consider deriving the rail's dossier from hoverNode only while viewMode === 'environment' (or clearing/hiding the rail card while this modal is open for the same id).

[Fix this →](https://claude.ai/code?q=In%20src%2Fpages%2Fai-atlas.tsx%2C%20focusId%20(around%20line%201389)%20is%20computed%20as%20%60hoverNode%20%7C%7C%20focusedNode%60%2C%20which%20means%20clicking%20a%20node%20in%20Environment%20view%20(setting%20focusedNode%20and%20opening%20the%20FeatureModal%20from%20commit%2019991d8)%20also%20makes%20the%20rail%20Dossier%20panel%20in%20the%20aside%20show%20the%20same%20card%20behind%20the%20modal%27s%20scrim.%20The%20commit%20intended%20%27hover%20keeps%20the%20rail%2C%20click%20opens%20the%20modal%27%2C%20so%20the%20rail%20should%20not%20mirror%20a%20click-triggered%20focusedNode%20while%20the%20modal%20for%20it%20is%20showing.%20Fix%20by%20gating%20the%20rail%27s%20dossier%20source%20on%20hoverNode%20only%20in%20environment%20view%2C%20or%20hiding%2Fclearing%20the%20rail%20dossier%20while%20FeatureModal%20is%20open%20for%20the%20same%20id.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7dab594. The rail now falls back to the intro while the modal holds a card, and takes the hovered card the moment the pointer picks one. The split the commit message described is the behaviour now.

@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

Claude finished @manager's task in 3m 20s —— View job


Review: async AI shelf roll (PR #227, sync on new commits)

  • Gather context (diff since previous review sync, 934530d..1cbf981: 62438d0, dfad950, 1cbf981)
  • Check whether prior review findings were addressed
  • Review new commits' changes (guide-stripping script, guide bundled off public/, internal-data guard)
  • Post inline comments for new issues found
  • Post final summary

What's new since the last pass

Three commits, all responding to the "guide.json publishes internal Terminal documentation" finding from the prior round:

  • 62438d0 adds scripts/ai-atlas/strip-guide.mjs, which strips the Terminal's guide export down to id/title/text/detail/children (entries), id/title/location/text/children (steps), and id/title/role/detail/basis (system nodes) — dropping source (file/line/sha256), where/when/note/category/sourceLabel.
  • dfad950 moves guide.json from public/ai-atlas/ to src/lib/aiAtlas/, imports it directly into ai-atlas.tsx (import guide from '@lib/aiAtlas/guide.json') instead of fetching it client-side, and removes the now-dead loading/fetch-error state. /ai-atlas/guide.json correctly 404s now. Clean removal, no leftover dataUrlFor/Lang cruft, no hydration concern (the import is static/deterministic).
  • 1cbf981 adds scripts/guard/no-internal-data.mjs, wired into lint-staged (package.json) on * so it runs on every commit, plus a new CLAUDE.md section ("This repository is public") documenting the incident and rule. The guard reads only added lines of the staged diff and blocks sha256 keys, file+line JSON pairs, specific internal path substrings, and (only under public/) where/when/basis/sourceLabel keys. Journal is correctly gitignored.

The main finding, posted inline on scripts/ai-atlas/strip-guide.mjs:26: this only strips metadata (source fingerprints, placement notes) — it does not touch detail/text, which is where the actually sensitive content lives. src/lib/aiAtlas/guide.json still ships, and is now bundled straight into the public /ai-atlas page, prose describing: the credential-boundary policy (secrets node), the SSH "server cockpit" gate, the Restic backup schedule, and the Telegram relay — the same content the prior review round flagged as the substantive disclosure. Moving the file out of public/ and dropping file/line/sha256 stops the narrow "source-fingerprint" leak and the direct-JSON-URL leak, but doesn't make this content private: it's still rendered to every visitor of /ai-atlas, and (independent of any runtime endpoint) still sits in this OSS repo's committed source either way. The commit title and new CLAUDE.md section read as though the incident is closed, but the highest-sensitivity content from the original finding is still live.

Related: no-internal-data.mjs's pattern rules (sha256, file+line, specific path substrings, and where/when/basis/sourceLabel scoped to public/ only) wouldn't catch a recurrence of this either — none of them match free-text prose like "carries the operations I allow over SSH", and the scoping to public/ means a similar export landing under src/ (as this one now does) isn't covered by rule 4 at all.

No other issues found in these three commits — the lint-staged wiring, .gitignore entry, and CLAUDE.md documentation are all correctly done for what they cover.

Still open from earlier rounds (unrelated to today's commits)

Library AI-shelf, unaddressed (no library files changed in this sync):

  • src/components/library/organisms/RecommendedShelf/RecommendedShelf.tsx (~line 398, ban button ~481) — lock/ban controls still gated on shelf.busy alone, not shelf.rolling/working.
  • src/lib/library/magic/store.ts:66rollingSince comment still overstates a cross-process guarantee.
  • src/hooks/library/useAiShelf.ts:100 — polling still goes silent after POLL_LIMIT (~6 min).
  • src/lib/library/magic/relay.ts:39JOB_DEADLINE_MS (330s) vs ROLL_STALE_MS (300s) can still falsely mark a running roll stale.
  • src/lib/library/aishelf/engine.ts:471CUT_OFF status matching vs a raw connection-reset still unverified against the real edge.
  • src/lib/library/magic/relay.ts:293 — resubmit-on-404 still assumes a single-instance relay.

Atlas, carried over unchanged by today's commits:

  • src/styles/ai-atlas.css .ring__circle transition still fights the per-frame --security-passage update; dead stroke/strokeWidth JSX props on <circle> still unreachable.
  • src/lib/aiAtlas/adapter.ts:7 — new file typed any throughout (transplanted from Terminal, not blocking).
  • public/ai-atlas/fonts.css — 7 raw TTF faces (~1.7MB), bypasses the yarn fonts:build/WOFF2 pipeline required by AGENTS.md.
  • src/pages/ai-atlas.tsx:652 — the feature modal has no Tab focus trap.
  • src/lib/aiAtlas/adapter.ts:22adaptGuide never sets .redacted, worth confirming nothing needed hiding was dropped in the capture (more relevant now that the content-level disclosure above is still open).

Did not run yarn typecheck/lint/check:library in this pass (no tool access for yarn/tsc in this session).

Per the release-lessons note in CLAUDE.md, this PR still needs Wolf's explicit go before it's merged/built, and per the "Staging and production need Wolf's word" rule, no merge should happen without that direct instruction in the conversation. Given the finding above, that go should specifically weigh whether the secrets/server cockpit/recovery/telegram prose is intended to be public, not just the general release batch or the narrower fingerprint leak this round's commits addressed.
· branch fix/library-ai-shelf-async-roll

pick(s, ['id', 'title', 'location', 'text', 'children']),
),
entries: guide.entries.map(e =>
pick(e, ['id', 'title', 'text', 'detail', 'children']),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This strip only removes the Terminal's metadatawhere/when/note/category/sourceLabel/source (file, line, sha256) — from entries, and additionally keeps basis on system.nodes (line 31). It does not touch detail/text, which is where the actually sensitive content lives.

Concretely, src/lib/aiAtlas/guide.json still ships (and is now bundled straight into /ai-atlas, plus visible in this public repo's source either way):

  • secrets node: "The Order keeps the shared credentials. A project operator gets the ones its grant names and none of the others." (guide.json:624)
  • server cockpit: "...carries the operations I allow over SSH. Changes to shared infrastructure go through The Order." (guide.json:633)
  • recovery: "Restic copies selected state off the server every night, encrypted. Coverage has exclusions..." (guide.json:714, 732)

This is exactly the class of content the earlier review round flagged as the substantive disclosure (credential-boundary policy, SSH gate, backup schedule, alert relay) — moving the file from public/ to src/lib/aiAtlas/ and stripping source fingerprints doesn't remove it, it's still rendered on the public page and still sitting in this OSS repo's source tree regardless of any runtime endpoint. The commit title ("a commit that adds private infrastructure data is refused") and the new CLAUDE.md section read as if this closes the incident, but the highest-sensitivity content from the original finding is still live.

scripts/guard/no-internal-data.mjs won't catch a future recurrence of this either — its rules match sha256, file+line, specific internal-path substrings, and (only under public/) the where/when/basis/sourceLabel keys. None of those patterns match free-text prose like "carries the operations I allow over SSH", so a similar export landing outside public/ (as this one now does) would sail through the guard.

If the intent is really "nothing describing private infrastructure goes into the tree" (per the new CLAUDE.md rule), this script needs to drop or redact detail/text/basis for sensitive nodes/entries too — not just their sourcing metadata. [Fix this →](https://claude.ai/code?q=In%20scripts%2Fai-atlas%2Fstrip-guide.mjs%20on%20branch%20fix%2Flibrary-ai-shelf-async-roll%2C%20the%20strip%20only%20removes%20source%20metadata%20(where%2Fwhen%2Fnote%2Fcategory%2FsourceLabel%2Ffile%2Fline%2Fsha256)%20from%20the%20Terminal%27s%20guide%20export%2C%20but%20keeps%20%60detail%60%2F%60text%60%2F%60basis%60%20prose%20verbatim.%20That%20prose%20still%20describes%20sensitive%20internal%20infrastructure%20(credential%20boundary%20policy%20on%20the%20%27secrets%27%20node%2C%20SSH%20%27server%20cockpit%27%20access%2C%20Restic%20backup%20schedule%2C%20Telegram%20relay)%20in%20src%2Flib%2FaiAtlas%2Fguide.json%2C%20which%20is%20now%20bundled%20into%20the%20public%20%2Fai-atlas%20page%20and%20also%20sits%20in%20this%20public%20repo%27s%20source.%20Add%20redaction%2Fremoval%20of%20detail%2Ftext%2Fbasis%20for%20sensitive%20nodes%20(secrets%2C%20ssh%2Fcockpit%2Frecovery%2Ftelegram)%2C%20or%20confirm%20with%20Wolf%20that%20this%20specific%20content%20is%20intentionally%20public%20before%20relying%20on%20this%20script%20as%20the%20fix%20for%20the%20earlier%20disclosure%20finding.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7dab594 and in the commit before it. The strip now also drops links and system edges, which is where the Terminal's own explanatory sentences lived. detail and text stay on purpose: that is the prose the page draws, written by the owner for a public page, and it is his call.

Every entry now carries the name printed on its card, and the twelve black
tiles on the outer ring are marked TILE so the one being edited is found by
its label rather than by an id. A header states the rules of the file: a
string is a paragraph, a key is never renamed.

check-features.mjs reports a key that matches no card, which would be words
that never show.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

Claude finished @manager's task in 3m 18s —— View job


Review: async AI shelf roll (PR #227, sync on new commit)

  • Gather context (diff since previous review sync, 1cbf981..327ce97)
  • Review new commit's changes (features.ts word-labelling, check-features.mjs)
  • Post inline comments for new issues found
  • Post final summary

What's new since the last pass

The new commit (327ce97) touches only src/lib/aiAtlas/features.ts and adds scripts/ai-atlas/check-features.mjs. No Library AI-shelf files changed.

  • features.ts: purely a labelling pass — every one of the ~65 keys gets a // card: <title> or // TILE: <title> comment above it, plus an expanded file-header explaining the TILE/card distinction and the "don't rename a key" warning. Confirmed via git diff … | grep -v '^[+-]//' that no prose string content changed — this is comments-only. Spot-checked several labels against guide.json (directory → "Project owners", system-tools → system node "Tools & workspace") and they're accurate.
  • check-features.mjs (new): a manual dev script that cross-checks features.ts keys against guide.json ids, reporting dead keys (match nothing), duplicates, and cards falling back to the guide's own text unmodified. Good idea for a hand-edited file that silently no-ops on a typo'd key — this is exactly the failure mode called out in the file's own new header comment.
    • One real gap, posted inline on check-features.mjs:21-25: the script's valid-id set adds every system node under both its plain id and a 'system-'-prefixed id, but adapter.ts's adaptGuide only actually produces the system--prefixed dossier id when that node's id collides with an entry id (today, only tools collides; terminal, wolf, access, etc. don't and keep their plain id). So the checker is more permissive than reality — a typo'd key like 'system-wolf' in features.ts would pass this check as valid even though it would never render anything in the real page. Not an active bug today (only the genuinely-colliding 'system-tools' key exists), but it undermines the tool's stated purpose for future hand-edits. Suggested fix inline: compute the system- prefix only for nodes whose id exists in guide.entries, mirroring adapter.ts's own collision check.
    • Not wired into package.json scripts, lint-staged, or CI — it's a standalone manual check per its own header (node scripts/ai-atlas/check-features.mjs), so nothing currently enforces it automatically. Not a blocker, just noting it won't catch a slip unless someone remembers to run it.

No App Router/Tailwind/CSS-in-JS/any/import-order issues — this commit touches no components or styles.

Still open from earlier rounds (unrelated to today's commit)

Library AI-shelf, unaddressed:

  • src/components/library/organisms/RecommendedShelf/RecommendedShelf.tsx (~line 398, ban button ~481) — lock/ban controls still gated on shelf.busy alone, not shelf.rolling/working.
  • src/lib/library/magic/store.ts:66rollingSince comment still overstates a cross-process guarantee.
  • src/hooks/library/useAiShelf.ts:100 — polling still goes silent after POLL_LIMIT (~6 min).
  • src/lib/library/magic/relay.ts:39JOB_DEADLINE_MS (330s) vs ROLL_STALE_MS (300s) can still falsely mark a running roll stale.
  • src/lib/library/aishelf/engine.ts:471CUT_OFF status matching vs a raw connection-reset still unverified against the real edge.
  • src/lib/library/magic/relay.ts:293 — resubmit-on-404 still assumes a single-instance relay.

Atlas, carried over unchanged by today's commit:

  • src/styles/ai-atlas.css .ring__circle transition still fights the per-frame --security-passage update; dead stroke/strokeWidth JSX props on <circle> still unreachable.
  • src/lib/aiAtlas/adapter.ts typed any throughout (transplanted from Terminal, not blocking).
  • public/ai-atlas/fonts.css — 7 raw TTF faces, bypasses the yarn fonts:build/WOFF2 pipeline.
  • src/pages/ai-atlas.tsx — feature modal has no Tab focus trap; rail dossier still mirrors the modal's card on click (focusId fallthrough).
  • scripts/ai-atlas/strip-guide.mjs — still only strips source metadata, not the sensitive detail/text/basis prose itself (credential-boundary policy, SSH cockpit, backup schedule, Telegram relay) — that content is still bundled into the public /ai-atlas page and this OSS repo's source.

Did not run yarn typecheck/lint/check:library or the new check-features.mjs script in this pass — no tool access for node/yarn in this session (verified the script's logic and its ids set by hand against guide.json and adapter.ts instead).

Per the release-lessons note in CLAUDE.md, this PR still needs Wolf's explicit go before it's merged/built, and per the "Staging and production need Wolf's word" rule, no merge should happen without that direct instruction in the conversation. The unresolved public-disclosure scope in guide.json's prose (flagged in earlier rounds) is still the item most worth a deliberate look before that go.
· branch fix/library-ai-shelf-async-roll

Comment thread scripts/ai-atlas/check-features.mjs Outdated
Comment on lines +21 to +25
const ids = new Set([
...guide.steps.map(s => 'stage-' + s.id),
...guide.entries.map(e => e.id),
...guide.system.nodes.map(n => n.id),
...guide.system.nodes.map(n => 'system-' + n.id),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The valid-ids set is more permissive than the ids adapter.ts actually produces, which creates a blind spot in exactly the check this script exists to run.

adapter.ts:16-21 only prefixes a system node's id with system- when that id also exists among guide.entries (a real collision — today just tools, since terminal appears in guide.entries... actually only as a step, which doesn't collide, so system terminal keeps its plain id). But here, every system node contributes both n.id and 'system-' + n.id to the valid set unconditionally (lines 24-25), regardless of whether it actually collides.

Concretely: guide.system.nodes includes wolf, access, terminal, secrets, apex, etc. — none of which collide with an entry — so this script would treat a typo'd key like 'system-wolf' or 'system-terminal' in features.ts as valid, even though adaptGuide would never produce that dossier id and the words behind it would silently never render. That's precisely the "dead key" failure mode this tool is meant to catch.

No key in features.ts hits this today (only the genuinely-colliding 'system-tools' is used), so it's latent rather than active, but worth tightening since Wolf edits this file by hand per the header comment and the check's whole job is to catch exactly this class of mistake:

const entryIds = new Set(guide.entries.map(e => e.id));
const ids = new Set([
  ...guide.steps.map(s => 'stage-' + s.id),
  ...guide.entries.map(e => e.id),
  ...guide.system.nodes.map(n =>
    entryIds.has(n.id) ? 'system-' + n.id : n.id,
  ),
]);

Fix this →

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7dab594. The script now mirrors adapter.ts exactly: a system node keeps its own id unless that id also names an entry, and only then is it prefixed. The blind spot is closed and the count it prints dropped from 89 to 71, which is the number of cards the page can actually reach.

…ows them

The tiles now carry the features he actually wants a reader to see, and the
prose on each is his: why the thing exists, what it does for him, what broke
before it existed.

Map changes that follow from that: Composites is Composite Keys, Attachments
gives its tile to Smart Queuing, Tracks folds into Engine switch and Live
Steering takes the free slot, Session resume becomes Session start, Local
CLAUDE.md joins Prepare as a third tile, and Doors leads the Work stage. A
stage carrying three tiles gets a wider arc so they do not overlap.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

Claude finished @manager's task in 3m 41s —— View job


Review: async AI shelf roll (PR #227, sync on new commit)

  • Gather context (diff since previous review sync, 327ce97..5573b3a)
  • Check whether prior review findings were addressed
  • Review new commit's changes (adapter.ts, features.ts, guide.json)
  • Post inline comments for new issues found
  • Post final summary

What's new since the last pass

The new commit (5573b3a) is Atlas-only — no Library AI-shelf files changed. It touches src/lib/aiAtlas/adapter.ts, src/lib/aiAtlas/features.ts, and src/lib/aiAtlas/guide.json, reshuffling which entries render as outer-ring tiles and rewriting several cards' prose in Wolf's own voice.

Correctness check (adapter.ts/guide.json wiring): verified by hand — every id in the new chosen arrays (engine-switch, steering, global, local, session-resume, work-checks, sendto, skills, queue, message, keys, backlog, history, decisions) exists in guide.entries, so (entries.get(child) as any).title at adapter.ts:102 won't throw. The guide.json step children edits are internally consistent with the moves described in the commit message (queue moved from the terminal step to task, local/work-checks added to prepare/agent, tracks stays a reachable topic under terminal even though it's folded out of the tile set). The territoryArc/childrenArc widening for 3-tile stages is correctly gated on chosen[index].length > 2.

Two issues found, posted inline on features.ts:

  1. (more significant) This commit substantially expands the first-person prose bundled into the public /ai-atlas page with specific detail about the operator's own internal tooling — subscription/track counts ("around five Claude subscriptions and five Codex subscriptions standing ready"), and a fairly precise description of this repo's own edit-time review gates ("Doors") including the exact categories of checks run after every edit (design-token/contrast checks, reduced-motion checks, layout-shift checks, AI-filler-language checks), plus new detail on the nightly transcript-audit/notice mechanism and the internal Composite Keys system. This is a direct continuation of the disclosure concern raised across several earlier review rounds on guide.json, which was never fully closed (strip-guide.mjs strips only source metadata, not detail/text, and scripts/guard/no-internal-data.mjs's rules don't match free-text prose). Worth a deliberate confirmation that this specific new content is intended to be public rather than assuming the earlier guide-stripping commits already covered it.
  2. (minor doc drift) The file's own header comment says "Twelve of them" and lists 13 tile names but omits "Doors" — which this commit turns into a genuine tile (work-checks now leads chosen[4], tagged // TILE: Doors at line 226). The actual current tile count is 14. The engine-switch entry's inline tag is also still // card: even though it's a tile now per chosen[2]. check-features.mjs doesn't validate this header/tag against the guide, so nothing currently catches the drift.

No App Router/Tailwind/CSS-in-JS/import-order issues — this commit touches no components or styles. No UI/styling changes touch the Library AI-shelf surfaces reviewed earlier.

Still open from earlier rounds (unrelated to today's commit)

Library AI-shelf, unaddressed:

  • src/components/library/organisms/RecommendedShelf/RecommendedShelf.tsx (~line 398, ban button ~481) — lock/ban controls still gated on shelf.busy alone, not shelf.rolling/working.
  • src/lib/library/magic/store.ts:66rollingSince comment still overstates a cross-process guarantee.
  • src/hooks/library/useAiShelf.ts:100 — polling still goes silent after POLL_LIMIT (~6 min).
  • src/lib/library/magic/relay.ts:39JOB_DEADLINE_MS (330s) vs ROLL_STALE_MS (300s) can still falsely mark a running roll stale.
  • src/lib/library/aishelf/engine.ts:471CUT_OFF status matching vs a raw connection-reset still unverified against the real edge.
  • src/lib/library/magic/relay.ts:293 — resubmit-on-404 still assumes a single-instance relay.

Atlas, carried over unchanged by today's commit:

  • src/styles/ai-atlas.css .ring__circle transition still fights the per-frame --security-passage update; dead stroke/strokeWidth JSX props on <circle> still unreachable.
  • src/lib/aiAtlas/adapter.ts typed any throughout (transplanted from Terminal, not blocking).
  • public/ai-atlas/fonts.css — 7 raw TTF faces, bypasses the yarn fonts:build/WOFF2 pipeline.
  • src/pages/ai-atlas.tsx — feature modal has no Tab focus trap; rail dossier still mirrors the modal's card on click (focusId fallthrough).
  • scripts/ai-atlas/strip-guide.mjs — still only strips source metadata, not the sensitive prose itself (now materially larger in scope after today's commit, see inline comment).
  • scripts/ai-atlas/check-features.mjs — valid-id set still more permissive than what adapter.ts actually produces for system--prefixed ids.

Did not run yarn typecheck/lint/check:library in this pass (no tool access for yarn/node/tsc in this session) — verified the id-wiring changes above by manual cross-reference against guide.json instead.

Per the release-lessons note in CLAUDE.md, this PR still needs Wolf's explicit go before it's merged/built, and per the "Staging and production need Wolf's word" rule, no merge should happen without that direct instruction in the conversation. Given today's commit further expands the operational detail rendered on the public /ai-atlas page, that go should specifically weigh this content, not just the general release batch.
· branch fix/library-ai-shelf-async-roll

],
// TILE: Doors
'work-checks': [
'I got tired of agents agreeing with a rule and then breaking it an hour later. Reading a rule and following it turned out to be two different things, so I stopped writing rules and started running them. Doors are small programs that sit at fixed moments of a turn. When one fires, the agent goes back and fixes the work. There is no way to skip it and no flag to turn it off.',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a substantial escalation of the disclosure concern raised repeatedly in earlier rounds on guide.json (never fully closed — strip-guide.mjs only strips source metadata, not detail/text). This commit adds much more specific first-person prose describing this project's own internal tooling directly into features.ts, which flows into the dossier text bundled onto the public /ai-atlas page via describe()/adaptGuide:

  • Line 92: exact subscription counts/scale ("three to five tracks... around five Claude subscriptions and five Codex subscriptions standing ready") — business/cost-scale detail about the operator's infrastructure.
  • Lines 228-232 (work-checks/"Doors"): a fairly precise description of this repo's own pre/post-edit review gates — including the specific checks run after every edit ("Type sizes and colours get checked against the project's passport... Animations get checked for reduced motion... text gets read for AI filler") — which reads as a description of the very review pipeline this PR is going through, now public.
  • The local/global/session-resume/keys entries similarly add detail on the nightly transcript-audit/notice mechanism, the memory-hook system, and the internal "Composite Keys" catalog mechanics.

None of scripts/guard/no-internal-data.mjs's rules (sha256/file+line/specific paths/where+when+basis+sourceLabel under public/) would catch any of this — it's exactly the free-text-prose gap the guard was already flagged as having. Worth a deliberate confirmation that this level of detail about the operator's own tooling is intended to be public, rather than assuming the earlier "guide is fixed" commits (62438d0/dfad950) already covered it — this commit adds new sensitive-adjacent prose after that point.

[Fix this →](https://claude.ai/code?q=In%20src%2Flib%2FaiAtlas%2Ffeatures.ts%20on%20branch%20fix%2Flibrary-ai-shelf-async-roll%2C%20commit%205573b3a%20added%20detailed%20first-person%20prose%20describing%20internal%20operator%20tooling%20(subscription%2Ftrack%20counts%20at%20line%2092%2C%20the%20Doors%20review-gate%20mechanics%20at%20lines%20228-232%2C%20nightly%20audit%2Fmemory-hook%20details%20on%20session-resume%2Fglobal%2Flocal%2Fkeys)%20that%20is%20bundled%20directly%20into%20the%20public%20%2Fai-atlas%20page%20via%20adaptGuide%2Fdescribe.%20This%20continues%20a%20disclosure%20concern%20flagged%20in%20earlier%20PR%20review%20rounds%20on%20guide.json%20that%20was%20never%20fully%20resolved%20(strip-guide.mjs%20only%20strips%20source%20metadata%2C%20not%20detail%2Ftext%2C%20and%20scripts%2Fguard%2Fno-internal-data.mjs%20does%20not%20match%20free-text%20prose).%20Confirm%20with%20the%20project%20owner%20whether%20this%20specific%20level%20of%20operational%20detail%20is%20intended%20to%20be%20public%2C%20or%20trim%2Fgeneralize%20the%20prose%20before%20merge.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same answer as on guide.json, and the same author. This prose is written by the owner about his own system for a public page: features, rules and how he works, with no host, no path, no credential and no tool inventory. The line about apartments that came close was cut before this push, and a commit-time guard refuses the class of data you are pointing at.

Involve an agent,
Conversation history, Saved decisions.
card: every other card, opened from a stage, a ring or the Topics list.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor doc drift introduced by this commit: the header says "Twelve of them" and lists 13 names, but omits "Doors" — which this same commit turns into a real TILE (adapter.ts's chosen[4] now starts with 'work-checks', and this file tags it // TILE: Doors at line 226). Actual current TILE count is 14 (keys, backlog, message, queue, engine-switch, steering, global, local, session-resume, work-checks, sendto, skills, history, decisions). Relatedly, the engine-switch entry's own comment (line 90) is still tagged // card: Engine switch even though it's also a TILE now per chosen[2].

Since scripts/ai-atlas/check-features.mjs only checks key validity against the guide, not this header list or the TILE/card tags, nothing currently catches this drift — worth updating the count/list and the engine-switch tag so a future hand-edit doesn't trust a stale map of which entries are on-map tiles.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. The header now reads fifteen and lists all fifteen, Doors and Discipline included.

Involve an agent becomes AI Collaboration and says the ownership rule first.
Human Collaboration is new: the roster carries people with handles, a line
addressed to one leaves for Telegram, and a granted person can hand a task
back and receive the result in the same chat. Skills leaves the tile row and
stays a card; it is a commodity now and the tile was spent on it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

Claude finished @manager's task in 2m 22s —— View job


Review: async AI shelf roll (PR #227, sync on new commit)

  • Gather context (diff since previous review sync, 5573b3a..17d5fb3)
  • Check whether prior review findings were addressed
  • Review new commit's changes (adapter.ts, features.ts, guide.json — Work stage tile reshuffle)
  • Post inline comment for the new issue found
  • Post final summary

What's new since the last pass

The new commit (17d5fb3) is Atlas-only, touching src/lib/aiAtlas/adapter.ts, src/lib/aiAtlas/features.ts, and src/lib/aiAtlas/guide.json. No Library AI-shelf files changed. It renames the "Agent works" stage's third tile slot: sendto is retitled "AI Collaboration" (was "Involve an agent") and a new human-collab topic ("Human Collaboration") takes the tile slot previously held by skills (adapter.ts:57: chosen[4] is now ['work-checks', 'sendto', 'human-collab'], was ['work-checks', 'sendto', 'skills']).

Verified human-collab is wired correctly: it exists in guide.json:439 as a real entry with children: ["notify"], is added to the agent step's children (so it still shows in the stage rail), and gets two new links entries (directory, notify) that resolve to existing dossier ids — adapter.ts:102's non-optional (entries.get(child) as any).title would throw on a missing id, and it doesn't, so this is safe.

One issue found, posted inline on src/lib/aiAtlas/features.ts:186: since skills is no longer in chosen[4], it's no longer rendered as an outer-ring tile — it's now reachable only as a regular card (still listed in the agent stage's rail via step.children, just not "filled"). But its inline tag comment is still // TILE: Skills, which is now inaccurate and should read // card: Skills. This is the same class of drift flagged in the last review round on engine-switch (tagged // card: while actually a tile) — now there are two mismatched tags pulling in opposite directions. The file's own header list (// TILE: ... Twelve of them:) was already stale before this commit (actual tile count is 14, unchanged by this commit) and remains uncorrected; scripts/ai-atlas/check-features.mjs only validates key-vs-guide existence, not these tags, so nothing catches this automatically.

No App Router/Tailwind/CSS-in-JS/import-order issues — this commit touches no components or styles.

Still open from earlier rounds (unrelated to today's commit)

Library AI-shelf, unaddressed:

  • src/components/library/organisms/RecommendedShelf/RecommendedShelf.tsx (~line 398, ban button ~481) — lock/ban controls still gated on shelf.busy alone, not shelf.rolling/working.
  • src/lib/library/magic/store.ts:66rollingSince comment still overstates a cross-process guarantee.
  • src/hooks/library/useAiShelf.ts:100 — polling still goes silent after POLL_LIMIT (~6 min).
  • src/lib/library/magic/relay.ts:39JOB_DEADLINE_MS (330s) vs ROLL_STALE_MS (300s) can still falsely mark a running roll stale.
  • src/lib/library/aishelf/engine.ts:471CUT_OFF status matching vs a raw connection-reset still unverified against the real edge.
  • src/lib/library/magic/relay.ts:293 — resubmit-on-404 still assumes a single-instance relay.

Atlas, carried over unchanged by today's commit:

  • src/styles/ai-atlas.css .ring__circle transition still fights the per-frame --security-passage update; dead stroke/strokeWidth JSX props on <circle> still unreachable.
  • src/lib/aiAtlas/adapter.ts typed any throughout (transplanted from Terminal, not blocking).
  • public/ai-atlas/fonts.css — 7 raw TTF faces, bypasses the yarn fonts:build/WOFF2 pipeline.
  • src/pages/ai-atlas.tsx — feature modal has no Tab focus trap; rail dossier still mirrors the modal's card on click (focusId fallthrough).
  • scripts/ai-atlas/strip-guide.mjs — still only strips source metadata, not the sensitive detail/text/basis prose (credential-boundary policy, SSH cockpit, backup schedule, Telegram relay) — that content is still bundled into the public /ai-atlas page and this OSS repo's source. The human-collab/sendto prose added by today's commit is a continuation of this same pattern (roster handles, Telegram delivery) rather than a new category of disclosure, but it's still worth weighing together with the rest before the go.
  • src/lib/aiAtlas/features.ts:10 header "Twelve of them" tile list still undercounts the actual 14 tiles, and now has two mismatched tags (engine-switch under-tagged as card, skills over-tagged as TILE — see inline comment).

Did not run yarn typecheck/lint/check:library/node in this pass — Bash execution for anything beyond plain git/grep read commands required approval that wasn't available in this session; verified the human-collab wiring by hand against guide.json and adapter.ts instead.

Per the release-lessons note in CLAUDE.md, this PR still needs Wolf's explicit go before it's merged/built, and per the "Staging and production need Wolf's word" rule, no merge should happen without that direct instruction in the conversation.
· branch fix/library-ai-shelf-async-roll

Comment thread src/lib/aiAtlas/features.ts Outdated
'MemPalace is the store where decisions survive. An agent files what we agreed, and a new session searches it before asking me the same question again.',
'It is organized by project and searchable by every agent.',
],
// TILE: Skills

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit's adapter.ts:57 change drops skills from chosen[4] (replaced by human-collab), so skills is no longer rendered as an outer-ring tile — it's demoted to a regular card, still reachable via the agent stage's rail but not "filled" anymore. This inline tag is still // TILE: Skills, which is now stale and should read // card: Skills.

This is the same class of drift already flagged in the previous review round on engine-switch (tagged // card: while it's actually a tile per chosen[2]) — now there are two tags pointing the wrong way, in opposite directions. Neither the file's own header list (line 10, "Twelve of them", already undercounting the real 14 tiles) nor scripts/ai-atlas/check-features.mjs (which only checks key-vs-guide existence) catches either mismatch.

[Fix this →](https://claude.ai/code?q=In%20src%2Flib%2FaiAtlas%2Ffeatures.ts%20on%20branch%20fix%2Flibrary-ai-shelf-async-roll%2C%20commit%2017d5fb3%20removed%20%27skills%27%20from%20adapter.ts%27s%20chosen%5B4%5D%20tile%20array%20(replaced%20by%20%27human-collab%27)%2C%20so%20the%20%27skills%27%20entry%20is%20no%20longer%20rendered%20as%20an%20outer-ring%20tile.%20Update%20the%20inline%20comment%20above%20the%20%27skills%27%20key%20(around%20line%20186)%20from%20%27%2F%2F%20TILE%3A%20Skills%27%20to%20%27%2F%2F%20card%3A%20Skills%27.%20Also%20fix%20the%20pre-existing%20mismatch%20on%20%27engine-switch%27%20(tagged%20%27%2F%2F%20card%3A%27%20but%20actually%20a%20tile%20per%20chosen%5B2%5D)%20and%20update%20the%20header%20comment%27s%20%27Twelve%20of%20them%27%20tile%20list%20(line%2010)%20to%20match%20the%20current%2014%20actual%20tiles.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7dab594. It reads // card: Skills now.

…the page has a night

The map now says what it is. The header reads WOLF'S TERMINAL and the banner
under it names the page. Ring II is Resources, not Context: it holds what every
stage draws on. Ring III finally prints its own name; it was filtered out of the
render. Project agents becomes Colleagues and carries both diamonds the legend
names, people and agents.

Result gains Discipline as its third tile: a result sent back is read overnight
and comes back as a rule. Conversation history says why the record is kept, the
monthly review that proposes what to change. The six stage cards were written
before the tiles existed and are rewritten to match them.

The four resource cards carry real content: the memory layers and what is
mined from them, the tools including the network, the engines and where local
models actually stand, the roster of colleagues. The Order is described as the
orchestrator it is.

The Terminal's own cross-links are gone from the guide and from the bundle:
34 sentences in another voice under cards written in Wolf's. Rows under a card
are its own contents, labelled by what they are.

Hover now works on every ring. One relation graph drives it: Wolf to The Order,
The Order to each resource, each resource to the stages and tiles that use it,
and the lines are drawn where they were missing. Stage-to-mechanism wires are
legible before a stage is picked.

Dark mode arrives on the site's own switch, no second theme: the palette flips
on the variables the sheet already draws with, and every hardcoded paper colour
gets its night value, modal, Security, dropdown and scrollbars included. Text
holds 4.5:1 or better throughout.

The rail card is capped at the height of the map and clips with a fade and
"click to read more". It used to grow past the sheet and offer a scrollbar that
could not be reached, because the card follows the pointer.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

Claude finished @manager's task in 4m 26s —— View job


Review: async AI shelf roll (PR #227, sync on new commit)

  • Gather context (diff since previous review sync, 17d5fb3..c7d570f)
  • Check whether prior review findings were addressed
  • Review new commit's changes (Atlas rings renamed, resource cards rewritten, relation graph, dark mode)
  • Post inline comment for the new issue found
  • Post final summary

What's new since the last pass

The new commit (c7d570f) is Atlas-only — no Library AI-shelf files changed. It touches scripts/ai-atlas/strip-guide.mjs, src/lib/aiAtlas/{adapter.ts,copy.json,features.ts,guide.json}, src/pages/ai-atlas.tsx, and src/styles/ai-atlas.css.

Real fix, worth crediting: the Terminal's own-voice cross-link sentences (guide.json's links array and system.edges, ~34 sentences per the commit message) are now dropped entirely from the guide and from strip-guide.mjs's output. adapter.ts rebuilds hover relations itself from a small hardcoded relations/resourceUses graph of bare ids (wolforder→resource→tile), verified by hand against guide.entries/guide.system.nodes — every id resolves, systemRef()'s collision logic (only tools collides with an entry id) is applied consistently, and points[a]/points[b] exist for every relation pair (Spoke no-ops safely on a miss anyway). This closes a real slice of the disclosure concern raised across earlier rounds without introducing a wiring bug.

Also verified: the "Fifteen of them" TILE count in features.ts's header now correctly matches the actual chosen array in adapter.ts (15 tile ids), fixing the undercount flagged in the last round. Spot-checked one of the new dark-mode contrast claims in ai-atlas.css by hand (--red: #e8705a on --paper: #1b1e26 → ratio ≈5.47, matching the comment's claimed "red 5.5"), so the "4.5:1 or better throughout" claim looks credible rather than asserted. New/changed font sizes (12px, 16px) stay within the CLAUDE.md passport. Ring III ("Task lifecycle") rendering again (previously filtered out) is a straightforward fix, confirmed ringLabels.projects was already defined and just wasn't reaching render.

One issue found, posted inline on src/lib/aiAtlas/features.ts:327: while this commit removes one disclosure vector (cross-links), it adds new operational detail that goes further than anything flagged in earlier rounds — network topology (Tailscale mesh, a personal VPN, and "my own pool of proxy servers" for the work that needs a different exit), a phone-call human-escalation path, and an internal usage metric (41% of sessions end with a PREP save). This is hand-written prose in features.ts, so it was never subject to strip-guide.mjs, and scripts/guard/no-internal-data.mjs's pattern rules don't match free-text prose either — the same gap flagged on guide.json in prior rounds. Worth a deliberate confirmation that this specific level of infrastructure detail is intended to ship publicly, since it's a step beyond what was already flagged (Doors/credentials/backup/Telegram) — it adds concrete network egress architecture.

No App Router/Tailwind/CSS-in-JS/import-order issues in this commit. any usage is consistent with the file's pre-existing transplanted style, not newly introduced.

Still open from earlier rounds (unaddressed, unrelated to today's commit)

Library AI-shelf:

  • src/components/library/organisms/RecommendedShelf/RecommendedShelf.tsx (~line 398, ban button ~481) — lock/ban controls still gated on shelf.busy alone, not shelf.rolling/working.
  • src/lib/library/magic/store.ts:66rollingSince comment still overstates a cross-process guarantee.
  • src/hooks/library/useAiShelf.ts:100 — polling still goes silent after POLL_LIMIT (~6 min).
  • src/lib/library/magic/relay.ts:39JOB_DEADLINE_MS (330s) vs ROLL_STALE_MS (300s) can still falsely mark a running roll stale.
  • src/lib/library/aishelf/engine.ts:471CUT_OFF status matching vs a raw connection-reset still unverified against the real edge.
  • src/lib/library/magic/relay.ts:293 — resubmit-on-404 still assumes a single-instance relay.

Atlas, carried over unchanged by today's commit:

  • src/styles/ai-atlas.css .ring__circle transition still fights the per-frame --security-passage update; dead stroke/strokeWidth JSX props still unreachable.
  • src/lib/aiAtlas/adapter.ts typed any throughout (transplanted, not blocking).
  • public/ai-atlas/fonts.css — 7 raw TTF faces, bypasses the yarn fonts:build/WOFF2 pipeline.
  • src/pages/ai-atlas.tsx — feature modal has no Tab focus trap; the rail dossier (focusId = hoverNode || focusedNode, still line ~1435) still mirrors the modal's card when a click leaves the node hovered.
  • scripts/ai-atlas/strip-guide.mjs — still only strips source metadata (plus now cross-link sentences), not the sensitive detail/text/basis prose itself (credential-boundary policy, SSH cockpit, backup schedule, Telegram relay) — see this round's inline comment for how that gap now extends beyond guide.json into features.ts too.
  • scripts/ai-atlas/check-features.mjs — valid-id set still more permissive than what adapter.ts actually produces for system--prefixed ids.
  • src/lib/aiAtlas/features.ts — the engine-switch (tagged // card: while it's a tile) and skills (tagged // TILE: while it's no longer one) tag mismatches from prior rounds are still present; the header's tile count is now correct, but these two individual tags weren't touched.

Did not run yarn typecheck/lint/check:library/node in this pass (no tool access for yarn/node/tsc in this session) — verified the relation-graph wiring and the dark-mode contrast claim by hand instead.

Per the release-lessons note in CLAUDE.md, this PR still needs Wolf's explicit go before it's merged/built, and per the "Staging and production need Wolf's word" rule, no merge should happen without that direct instruction in the conversation. Given today's commit further expands the operational/infrastructure detail rendered on the public /ai-atlas page, that go should specifically weigh this content (network topology in particular), not just the general release batch.
· branch fix/library-ai-shelf-async-roll

'system-tools': [
'What a tool can do and what the agent may do are two different questions. The rules answer the second.',
'Tools are how an agent acts, and most of mine are my own. CodeGraph indexes every symbol of a project, so the agent reads the one function it needs instead of the file. MemPalace holds the decisions. A headless browser reads the pages a plain request cannot. Each project has a lever for its own containers, deploy, restart, logs, a shell, within what I granted it.',
'The network is a tool too. Tailscale joins my servers into one mesh, so an agent on one of them reaches another by hostname, wherever it stands. My own VPN and my own pool of proxy servers are there for the work that needs a different exit.',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit is a genuine improvement on the cross-link front — the Terminal's own-voice links/system.edges sentences (34 of them, per the commit message) are now dropped from guide.json and strip-guide.mjs, and adapter.ts builds its hover relations from bare ids instead. That closes one real slice of the disclosure concern raised across earlier rounds.

But this same commit also adds new operational detail that goes further than anything flagged before, all still bundled straight into the public /ai-atlas page via describe()/adaptGuide (no gate in middleware.ts):

  • Network topology (line 327): "Tailscale joins my servers into one mesh... My own VPN and my own pool of proxy servers are there for the work that needs a different exit." This is a new category — infrastructure/network architecture — not present in prior rounds' disclosure.
  • Human escalation path (line 310): "when it cannot wait, it calls my phone and wakes me up."
  • Internal operational metrics (line 321): "41% of sessions end with a save, and most of the palace turned out to be auto-mined noise..." — an internal usage/reliability statistic about the operator's own tooling.

None of scripts/guard/no-internal-data.mjs's rules (sha256/file+line/specific path substrings/where+when+basis+sourceLabel under public/) would catch any of this — it's the same free-text-prose gap flagged on guide.json in earlier rounds, now also true of hand-written prose in features.ts that never went through strip-guide.mjs at all. Worth a deliberate confirmation that this level of detail (VPN/proxy topology in particular) is intended to be public, since it's a step beyond the "Doors/credentials/backup" content already flagged — this adds concrete network egress/exit infrastructure to what a visitor to /ai-atlas can learn.

[Fix this →](https://claude.ai/code?q=In%20src%2Flib%2FaiAtlas%2Ffeatures.ts%20on%20branch%20fix%2Flibrary-ai-shelf-async-roll%2C%20commit%20c7d570f%20added%20new%20first-person%20prose%20describing%20internal%20network%20topology%20(Tailscale%20mesh%2C%20VPN%2C%20proxy%20server%20pool%20at%20line%20327)%2C%20a%20phone-call%20escalation%20path%20(line%20310)%2C%20and%20internal%20usage%20metrics%20(41%25%20PREP%20save%20rate%20at%20line%20321).%20This%20is%20bundled%20directly%20into%20the%20public%20%2Fai-atlas%20page%20via%20adaptGuide%2Fdescribe%2C%20continuing%20a%20disclosure%20concern%20flagged%20repeatedly%20in%20earlier%20PR%20review%20rounds%20on%20guide.json%2C%20now%20extended%20to%20hand-written%20prose%20in%20features.ts%20that%20scripts%2Fguard%2Fno-internal-data.mjs%20does%20not%20cover%20either.%20Confirm%20with%20the%20project%20owner%20whether%20this%20specific%20network%2Finfrastructure%20detail%20is%20intended%20to%20be%20public%2C%20or%20trim%2Fgeneralize%20the%20prose%20before%20merge.&repo=keepsimpleio/KeepSimpleOSS

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deliberate confirmation, since that is what you asked for.

The three lines are the owner's own words about his own system, written for this page in this session, and they stay. What they say is categorical, not operational: that a mesh VPN, a private VPN and a pool of proxies exist. No hostname, no address, no port, no tailnet name, no credential, no route. A visitor learns the shape of how the system is built, which is the entire purpose of the page, and gains nothing they could act on. Same for the phone call: it names an escalation path, not a number or a channel.

The usage figure (41% of sessions ending in a save, and the auto-mined noise that was dropped from search) is the one item the owner did not dictate line by line, and it is being put in front of him as part of this release rather than quietly kept or quietly cut. It is a statistic about how often a habit is used, with no system detail attached.

You are right that the commit guard does not read prose, and it is not meant to. It catches the mechanical leak class (source references, file fingerprints, internal paths, tool inventories) that actually escaped once. Prose on this page is authored, reviewed and owned by one person, and that is the control on it.

…e atlas fonts ship compressed

From the review on #227.

A roll now polls a relay job, so one model call can legitimately run for
minutes and a whole roll can pass the five-minute staleness window while it is
still working. The running roll renews its marker every minute, so the window
means "no heartbeat", not "started long ago", and a live roll is never rolled a
second time under itself.

A roll answers at once and works on in the background, so `busy` no longer
covers it. Preference, lock, ban and unban now wait on the roll as well: the
roll replaces the whole board from the snapshot it started with, so a write
made while it ran was silently overwritten when it landed.

When the poll allowance runs out the shelf says the roll is still working and
a reload will show where it got to, instead of saying "stocking" for the rest
of the session.

Two comments claimed more than the code does: the store's serial queue and the
shelf's roll set are one process, and the relay's resubmit-on-404 assumes the
single relay container. Both now say so, and say what replicas would need.

On the atlas: the seven faces shipped as raw TTF against the repo's own web
font rule, 1.72 MB on a public page. They are WOFF2 now, 0.60 MB, and the
stylesheet version is bumped so nobody keeps the old sheet. Skills lost its
tile in the last commit and its tag said otherwise. The features checker
mirrors the adapter's id rule instead of accepting ids the page never reads.
The security rings no longer carry a transition the animation frame fights.
The rail no longer shows the same card that the modal holds open in front of
it, and Tab stays inside the modal.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@manager manager changed the title Library: the AI shelf is stocked without holding the request open Library: the AI shelf is stocked without holding the request open, and the AI Atlas is finished Sep 10, 2026
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

Claude finished @manager's task in 3m 51s —— View job


Review: async AI shelf roll (PR #227, sync on new commit)

  • Gather context (diff since previous review sync, c7d570f..7dab594)
  • Check whether prior review findings were addressed
  • Review new commit's changes (rollingSince heartbeat, mid-roll write guard, atlas fonts/focus fixes)
  • Post inline comments for new issues found
  • Post final summary

What's new since the last pass

This commit (7dab594) is explicitly a review-response commit and closes out most of the open findings from earlier rounds, across both halves of the PR.

Library AI shelf — all three open findings addressed:

  • RecommendedShelf.tsx:294,399,485 — lock/ban toggles, the preference buttons, and the unban button are now all also gated on working (shelf.loading || shelf.rolling), not just shelf.busy. This closes the "mid-roll edit silently dropped" race from the first review round.
  • useAiShelf.ts:100-106 — hitting POLL_LIMIT now calls setError(...) instead of silently going quiet, so a roll that's still running past ~6 minutes tells the owner to reload instead of leaving "Stocking the shelf" up forever.
  • ai-shelf.ts:60-90,431-441,563 / store.ts:78-86 — this is the real fix for the JOB_DEADLINE_MS (330s) vs ROLL_STALE_MS (300s) race flagged twice: stockTheBoard now runs a setInterval heartbeat that renews rollingSince every ROLL_BEAT_MS (60s) while the roll is in flight, and the interval callback re-reads current.rollingSince at execution time (not a captured value), so if the real completion write already nulled it out first in the serialized queue, the heartbeat correctly no-ops instead of reviving a cleared marker. Verified the ordering: serial()'s promise chain enqueues by call order, and the ternary check (current.rollingSince ? renew : current) inside the heartbeat task makes the fix correct regardless of whether the heartbeat or the final write happens to be scheduled first. clearInterval(beat) in the finally block stops further beats once the roll ends. The store/route comments were also corrected to state the in-process-only guarantee plainly instead of the previously-flagged "carries the same fact across processes" overstatement.
  • relay.ts:283-289 — the resubmit-on-404 single-instance assumption is now documented explicitly in a comment, as requested (doc-only, no behavior change — reasonable since the relay is a single container today per the file's own model).

Atlas — several open findings addressed:

  • public/ai-atlas/fonts.css + font-*.woff2 — all 7 faces now ship as WOFF2 (the TTF files were removed rather than kept as build sources, but that's fine here since, unlike the Library thumbnail case in AGENTS.md, nothing in this port re-renders from the TTF originals).
  • scripts/ai-atlas/check-features.mjs:21-28 — the valid-id set now mirrors adapter.ts's actual system-node collision logic (entryIds.has(n.id) ? 'system-' + n.id : n.id) instead of adding both forms unconditionally, closing the "checker is more permissive than reality" gap.
  • features.ts:186skills's stale // TILE: tag corrected to // card: (it stopped being a tile a few commits ago).
  • ai-atlas.tsx:640-665FeatureModal now traps Tab/Shift+Tab inside cardRef, closing the missing-focus-trap a11y gap. Implementation looks correct: computed stops excludes the -1-tabindex card itself, wraps at both ends, and also recovers focus if it somehow lands outside the card.
  • ai-atlas.tsx:1461-1466 — the rail-mirrors-modal bug is fixed: railFocusId now falls back to null (not focusedNode) while the environment-view modal is open for that same id, so a click no longer shows the identical card twice.
  • ai-atlas.css:1843-1853.ring__circle's transition is now scoped to opacity only, so it no longer fights the per-frame --security-passage update on stroke/stroke-width.

Two small items from earlier rounds are still open (not touched by this commit, low priority):

  • features.ts:96engine-switch is still tagged // card: Engine switch even though it's been a tile since 327ce97/17d5fb3 (adapter.ts's chosen[2]). Same class of drift as the skills tag this commit just fixed.
  • ai-atlas.tsx:1007-1008 — the stroke={isHovered ? 'var(--red)' : 'var(--rule)'} / strokeWidth={isHovered ? 1.6 : 1} presentation attributes on the ring <circle> are still dead code: .ai-atlas-root .security-canvas .ring__circle (base rule) always sets stroke-width from --security-passage, and .ring__circle.is-glow (not the JSX attribute) is what actually drives the hover color. The transition fix in this commit didn't remove these now-provably-unreachable props.

Unrelated to this commit, still open from earlier rounds:

  • src/lib/library/aishelf/engine.ts:471CUT_OFF status matching vs a raw connection-reset (status: 0) still unverified against the real edge.
  • The public-disclosure content in src/lib/aiAtlas/guide.json / features.ts (credential-boundary policy, SSH cockpit, backup schedule, Telegram relay, network/VPN topology, subscription counts) flagged across several rounds — strip-guide.mjs still only strips source metadata, not detail/text/basis prose, and this content is unchanged by today's commit.
  • src/lib/aiAtlas/adapter.ts — still typed any throughout (transplanted file, not blocking).

No App Router/Tailwind/CSS-in-JS/import-order issues in this commit. Did not run yarn typecheck/lint/check:library in this pass (no tool access for yarn/tsc in this session) — verified the heartbeat ordering and the focus-trap/rail-fallback logic by hand instead.

Per the release-lessons note in CLAUDE.md, this PR still needs Wolf's explicit go before it's merged/built, and per the "Staging and production need Wolf's word" rule, no merge should happen without that direct instruction in the conversation.
· branch fix/library-ai-shelf-async-roll

@manager
manager merged commit 8bd2626 into dev Sep 10, 2026
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant