Skip to content

v0.2.54.0 feat: a run is refused up front when saved studies and test replays fill the limit, and a server replay has storage to land in (Cycle 0059 pt1) - #148

Merged
ryota-murakami merged 16 commits into
mainfrom
codex/cycle-0059-pt1
Oct 28, 2026
Merged

ryota-murakami merged 16 commits into
mainfrom
codex/cycle-0059-pt1

Conversation

@ryota-murakami

@ryota-murakami ryota-murakami commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor
{
  "schema_version": 1,
  "pr": 148,
  "head": "1972b17057cfd3ccc7ad5da78f65c698c4bd6a5a",
  "version": "0.2.54.0",
  "base": "main",
  "gate": {
    "tests": {
      "pass": 8006,
      "fail": 0,
      "skip": 10
    },
    "test:package": {
      "pass": 1083,
      "fail": 0,
      "skip": 10
    }
  },
  "session_kind": "interactive",
  "pregate": "pass @5626062b8c8e (6 checks, current for 1972b17057cf)",
  "issues": {
    "fixes": [],
    "refs": [],
    "flakes": []
  },
  "docs": {
    "status": "current",
    "scope": "AGENTS.md, CHANGELOG.md, PRODUCT_GOAL.md, README.md, extension/README.md, extension/skills/codebook/SKILL.md, the v9 PNG; base 250ade84 to the candidate"
  },
  "review_loop": {
    "passes": 2,
    "blocking_found": 2,
    "deferred": 0,
    "noted": 1,
    "fixed": 2,
    "scope": "frozen"
  }
}

Summary

A learner who explains their own Node server with /codebook loses everything the run captured when the server stops: Save answers that server runs are not saved yet. Cycle 0059 (Issue 147) makes that explanation a learning replay in three pull requests. This first one lays the storage a server replay is written to and read from, so the next two can read it back (pt2) and record into it during the run (pt3). Nothing writes a server history yet, so a server run is still refused at Save learning replay.

What a learner notices today is that the saved study limit now counts the same way everywhere. Before, a test's start counted saved studies, but a new study's save, the check at a run's start and a recording's start did not count test replays. A run could start, finish its explanation and then be refused at its save. Now that run is refused before the application receives any input, with the study library's full-library sentence.

Under the hood:

  • One history store over two profiles. The node-test profile wraps today's behavior unchanged; its existing tests pass unedited, and a history saved by the previous release opens and lists unchanged (a committed fixture).
  • A node-server profile with its own codec and writer rules: request attempts, the executed source stored once per file and hash and restored on read, output and queue bounds that never drop the request, stops or response.
  • A writer that stops taking new entries the moment its seal begins, writes every entry it accepted, writes end once, and never publishes a seal after it was abandoned.
  • A structured seal with one capture evaluator, and journal rules shared by the writer, the incremental scan and the full read.

Blast radius

The refactor runs under every saved test replay (NodeTestHistoryStore now extends a generic store) and the admission change runs under every browser save, run start and recording start. That is safe because the 7,970-odd existing tests, node-test store, summary, citation and other-window suites included, pass with no edit to their files, and new tests drive both admission directions from two windows (script ran: pnpm --dir extension test, 8016 tests, 0 failures).

Test Coverage

CODE PATHS                                                   USER FLOWS
[+] nodeServerHistoryWriter.ts                               [+] Save a server run as replay (storage level)
  ├── appendEntry / closedToAppends   [★★★ TESTED] SealCoordinator.test      ├── [★★★ TESTED] Whole run seals complete — Seal.test
  ├── output bounds (entries/bytes/queue) [★★★ TESTED] Bounds.test           ├── [★★★ TESTED] Stopped before answer / exit / failure reasons
  ├── appendRequest (query mask)      [★★ TESTED] Store/Request              ├── [★★★ TESTED] Torn/edited journal -> interrupted/unavailable
  ├── appendRequestWithdrawn          [★★ TESTED] Integrity rows             ├── [★★★ TESTED] Two windows, one free place — Admission.test
  ├── appendStop + source entry       [★★★ TESTED] Writer.test               └── [★★★ TESTED] Restart reads the same reasons
  ├── seal / finish / closed flag     [★★★ TESTED] SealCoordinator.test
  └── buildSeal complete|partial (long cause cut) [★★★ TESTED] Seal.test
[+] nodeServerJournalRules.ts (all ten entry rules)  [★★★/★★ TESTED] Integrity table + Writer refusals
[+] evaluateNodeServerCapture / evaluateNodeServerLearning / capMissingList / isPresentableServerStop  [★★★ TESTED] Seal.test
[+] mask* / dehydrate / rehydrate     [★★★ TESTED] maskNodeServerRequest / Writer / Store tests
[+] nodeHistoryJournalWriter generalization  [★★★ TESTED] SealCoordinator + previous-release fixture
[+] Repository / admission / descriptors / RecordingStore admission  [★★★ TESTED] Admission / Store tests

COVERAGE: 30/30 paths tested (100%) | value-weighted 30/30 (100%)
QUALITY: ★★★:24 ★★:6 | GAPS: 0 (no E2E, no eval)

Tests: 533 → 533 test files at audit time (the branch's ten new test files were already counted)
Coverage: 100% value-weighted (100% including 0 weakly covered paths)
Test value: 0 tests written, 1 rejected by the authoring gate, 1 existing test extended, 0 paths weakly covered (weak = ★, gate-failing or unrated).
Tests ran: tests count unavailable (receipt); the runner's own summary on the final tree reads 8016 tests, 8006 pass, 0 fail, 10 skipped.

Pre-Landing Review

Pass 1 (whole diff, 6,526 lines): core checklist (a fresh subagent on the same checklist), testing, maintainability, security, performance, simplification and state-machines specialists, the red team, and Codex's structured review (codex review --base origin/main). Two critical findings, both fixed with a test that failed first:

  • nodeServerHistoryWriter.ts: a long append-failure cause made the partial seal unwritable forever (the seal schema caps a reason's param at 1,024 characters). Fixed: the cause is cut to the seal's bound.
  • recordingStore.ts (caused by the shared-limit change in learningLibrary.ts): the run-start check and a recording's start did not count histories, so a run was admitted and then refused at its save. Fixed: both count test and server histories.

Ten informational findings were answered Skip under the repository rule for feature cycles (non-critical findings are not fixed by the ship review). They are listed below and tracked on Issue 147 to be fixed in pt2 and pt3 of this cycle, each with a test that fails without its fix. Pass 2 (fix range, 170 lines): testing, maintainability, performance, red team NO FINDINGS; state-machines one informational note. Converged.

REVIEW_LOOP: passes=2 blocking_found=2 deferred=0 noted=1 fixed=2 scope=frozen

Gate edits: none detected (13 listed, 0 read; patterns: RH-1,RH-3,RH-4,RH-12,RH-13,RH-14,RH-15,RH-16; unlisted idioms: none)

Outside review: Codex structured review of the whole diff in pass 1 completed with three P2 findings and no P0 or P1 (all three are informational notes below, two reproduced by Codex). On the fix range, the Codex adversarial challenge and the Codex structured review both reported no findings in Japanese prose without severity tags, so the result parser reads them as unverified: that is missing outside coverage for the fix range, not a pass. The native adversarial pass on the fix range completed with three low notes and recommends shipping as is.

Local reviews before /ship (repository rule): CodeRabbit CLI 0.9.1 on the range before the fixes (--committed --base-commit 250ade84 -c .coderabbit.yaml, included reviews, no credits) completed with 3 findings; open-code-review-delegate reviewed 36 of 36 reviewable files (13 default exclusions: README, fixture files, 9 test files) with 1 high, 5 medium, 5 low findings. All of them were fixed before /ship with red-first tests. Hosted CodeRabbit is off for this repository (.coderabbit.yaml reviews.auto_review.enabled: false); nobody asked it to review this PR.

Review notes

Below the blocking floor (advisory, informational, test-gap or wording items); recorded here and not fixed in this PR.

  • INFORMATIONAL extension/src/nodeServerHistoryWriter.ts:416 finish() takes end's elapsedMs from durable entries only; a queued later entry makes end move time backward and a clean run seals partial (reproduced by Codex) — MULTI-SOURCE CONFIRMED (state-machines + codex)
  • INFORMATIONAL extension/src/utils/evaluateNodeServerCapture.ts:62 an interrupted history without end and without a terminal entry reads captureComplete true on re-read (no appendFailure passed), reproduced by Codex
  • INFORMATIONAL extension/src/nodeServerStorageDescriptor.ts:85 the incremental scan drops source entries, so the pre-seal summary misses SERVER_HISTORY_SOURCE_UNAVAILABLE that the seal then reports (reproduced by Codex)
  • INFORMATIONAL extension/src/utils/maskSecretFormPairs.ts:18 undecodable-name branch untested
  • INFORMATIONAL extension/src/utils/maskNodeServerConfiguration.ts:22 no direct test (body-absent branch and composition)
  • INFORMATIONAL extension/src/nodeServerHistoryWriter.ts:115 seal version written as literal 1 instead of NODE_SERVER_HISTORY_FORMAT_VERSION
  • INFORMATIONAL extension/src/utils/cutHistoryReason.ts:7 JSDoc cites internal id F265
  • INFORMATIONAL extension/src/historyStorageDescriptor.ts:143 JSDoc says pt1's store
  • INFORMATIONAL extension/src/nodeHistoryJournalWriter.ts:160 untested writer cells: sealed x abandon, abandoned x append/finish/abandon, seal-failed x abandon (behavior correct by reading; downgraded from CRITICAL by lead judgment: no defect named)
  • INFORMATIONAL extension/src/utils/nodeServerJournalRules.ts:138 untested cell: a second request-sent or one with no open attempt is refused (downgraded: no defect named)
  • INFORMATIONAL extension/src/nodeTestHistoryStore.ts:319 history start counts archives + histories, recording start counts recordings + histories, save/pre-check count archives + histories; a history can take the last place after a recording was admitted and before its archive is saved (pre-existing overtaking, not made worse)
  • INFORMATIONAL extension/src/nodeHistoryRepository.ts:82 countHistories counts an unreadable history directory as 0 for every admission caller (deliberate: an unreadable store never blocks browser saves), so the shared limit is not enforced while a history directory is unreadable
  • INFORMATIONAL extension/src/studyWorkspace.ts:10727 lazy non-null assertion over nodeHistoryRepository; not reachable today
  • INFORMATIONAL extension/src/nodeTestHistoryStore.ts:319 recordings vs archives counted differently by entry point (pre-existing; same as pass-2 note)

Exploratory QA

Functional surface only (pt1 has no producer, UI or MCP surface), probed through the repository's own test files with node --test on the compiled extension. Pass 1 smoke (before the fixes): the server history store's write, seal and read path passed 7 of 7, and the cross-window admission edge passed 9 of 9. Those captures were superseded by the pass-1 fixes. On the current inputs the same two charters reran and passed: 7 of 7, and 13 of 13 for the admission edge. That run also covers the run-start check and a recording's start against test and server histories. Findings: none. Limits: no native editor check runs in pt1; the cycle's native trial N59 (VS Code and Cursor) runs on pt3.

Design Review

No frontend files changed — design review skipped.

Eval Results

No prompt-related files changed — evals skipped.

Scope Drift

Scope Check: CLEAN. Intent: Cycle 0059 pt1 storage foundation with no producer. Delivered: that, plus the shared saved study limit the plan's admission contract requires, its packaged README sentence, and shard timings for the new test files.

Plan Completion

Plan: the plan /autoplan approved for Cycle 0059 (Issue 147 body and its pinned comment), audited for the part this PR owns (tasks 1, 2 and the writer and seal parts of task 4, plus engineering tests E1 to E4). Result: 21 items, 17 done, 3 partial, 1 not done, 0 unverifiable.

  • Storage descriptor seam and one store over profiles (node-test unchanged, the previous release's fixture opens)
  • node-server profile folder, seven files, fsync, writer lock, bounds
  • Header union discriminated by profile
  • One saved study limit counted in both directions under one lock
  • PARTIAL: a live server history lists as recording in another window (done); cannot be deleted there (moved to pt2 with the Saved studies Trash path)
  • Server entry schemas and writer order rules, request attempt rules
  • Source dedupe, rehydration, oversized source stored as hash only
  • Writer closed vs draining, abandon token, one seal promise
  • Structured seal with reasons, params, one capture evaluator, presentable-snapshot rule
  • Seal reason table, 5xx complete, no-request removal
  • Journal-integrity rules shared by writer, scan and read
  • PARTIAL: bounds (output budget, structural headroom, queue entries done; the 4 MiB pending-byte bound and the 2 s history start need the producer, pt3; the 16-page events test belongs to the MCP reads, pt2)
  • Learning-complete rule as a function with capped missing lists
  • History repository resolving study ids with validation
  • Redaction at rest of secret-named body fields and query tokens of the declaration
  • NOT DONE here: a token in output reaches no file. The plan's task 3 owns this test and the run's output redactor feeds the history sink in pt3; deferred within the cycle (tracked on Issue 147, no TODO file).
  • E1 seal coordinator tests
  • E2 admission tests
  • E3 journal integrity tests
  • E4 structured seal tests
  • PARTIAL: task 4 seal tests (store-level cases done; learner Stop, self-exit closed, dispose past the close bound and the status naming the cause need the service wiring, pt3)

Acceptance edits: baseline unavailable (plan is not tracked in this repository).

Verification Results

Plan checks executed in this ship: pnpm --dir extension health:static PASS, pnpm --dir extension test PASS (8016 tests, 0 failures), pnpm --dir extension test:package PASS (1093 tests, 0 failures), pre-gate preflight PASS (secrets scan: 6124 added lines clean). Still to run before merge (repository delivery chain): required CI, the exact-HEAD gate pnpm --dir extension gate (its fifteen checks include health), and the local CodeRabbit CLI on the final range. 3 PASS, 0 FAIL, 0 BLOCKED, 3 NOT RUN yet (they run after this PR opens).

Documentation

Status: current - no documentation edits needed, no blockers.

Scope: base 250ade8 to head 1c3c3d9 plus unstaged working-tree changes (CHANGELOG.md, README.md, VERSION, extension/package.json, extension/scripts/tests/package.test.mjs, extension/src/constants.ts, one test). Candidate docs: AGENTS.md, CHANGELOG.md, PRODUCT_GOAL.md, README.md, extension/README.md, extension/skills/codebook/SKILL.md, the v9 PNG (binary, not edited). Discovery found the remaining .md/.txt files (docs/agents/*, test fixtures) unrelated to the diff.

Documentation health

  • README.md: current (Implementation status paragraph verified against the diff and CHANGELOG; storage-only claim confirmed in source)
  • CHANGELOG.md: current (0.2.54.0 entry matches the diff; not modified by policy)
  • extension/README.md: current (shared saved-study-limit sentence present near line 630; server-run not-saved status-bar line still true)
  • AGENTS.md: current (no statement falsified)
  • PRODUCT_GOAL.md: current (not edited by policy)
  • extension/skills/codebook/SKILL.md: current (1.0.52, unchanged and still true)
  • docs/designs/assets/codebook-product-spec-v9.png: binary, untouched

Coverage debt: no new public surface (no commands, settings, MCP tools or user-visible flows); the storage foundation is internal and covered by tests only. The one user-visible behavior, the shared study limit, has reference coverage in extension/README.md and CHANGELOG.md. No diagram drift (no architecture diagrams affected).

Test plan

  • pnpm --dir extension test (Node 24.22.1, LANG=en_US.UTF-8, macOS): 8016 tests, 8006 pass, 0 fail, 10 skipped
  • pnpm --dir extension test:package: 1093 tests, 1083 pass, 0 fail, 10 skipped
  • pnpm --dir extension typecheck, lint, health:static, dupes, dead-code, format:check: exit 0 (run by the implementers after each fix and re-run by the driving agent on the pre-fix tree)
  • node --test extension/scripts/tests/testShards.test.mjs: 101 pass, 0 fail (new test files carry measured timings, so the shard plan splits the 20 s bounds file)
  • gstack-pregate --stage preflight --allow-repo-commands: pass, 6 checks, secrets scan clean
  • Required CI on this head
  • Exact-HEAD gate pnpm --dir extension gate on this head
  • Local CodeRabbit CLI on the final range

Refs Issue 147 (Cycle 0059; this PR is part 1 of 3 and closes nothing).

🤖 Generated with Claude Code

ryota-murakami and others added 16 commits October 28, 2026 12:45
…le with its writer rules and bounds, and a writer that drains accepted entries before its seal
…server histories together, in both directions
…on baselines (moved writer identity, shared pair masker, pt3-only members suppressed with a reason)
…plits the 20-second bounds file instead of weighing it as one second
…and never publishes a seal without end

An output offered after the seal began is no longer queued behind end (it is dropped, not a failure), a retried seal keeps its single end, the inherited finish goes through the end-writing seal, an abandon during the final publish removes the published sealed.json, and the rules count an entry only once its line is durable.
…stop whose executed source was not stored seals the capture incomplete

The seal keeps every reason uncut, so the stop's unavailable list is bounded by the declaration's inspection limit (the largest legal capture seals inside the 256 KiB seal limit). A stop whose source entry says too large or not captured adds SERVER_HISTORY_SOURCE_UNAVAILABLE right after the snapshot reason.
…browser save, as an unreadable Node history store never blocked one
… that no longer match a finding

seal is now used by the server writer's finish, and the three NodeTestHistoryWriter members that moved out of nodeTestHistoryStore.ts match no finding; the two writer entries that still match stay.
…o the partial server history seal is always written
…erver histories that share the saved-study limit
…ft, a request after the terminal entry and output after its truncation, in writer, scan and read
Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 28, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@ryota-murakami

Copy link
Copy Markdown
Contributor Author

Local CodeRabbit CLI on the final range (repository delivery rule; hosted CodeRabbit stays off)

  • Command: coderabbit review --agent --committed --base-commit 250ade84 -c .coderabbit.yaml (CLI 0.9.1, included reviews, no credits).
  • Source cut: 250ade84..1972b170, this PR's head. Completed; 55 files reviewed.
  • Result: one finding, severity minor, in extension/src/constants.ts:1524. The per-stop unavailable bound does not bound the seal's serialized bytes. A path with quotes or backslashes doubles under JSON escaping. Non-ASCII paths take up to 3 bytes per character. The path appears in both the reason text and its params.
    • The finding is valid: 160 unavailable values with 512-character CJK paths can pass the 256 KiB seal limit.
    • Disposition: not critical, because it needs that extreme input. Under the feature-cycle rule it is not fixed in this PR. It is tracked on Issue 147 for pt3 of this cycle, where the producer first writes real captures. The fix is a byte-bounded seal (cut each reason's path to a byte bound) with a regression test at the maximum escaped, non-ASCII path.

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