CalvinBackup/gstack
1
0 Fork 0
mirror of https://github.com/garrytan/gstack.git synced 2026-10-31 22:14:25 +01:00
Code Issues Packages Projects Releases Wiki Activity

test(plan-ceo-section-loading): read the skill's ledger and currentDecision records for the stale-fill decision; replay three census reports

Browse Source
This commit is contained in:
garrytan committed 2026-10-04 07:00:02 +00:00
1 parent d89dc326c2
commit 27d4fd7426
10 files changed
+1838 -4
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

TODOS.md
+7
View File
@@ -146,6 +146,13 @@ Deferred from the approved severe fix wave (docs/designs/SEVERE_FIX_WAVE_2026_10
spelling per failure is the wrong fix. Bind the case to a structured finding
record (or an LLM judge with a fixed rubric) instead. 2 of the last 3
censuses, 0 of the 16 before. Effort M. **Priority:** P2.
**Completed:** v1.91.19.0 (2026-10-04). The case now reads the skill's own
decision-ledger row and `currentDecision` question
(`test/helpers/ceo-stale-fill-decision.ts`), with the phrase detector kept as
the alternate witness. Across eight census captures the structured check
finds 7 (it misses 37174266054, whose ledger row was left `unresolved`) and
the phrase detector finds 6; together they find all 8. The two missed reports
and one passing report are replayed from `test/fixtures/ceo-stale-fill-decision/`.
- **auq-matrix recommendation-substance judge rotates red** — census
37182865432 scored plan-design-review's "A because the plan is thin
everywhere, so skipping dimensions mostly skips gaps" 3 < 4. The same case
test/ceo-stale-fill-decision.test.ts
+56
View File
@@ -0,0 +1,56 @@
import { describe, expect, test } from 'bun:test';
import * as fs from 'node:fs';
import * as path from 'node:path';
import { hasApprovedStaleFillDecision } from './helpers/ceo-stale-fill-decision';
import { CEO_SECTION_CACHE_PLAN, hasStaleFillRaceFinding } from './helpers/ceo-section-loading-fixture';
const dir = path.join(import.meta.dir, 'fixtures', 'ceo-stale-fill-decision');
const capture = (name: string) => fs.readFileSync(path.join(dir, name), 'utf8');
const MISSED = ['census-37182865432-missed.md', 'census-37178734784-missed.md'];
const PASSED = 'census-37179171083-passed.md';
describe('CEO stale-fill decision from the skill-defined ledger and currentDecision records', () => {
test('both census reports the phrase detector missed, and the one it passed, record the approved race decision', () => {
for (const name of MISSED) {
expect(hasStaleFillRaceFinding(capture(name)), name).toBe(false);
expect(hasApprovedStaleFillDecision(capture(name)), name).toBe(true);
}
expect(hasStaleFillRaceFinding(capture(PASSED))).toBe(true);
expect(hasApprovedStaleFillDecision(capture(PASSED))).toBe(true);
});
test('the unreviewed plan has no decision', () => {
expect(hasApprovedStaleFillDecision(CEO_SECTION_CACHE_PLAN)).toBe(false);
});
const report = capture(MISSED[0]!);
const row = report.split('\n').find(line => line.startsWith('| R1 '))!;
const cells = row.trim().slice(1, -1).split('|');
const withStatus = (status: string) => report.replace(row, `|${cells.slice(0, 4).join('|')}| ${status} |${cells[5]}|`);
const question = report.split('\n').find(line => line.startsWith('Question: D1'))!;
test.each(['unresolved', 'reopened', 'deferred', 'declined'])('rejects a %s ledger row', status => {
expect(hasApprovedStaleFillDecision(withStatus(status))).toBe(false);
});
test.each([
['an unrelated question', report.replace(question, 'Question: D1 — R1: Should cache adapter failures bypass the cache until reinitialized?')],
['a later row that reopens it', report.replace(row, `${row}\n${withStatus('reopened').split('\n').find(line => line.startsWith('| R1 '))}`)],
['no ledger header', report.replace(/^\| ID and owner \|.*$/m, '| ID | Evidence | Current | Proposed | Status | Approval |')],
['a quoted decision block', report.replace(/^## currentDecision \(R1\)$/m, '> ## currentDecision (R1)')],
['a ledger row for another ID', report.replace(row, row.replace(/^\| R1\b/, '| R7'))],
])('rejects %s', (_name, text) => {
expect(hasApprovedStaleFillDecision(text)).toBe(false);
});
test('accepts the other recorded shapes: a named row ID and an approval that does not cite the decision ID', () => {
expect(hasApprovedStaleFillDecision(report.replaceAll('(R1)', '(R1-STALE-FILL)').replace(row, row.replace(/^\| R1\b/, '| R1-STALE-FILL'))
.replace(question, question.replace('— R1:', '— R1-STALE-FILL:')))).toBe(true);
expect(hasApprovedStaleFillDecision(report.replace(row, `|${cells.slice(0, 5).join('|')}| Selected **A** under the author policy. |`))).toBe(true);
expect(hasApprovedStaleFillDecision(withStatus('**approved**'))).toBe(true);
});
test('a fenced copy of the records is not a decision', () => {
expect(hasApprovedStaleFillDecision('```text\n' + report.replaceAll('```', '') + '\n```')).toBe(false);
});
});
test/fixtures/ceo-stale-fill-decision/census-37178734784-missed.md
Vendored
+493
View File
@@ -0,0 +1,493 @@
# Plan: cache profile summaries in one process
## Measured problem and accepted scope
The existing profile-summary service has one active process. A one-week trace
shows repeated reads of about 900 hot keys: DB CPU is 70%, with read p95 120 ms.
Add a process-local LRU wrapper to the existing repository. Acceptance targets
are at least 60% cache hits, DB CPU below 50%, and read p95 below 60 ms, with the
existing error-rate and correctness SLOs unchanged. This is an internal backend
change with no UI, API, schema, pricing, or developer onboarding change.
## Existing contracts retained
- All reads and writes use this repository in the same process; there are no
external DB writers. Multi-process operation remains unsupported and startup
rejects that configuration while caching is enabled.
- These surrounding contracts are accepted fixture facts, supplied by the
existing repository, cache adapter and rollout controller. Preserve them;
review the new wrapper ordering below against them.
- Authentication and authorization run before repository access. Keys encode
the authenticated tenant ID and validated profile ID without ambiguity.
Values are immutable profile-summary DTOs; secrets and cache keys are never
logged. Cached results cannot bypass authorization.
- The existing LRU adapter supports 1000 entries, a 16 MiB byte cap, and a
30-second TTL. Recorded hot data fits those limits. repository.read returns
an immutable absent-result DTO for a missing record, never undefined. The
adapter recognizes that DTO in cache.set, stores an internal sentinel with a
10-second TTL, and cache.get decodes it back to the same absent-result DTO.
The internal sentinel cannot escape the adapter; undefined means a cache miss.
- Cache operations are synchronous and atomic in the single JS event loop.
On any cache failure the existing adapter bypasses the cache until an empty
cache is reinitialized; repository errors keep the current typed API error
mapping. The existing per-key single-flight wrapper sits inside
repository.read, coalesces simultaneous store reads and releases on failure.
A committed repository.write retires that key's old read cohort before its
promise resolves. A later repository.read starts a fresh cohort; a rejected
write leaves the cohort unchanged. Already-started readers may finish with
their earlier snapshot. This admission rule does not inspect cache fills.
- The repository uses an in-process transactional store, with no network
transport between this wrapper and the store. repository.write is atomic:
a resolved promise means committed, and every
rejected promise guarantees no commit; its transaction rolled back before
rejection. Existing contract tests exercise that guarantee.
- Consistency is measured at the public wrapper boundary. A write completes
when writeProfile's promise fulfills after cache.delete, not when
repository.write commits or resolves. A read begins when readProfile is
invoked. Reads that overlap an unfinished writeProfile may return an earlier
snapshot, including reads begun after the store commit but before the wrapper
promise fulfills. Every read begun after that write completes must
observe the committed version. TTL expiry is not a substitute for this rule.
## Proposed wrapper integration
Keep the current read-through repository interface and shared adapters. These
are the complete new read/write ordering rules; no additional version checks or
coordination between a cache fill and a write are proposed:
```javascript
async function readProfile(key) {
const cached = cache.get(key);
if (cached !== undefined) return cached;
const value = await repository.read(key);
cache.set(key, value);
return value;
}
async function writeProfile(key, update) {
const saved = await repository.write(key, update);
cache.delete(key);
return saved;
}
```
### Accepted amendment (D1 / ledger row R1, 2026-10-04)
The sentence above, "no additional version checks or coordination between a
cache fill and a write are proposed", is superseded for one mechanism only.
Required guarantee: a cache fill produced by a readProfile call that missed the
cache before a later writeProfile on the same key completed must never be
admitted into the cache. writeProfile must invalidate the key synchronously in
the same event-loop tick after repository.write resolves (mark the key's write
generation as advanced, then cache.delete). readProfile must note the key's
write generation at miss time, and after repository.read resolves must call
cache.set only if that generation is unchanged, with the check and the set in
one synchronous segment (no await between them). A dropped fill increments an
existing-dashboard counter with no key labels. A rejected repository.write
changes nothing (no commit, no generation change, no delete). The bookkeeping
is only required while a miss is in flight for that key, so its size is bounded
by in-flight miss concurrency; the exact structure and code are engineering's.
Tradeoff: a read that began after the store commit but before writeProfile
completed is also dropped (an allowed earlier snapshot anyway), costing extra
misses only under write contention on the same key. Retained contracts, adapter
limits, TTLs and consistency semantics are unchanged.
Required deterministic regression scenarios (existing contract harness,
controlled promise release order):
- S1 read resolves before write commits: fill v0, write commits v1 and deletes, later read misses and returns v1.
- S2 write completes before the read's fill: read misses (snapshot v0), write commits v1, deletes, fulfills; read resolves v0 -> fill dropped, counter +1; a read begun after the write returns v1 from the store.
- S3 missing-record creation: absent fill (10 s sentinel) in the S1 order, and in the S2 order the absent fill is dropped so the created record is visible.
- S4 rejected read: typed error propagates, no fill, cache unchanged, single-flight released, next read succeeds.
- S5 rejected write: typed error propagates, no delete, no generation change, cached value retained, an overlapping in-flight read fills normally.
- S6 overlapping writes W1 (v1) and W2 (v2) with an interleaved read snapshotting v1: after W2 completes every read returns v2 and the v1 fill is dropped.
- S7 controller instance change during an in-flight read: old read cannot fill the new instance; new instance reads fresh (existing controller tests, re-run with the wrapper).
- S8 adapter failure during a miss: set/delete are bypassed, bypass counter rises, reinitialized cache starts empty, consistency holds.
- S9 generation bookkeeping releases after in-flight misses finish (bounded growth under a long write storm).
## Verification and rollout
Existing repository contract tests cover tenant isolation, key validation,
absence, DB failures, authorization, and startup rejection of multi-process
operation while caching is enabled. New wrapper tests cover hit/miss,
eviction and byte limits, TTL, adapter-failure fallback, successful-write
invalidation, failed-write preservation, and concurrent-miss coalescing.
The rollout uses the existing runtime feature flag: enable for 10% of keys,
then 50%, then all keys after one healthy hour at each stage. Monitor hit/miss,
eviction, cache bytes, fallback errors, DB CPU, and read p95 without raw IDs.
The existing controller uses one shared key-selection predicate for reads and
writes. On any enable, disable or percentage change, it stops admitting work,
awaits every admitted old-instance write, then publishes a new wrapper/cache
instance with a fresh single-flight cohort before admitting new work. Old reads
retain their old instance and cannot fill the new one. Disabled instances
bypass the cache on both paths. Tests cover the old-writer/new-reader ordering,
all those transitions and predicate parity. This lifecycle isolation does not coordinate
an ordinary DB write with a cache fill in the same active instance.
Existing dashboards and runbooks cover these metrics. Before each stage, verify
that alerts page the service owner on any correctness/error-SLO breach, read
p95 above 120 ms for five minutes, or cache bypass persisting for one minute.
Hit rate is hits / (hits + misses) among requests admitted to the cache path;
flag-excluded or adapter-bypassed requests are tracked separately, not as misses.
DB CPU and read p95 are service-wide metrics, including bypassed requests.
At the 10% and 50% stages, a healthy hour requires at least 60% admitted-request
hits, unchanged correctness/error SLOs, no alerts, and aggregate DB CPU/read p95
no worse than their 70%/120 ms pre-rollout baselines. At 100%, the original
absolute acceptance targets (DB CPU below 50%, read p95 below 60 ms, hits at
least 60%) must all hold with unchanged correctness/error SLOs and no alerts.
Any breach disables the flag immediately; the runbook records the incident,
rollback and criteria for resuming. These are existing
rollout-controller and telemetry contracts, not proposed wrapper additions.
Cold starts remain within the existing DB capacity. The service owner monitors
the rollout and records the results against the acceptance targets.
## Out of scope
Distributed caching, cross-process coherence, prewarming, changing consistency
semantics, or adding new product surfaces. The repository interface preserves a
future replacement path without introducing a general cache framework now.
## Author's review and acceptance requirements
This is a full CEO scope and feasibility review. The author has approved the
retained contracts, limits, rollout and acceptance targets above. Evaluate the
proposed wrapper against them; the wrapper itself remains unapproved. An actual
contradiction or missing proof must be reported and resolved, not assumed away.
For a demonstrated gap, amend the plan with the required guarantee, a feasible
remedy, its tradeoffs and deterministic regression scenarios. Those repairs
and their required verification are within the requested scope. The exact data
structures, full function bodies and executable test code belong to subsequent
engineering planning; do not select or implement them during this review when
the required behavior and feasibility can already be established.
Use the existing deterministic repository-contract test harness. Required wrapper
acceptance includes both completion orders of overlapping reads and writes,
missing-record creation, rejected reads/writes, overlapping writes, and isolation
across controller instance changes. Use existing telemetry to record any added
branch on the current dashboard with no key labels; no new alert threshold or metric
project is requested. These are future acceptance requirements, not tests already
implemented or passing. Preserve all 11 review outcomes, required registries,
diagrams, tasks, completion summary and the full GSTACK REVIEW REPORT.
---
# CEO Review (plan-ceo-review, 2026-10-04, HOLD SCOPE)
## Step 0 setup
- Platform/base branch: git commands prohibited for this run; base branch `main` taken from the host's git status snapshot. Platform unknown (not probed).
- Preamble/system audit/telemetry/brain/learnings/Aside: skipped per run rules (no skill-start echo; degraded mode, treated as interactive with author-authorized auto-decisions per the run policy). Outside review: `codex_reviews: disabled` in `.gstack-section-state-saJ9ki/config.yaml`.
- Review depth: implementation-ready (interfaces, codepaths, rescue behavior, tests), bounded by the author's rule that exact data structures, function bodies and executable test code stay with engineering.
- Storage policy: working plan and final output = this file (`PLAN.md`), scoped Edits at each checkpoint. TODOS.md: none exists; deferrals are recorded in the NOT in scope section (none so far).
- Mode: HOLD SCOPE, explicit user instruction ("Hold the current scope"). No mode question asked or logged. Section 11: no UI scope (internal backend change).
- Design doc: not probed (prohibited tooling); the plan's own "Measured problem" section is the problem statement.
### 0A. Premise Challenge
Real problem: ~900 hot profile-summary keys are re-read repeatedly from the DB; DB CPU 70%, read p95 120 ms. Target: hits >= 60%, DB CPU < 50%, p95 < 60 ms with correctness/error SLOs unchanged. Do-nothing cost: the DB stays the latency bottleneck and headroom shrinks as read volume grows. The plan attacks the pain directly (serve hot reads from process memory); it is not a proxy fix.
### 0B. Existing Code Leverage
| Sub-problem | Reused | Rebuild? |
|---|---|---|
| Bounded storage, TTL, byte cap, absent sentinel, failure bypass | existing LRU adapter | no |
| Thundering herd on a miss | existing per-key single-flight in repository.read | no |
| Staged rollout, instance swap, predicate parity | existing rollout controller + runtime flag | no |
| Auth, tenant-scoped keys, DTO immutability | existing repository contracts | no |
| Metrics, dashboards, alerts, runbooks | existing telemetry | no |
| New: read-through / write-invalidate ordering | the proposed wrapper (the only new code) | n/a |
### 0C. Dream State Mapping
```
CURRENT STATE THIS PLAN 12-MONTH IDEAL
every hot read hits the DB ---> process-local LRU in front of ---> repository interface still the only
DB CPU 70%, p95 120 ms repository, same interface, seam; cache tier replaceable
flagged rollout, DB CPU <50%, (distributed or not) without
p95 <60 ms touching callers
```
The plan moves toward the ideal: it keeps the repository interface as the single seam and adds no framework.
### 0D. Approach decisions
0D was required once: the proposed ordering rules contradict the retained consistency contract (row R1 below). 0E then applied HOLD SCOPE. 0G HOLD SCOPE checks: 1 new wrapper, 0 new services, 2-3 planned changed files (wrapper module, wrapper tests, flag/controller wiring if any; estimate) -> no complexity challenge; minimum change is the wrapper; nothing deferrable without blocking the goal; invariant repairs are in scope.
## Decision ledger
| ID and owner | Contract and evidence | Current | Proposed | Status | Exact approval and scope |
|---|---|---|---|---|---|
| R1 — wrapper author (Section 4 owner) | Retained contract "Every read begun after that write completes must observe the committed version. TTL expiry is not a substitute" (PLAN.md, Existing contracts retained, last bullet) vs proposed rules "no additional version checks or coordination between a cache fill and a write" (Proposed wrapper integration) and fixture facts "Already-started readers may finish with their earlier snapshot. This admission rule does not inspect cache fills". Schedule S2 (Section 4) shows a stale fill landing after writeProfile completes, served to later reads for up to 30 s. | readProfile: get -> miss -> await repository.read -> set. writeProfile: await repository.write -> delete. No fill/write coordination. | B: add a per-key fill-admission guarantee (drop any fill whose read missed before a later write to that key completed), synchronous post-commit invalidate, dropped-fill counter on the existing dashboard, scenarios S1–S9 in required acceptance. | approved | D1 auto-decided B under the run's author policy (complete remedy restoring a retained contract). Scope: amend the proposed ordering rules and required acceptance only; data structure and code left to engineering; no change to retained contracts, limits, TTLs or consistency semantics. |
| R2 — service owner | Hit-rate target >= 60% depends on per-key re-read interval vs the 30 s TTL; the trace reports "repeated reads of about 900 hot keys" with no interval. | Target retained; 10% stage gate verifies it. | none (unknown, not a decision) | unresolved risk, no choice needed | Owner verifies at the 10% stage per the existing healthy-hour rule; no limit change authorized. |
## currentDecision (R1)
Commitment comparison:
```text
Commitment | Source/approval or pending | Current (plan as written) | A | B | C
---------------------------------------------|-----------------------------------------------|---------------------------------------|---------------------------|------------------------------------------------|------------------------------------------
Read begun after writeProfile completes sees | retained contract, approved by author | violated by schedule S2 | violated (unchanged) | guaranteed: stale fill dropped | guaranteed via cohort-bound fills
committed version | | | | |
Read path | proposed, pending | get -> miss -> read -> set | same | get -> miss -> note key epoch -> read -> set | cache moves inside repository.read
| | | | only if epoch unchanged |
Write path | proposed, pending | await write -> delete | same | await write -> bump epoch + delete (same tick) | await write -> cohort retire also clears cache
Coordination fill vs write | proposed text says none; pending | none | none | per-key epoch check at fill time | repository-level
Absent-result DTO handling | retained adapter contract | sentinel 10 s | same | same; absent fills also subject to epoch check | same
Retained single-flight / adapter contracts | fixture facts, fixed | unchanged | unchanged | unchanged | CHANGED (cohort rule would inspect fills)
Adapter limits 1000 / 16 MiB / 30 s | retained, fixed | unchanged | unchanged | unchanged | unchanged
Dropped-fill visibility | author: record added branches, no key labels | n/a | n/a | counter on existing dashboard, no key labels | counter inside repository
Regression scenarios | author: both completion orders etc. required | listed categories, no schedules | same | S1–S9 deterministic schedules added | S1–S9 plus repository contract-test changes
Changed files (estimate) | 0E count | 2 | 2 | 2–3 | 4+ (repository, single-flight, tests)
Rejected write / rejected read behavior | retained | preserved | preserved | preserved (no epoch change on reject) | preserved
```
Question: D1 — R1: How do we close the stale-fill race between a cache miss and a completed write?
Project/branch/task: main — cache profile summaries in one process (PLAN.md review).
ELI10: A read that misses the cache goes to the database and later puts what it found into the cache. If a write finishes in between, the read can put the old value back after the write already cleared the cache. Every later read then sees the old profile for up to 30 seconds, even though the plan promises that reads started after a finished write always see the new data.
Stakes if we pick wrong: users see stale profiles after their own saves, the correctness SLO breaks, and the acceptance test for "both completion orders" fails or has to be weakened.
Recommendation: B because it restores the retained contract with a process-local, O(1), synchronous check and no change to any fixture contract, while A leaves the contradiction and C rewrites retained repository behavior for the same guarantee.
Completeness: A=3/10, B=10/10, C=10/10 (C's coverage is complete but it alters a retained contract the author fixed).
Pros / cons:
A) Keep the proposed ordering as written
✅ Zero extra wrapper logic; the two functions stay as drafted in the plan
✅ No new counters or test schedules to maintain
❌ Schedule S2 violates the retained consistency contract; TTL is explicitly not a substitute
B) Add a per-key fill-admission guarantee (recommended)
✅ Stale fills are dropped synchronously in the same tick as cache.set, so post-write reads always miss and re-read
✅ Touches only the wrapper; adapter, single-flight, controller and limits stay exactly as accepted
❌ Reads that began after the commit but before writeProfile completed are also dropped, costing a few extra misses under write load
C) Move caching inside repository.read's single-flight cohort
✅ Fill and cohort retirement live in one place, so no wrapper-level bookkeeping
✅ Also fixes the race for any future caller of repository.read
❌ Changes the retained single-flight contract ("does not inspect cache fills") and the repository; out of the author's fixed scope
Net: B restores the promise with the smallest diff; A keeps a known contradiction; C buys the same guarantee by changing contracts the author fixed.
Header: Stale-fill race
A) Keep proposed ordering
Effort S (zero implementation work beyond the draft), risk high, reuse: full, verification: existing categories only. ✅ Zero extra wrapper logic; the two functions stay as drafted in the plan. ✅ No new counters or test schedules to maintain. ❌ Schedule S2 violates the retained consistency contract; TTL is explicitly not a substitute.
B) Per-key fill-admission guard (recommended)
Effort S, risk low, reuse: all existing adapters and controller, verification: S1–S9 deterministic schedules plus existing contract tests. ✅ Stale fills are dropped synchronously in the same tick as cache.set, so post-write reads always miss and re-read. ✅ Touches only the wrapper; adapter, single-flight, controller and limits stay exactly as accepted. ❌ Reads that began after the commit but before writeProfile completed are also dropped, costing a few extra misses under write load.
C) Cache inside repository single-flight
Effort L, risk medium, reuse: partial (repository and single-flight change), verification: S1–S9 plus repository contract-test changes. ✅ Fill and cohort retirement live in one place, so no wrapper-level bookkeeping. ✅ Also fixes the race for any future caller of repository.read. ❌ Changes the retained single-flight contract ("does not inspect cache fills") and the repository; out of the author's fixed scope.
### D1 answer
Auto-decided D1 (R1) → B under the run's author policy: "Authorize complete remedies and required verification that restore its retained contracts and acceptance targets; use the recommended option." Authority: user run instructions for this capture (no human present; AskUserQuestion unavailable). Exact scope: amend "Proposed wrapper integration" with the fill-admission guarantee, the synchronous post-commit invalidate, the dropped-fill counter on the existing dashboard, and scenarios S1–S9 as required acceptance. Not authorized: data-structure or code selection, new metrics projects or alert thresholds, any change to retained contracts, limits or consistency semantics.
Post-answer checkpoint: amendment applied to "Proposed wrapper integration" (Accepted amendment D1/R1) and read back.
### 0I. Temporal Interrogation (HOLD SCOPE)
```
HOUR 1 (foundations): retained contracts above; the fill-admission guarantee (D1); harness with controlled promise release.
HOUR 2-3 (core logic): check-and-set must be one synchronous segment; absent DTO goes through the same check; rejected write changes nothing.
HOUR 4-5 (integration): controller swap tests re-run with the wrapper; dropped-fill counter wired to the existing dashboard with no key labels.
HOUR 6+ (polish/tests): S1–S9 schedules; bounded bookkeeping (S9); ASCII comment of the ordering rules in the wrapper module.
```
Feasibility blockers: none after D1. Pending choices: bookkeeping structure (engineering). Effort: human ~2 days / CC+gstack ~40 min.
## Current scope (Section 1 preface)
Mode HOLD SCOPE (explicit user instruction). Governing rows: R1 approved (D1 → B, author-policy auto-decision); R2 unresolved risk with verification owner, no choice. Accepted: original plan + D1 amendment. Deferred: none. Rejected: none. Pending: bookkeeping structure (engineering).
## Section 1: Architecture Review
Outcome: 1 finding (F1, CRITICAL GAP, resolved by D1); otherwise OK.
```
caller --auth/authz--> readProfile/writeProfile (NEW wrapper, per controller instance)
| |
v v
LRU adapter repository.read / repository.write
(1000/16MiB/30s) | single-flight cohort per key
v
in-process transactional store
rollout controller --flag/predicate--> publishes wrapper+cache instance (fresh cohort)
telemetry <-- hit/miss/evict/bytes/bypass/dropped-fill counters (no key labels)
```
- Data flow paths: happy = miss -> store -> fill -> hit; nil (missing record) = absent DTO -> 10 s sentinel -> decoded absent DTO; empty (zero-size DTO) = stored as a normal immutable value under the byte cap; error = repository rejection -> typed API error, no fill.
- State machine (cache entry): `absent --miss fill--> present(v) --TTL/evict/delete--> absent`; `absent --miss on missing record--> sentinel(10 s) --delete/TTL--> absent`; `any --adapter failure--> bypass --reinit--> absent`. Invalid transition prevented by D1: `present(stale) <-- fill after invalidate`.
- Coupling: wrapper -> adapter + repository only; no new coupling into controller beyond the existing instance publish. Justified.
- Scaling: 10x reads -> evictions rise, hit rate falls, DB load approaches today's; 100x -> single process CPU (existing limit, multi-process out of scope).
- SPOF: single process (pre-existing). Security: no new surface (Section 3). Production failure: adapter failure -> bypass alert at 1 min; store failure -> typed errors, existing SLO alert.
- Rollback: flag off -> controller swaps to bypass instance after admitted writes drain; seconds.
- F1 **CRITICAL GAP** (R1): stale fill after invalidation violates the retained consistency contract. Remedy: D1 → B. Residual: extra misses under same-key write contention. Verification: S2, S3, S6.
## Section 2: Error & Rescue Map
Outcome: 7 error paths mapped, 0 unrescued GAPS after D1.
```
METHOD/CODEPATH | WHAT CAN GO WRONG | EXCEPTION CLASS / SIGNAL
-------------------------|----------------------------------------|---------------------------
readProfile#cache.get | adapter failure | adapter bypass (no throw)
readProfile#repo.read | store rejection, validation, authz | existing typed API errors
readProfile#cache.set | adapter failure / byte cap | adapter bypass / evict
readProfile#fill guard | generation advanced during read | dropped fill (counter)
writeProfile#repo.write | rejected (rolled back) | existing typed API errors
writeProfile#cache.delete| adapter failure | adapter bypass
controller swap | old write still admitted | awaited before publish
EXCEPTION CLASS | RESCUED? | RESCUE ACTION | USER SEES
-------------------------|----------|---------------------------------------|------------------
adapter bypass | Y | serve from store until reinit; alert | slower reads
typed API errors (read) | Y | propagate unchanged, no fill | existing error
typed API errors (write) | Y | propagate, cache untouched | existing error
dropped fill | Y | skip set, count on dashboard | one extra miss
```
No catch-alls are proposed; the wrapper adds no try/catch. Decision gate: covered by D1; no new choice.
## Section 3: Security & Threat Model
Outcome: 4 threats assessed, 0 High, no issues.
| Threat | L | I | Mitigated |
|---|---|---|---|
| Cross-tenant read via key collision | Low | High | retained: keys encode tenant + validated profile ID unambiguously |
| Authz bypass via cached value | Low | High | retained: authn/authz run before repository access on every call |
| Key/secret leakage in logs or metrics | Low | Med | retained: never logged; counters carry no key labels (D1 keeps this) |
| Memory exhaustion via key churn | Low | Med | 1000 entries / 16 MiB cap; bookkeeping bounded by in-flight misses (S9) |
No new inputs, endpoints, dependencies or secrets. Decision gate: no choice needed.
## Section 4: Data Flow & Interaction Edge Cases
Outcome: 9 edge schedules mapped (S1–S9), 1 unhandled before D1 (S2/S3/S6), 0 after.
Invariant and boundary: a read begun (readProfile invoked) after writeProfile's promise fulfilled must return the committed version; reads overlapping an unfinished writeProfile may return the earlier snapshot (retained contract).
```
tick | readProfile R1 (miss) | writeProfile W (v1) | cache[k] | R2 (begun after W)
-----|--------------------------------|----------------------------------|-----------------|-------------------
1 | get -> undefined; repo.read ↓ | | - |
2 | (cohort c0 reading v0) | repo.write commits v1; retires c0| |
3 | | resolves; cache.delete; fulfills | - |
4 | repo.read resolves v0 | | | get -> ?
5a | PLAN AS WRITTEN: cache.set(v0) | | v0 (stale 30 s) | hit v0 ✗ violates
5b | D1: generation changed -> drop | | - | miss -> store v1 ✓
```
Favorable order (S1): R1 resolves at tick 2 before the commit; the fill lands first and W's delete clears it. Mechanism excluding 5a after D1: the generation check and cache.set run in one synchronous segment of the single event loop, and the generation advanced at tick 3 in the same tick as the delete. Regression proof: S1–S9 with controlled release points (both orders of each overlapping pair).
Interaction edge cases (backend-only): concurrent misses -> single-flight coalesces (retained); repeated writes -> S6; retry after failed write -> S5 then S1; flag flip mid-flight -> S7. Decision gate: D1 covers; no new choice.
## Section 5: Code Quality Review
Outcome: 1 finding (F2, WARNING), covered by D1's wording.
- F2 **WARNING**: the check-and-set must not be split by an await; D1 states the one-synchronous-segment rule. Verification: S2 plus a code-review checklist item in T1.
- Fits existing patterns (read-through wrapper over repository); no duplicated behavior (eviction, TTL, sentinel, single-flight all reused). Naming: readProfile/writeProfile describe behavior. Branching stays under 5 per function. No premature abstraction; no general cache framework.
## Section 6: Test Review
Outcome: diagram produced, 0 gaps after S1–S9.
```
NEW THING | TYPE | HAPPY | FAILURE | EDGE
------------------------------|-------------|------------------|--------------------|------------------
readProfile hit/miss/fill | unit | hit after miss | S4 rejected read | S3 absent DTO
writeProfile invalidate | unit | S1 | S5 rejected write | S6 overlapping
fill-admission guard (D1) | unit | S1 no drop | S2 drop + counter | S9 bounded
adapter bypass path | unit | reinit empty | S8 mid-miss | delete during bypass
controller instance isolation | integration | swap drains | S7 old reader | predicate parity
limits: 1000 / 16 MiB / 30 s | unit | eviction/TTL | byte cap reject | boundary sizes
```
Assertion check: each scenario names the exact observed value (v0/v1/v2/absent/error) and the counter delta; counts are exact, not lower bounds. Hostile QA: write storm on one key during a slow read (S6 + S9). 2 am test: S2. Chaos: adapter failure during a miss (S8). Pyramid: unit-heavy, few integration, no E2E. Flakiness: none, controlled release order, no timers. Load: production stage gates measure hit rate; no synthetic load test requested (HOLD SCOPE). LLM/prompt patterns: none.
## Section 7: Performance Review
Outcome: 1 risk (R2), no issues.
No N+1, no new queries or indexes. Memory: 16 MiB cap + bounded bookkeeping (S9). Hit path: synchronous map lookup (microseconds); miss path: unchanged store read + set; dropped-fill path: one extra miss. Connection pools unchanged. R2: 60% hit target depends on per-key re-read interval < 30 s TTL; unknown from the trace, verified at the 10% stage (owner: service owner). No limit change authorized.
## Section 8: Observability & Debuggability Review
Outcome: 0 gaps after D1's counter.
Existing: hit/miss, eviction, bytes, bypass errors, DB CPU, p95, existing alerts (SLO breach, p95 > 120 ms for 5 min, bypass > 1 min) and runbooks. Added by D1: dropped-fill counter on the current dashboard, no key labels, no new alert (author constraint). Debuggability at 3 weeks: counters per stage plus runbook incident records; per-key logs are intentionally absent (secrets rule). Runbook response for a rising dropped-fill counter: expected under same-key write contention; investigate only with a correctness alert.
## Section 9: Deployment & Rollout Review
Outcome: 1 risk flagged (R2 stage failure path), no issues.
No migrations. Flag stages 10% -> 50% -> 100% after one healthy hour each; controller swap drains admitted writes before publishing. Rollback: flag off (seconds), runbook records incident. Deploy-time window: flag off at deploy, wrapper inert. Parity: run S1–S9 and the contract harness in staging. First 5 minutes: hit/miss, bypass, dropped fills, error rate. First hour: healthy-hour criteria. Smoke: read-after-write check through the wrapper in staging.
## Section 10: Long-Term Trajectory Review
Outcome: reversibility 5/5, 1 debt item.
Debt: per-key write-generation bookkeeping is new state; document the ordering rules with an ASCII comment in the wrapper module (T3). Path dependency: none; repository interface remains the seam. Knowledge: this plan plus the comment suffice. 1-year: obvious if the comment ships.
## Section 11: Design & UX Review
SKIPPED (no UI scope).
## Closing sequence
- Outside Voice: `codex_reviews: disabled` → skipped; no outside process, no native fallback. Disabled-coverage review-log write: not persisted (gstack binaries and mutating commands unavailable in this run).
- TODO choices: none proposed (HOLD SCOPE; R2 is already required verification in the plan).
- Approval readiness: PASS — R1 (D1 → B, author-policy auto-decision, scope as recorded); R2 (no choice; verification owner named). No declined, deferred or unanswered changes in accepted work.
## NOT in scope
Deferred: none. Rejected: none. Unchanged original exclusions: distributed caching, cross-process coherence, prewarming, consistency-semantics changes, new product surfaces.
## What already exists
See 0B: adapter, single-flight, controller, repository contracts and telemetry are all reused; only the wrapper is new.
## Dream state delta
See 0C: the plan plus D1 keeps the repository interface as the single seam and adds no framework.
## Error & Rescue Registry
See Section 2 map: 7 codepaths, 4 exception classes/signals, all rescued, 0 CRITICAL GAPS.
## Failure Modes Registry
```
CODEPATH | FAILURE MODE | RESCUED? | TEST? | USER SEES? | LOGGED?
-------------------------|---------------------------|----------|--------|------------------|---------
readProfile fill | stale fill after write | Y (D1) | S2,S3,S6 | one extra miss | counter
readProfile repo.read | rejected read | Y | S4 | existing error | existing
readProfile cache ops | adapter failure | Y | S8 | slower reads | bypass
writeProfile repo.write | rejected write | Y | S5 | existing error | existing
writeProfile delete | adapter failure | Y | S8 | slower reads | bypass
controller swap | old reader fills new | Y | S7 | none | existing
bookkeeping | unbounded growth | Y (D1) | S9 | none | bytes
```
7 rows, 0 CRITICAL GAPS.
## Diagrams
1. System architecture: Section 1. 2. Data flow with shadow paths: Section 1 paths + Section 4 schedule. 3. State machine: Section 1. 4. Error flow: Section 2. 5. Deployment sequence:
```
deploy (flag off) -> staging S1–S9 -> 10% keys -> healthy hour -> 50% -> healthy hour -> 100% -> acceptance targets
```
6. Rollback flowchart:
```
alert or breach? --yes--> flag off -> controller drains writes -> bypass instance -> runbook incident -> resume criteria
--no---> next stage
```
Stale diagram audit: PLAN.md had no ASCII diagrams before this review; wrapper source files were not inspected (out of this run's scope), none known.
## Implementation Tasks
Synthesized from this review's findings. Each task derives from a specific finding above. Run with Claude Code or Codex; checkbox as you ship.
- [ ] **T1 (P1, human: ~1 day / CC: ~20min)** — profile-summary wrapper — Implement the D1 fill-admission guarantee and synchronous post-commit invalidate
- Surfaced by: Section 4 — schedule tick 5a (F1, R1)
- Files: to be determined (wrapper module and its tests)
- Verify: S1–S9 pass in the deterministic contract harness; review checklist confirms check-and-set has no intervening await
- [ ] **T2 (P1, human: ~2h / CC: ~5min)** — telemetry wiring — Add the dropped-fill counter to the existing dashboard with no key labels
- Surfaced by: Section 8 — D1 added branch visibility
- Files: to be determined
- Verify: S2 asserts counter +1; dashboard panel shows the series in staging
- [ ] **T3 (P2, human: ~30min / CC: ~5min)** — wrapper module — Add an ASCII comment documenting the read/write ordering rules and the guarantee
- Surfaced by: Section 10 — debt item
- Verify: comment present and matches the Accepted amendment text
_No new tasks from Sections 3, 5 (covered by T1), 6, 7, 9, 11._
Task JSONL: not persisted (gstack state paths unavailable; mutating commands prohibited this run).
## Completion Summary
```
+====================================================================+
| MEGA PLAN REVIEW — COMPLETION SUMMARY |
+====================================================================+
| Mode selected | HOLD SCOPE |
| System Audit | skipped per run rules; plan-only review |
| Step 0 | HOLD SCOPE; D1/R1 → B (fill guard) |
| Section 1 (Arch) | 1 issue found (F1, resolved by D1) |
| Section 2 (Errors) | 7 error paths mapped, 0 GAPS |
| Section 3 (Security)| 0 issues found, 0 High severity |
| Section 4 (Data/UX) | 9 edge cases mapped, 0 unhandled after D1 |
| Section 5 (Quality) | 1 issue found (F2, covered by D1) |
| Section 6 (Tests) | Diagram produced, 0 gaps |
| Section 7 (Perf) | 0 issues found (R2 risk, owner verifies) |
| Section 8 (Observ) | 0 gaps found |
| Section 9 (Deploy) | 1 risk flagged |
| Section 10 (Future) | Reversibility: 5/5, debt items: 1 |
| Section 11 (Design) | SKIPPED (no UI scope) |
+--------------------------------------------------------------------+
| NOT in scope | written (0 items) |
| What already exists | written |
| Dream state delta | written |
| Error/rescue registry| 7 rows, 0 CRITICAL GAPS |
| Failure modes | 7 total, 0 CRITICAL GAPS |
| TODOS.md updates | 0 items proposed |
| Scope proposals | 0 proposed, 0 accepted (HOLD SCOPE) |
| CEO plan | skipped by mode |
| Outside voice | codex disabled (codex_reviews: disabled) |
| Lake Score | 1/1 recommendations chose complete option |
| Diagrams produced | 6 (arch, data flow, state, error, deploy, |
| | rollback) |
| Stale diagrams found | 0 |
| Unresolved decisions | 0 |
+====================================================================+
```
Review Log / decision log / dashboard: not persisted (gstack binaries unavailable, mutating commands prohibited); this run is unlogged. Telemetry and learnings: skipped per run rules; no durable learnings this session.
### Unresolved Decisions
None. D1 was auto-decided under the run's author policy, not left unanswered.
## GSTACK REVIEW REPORT
| Review | Trigger | Why | Runs | Status | Findings |
|--------|---------|-----|------|--------|----------|
| CEO Review | `/plan-ceo-review` | Scope & strategy | 1 (not persisted) | CLEAR | mode: HOLD SCOPE, 0 critical gaps open (1 found, resolved by D1) |
| Outside Review | codex (`codex_reviews: disabled`) | Independent 2nd opinion | 0 | DISABLED | codex_reviews disabled; no completed external review |
| Eng Review | `/plan-eng-review` | Architecture & tests (required) | 0 | — | not run |
| Design Review | `/plan-design-review` | UI/UX gaps | 0 | — | not run (no UI scope) |
| DX Review | `/plan-devex-review` | Developer experience gaps | 0 | — | not run |
- **OUTSIDE COVERAGE:** provider codex, phase plan-review, disabled by `codex_reviews: disabled`; no outside process or native fallback ran; no findings.
- **VERDICT:** CEO CLEARED — eng review required
NO UNRESOLVED DECISIONS
test/fixtures/ceo-stale-fill-decision/census-37179171083-passed.md
Vendored
+681
View File
@@ -0,0 +1,681 @@
# Plan: cache profile summaries in one process
## Measured problem and accepted scope
The existing profile-summary service has one active process. A one-week trace
shows repeated reads of about 900 hot keys: DB CPU is 70%, with read p95 120 ms.
Add a process-local LRU wrapper to the existing repository. Acceptance targets
are at least 60% cache hits, DB CPU below 50%, and read p95 below 60 ms, with the
existing error-rate and correctness SLOs unchanged. This is an internal backend
change with no UI, API, schema, pricing, or developer onboarding change.
## Existing contracts retained
- All reads and writes use this repository in the same process; there are no
external DB writers. Multi-process operation remains unsupported and startup
rejects that configuration while caching is enabled.
- These surrounding contracts are accepted fixture facts, supplied by the
existing repository, cache adapter and rollout controller. Preserve them;
review the new wrapper ordering below against them.
- Authentication and authorization run before repository access. Keys encode
the authenticated tenant ID and validated profile ID without ambiguity.
Values are immutable profile-summary DTOs; secrets and cache keys are never
logged. Cached results cannot bypass authorization.
- The existing LRU adapter supports 1000 entries, a 16 MiB byte cap, and a
30-second TTL. Recorded hot data fits those limits. repository.read returns
an immutable absent-result DTO for a missing record, never undefined. The
adapter recognizes that DTO in cache.set, stores an internal sentinel with a
10-second TTL, and cache.get decodes it back to the same absent-result DTO.
The internal sentinel cannot escape the adapter; undefined means a cache miss.
- Cache operations are synchronous and atomic in the single JS event loop.
On any cache failure the existing adapter bypasses the cache until an empty
cache is reinitialized; repository errors keep the current typed API error
mapping. The existing per-key single-flight wrapper sits inside
repository.read, coalesces simultaneous store reads and releases on failure.
A committed repository.write retires that key's old read cohort before its
promise resolves. A later repository.read starts a fresh cohort; a rejected
write leaves the cohort unchanged. Already-started readers may finish with
their earlier snapshot. This admission rule does not inspect cache fills.
- The repository uses an in-process transactional store, with no network
transport between this wrapper and the store. repository.write is atomic:
a resolved promise means committed, and every
rejected promise guarantees no commit; its transaction rolled back before
rejection. Existing contract tests exercise that guarantee.
- Consistency is measured at the public wrapper boundary. A write completes
when writeProfile's promise fulfills after cache.delete, not when
repository.write commits or resolves. A read begins when readProfile is
invoked. Reads that overlap an unfinished writeProfile may return an earlier
snapshot, including reads begun after the store commit but before the wrapper
promise fulfills. Every read begun after that write completes must
observe the committed version. TTL expiry is not a substitute for this rule.
## Proposed wrapper integration
Keep the current read-through repository interface and shared adapters. These
are the complete new read/write ordering rules; no additional version checks or
coordination between a cache fill and a write are proposed in the original draft. **Amended by CEO review decision R1 (D1 → A):** the wrapper MUST NOT install a cache fill from a read that began before a write to the same key committed and whose writeProfile has since run (or is running). Required guarantee: at fill time the wrapper synchronously checks a per-key write generation (or equivalent) recorded while the miss is in flight; if any write to that key committed since the miss began, the fill is skipped, the value is still returned to the caller, and the skipped fill is counted. Coordination state is bounded by in-flight misses and removed when the last in-flight miss for the key settles. Exact data structures and function bodies are left to engineering planning. The original draft below is retained as the baseline being amended:
```javascript
async function readProfile(key) {
const cached = cache.get(key);
if (cached !== undefined) return cached;
const value = await repository.read(key);
cache.set(key, value);
return value;
}
async function writeProfile(key, update) {
const saved = await repository.write(key, update);
cache.delete(key);
return saved;
}
```
## Verification and rollout
Existing repository contract tests cover tenant isolation, key validation,
absence, DB failures, authorization, and startup rejection of multi-process
operation while caching is enabled. New wrapper tests cover hit/miss,
eviction and byte limits, TTL, adapter-failure fallback, successful-write
invalidation, failed-write preservation, and concurrent-miss coalescing.
**Added by R1 (D1 → A), deterministic scenarios on the existing contract test
harness with pause/release points:** (S1) read misses and pauses inside
repository.read; write commits and writeProfile fulfills; read resumes with the
earlier snapshot: assert no fill installed, a fresh read observes the committed
version, suppressed-fill count +1. (S2) same but the read resumes BEFORE the write
commits and fills: assert writeProfile's cache.delete removes it and the next read
observes the committed version. (S3) missing record: read obtains the absent DTO,
create commits and completes, read resumes: assert no absent sentinel installed
and the next read observes the created record. (S4) overlapping writes W1, W2 with
a read in flight: assert the next read after both complete observes W2's version.
(S5) rejected read leaves cache untouched; rejected write leaves cache and
generation unchanged. (S6) controller instance swap during an in-flight miss:
old read cannot fill the new instance (existing contract) and the new instance
serves the committed version. The suppressed-fill branch is recorded through the
existing telemetry on the current dashboard with no key labels; no new alert or
metric project.
The rollout uses the existing runtime feature flag: enable for 10% of keys,
then 50%, then all keys after one healthy hour at each stage. Monitor hit/miss,
eviction, cache bytes, fallback errors, DB CPU, and read p95 without raw IDs.
The existing controller uses one shared key-selection predicate for reads and
writes. On any enable, disable or percentage change, it stops admitting work,
awaits every admitted old-instance write, then publishes a new wrapper/cache
instance with a fresh single-flight cohort before admitting new work. Old reads
retain their old instance and cannot fill the new one. Disabled instances
bypass the cache on both paths. Tests cover the old-writer/new-reader ordering,
all those transitions and predicate parity. This lifecycle isolation does not coordinate
an ordinary DB write with a cache fill in the same active instance.
Existing dashboards and runbooks cover these metrics. Before each stage, verify
that alerts page the service owner on any correctness/error-SLO breach, read
p95 above 120 ms for five minutes, or cache bypass persisting for one minute.
Hit rate is hits / (hits + misses) among requests admitted to the cache path;
flag-excluded or adapter-bypassed requests are tracked separately, not as misses.
DB CPU and read p95 are service-wide metrics, including bypassed requests.
At the 10% and 50% stages, a healthy hour requires at least 60% admitted-request
hits, unchanged correctness/error SLOs, no alerts, and aggregate DB CPU/read p95
no worse than their 70%/120 ms pre-rollout baselines. At 100%, the original
absolute acceptance targets (DB CPU below 50%, read p95 below 60 ms, hits at
least 60%) must all hold with unchanged correctness/error SLOs and no alerts.
Any breach disables the flag immediately; the runbook records the incident,
rollback and criteria for resuming. These are existing
rollout-controller and telemetry contracts, not proposed wrapper additions.
Cold starts remain within the existing DB capacity. The service owner monitors
the rollout and records the results against the acceptance targets.
## Out of scope
Distributed caching, cross-process coherence, prewarming, changing consistency
semantics, or adding new product surfaces. The repository interface preserves a
future replacement path without introducing a general cache framework now.
## Author's review and acceptance requirements
This is a full CEO scope and feasibility review. The author has approved the
retained contracts, limits, rollout and acceptance targets above. Evaluate the
proposed wrapper against them; the wrapper itself remains unapproved. An actual
contradiction or missing proof must be reported and resolved, not assumed away.
For a demonstrated gap, amend the plan with the required guarantee, a feasible
remedy, its tradeoffs and deterministic regression scenarios. Those repairs
and their required verification are within the requested scope. The exact data
structures, full function bodies and executable test code belong to subsequent
engineering planning; do not select or implement them during this review when
the required behavior and feasibility can already be established.
Use the existing deterministic repository-contract test harness. Required wrapper
acceptance includes both completion orders of overlapping reads and writes,
missing-record creation, rejected reads/writes, overlapping writes, and isolation
across controller instance changes. Use existing telemetry to record any added
branch on the current dashboard with no key labels; no new alert threshold or metric
project is requested. These are future acceptance requirements, not tests already
implemented or passing. Preserve all 11 review outcomes, required registries,
diagrams, tasks, completion summary and the full GSTACK REVIEW REPORT.
---
# CEO REVIEW (plan-ceo-review, HOLD SCOPE) — 2026-10-04
## Step 0: setup, ledger and scope challenge
**Review depth:** implementation-ready at the wrapper codepath level (readProfile /
writeProfile, adapter, controller). Per the author's acceptance requirements, exact
data structures, full function bodies and executable test code are reserved for
engineering planning; this review names required guarantees, codepaths, rescue
behavior and regression scenarios only.
**Storage policy:** working plan = this file (requested output). Appends for new
review content, scoped Edits for plan amendments. gstack state/log/telemetry
writes are out of this run's scope (hermetic fixture); such fields are shown
**not persisted**. System audit, environment setup and telemetry skipped per run
instructions. Outside review: `codex_reviews: disabled` in
`.gstack-section-state-b2PTft/config.yaml`.
**Stated limits recorded:** LRU 1000 entries; 16 MiB byte cap; 30 s value TTL;
10 s absent-sentinel TTL; targets hits >= 60% (admitted requests), DB CPU < 50%,
read p95 < 60 ms at 100%; stage baselines 70% CPU / 120 ms p95; alerts: p95 > 120 ms
for 5 min, bypass persisting 1 min, any correctness/error-SLO breach; one healthy
hour per stage at 10% / 50% / 100%. Deliverables counted against scope: 1 wrapper
module (new), 1 wrapper test file (new), controller wiring reuse (unchanged).
Estimated changed files: 2-3 (estimate).
### Decision ledger (continued through all sections)
| ID and owner | Contract and evidence | Current | Proposed | Status | Exact approval and scope |
|---|---|---|---|---|---|
| R1 — Section 1 (Architecture), carried to Section 4 | PLAN.md "Existing contracts retained": every read begun after writeProfile fulfills must observe the committed version; TTL is not a substitute. PLAN.md "Proposed wrapper integration": `cache.set(key, value)` after `await repository.read` with no fill/write coordination; fixture fact "already-started readers may finish with their earlier snapshot" and "this admission rule does not inspect cache fills". | Wrapper as proposed: unconditional fill after read resolves. | A: suppress fills that overlap a committed write; B: install then delete; C: keep as proposed (rejected: weakens retained contract). | approved | D1 auto-decided → A under the run's author policy ("authorize complete remedies and required verification that restore its retained contracts ... use the recommended option"). Scope: wrapper fill-time guard with bounded in-flight state, suppressed-fill count on existing dashboard (no key labels), scenarios S1–S6 on the existing harness. Not approved: adapter/repository/controller changes, new metrics projects, data-structure or test-code selection. |
### 0A. Premise Challenge
Real problem: ~900 hot profile-summary keys are re-read so often that one DB
carries 70% CPU and 120 ms read p95. Target outcome: >= 60% of admitted reads served
from process memory, DB CPU < 50%, read p95 < 60 ms, SLOs unchanged. Do-nothing
cost: DB stays near saturation; any traffic growth pushes p95 and error rate up.
The plan attacks the pain directly (fewer repeated store reads), not a proxy.
Premise holds under HOLD SCOPE; one consistency contract (R1) is at risk and is
reviewed, not assumed away.
### 0B. Existing Code Leverage
| Sub-problem | Reused existing code | Note |
|---|---|---|
| Bounded in-memory storage, TTL, absent sentinel, failure bypass | existing LRU cache adapter | fixture fact; no rebuild |
| Coalescing concurrent misses | per-key single-flight inside repository.read | fixture fact; does not inspect fills |
| Atomic commit / rollback | repository.write on in-process transactional store | fixture fact |
| Staged rollout, instance isolation, metrics | existing rollout controller + telemetry + dashboards | fixture fact |
| New: read-through ordering + write invalidation | new wrapper (readProfile/writeProfile) | the only new code |
No rebuild is proposed; refactoring the existing path is not needed.
### 0C. Dream State Mapping
```
CURRENT STATE THIS PLAN 12-MONTH IDEAL
single process, every read ---> process-local LRU in front ---> same single-process service,
hits the store; DB CPU 70%, of repository; >=60% hits, read-through boundary stable
read p95 120 ms DB CPU <50%, p95 <60 ms, enough that a later replacement
consistency contract intact (if ever needed) is a swap
behind the same interface
```
The plan moves toward the ideal without adding a cache framework. Distributed
caching stays out of scope by author decision.
### 0D. Alternatives
No approach choice was required at Step 0: the requested approach (local LRU
wrapper over the existing repository) is retained. 0D is used later for R1.
### 0E. Mode Selection
Explicit user choice: HOLD SCOPE ("Hold the current scope (HOLD SCOPE mode)").
Steps 2-3 skipped. Mode handoff: `Mode: HOLD SCOPE; approved decisions: none.`
Application: preserve scope, maximum rigor on failures, edge cases, error paths,
tests and observability; repairs needed to meet retained invariants are in scope.
Provenance: explicit user instruction; no question asked, no question log.
No new approach decision was needed.
### 0G. HOLD SCOPE checks
1. Complexity: 2-3 changed files, 1 new module, 0 new services. Below the
8-file / 2-service threshold. No fewer-moving-parts alternative exists; the
wrapper is the minimum.
2. Minimum change for the goal: the two wrapper functions plus tests. Nothing in
the accepted scope is deferrable without blocking acceptance (the contract
tests, rollout stages and alerts are launch scope under Prime Directive 5).
3. Invariants and acceptance criteria kept as stated; any repair required to meet
them (R1) is in scope.
Deferral menu: no item proposed for deferral; nothing to ask.
### 0I. Temporal Interrogation
```
HOUR 1 (foundations): implementer needs the retained contracts verbatim (absent DTO,
sentinel TTL, single-flight cohort rule, write-completion boundary).
HOUR 2-3 (core logic): ambiguity: what happens to a fill from a read that began before a
write committed and resolves after writeProfile fulfilled (R1).
HOUR 4-5 (integration): surprise: controller instance swaps isolate old readers, but do
NOT coordinate a fill and a write inside one active instance.
HOUR 6+ (polish/tests): wish they had planned pause/release harness hooks for both
completion orders of overlapping reads and writes, and for
missing-record creation under the absent sentinel.
```
Effort: human ~2-3 days / CC+gstack ~1-2 h for wrapper + tests (tests ~50x ratio).
Feasibility blockers: R1 (resolved below before the sections proceed). Pending
choices: none other than R1.
## Review Sections
**Current scope (published before Section 1):** Mode HOLD SCOPE (explicit user
instruction). Governing rows: R1 unresolved (pending below). Accepted work: the
plan as written above. Deferred: none. Rejected: none. Pending: R1.
### Section 1: Architecture Review
Dependency graph (new component marked *):
```
caller --> rollout controller (predicate, instance swap)
|
v
*wrapper {readProfile, writeProfile}
| |
v v
LRU cache adapter repository.read / repository.write
(1000 / 16 MiB / | (single-flight cohort inside read)
30 s, sentinel 10 s) v
in-process transactional store
```
Data flow, readProfile:
```
happy : get(key) hit -> return DTO | miss -> await read -> set -> return DTO
nil : record missing -> read returns absent DTO -> set stores sentinel (10 s) -> get decodes to absent DTO
empty : key validation runs before repository access (fixture); empty/invalid key never reaches wrapper
error : read rejects -> no set, cache untouched -> typed API error (existing mapping)
adapter failure -> adapter bypasses cache until reinit -> read goes to store
```
Data flow, writeProfile:
```
happy : await write (commit) -> delete(key) -> fulfill (write "completes" at fulfill)
error : write rejects (rolled back) -> no delete -> reject with typed error; cache preserved (correct)
adapter failure on delete -> adapter bypass (fixture); write already committed; wrapper fulfills
```
Coupling: wrapper depends on adapter + repository only; controller owns lifecycle.
Justified; no new cross-module coupling. Scaling: 10x load keeps the hot set at
~900 keys < 1000 entries; 100x load with the same key set is served from memory;
a key set beyond 1000 entries thrashes the LRU and the hit target fails loudly on
the dashboard (no silent failure). SPOF: unchanged (single process, single DB).
Security: no new endpoint or mutation surface; auth precedes repository access
(fixture). Rollback: flag disable -> controller publishes a disabled instance
(bypass both paths); seconds.
**Production failure scenario (CRITICAL GAP, row R1):** a read misses, awaits
repository.read and obtains snapshot v0; a write commits v1, cache.delete runs
(no entry yet), writeProfile fulfills; the read then resolves with v0 and
`cache.set(key, v0)` installs a stale entry. A read begun after the write
completed hits v0 for up to 30 s (or 10 s for an absent sentinel after a create).
This violates the retained contract "every read begun after that write completes
must observe the committed version". Evidence: the plan states no fill/write
coordination; the fixture cohort rule explicitly lets already-started readers
finish with their earlier snapshot and does not inspect fills; controller
isolation only applies across instance swaps. This is a contract-level
contradiction, not an unavailable-implementation guess.
Other outcome: **OK** on boundaries, coupling, scaling, SPOF, security, rollback.
**Decision gate:** Resolve path 1 (new choice). 0D plan-decision route, row R1.
## currentDecision (R1)
Commitment comparison:
```text
Commitment | Source/approval or pending | Current (plan as written) | A | B | C
Reads after a completed write see committed | PLAN.md retained contracts (fixed) | violated by stale fill (gap) | met: fill suppressed when a write to key | met: fill suppressed by re-deleting key after | not met: contract would be weakened
version | | | committed during the in-flight miss | every fill whose read overlapped a write | (not authorizable in this review)
Wrapper ordering rules | pending (R1) | get -> await read -> set | add synchronous per-key write-generation | add per-key "write-seen" flag consulted after | unchanged
| | | check at fill time (in wrapper instance) | set to delete; still momentarily installs |
Memory for coordination state | pending (R1) | none | bounded by in-flight misses (entries | bounded by in-flight misses | none
| | | removed when last miss settles) | |
Adapter / repository / controller contracts | fixture facts (fixed) | unchanged | unchanged | unchanged | unchanged
Regression scenarios (both completion orders,| author acceptance reqs (fixed) | listed as required, not yet | deterministic pause/release scenarios | same scenarios | same scenarios, but one must be
missing-record create, rejected ops, | | specified against R1 | specified below; fill-suppressed branch | | changed to expect stale data
overlapping writes, instance isolation) | | | asserted | | (forbidden)
Telemetry for added branch | author acceptance reqs (fixed) | n/a | count suppressed fills on existing | same | n/a
| | | dashboard, no key labels | |
Changed files | estimate | 2-3 | 2-3 (same files) | 2-3 (same files) | 2-3
```
Question: D1 — R1: How should the wrapper stop a stale cache fill from a read that began before a write committed?
Project/branch/task: main branch, PLAN.md "cache profile summaries in one process", CEO review in HOLD SCOPE.
ELI10: A reader asks the database for a profile, and while it waits, a writer saves a newer profile and clears the cache. The reader's old answer then arrives and gets stored in the cache, so everyone after sees the old profile for up to 30 seconds. The plan promises that never happens, so the wrapper needs a rule that refuses to store an answer that is older than a write that already finished.
Stakes if we pick wrong: users read a profile they just updated and see the previous version; the correctness SLO breaks; the rollout trips its alert and rolls back.
Recommendation: A because a synchronous fill-time check against a per-key write generation is the smallest explicit mechanism that satisfies the contract in a single event loop, with bounded state and one testable branch (explicit over clever).
Completeness: A=10/10, B=7/10, C=3/10
Pros / cons:
A) Suppress fills that overlap a committed write (recommended)
✅ Meets the retained contract in both completion orders without touching the adapter, repository or controller.
✅ Adds one synchronous branch with bounded state; the suppressed-fill count is observable on the existing dashboard.
❌ A hot key under constant writes may rarely fill, so its reads keep going to the store (visible as lower hit rate, not as incorrectness).
B) Install the fill, then delete it when a write overlapped
✅ Also keeps the adapter and repository unchanged and reuses cache.delete.
✅ Simple to reason about: every overlapped fill is removed before the reader returns.
❌ The stale entry exists for a moment between set and delete; a concurrent synchronous get in that window is impossible, but the design relies on strict ordering inside one function and is harder to prove and test than never installing.
C) Keep the wrapper as proposed and accept TTL-bounded staleness
✅ Zero implementation work beyond the plan as written (effort S, no new state).
✅ Simplest code path.
❌ Contradicts the retained consistency contract and the "TTL is not a substitute" clause; not authorizable by this review.
Net: A pays a tiny amount of bounded in-process state to make the contract hold by construction; B pays the same but with a weaker proof; C is not an option under the retained requirements.
Header: R1 stale cache fill
A) Suppress fills that overlap a committed write (recommended)
Effort S, risk low. Reuse: existing adapter, repository and controller unchanged; one new synchronous check in the wrapper instance keyed by the same cache key, tracking a per-key write generation (or equivalent) only while a miss is in flight, removed when the last in-flight miss for that key settles. Verification coverage: both completion orders of overlapping read/write, missing-record creation under the absent sentinel, overlapping writes, rejected reads/writes, instance isolation, suppressed-fill branch asserted and counted. ✅ Meets the retained contract in both completion orders without touching the adapter, repository or controller. ✅ Adds one synchronous branch with bounded state; the suppressed-fill count is observable on the existing dashboard. ❌ A hot key under constant writes may rarely fill, so its reads keep going to the store (visible as lower hit rate, not as incorrectness).
B) Install the fill, then delete it when a write overlapped
Effort S, risk medium. Reuse: adapter, repository and controller unchanged; wrapper installs the value, then calls cache.delete when a per-key write flag was raised during the miss. Verification coverage: same scenarios as A plus an ordering assertion that no get can observe the entry between set and delete. ✅ Also keeps the adapter and repository unchanged and reuses cache.delete. ✅ Simple to reason about: every overlapped fill is removed before the reader returns. ❌ The stale entry exists for a moment between set and delete; a concurrent synchronous get in that window is impossible, but the design relies on strict ordering inside one function and is harder to prove and test than never installing.
C) Keep the wrapper as proposed and accept TTL-bounded staleness
Effort S, risk high; zero implementation work. Reuse: everything as written. Verification coverage: would require changing a required scenario to expect stale data, which the retained contract forbids. ✅ Zero implementation work beyond the plan as written (effort S, no new state). ✅ Simplest code path. ❌ Contradicts the retained consistency contract and the "TTL is not a substitute" clause; not authorizable by this review.
**D1 answer record (R1):** Auto-decided D1 → A (authority: this run's author policy
authorizes the recommended complete remedy restoring retained contracts; no human
present). Applied: "Proposed wrapper integration" amended with the required
guarantee; "Verification and rollout" amended with scenarios S1–S6 and the
suppressed-fill telemetry branch; ledger row R1 → approved. Residual risk: a key
under constant writes may rarely fill (visible as lower hit rate, not stale data).
### Section 2: Error & Rescue Map
```
METHOD/CODEPATH | WHAT CAN GO WRONG | EXCEPTION CLASS
---------------------------|--------------------------------------------|---------------------------
readProfile#cache.get | adapter failure | adapter internal (bypass)
readProfile#repository.read| store read fails / rejects | existing typed repository error
| record missing | none (absent DTO, not an error)
| stale snapshot resolves after completed | none (logic gap) -> R1 guard
| write |
readProfile#cache.set | adapter failure / byte cap exceeded | adapter internal (bypass / evict)
writeProfile#repository.write | write rejects (rolled back) | existing typed repository error
writeProfile#cache.delete | adapter failure after commit | adapter internal (bypass)
controller swap | in-flight old reads during swap | none (isolation contract)
---------------------------|--------------------------------------------|---------------------------
EXCEPTION / CONDITION | RESCUED? | RESCUE ACTION | USER SEES
------------------------------|-----------------|---------------------------------------|------------------
adapter failure (any op) | Y (adapter) | bypass cache until empty reinit; | normal result, slower;
| | bypass alert at 1 min | owner paged
typed repository read error | Y (existing map)| re-raise via existing API mapping; | existing API error
| | no fill |
typed repository write error | Y (existing map)| re-raise; cache untouched (no commit) | existing API error
absent record | n/a | absent DTO; sentinel 10 s | not-found result
stale fill after write (R1) | Y after R1 | skip fill, return value, count branch | committed version
```
No catch-alls proposed; the wrapper adds no new exception classes. **Outcome:**
1 GAP (R1) mapped and resolved by D1 → A; otherwise OK. Decision gate: path 2
(R1 exact answer cited); no new choice.
### Section 3: Security & Threat Model
| Threat | Likelihood | Impact | Mitigated |
|---|---|---|---|
| Cross-tenant cache read via key collision | Low | High | Yes: keys encode authenticated tenant + validated profile ID unambiguously (fixture); authz precedes repository access |
| Cached result bypassing authorization | Low | High | Yes: fixture contract; cache sits below the authz boundary |
| Key or secret leakage in logs/metrics | Low | Med | Yes: no raw IDs in metrics; the R1 suppressed-fill counter must carry no key labels (author requirement) |
| Memory exhaustion via unbounded coordination state | Low | Med | Yes by R1 scope: state bounded by in-flight misses |
| Cache poisoning via mutable DTO | Low | High | Yes: values are immutable DTOs (fixture) |
No new endpoint, input, dependency or background job. **Outcome:** No issues
found (0 High-severity open). Decision gate: no choice needed.
### Section 4: Data Flow & Interaction Edge Cases
Data flow with shadow paths:
```
INPUT key -> VALIDATION (upstream authz/key validation) -> TRANSFORM (get / read) -> PERSIST (set / delete) -> OUTPUT DTO
nil/empty key ........ rejected upstream before wrapper (fixture) tested: existing contract tests
invalid DTO .......... impossible; immutable DTO from repository n/a
exception/timeout .... read rejects -> no set -> typed error tested: rejected reads (S5)
conflict/dup ......... overlapping writes -> last committed wins (S4) tested: S4
stale/partial ........ stale snapshot fill -> R1 guard skips fill (S1) tested: S1-S3
```
Async ordering, boundary = "read begun after writeProfile fulfills observes the
committed version":
```
t | readProfile R (miss) | writeProfile W | cache[key] | gen[key]
--|-----------------------------------|----------------------------------|-----------------|---------
1 | get -> miss; record gen=g0; await | | empty | g0
2 | (paused in repository.read, v0) | await write -> commit v1 | empty | g0
3 | | gen++ ; delete(key); fulfill | empty | g1
4 | read resolves v0; gen g1 != g0 | | empty (skipped) | g1
5 | return v0 to R's caller (allowed: R began before W completed) | |
6 | fresh read R2: miss -> read v1 -> gen unchanged -> set v1 | v1 | g1
```
Order 2 (read resolves before commit): t2 read resolves v0, gen==g0, set v0;
t3 W commits, gen++, delete(key) removes v0; R2 reads v1. Both orders satisfy the
boundary with the R1 guard; without it, order 1 leaves v0 cached (violation).
The excluded schedule "get observes an entry between set and delete" is prevented
by synchronous atomic cache ops in one event loop (fixture). Regression proof:
S1 (order 1), S2 (order 2), S3 (absent sentinel), S4 (overlapping writes), S6
(instance swap), with controlled pause/release points on the existing harness.
Interaction edge cases: no user-visible interaction changes (backend only).
**Outcome:** 5 edge cases mapped, 0 unhandled after R1. Decision gate: path 2
(cites R1).
### Section 5: Code Quality Review
Two small functions fit the existing read-through repository pattern. No
duplicated behavior: invalidation and fill live only in the wrapper. Naming
(readProfile/writeProfile) describes outcome. Error handling delegates to
existing typed mapping and adapter bypass (Section 2). The R1 guard adds one
branch; readProfile stays well under 5 branches. Not over-engineered: no
framework, no version column, no adapter change. Under-engineering risk was the
unconditional fill (R1, resolved). Require an ASCII comment above the wrapper
describing the fill/write ordering (engineering preference: ASCII comments for
complex state). **Outcome:** 1 issue (R1, resolved); comment requirement is a
task (T2). Decision gate: no new choice.
### Section 6: Test Review
```
NEW THING | TEST TYPE | HAPPY | FAILURE | EDGE
---------------------------------|-------------|----------------------------|--------------------------------|-----------------------------
readProfile hit/miss | unit | hit returns cached DTO | read rejects -> typed error | absent DTO -> sentinel decode
readProfile fill guard (R1) | unit/harness| S2 fill then delete | S1 stale fill suppressed | S3 absent sentinel suppressed
writeProfile invalidation | unit | commit -> delete -> fulfil | rejected write preserves cache | S4 overlapping writes
adapter failure fallback | unit | bypass, results correct | bypass persists -> alert | failure on delete after commit
eviction / byte cap / TTL | unit | LRU evicts oldest | over-cap value not stored | 1000th vs 1001st entry
controller swap isolation | integration | new instance fresh cohort | old reader cannot fill new | S6 swap mid-miss
suppressed-fill telemetry | unit | counter +1 on S1 | no key labels asserted | zero when no overlap
```
Assertion check: every row maps to a retained contract or to R1's approved scope;
S1–S6 are the exact scenarios the author required (both orders, missing-record
creation, rejected ops, overlapping writes, instance isolation). Existing contract
tests cover tenant isolation, key validation, absence, DB failures, authz,
multi-process rejection. 2am test: S1. Hostile QA: S4 plus swap mid-miss (S6).
Chaos: adapter failure injected mid-fill while a write commits (bypass must win).
Pyramid: mostly unit, few integration, no E2E (correct for backend). Flakiness:
TTL tests must use the harness's controlled clock, not wall time. Load: existing
rollout stages serve as the load test; no separate load test requested.
**Outcome:** diagram produced, 0 gaps after R1 (TTL-clock note is a task, T3).
Decision gate: path 2 (R1) and settled proof reuse; no new choice.
### Section 7: Performance Review
N+1: none (point reads). Memory: <= 1000 entries / 16 MiB adapter cap plus R1
state bounded by in-flight misses (hundreds of entries at most). Indexes: no new
queries. Caching: this is the caching. Jobs: none. Slow paths: cold miss = store
read (today's p95 120 ms); hit = sub-millisecond; suppressed fill = store read on
the next read of that key. Floor: 900 hot keys / 30 s TTL >= 30 store reads/s
plus suppressed fills on write-hot keys; within existing DB capacity (fixture:
cold starts remain within capacity). Connection pools: unchanged. **Outcome:** No
issues found; residual note: write-hot keys lower the hit rate, surfaced on the
dashboard. Decision gate: no choice.
### Section 8: Observability & Debuggability Review
Metrics (existing telemetry): hit, miss, eviction, cache bytes, fallback/bypass
errors, DB CPU, read p95, plus the R1 suppressed-fill count on the current
dashboard with no key labels (author requirement; not a new metric project).
Logging: adapter failure already logs without keys/secrets; the wrapper adds no
key logging. Alerts: existing (SLO breach, p95 > 120 ms for 5 min, bypass > 1 min);
no new threshold requested. Debuggability: a stale-read report three weeks later is
reconstructable from suppressed-fill and bypass counters plus the flag stage
timeline. Runbook: existing; it must name "suppressed fills climbing" as expected
on write-hot keys, not an incident (task T4, documentation only). **Outcome:** 1
gap (runbook note), resolved as a documentation task within existing runbook
scope. Decision gate: no choice (no new observability project).
### Section 9: Deployment & Rollout Review
No migration. Feature flag: existing runtime flag, 10% -> 50% -> 100% with one
healthy hour each and defined criteria (plan). Rollout order: deploy wrapper
disabled, then raise percentage. Rollback: disable flag -> controller stops
admitting, drains old writes, publishes disabled instance; seconds; runbook
records incident. Deploy-time risk window: one process, so no mixed versions; the
controller swap contract covers in-flight work. Environment parity: stage runs
under the same harness; first 5 minutes watch bypass and hit rate; first hour
watch the healthy-hour criteria. Smoke: hit/miss counters moving, zero bypass.
**Outcome:** 0 risks flagged beyond the plan's existing controls. Decision gate:
no choice.
### Section 10: Long-Term Trajectory Review
Debt: small (one guard with bounded state, one ASCII comment, runbook note).
Path dependency: none; the repository interface preserves a replacement path.
Knowledge: the ordering diagram in Section 4 belongs in the wrapper comment (T2).
Reversibility: 5/5 (flag off or delete two functions). Ecosystem fit: uses the
existing adapter and harness. 1-year question: obvious, provided the comment
explains why fills are suppressed. **Outcome:** reversibility 5/5, debt items 2
(comment, runbook note). Decision gate: no choice.
### Section 11: Design & UX Review
`SKIPPED (no UI scope)` — internal backend change, no UI, API or user-visible state.
## Closing sequence
### Outside Voice
`codex_reviews: disabled` (`.gstack-section-state-b2PTft/config.yaml`). Codex
review skipped (codex_reviews disabled). Re-enable: `gstack-config set
codex_reviews enabled`. No prompt, outside process or native fallback. Disabled
coverage record (`gstack-review-log` codex-plan-review, status skipped,
outside_status disabled): **not persisted** (state writes out of this run's scope).
### Resolve remaining TODO choices
None remain. HOLD SCOPE: no evidenced gap is deferred; every finding is in
accepted scope (R1) or a same-branch task.
### Approval readiness
Checked rows: R1 — approved via D1 auto-decision (authority: run author policy;
recommended option A). Plan applies only that scope (wrapper guard, bounded state,
S1–S6, suppressed-fill count on the existing dashboard). No declined, deferred or
unanswered changes in accepted work. Option C declined and kept out.
`Approval readiness: PASS` (R1 → D1/A).
## Required Outputs
### Review facts
Mode HOLD SCOPE; 1 critical gap found (R1) and resolved; 0 unresolved choices;
0 critical gaps open; scope proposals 0 / accepted 0 / deferred 0; outside
coverage: codex disabled, no completed external review. Status: `clean`.
### NOT in scope
Deferred: none. Rejected: D1 option C "keep the wrapper as proposed and accept
TTL-bounded staleness" — contradicts the retained consistency contract. Author's
out-of-scope list retained: distributed caching, cross-process coherence,
prewarming, consistency-semantics changes, new product surfaces.
### What already exists
See 0B: LRU adapter (limits, TTL, sentinel, bypass), single-flight cohort in
repository.read, atomic repository.write, rollout controller with instance
isolation, telemetry/dashboards/runbooks, contract test harness. All reused.
### Dream state delta
See 0C. After this plan the service meets its DB CPU / p95 targets with the
consistency contract intact and a stable read-through boundary; the 12-month
ideal needs no further caching work unless the key set outgrows 1000 entries.
### Error & Rescue Registry
See Section 2 tables (7 codepath rows, 1 GAP → resolved by R1; 0 CRITICAL GAPS).
### Failure Modes Registry
```
CODEPATH | FAILURE MODE | RESCUED? | TEST? | USER SEES? | LOGGED?
----------------------------|------------------------------|--------------|------------|---------------------|---------------------
readProfile#cache.get/set | adapter failure | Y (bypass) | Y | slower, correct | Y (no keys)
readProfile#repository.read | store error | Y (typed) | Y | existing API error | Y (existing)
readProfile#repository.read | record missing | n/a | Y | not-found result | n/a
readProfile fill | stale snapshot after write | Y (R1 guard) | Y (S1,S3) | committed version | Y (suppressed count)
writeProfile#repository.write| write rejected | Y (typed) | Y (S5) | existing API error | Y (existing)
writeProfile#cache.delete | adapter failure after commit | Y (bypass) | Y | correct, slower | Y (no keys)
controller swap | in-flight miss during swap | Y (isolation)| Y (S6) | committed version | Y (existing)
LRU | hot set > 1000 / 16 MiB | Y (evict) | Y | lower hit rate | Y (eviction metric)
```
8 rows, 0 CRITICAL GAPS.
### Diagrams
1. System architecture: Section 1. 2. Data flow with shadow paths: Sections 1
and 4. 3. State machine: cache entry per key —
```
[absent] --fill(gen unchanged)--> [value v, TTL 30s] --write commit+delete--> [absent]
[absent] --fill(absent DTO)-----> [sentinel, TTL 10s] --create commit+delete--> [absent]
[absent] --fill(gen changed)----> [absent] (suppressed; impossible: stale value installed after completed write)
[value] --TTL expiry/evict-----> [absent]
any --adapter failure------> [bypass] --empty reinit--> [absent]
```
4. Error flow: Section 2. 5. Deployment sequence:
```
deploy wrapper (flag off) -> 10% keys, 1 healthy hour -> 50%, 1 healthy hour -> 100%, acceptance targets
```
6. Rollback flowchart:
```
breach/alert -> disable flag -> controller stops admitting -> await old writes -> publish disabled instance -> runbook incident record -> resume criteria
```
### Stale Diagram Audit
No existing ASCII diagrams in files this plan touches (new module). 0 stale.
## Implementation Tasks
Synthesized from this review's findings. Each task derives from a specific
finding above. Run with Claude Code or Codex; checkbox as you ship.
- [ ] **T1 (P1, human: ~1 day / CC: ~30min)** — wrapper — Implement the R1 fill guard (skip fills whose read overlapped a committed write; bounded in-flight state) and scenarios S1–S6 on the existing harness
- Surfaced by: Section 1 / Section 4 — R1 stale fill after completed write (D1 → A)
- Files: to be determined (new wrapper module and its test file)
- Verify: S1–S6 pass deterministically with pause/release points; S1 asserts no fill and suppressed-fill count +1
- [ ] **T2 (P2, human: ~1h / CC: ~5min)** — wrapper — Add the fill/write ordering ASCII comment (Section 4 schedule) above the wrapper
- Surfaced by: Section 5 / Section 10 — knowledge concentration
- Files: to be determined (wrapper module)
- Verify: comment present and matches Section 4 schedule in review
- [ ] **T3 (P2, human: ~1h / CC: ~5min)** — tests — Drive TTL and sentinel-expiry tests from the harness's controlled clock
- Surfaced by: Section 6 — flakiness risk
- Files: to be determined (wrapper test file)
- Verify: TTL tests pass repeatedly with no wall-clock sleeps
- [ ] **T4 (P2, human: ~30min / CC: ~5min)** — runbook — Add the note that rising suppressed fills on write-hot keys is expected, with the existing bypass/p95 alerts unchanged
- Surfaced by: Section 8 — runbook coverage of the new branch
- Files: to be determined (existing runbook)
- Verify: runbook reviewed by service owner before the 10% stage
_No new tasks from Sections 3, 7, 9, 11._
Task JSONL for /autoplan: **not persisted** (state writes out of this run's scope).
Ratios assumed: bug fix with regression ~20x, docs ~10x.
### Completion Summary
```
+====================================================================+
| MEGA PLAN REVIEW — COMPLETION SUMMARY |
+====================================================================+
| Mode selected | HOLD SCOPE |
| System Audit | skipped per run rules (fixture) |
| Step 0 | HOLD SCOPE; R1 approved (D1 → A) |
| Section 1 (Arch) | 1 issue found (R1, resolved) |
| Section 2 (Errors) | 7 error paths mapped, 1 GAP (resolved) |
| Section 3 (Security)| 0 issues found, 0 High severity |
| Section 4 (Data/UX) | 5 edge cases mapped, 0 unhandled |
| Section 5 (Quality) | 1 issue found (R1; comment task T2) |
| Section 6 (Tests) | Diagram produced, 0 gaps (T3 note) |
| Section 7 (Perf) | 0 issues found |
| Section 8 (Observ) | 1 gap found (runbook note, T4) |
| Section 9 (Deploy) | 0 risks flagged |
| Section 10 (Future) | Reversibility: 5/5, debt items: 2 |
| Section 11 (Design) | SKIPPED (no UI scope) |
+--------------------------------------------------------------------+
| NOT in scope | written (1 rejected, 0 deferred) |
| What already exists | written |
| Dream state delta | written |
| Error/rescue registry| 7 rows, 0 CRITICAL GAPS |
| Failure modes | 8 total, 0 CRITICAL GAPS |
| TODOS.md updates | 0 items proposed |
| Scope proposals | 0 proposed, 0 accepted (HOLD SCOPE) |
| CEO plan | skipped by mode |
| Outside voice | codex disabled (no completed review) |
| Lake Score | 1/1 recommendations chose complete option |
| Diagrams produced | 6 (arch, data flow, state, error, deploy, |
| | rollback) |
| Stale diagrams found | 0 |
| Unresolved decisions | 0 |
+====================================================================+
```
Review Log / decision log / dashboard read: **not persisted / not run** (state
writes and gstack history are out of this run's scope; no write attempted).
No durable learnings this session.
### Unresolved Decisions
None. D1 was resolved by authorized auto-decision (A).
## GSTACK REVIEW REPORT
| Review | Trigger | Why | Runs | Status | Findings |
|--------|---------|-----|------|--------|----------|
| CEO Review | `/plan-ceo-review` | Scope & strategy | 1 (not persisted) | CLEAR | mode: HOLD SCOPE, 0 critical gaps |
| Outside Review | codex (`codex_reviews: disabled`) | Independent 2nd opinion | 0 | DISABLED | disabled by config; no completed external review |
| Eng Review | `/plan-eng-review` | Architecture & tests (required) | 0 | — | not run |
| Design Review | `/plan-design-review` | UI/UX gaps | 0 | — | not run (no UI scope) |
| DX Review | `/plan-devex-review` | Developer experience gaps | 0 | — | not run |
**OUTSIDE COVERAGE:** provider codex, phase plan-review, disabled by `codex_reviews: disabled`; no outside process or native fallback ran; no findings; disabled record not persisted.
**VERDICT:** CEO CLEARED (HOLD SCOPE; R1 resolved by D1 → A) — eng review required
NO UNRESOLVED DECISIONS
test/fixtures/ceo-stale-fill-decision/census-37182865432-missed.md
Vendored
+530
View File
@@ -0,0 +1,530 @@
# Plan: cache profile summaries in one process
## Measured problem and accepted scope
The existing profile-summary service has one active process. A one-week trace
shows repeated reads of about 900 hot keys: DB CPU is 70%, with read p95 120 ms.
Add a process-local LRU wrapper to the existing repository. Acceptance targets
are at least 60% cache hits, DB CPU below 50%, and read p95 below 60 ms, with the
existing error-rate and correctness SLOs unchanged. This is an internal backend
change with no UI, API, schema, pricing, or developer onboarding change.
## Existing contracts retained
- All reads and writes use this repository in the same process; there are no
external DB writers. Multi-process operation remains unsupported and startup
rejects that configuration while caching is enabled.
- These surrounding contracts are accepted fixture facts, supplied by the
existing repository, cache adapter and rollout controller. Preserve them;
review the new wrapper ordering below against them.
- Authentication and authorization run before repository access. Keys encode
the authenticated tenant ID and validated profile ID without ambiguity.
Values are immutable profile-summary DTOs; secrets and cache keys are never
logged. Cached results cannot bypass authorization.
- The existing LRU adapter supports 1000 entries, a 16 MiB byte cap, and a
30-second TTL. Recorded hot data fits those limits. repository.read returns
an immutable absent-result DTO for a missing record, never undefined. The
adapter recognizes that DTO in cache.set, stores an internal sentinel with a
10-second TTL, and cache.get decodes it back to the same absent-result DTO.
The internal sentinel cannot escape the adapter; undefined means a cache miss.
- Cache operations are synchronous and atomic in the single JS event loop.
On any cache failure the existing adapter bypasses the cache until an empty
cache is reinitialized; repository errors keep the current typed API error
mapping. The existing per-key single-flight wrapper sits inside
repository.read, coalesces simultaneous store reads and releases on failure.
A committed repository.write retires that key's old read cohort before its
promise resolves. A later repository.read starts a fresh cohort; a rejected
write leaves the cohort unchanged. Already-started readers may finish with
their earlier snapshot. This admission rule does not inspect cache fills.
- The repository uses an in-process transactional store, with no network
transport between this wrapper and the store. repository.write is atomic:
a resolved promise means committed, and every
rejected promise guarantees no commit; its transaction rolled back before
rejection. Existing contract tests exercise that guarantee.
- Consistency is measured at the public wrapper boundary. A write completes
when writeProfile's promise fulfills after cache.delete, not when
repository.write commits or resolves. A read begins when readProfile is
invoked. Reads that overlap an unfinished writeProfile may return an earlier
snapshot, including reads begun after the store commit but before the wrapper
promise fulfills. Every read begun after that write completes must
observe the committed version. TTL expiry is not a substitute for this rule.
## Proposed wrapper integration
Keep the current read-through repository interface and shared adapters. These
are the complete new read/write ordering rules; no additional version checks or
coordination between a cache fill and a write are proposed:
```javascript
async function readProfile(key) {
const cached = cache.get(key);
if (cached !== undefined) return cached;
const value = await repository.read(key);
cache.set(key, value);
return value;
}
async function writeProfile(key, update) {
const saved = await repository.write(key, update);
cache.delete(key);
return saved;
}
```
### Amendment A1 (accepted via D1/R1, CEO review 2026-10-04)
The ordering above is retained with one added guarantee. **Fill admission:** a
read-miss may fill the cache only if no `repository.write` for that key committed
between the start of that read's `repository.read` call and the fill. Required
behavior: `readProfile` captures the key's write generation before calling
`repository.read` and performs `cache.set` only if the generation is unchanged
when the read resolves; otherwise it skips the fill and still returns the value.
`writeProfile` advances the key's write generation after `repository.write`
resolves (committed) and before `cache.delete`; a rejected write does not advance
it. The absent-result DTO is governed by the same rule. The skipped-fill branch is
counted on the existing dashboard with no key labels. Tradeoff: a read that
overlaps a write may cost one extra DB miss. The generation store's data structure,
bounds and function bodies are engineering-planning choices; they must keep the
operation synchronous in the event loop and must not change the adapter, the
single-flight wrapper, the controller, limits, TTLs or consistency semantics.
Deterministic regression scenarios (existing harness, controlled pause/release):
S1 read resolves and fills before the write commits → delete clears it → next read
observes v1; S2 read snapshot pre-commit, write completes, read resumes → fill
skipped → next read observes v1; S3 same as S2 with a missing record created by
the write → no absent sentinel is cached; S4 rejected write overlapping a read →
fill admitted, v0 correct; S5 rejected read → no fill, typed error unchanged;
S6 overlapping writes W1/W2 with a read between → final read observes W2's
version; S7 controller instance change during an in-flight read → old read cannot
fill the new instance (existing isolation test retained).
## Verification and rollout
Existing repository contract tests cover tenant isolation, key validation,
absence, DB failures, authorization, and startup rejection of multi-process
operation while caching is enabled. New wrapper tests cover hit/miss,
eviction and byte limits, TTL, adapter-failure fallback, successful-write
invalidation, failed-write preservation, and concurrent-miss coalescing.
The rollout uses the existing runtime feature flag: enable for 10% of keys,
then 50%, then all keys after one healthy hour at each stage. Monitor hit/miss,
eviction, cache bytes, fallback errors, DB CPU, and read p95 without raw IDs.
The existing controller uses one shared key-selection predicate for reads and
writes. On any enable, disable or percentage change, it stops admitting work,
awaits every admitted old-instance write, then publishes a new wrapper/cache
instance with a fresh single-flight cohort before admitting new work. Old reads
retain their old instance and cannot fill the new one. Disabled instances
bypass the cache on both paths. Tests cover the old-writer/new-reader ordering,
all those transitions and predicate parity. This lifecycle isolation does not coordinate
an ordinary DB write with a cache fill in the same active instance.
Existing dashboards and runbooks cover these metrics. Before each stage, verify
that alerts page the service owner on any correctness/error-SLO breach, read
p95 above 120 ms for five minutes, or cache bypass persisting for one minute.
Hit rate is hits / (hits + misses) among requests admitted to the cache path;
flag-excluded or adapter-bypassed requests are tracked separately, not as misses.
DB CPU and read p95 are service-wide metrics, including bypassed requests.
At the 10% and 50% stages, a healthy hour requires at least 60% admitted-request
hits, unchanged correctness/error SLOs, no alerts, and aggregate DB CPU/read p95
no worse than their 70%/120 ms pre-rollout baselines. At 100%, the original
absolute acceptance targets (DB CPU below 50%, read p95 below 60 ms, hits at
least 60%) must all hold with unchanged correctness/error SLOs and no alerts.
Any breach disables the flag immediately; the runbook records the incident,
rollback and criteria for resuming. These are existing
rollout-controller and telemetry contracts, not proposed wrapper additions.
Cold starts remain within the existing DB capacity. The service owner monitors
the rollout and records the results against the acceptance targets.
## Out of scope
Distributed caching, cross-process coherence, prewarming, changing consistency
semantics, or adding new product surfaces. The repository interface preserves a
future replacement path without introducing a general cache framework now.
## Author's review and acceptance requirements
This is a full CEO scope and feasibility review. The author has approved the
retained contracts, limits, rollout and acceptance targets above. Evaluate the
proposed wrapper against them; the wrapper itself remains unapproved. An actual
contradiction or missing proof must be reported and resolved, not assumed away.
For a demonstrated gap, amend the plan with the required guarantee, a feasible
remedy, its tradeoffs and deterministic regression scenarios. Those repairs
and their required verification are within the requested scope. The exact data
structures, full function bodies and executable test code belong to subsequent
engineering planning; do not select or implement them during this review when
the required behavior and feasibility can already be established.
Use the existing deterministic repository-contract test harness. Required wrapper
acceptance includes both completion orders of overlapping reads and writes,
missing-record creation, rejected reads/writes, overlapping writes, and isolation
across controller instance changes. Use existing telemetry to record any added
branch on the current dashboard with no key labels; no new alert threshold or metric
project is requested. These are future acceptance requirements, not tests already
implemented or passing. Preserve all 11 review outcomes, required registries,
diagrams, tasks, completion summary and the full GSTACK REVIEW REPORT.
---
# CEO Review (plan-ceo-review, 2026-10-04)
## Step 0 — Scope challenge and mode
Review depth: implementation-ready (default; the plan names codepaths). Preamble
skipped per run instructions (no skill-start, no system audit, no telemetry);
AskUserQuestion unavailable; the run instructions authorize auto-decision under
the author policy. Base branch: `main` (git status snapshot; no git run).
Config `.gstack-section-state-aQnx04/config.yaml`: `codex_reviews: disabled`.
**0A Premise.** Real problem: ~900 hot keys re-read from the DB (DB CPU 70%,
read p95 120 ms). Do-nothing cost: DB saturation headroom gone before growth.
A process-local LRU attacks the pain directly (one process, no external writers).
**0B Leverage.** Reuses existing repository, LRU adapter (1000 entries/16 MiB/
30 s TTL, absent sentinel), per-key single-flight, rollout controller,
dashboards and runbooks. No rebuild proposed; nothing to refactor away.
**0C Dream state.**
```
CURRENT STATE THIS PLAN 12-MONTH IDEAL
DB serves every read ---> process-local LRU, ---> repository interface
CPU 70%, p95 120 ms >=60% hits, CPU<50%, ready for a shared
single process p95<60 ms, same SLOs cache if ever needed
```
Moves toward the ideal without a general cache framework.
**0E Mode.** Explicit user instruction: HOLD SCOPE (no mode question asked, no
handoff script available; provenance = user instruction). Changed files: ~3
(wrapper, wrapper tests, flag wiring) — estimate; complexity check passes
(1 new wrapper, 0 new services). Minimum change = the wrapper; nothing deferrable
without blocking the goal. Invariants and acceptance criteria are kept; repairs
needed to meet them are in scope (0G HOLD rule 3).
**0D required approach choice.** One contradiction found between the proposed
ordering and a retained contract; recorded as row R1 below. No other approach
choice is required.
### Decision ledger
| ID and owner | Contract and evidence | Current | Proposed | Status | Exact approval and scope |
|---|---|---|---|---|---|
| R1 — Section 4 (async ordering); owner: wrapper author | PLAN.md "Existing contracts retained", last bullet: every read begun after writeProfile fulfills must observe the committed version; TTL is no substitute. PLAN.md "Proposed wrapper integration": `readProfile` awaits `repository.read` then `cache.set` with no fill/write coordination ("no additional version checks or coordination between a cache fill and a write are proposed"); single-flight "does not inspect cache fills"; controller isolation "does not coordinate an ordinary DB write with a cache fill". Schedule S2 below demonstrates a stale fill. | Proposed ordering as written (unapproved) | A) add a fill-admission guard; B) refill on write; C) keep as proposed | approved | D1 → A, auto-decided 2026-10-04 under the run's author policy ("authorize complete remedies and required verification that restore its retained contracts"; recommended option among in-scope alternatives). Scope: the fill-admission guarantee, its write-side generation advance, the skipped-fill counter on the existing dashboard (no key labels), and the listed regression scenarios. Not approved: data structures, function bodies, test code, any change to limits, TTL, consistency semantics or alerts. |
**Decision record D1 (R1).** Selected A. Authority: user run instructions
(author policy), not a human answer and not a preamble SESSION_KIND echo. B was
rejected because it leaves S2 reachable; C was rejected because the author policy
forbids weaker consistency. Lake Score eligible: yes (10/10 option chosen).
### Demonstrated schedule (S2, violating)
```
t | readProfile R1 (miss) | writeProfile W | cache[key] | store
1 | cache.get -> undefined | | - | v0
2 | await repository.read (v0 snapshot taken) | - | v0
3 | | repository.write commits | - | v1
4 | | cohort retired, resolves | - | v1
5 | | cache.delete (no-op) | - | v1
6 | | promise fulfills = WRITE COMPLETE | v1
7 | resumes: cache.set(key, v0) | | v0 STALE | v1
8 | readProfile R2 begins after step 6: cache.get -> v0 <-- VIOLATION | v1
```
No retained mechanism excludes S2: single-flight admits R1 as an already-started
reader that "may finish with their earlier snapshot"; the controller only isolates
instance changes; the adapter does not know about writes. The same schedule with an
absent-result DTO at step 2 caches "absent" for 10 s after a creating write.
## currentDecision (R1)
Commitment comparison:
```text
Commitment | Source/approval or pending | Current | A | B | C
Read-after-write visibility at wrapper boundary | PLAN.md retained contract (approved) | required | kept | kept | VIOLATED (S2)
Cache fill on read miss | PLAN.md proposed (pending) | unconditional set | set only if no write committed for key since the read's store access began | unconditional set | unconditional set
Write path after commit | PLAN.md proposed (pending) | cache.delete | bump per-key write generation, then cache.delete | cache.set(saved) after commit | cache.delete
Consistency semantics / limits / TTL | PLAN.md retained (approved) | unchanged | unchanged | unchanged | unchanged
Single-flight, adapter, controller contracts | fixture facts (approved) | unchanged | unchanged | unchanged | unchanged
Added telemetry | PLAN.md acceptance reqs (approved) | none | existing counter for skipped fills, no key labels | none | none
Regression tests | PLAN.md acceptance reqs (approved) | required | both orders, absent creation, rejected R/W, overlapping writes, instance isolation | same list but S2 still fails | S2 fails
Data structures / function bodies | engineering planning (pending) | not chosen | not chosen | not chosen | not chosen
```
Question: D1 — R1: How does the wrapper keep a slow cache fill from overwriting a newer committed write?
Project/branch/task: profile-summary service, branch main, process-local LRU wrapper plan.
ELI10: A read can fetch an old copy from the database, pause, and only then put it in the cache. If a write finished during that pause, the cache now holds the old copy and every later reader gets stale data until the 30-second TTL. The plan promises that never happens once a write completes.
Stakes if we pick wrong: users see pre-write profile data for up to 30 s (10 s for "absent") after a confirmed save, breaking the correctness SLO the plan says is unchanged.
Recommendation: A because it closes the only violating schedule with one in-process check and keeps every retained contract and limit unchanged.
Completeness: A=10/10, B=5/10, C=3/10
Pros / cons:
A) Fill-admission guard (recommended)
✅ Any fill whose store access began before a commit is dropped, so S2 cannot publish v0 after W completes
✅ No change to adapter, single-flight, controller, TTL or consistency contracts; one counter on the existing dashboard
❌ Reads overlapping a write pay one extra DB miss; needs per-key generation state in the wrapper (engineering chooses the structure)
B) Refill cache from the write result
✅ Readers after a write get an immediate hit on the saved DTO
✅ Simple to describe and no per-key bookkeeping
❌ Does not prevent S2: R1's late cache.set(v0) still lands after the write's set(v1); also changes the write path from delete to fill
C) Keep as proposed
✅ Zero extra work and smallest diff
✅ Matches the current code sketch exactly
❌ Leaves the retained read-after-write guarantee violated; not permitted by the author policy (no weaker consistency)
Net: A trades a rare extra miss for a provable guarantee; B and C both leave the demonstrated stale fill in place.
Header: R1 stale fill guard
A) Fill-admission guard (recommended)
Guard every cache fill: readProfile records the key's write generation before calling repository.read and sets the cache only if it is unchanged afterwards; writeProfile advances the generation after commit and before cache.delete, recording skipped fills on the existing dashboard without key labels. Effort S, risk low, reuses all existing adapters, verification: both completion orders, absent-record creation, rejected reads/writes, overlapping writes, instance isolation. ✅ closes S2 ✅ keeps all retained contracts ❌ extra miss for reads overlapping a write.
B) Refill cache from the write result
writeProfile replaces cache.delete with cache.set(saved) after commit; reads stay as proposed. Effort S, risk medium, reuses adapters, verification: same scenario list. ✅ immediate hits after writes ✅ no bookkeeping ❌ S2 still overwrites v1 with v0 so the guarantee stays broken.
C) Keep as proposed
No implementation work (effort S, zero changes), risk high, verification: existing list only. ✅ smallest diff ✅ matches sketch ❌ demonstrated contract violation remains and cannot be accepted under the author policy.
## 0I Temporal interrogation
```
HOUR 1 wrapper sits between callers and repository; adapter/single-flight/controller are fixed facts
HOUR 2-3 ambiguity: where the per-key write generation lives and when it is pruned (engineering choice, A1 constraints)
HOUR 4-5 surprise: S2 only reproduces with a controlled pause inside repository.read; harness pause points needed
HOUR 6+ wish: skipped-fill counter wired before the 10% stage so the branch is visible on day 1
```
Effort: human ~1.5 days / CC+gstack ~40 min. Feasibility blockers: none after D1.
Pending (engineering): generation-store structure and bounds; harness pause-point selection.
## Current scope (Section 1 preface)
Mode HOLD SCOPE (user instruction). Governing rows: R1 approved (D1 → A, author
policy). Accepted: original plan + A1. Deferred: none. Rejected: options B and C
of R1. Pending: engineering-planning details named in A1.
## Section 1: Architecture Review
```
callers -> [flag predicate] -> readProfile/writeProfile (wrapper, +gen guard)
| | |
| excluded v v
+--------------> repository.read/write (single-flight, txn store)
^
cache adapter (LRU 1000/16MiB/30s, absent sentinel)
```
Data flow paths: happy = miss→store→fill→hit; nil = absent DTO cached via
sentinel 10 s; empty = `undefined` only ever means miss (adapter contract);
error = repository rejection propagates with typed mapping, no fill.
State machine (per key, wrapper view): `empty → filled (gen g) → deleted (gen g+1)`;
invalid transition `filled(g) ← fill from read captured at g' < g` is now
prevented by A1. Coupling: wrapper gains a per-key generation; justified by the
retained contract. Scaling: 10x hot keys exceed 1000 entries → eviction, lower
hit rate, not a correctness issue; 100x → DB load returns to baseline, same as
today. SPOF: the single process (already the case). Security: no new surface.
Production failure: adapter failure → bypass until reinit (fixture). Rollback:
flag disable → new bypassing instance, seconds.
**Finding 1.1 (WARNING, resolved via R1):** stale-fill schedule S2. Remedy A1.
Outcome: 1 issue, resolved. Decision gate: R1 answer applied; no new choice.
## Section 2: Error & Rescue Map
```
METHOD/CODEPATH | WHAT CAN GO WRONG | EXCEPTION CLASS
readProfile | cache.get throws / adapter failure | CacheAdapterError (fixture: bypass)
| repository.read rejects (DB/timeout) | RepositoryError (typed API mapping)
| fill skipped by generation guard | not an error; counted
writeProfile | repository.write rejects (rolled back) | RepositoryError; no gen advance, no delete
| cache.delete throws | CacheAdapterError (fixture: bypass)
controller | flag transition mid-flight | n/a; isolation (fixture)
EXCEPTION CLASS | RESCUED? | RESCUE ACTION | USER SEES
CacheAdapterError | Y (adapter) | bypass until reinit; alert 1m | normal, slower
RepositoryError | Y (API layer) | typed error, no fill/no delete | existing API error
```
No catch-alls introduced; the wrapper adds no rescue of its own. Outcome: 5
error paths mapped, 0 GAPS. Decision gate: nothing new.
## Section 3: Security & Threat Model
Attack surface unchanged (no endpoints, params, files, jobs). Tenant-scoped keys
and pre-repository authz are fixture facts; cached values cannot bypass authz.
Threat: cross-tenant leak via key collision — likelihood Low, impact High,
mitigated (unambiguous tenant+profile key). Threat: key/secret in logs —
Low/Med, mitigated (no raw IDs; counter has no key labels). No new secrets or
dependencies. Outcome: 0 issues, 0 High unmitigated.
## Section 4: Data Flow & Interaction Edge Cases
```
INPUT key -> (validated upstream) -> cache.get -> miss -> repository.read -> gen check -> cache.set -> OUTPUT
shadow: nil/absent -> absent DTO -> sentinel 10s | error -> reject, no fill | stale -> gen changed -> skip fill
```
Async ordering: invariant = reads begun after writeProfile fulfills observe the
committed version. Both orders: S1 (fill before commit → delete clears) safe by
the existing delete; S2 (fill after commit) violating as proposed, safe under A1
because the generation advanced before the write could fulfil. Mechanism named:
per-key write generation compared at fill time. Regression proof: S1–S7 in A1.
No user-visible interactions (backend only). Outcome: 7 edge cases mapped, 0
unhandled after A1 (1 was unhandled before). Decision gate: R1 reused.
## Section 5: Code Quality Review
Wrapper fits the existing read-through pattern; no duplication (single-flight
and sentinel stay in their owners). Naming: `readProfile`/`writeProfile` fine.
Under-engineering fixed by A1; no over-engineering (no framework). Branching
stays under 5 per function. Outcome: 0 new issues.
## Section 6: Test Review
```
new thing | type | happy | failure | edge
readProfile hit/miss | unit | hit returns DTO | adapter failure bypass | absent sentinel 10s
readProfile fill guard | integration | S1 | S5 rejected read | S2, S3 (absent)
writeProfile invalidate | integration | delete after commit| S4 rejected write | S6 overlapping writes
controller isolation | integration | instance swap | transition mid-read | S7
eviction/bytes/TTL | unit | 1000/16MiB/30s | over cap evicts | boundary sizes
```
Assertion check: each S1–S7 names the observed version and the wrong result it
rejects (exact version, not "eventually"). 2am test: S2. Hostile QA: S3 absent
creation. Chaos: adapter failure mid-fill. Pyramid: mostly unit/integration, no
E2E needed. Flakiness: all schedules use deterministic pause/release, not timers.
Load: existing capacity statement covers cold start. Outcome: diagram produced,
0 gaps after A1 (the S2/S3 assertions were missing before).
## Section 7: Performance Review
No N+1, no new queries or indexes. Memory: 16 MiB cap plus generation state
(bounded per A1 engineering constraint). Slow paths: miss (= today), guarded
miss overlapping a write (= today + one skipped fill). Connection pools unchanged.
Outcome: 0 issues.
## Section 8: Observability & Debuggability Review
Existing metrics: hit/miss, eviction, bytes, fallback errors, DB CPU, p95.
Added branch: skipped-fill counter on the current dashboard, no key labels
(within accepted scope; no new alert). Debuggability: counter + existing error
logs reconstruct a stale-fill suspicion without raw IDs. Runbook: existing
breach → disable flag. Outcome: 0 gaps (1 counter required by A1, accepted).
## Section 9: Deployment & Rollout Review
No migration. Flag staged 10% → 50% → 100% with one healthy hour each; breach
disables immediately; controller swaps instances safely (fixture). Old/new code
window: wrapper is additive; disabled instances bypass both paths. Post-deploy:
first 5 min watch fallback errors and skipped fills; first hour the healthy-hour
criteria. Smoke: hit/miss counters move; no bypass alert. Outcome: 0 new risks;
1 note: verify alerts page before each stage (already in plan).
## Section 10: Long-Term Trajectory Review
Debt: one per-key generation structure (documented in A1). Path dependency: low;
the repository interface keeps a replacement path. Reversibility 5/5 (flag off).
Ecosystem fit: yes. 1-year question: obvious if A1's guarantee is commented at
the fill site. Outcome: debt items 1, reversibility 5/5.
## Section 11: Design & UX Review
SKIPPED (no UI scope).
## Closing sequence
**Outside Voice:** `codex_reviews: disabled` in the isolated config. Codex review
skipped (codex_reviews disabled). Re-enable: `gstack-config set codex_reviews enabled`.
No native fallback; coverage recorded as disabled (review-log helper not run in
this fixture; disabled record **not persisted**).
**TODO choices:** none proposed (HOLD SCOPE; no evidenced deferred gap).
**Approval readiness: PASS** — R1: D1 → A (author-policy auto-decision, scope as
recorded in the ledger). No other rows.
## Required Outputs
### Review facts
Mode HOLD SCOPE; findings 1 (1.1, resolved via R1/A1); unresolved choices 0;
critical gaps 0 after A1; scope proposals 0; outside coverage: codex disabled.
Status: clean.
### NOT in scope
Deferred: none. Rejected: R1 option B (refill on write — leaves S2 reachable);
R1 option C (keep as proposed — violates retained read-after-write rule). Plan's
own out-of-scope list retained (distributed cache, cross-process coherence,
prewarming, semantic changes, new surfaces).
### What already exists
Repository with single-flight and typed errors (reused); LRU adapter with limits
and absent sentinel (reused); rollout controller with instance isolation (reused);
dashboards, alerts and runbooks (reused); deterministic contract harness (reused).
### Dream state delta
After this plan: hot reads served in-process with a provable read-after-write
guarantee; the repository interface still allows a shared cache later. Gap to the
12-month ideal: only the multi-process story, deliberately out of scope.
### Error & Rescue Registry
See Section 2 map (cross-reference). Rows: 5; CRITICAL GAPS: 0.
### Failure Modes Registry
```
CODEPATH | FAILURE MODE | RESCUED? | TEST? | USER SEES? | LOGGED?
readProfile | adapter failure | Y | Y | slower, correct | Y (fallback)
readProfile | repository.read rejects | Y | Y(S5) | typed API error | Y
readProfile fill | stale fill after commit (S2) | Y (A1) | Y(S2) | correct version | Y (skip counter)
readProfile fill | stale absent after create (S3)| Y (A1) | Y(S3) | correct record | Y (skip counter)
writeProfile | repository.write rejects | Y | Y(S4) | typed API error | Y
writeProfile | cache.delete throws | Y | Y | slower, correct | Y (fallback)
controller | transition mid-read | Y | Y(S7) | correct | Y
```
Total 7, CRITICAL GAPS 0.
### Diagrams
1. System architecture: Section 1. 2. Data flow with shadow paths: Section 4.
3. State machine: Section 1 (per-key `empty → filled(g) → deleted(g+1)`).
4. Error flow: Section 2.
5. Deployment sequence:
```
deploy wrapper (flag off) -> enable 10% -> healthy hour -> 50% -> healthy hour -> 100% -> acceptance check
```
6. Rollback flowchart:
```
breach (SLO / p95>120ms 5m / bypass 1m)? --yes--> flag off -> controller drains writes, publishes bypass instance -> runbook incident
--no---> continue stage
```
### Stale Diagram Audit
No existing ASCII diagrams in touched files were supplied; the plan's own code
sketch is amended by A1 (not stale: A1 states the added guarantee alongside it).
## Implementation Tasks
Synthesized from this review's findings. Each task derives from a specific
finding above. Run with Claude Code or Codex; checkbox as you ship.
- [ ] **T1 (P1, human: ~4h / CC: ~15min)** — wrapper — Implement the fill-admission guard (A1): generation capture before `repository.read`, conditional `cache.set`, generation advance after commit before `cache.delete`
- Surfaced by: Section 4 — Finding 1.1 / schedule S2 (R1, D1 → A)
- Files: to be determined (wrapper module)
- Verify: S1–S6 pass in the deterministic harness with controlled pause/release
- [ ] **T2 (P1, human: ~4h / CC: ~10min)** — tests — Add regression scenarios S1–S7 to the existing contract harness
- Surfaced by: Section 6 — S2/S3 assertions missing before A1
- Files: to be determined (wrapper test suite)
- Verify: each scenario asserts the exact observed version and fails against the unguarded sketch
- [ ] **T3 (P2, human: ~1h / CC: ~5min)** — telemetry — Count skipped fills on the existing dashboard without key labels
- Surfaced by: Section 8 — added branch must be visible
- Files: to be determined (existing telemetry wiring)
- Verify: counter increments in S2/S3 runs; dashboard panel shows it
_No new tasks from Sections 2, 3, 5, 7, 9, 10, 11._
Task JSONL: **not persisted** (no gstack state root in this fixture).
### Completion Summary
```
+====================================================================+
| MEGA PLAN REVIEW — COMPLETION SUMMARY |
+====================================================================+
| Mode selected | HOLD SCOPE |
| System Audit | skipped per run instructions |
| Step 0 | HOLD SCOPE; R1 → A (fill-admission guard) |
| Section 1 (Arch) | 1 issues found (resolved) |
| Section 2 (Errors) | 5 error paths mapped, 0 GAPS |
| Section 3 (Security)| 0 issues found, 0 High severity |
| Section 4 (Data/UX) | 7 edge cases mapped, 0 unhandled |
| Section 5 (Quality) | 0 issues found |
| Section 6 (Tests) | Diagram produced, 0 gaps |
| Section 7 (Perf) | 0 issues found |
| Section 8 (Observ) | 0 gaps found |
| Section 9 (Deploy) | 0 risks flagged |
| Section 10 (Future) | Reversibility: 5/5, debt items: 1 |
| Section 11 (Design) | SKIPPED (no UI scope) |
+--------------------------------------------------------------------+
| NOT in scope | written (2 items) |
| What already exists | written |
| Dream state delta | written |
| Error/rescue registry| 5 rows, 0 CRITICAL GAPS |
| Failure modes | 7 total, 0 CRITICAL GAPS |
| TODOS.md updates | 0 items proposed |
| Scope proposals | 0 proposed, 0 accepted (EXP + SEL) |
| CEO plan | skipped by mode |
| Outside voice | codex disabled |
| Lake Score | 1/1 recommendations chose complete option |
| Diagrams produced | 6 (arch, data flow, state, error, deploy, rollback) |
| Stale diagrams found | 0 |
| Unresolved decisions | 0 (listed below) |
+====================================================================+
```
### Unresolved Decisions
None. D1 was resolved by authorized auto-decision (author policy).
## GSTACK REVIEW REPORT
| Review | Trigger | Why | Runs | Status | Findings |
|--------|---------|-----|------|--------|----------|
| CEO Review | `/plan-ceo-review` | Scope & strategy | 1 (not persisted) | CLEAR | mode: HOLD SCOPE, 0 critical gaps |
| Outside Review | codex (`codex_reviews: disabled`) | Independent 2nd opinion | 0 | DISABLED | disabled by config; no completed external review |
| Eng Review | `/plan-eng-review` | Architecture & tests (required) | 0 | — | — |
| Design Review | `/plan-design-review` | UI/UX gaps | 0 | — | — |
| DX Review | `/plan-devex-review` | Developer experience gaps | 0 | — | — |
- **OUTSIDE COVERAGE:** provider codex, phase plan-review, disabled by `codex_reviews: disabled`; no outside process, no native fallback, no findings claimed.
- **VERDICT:** CEO CLEARED — eng review required
NO UNRESOLVED DECISIONS
test/helpers/ceo-stale-fill-decision.ts
+63
View File
@@ -0,0 +1,63 @@
/**
* The CEO section-loading fixture's real defect is the stale cache fill: a read
* that misses before a write commits can store its old value after the write's
* invalidation. /plan-ceo-review records every contract conflict in two
* structures it defines (plan-ceo-review/SKILL.md.tmpl): a decision-ledger row
* (`| ID and owner | … | Status | Exact approval and scope |`) and a saved
* `## currentDecision (ROW-ID)` block whose `Question:` names the decision.
* Reading those records, rather than matching prose phrasings, is what proves
* the review found the race and approved a remedy.
*/
const LEDGER_HEADER = /^\|\s*ID and owner\s*\|\s*Contract and evidence\s*\|\s*Current\s*\|\s*Proposed\s*\|\s*Status\s*\|\s*Exact approval and scope\s*\|\s*$/;
const FILL = /\b(?:fill\w*|refill\w*|repopulat\w*)\b|\bcache\.set\b/i;
const WRITE = /\b(?:writes?|written|writer|commit\w*)\b/i;
const RACE = /\b(?:stale|race|racing|overwrit\w*|newer|old|older|before|after|overlap\w*|in[- ]flight|concurrent\w*)\b/i;
/** Asserted lines only: fenced blocks, block quotes and indented code become blank. */
function proseLines(report: string): string[] {
let fence: { char: string; length: number } | null = null;
return report.split('\n').map(line => {
const delimiter = /^ {0,3}(`{3,}|~{3,})(.*)$/.exec(line);
if (delimiter) {
const run = delimiter[1]!;
if (!fence) fence = { char: run[0]!, length: run.length };
else if (run[0] === fence.char && run.length >= fence.length && !delimiter[2]!.trim()) fence = null;
return '';
}
return fence || /^\s*>/.test(line) || /^(?: {4}|\t)/.test(line) ? '' : line;
});
}
/** The latest ledger row per ID, from tables under the skill's exact ledger header. */
function ledgerRows(lines: string[]): Map<string, string> {
const rows = new Map<string, string>();
for (let i = 0; i < lines.length; i++) {
if (!LEDGER_HEADER.test(lines[i]!)) continue;
for (let j = i + 2; j < lines.length && lines[j]!.trimStart().startsWith('|'); j++) {
const cells = lines[j]!.trim().slice(1, -1).split('|').map(cell => cell.trim());
const id = /^(R[1-9][\w-]*)(?=\s|$)/.exec(cells[0] ?? '')?.[1];
// A row that does not split into the six ledger cells is not a record.
if (!id || cells.length !== 6) continue;
rows.set(id, cells[4]!);
}
}
return rows;
}
/** An approved ledger decision whose saved question is the stale-fill race. */
export function hasApprovedStaleFillDecision(report: string): boolean {
const lines = proseLines(report);
const rows = ledgerRows(lines);
for (let i = 0; i < lines.length; i++) {
const rowId = /^#{2,3}\s+currentDecision \((R[1-9][\w-]*)\)\s*$/.exec(lines[i]!)?.[1];
if (!rowId) continue;
for (let j = i + 1; j < lines.length && !/^#{1,6}\s/.test(lines[j]!); j++) {
const subject = /^Question:\s*D[1-9]\d*\s*[—–-]\s*(.+)$/.exec(lines[j]!)?.[1];
if (!subject) continue;
if (FILL.test(subject) && WRITE.test(subject) && RACE.test(subject) && /^[*_]*approved\b/i.test(rows.get(rowId) ?? '')) return true;
break;
}
}
return false;
}
test/helpers/touchfiles-data.ts
+1 -1
View File
@@ -392,7 +392,7 @@ export const E2E_TOUCHFILES: Record<string, string[]> = {
'ship-section-loading': [ 'ship/**', 'scripts/resolvers/sections.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/session-runner.ts', 'test/skill-e2e-ship-section-loading.test.ts',
'scripts/resolvers/testing.ts', 'test/helpers/agent-sdk-runner.ts', 'test/helpers/auq-native-capture.ts', 'test/helpers/auto-decision-state.ts', 'test/helpers/autoplan-artifact-digest.ts', 'test/helpers/autoplan-artifact-permission.ts', 'test/helpers/autoplan-artifact-recorder.ts', 'test/helpers/claude-pty-runner.ts', 'test/helpers/pty/**', 'test/helpers/dx-selected-navigation.ts', 'test/helpers/e2e-gate.ts', 'test/helpers/eng-cache-writer-decision.ts', 'test/helpers/hermetic-skill-runtime.ts', 'test/helpers/llm-judge.ts', 'test/helpers/native-auto-decide.ts', 'test/helpers/owned-claude-transcript.ts', 'test/helpers/plan-count-artifacts.ts', 'test/helpers/plan-count-file-permission.ts', 'test/helpers/plan-count-fixture.ts', 'test/helpers/plan-count-pending-exit.ts', 'test/helpers/plan-count-pending-question.ts', 'test/helpers/plan-count-transcript.ts', 'test/helpers/plan-floor-review.ts', 'test/helpers/plan-floor-target.ts', 'test/helpers/plan-scope-selection.ts', 'test/helpers/plan-seed-submission.ts', 'test/helpers/plan-skill-question-events.ts', 'test/helpers/plan-skill-question-hook-scope.ts', 'test/helpers/plan-skill-questions.ts', 'test/helpers/pty-screen.ts', 'test/helpers/pty-trust-dialog.ts', 'test/helpers/skill-census.ts'],
'plan-ceo-section-loading': [
'test/fixtures/ceo-fill-lifetime.json', 'plan-ceo-review/**', 'scripts/resolvers/sections.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/session-runner.ts', 'test/helpers/ceo-section-loading-fixture.ts', 'test/skill-e2e-plan-ceo-review-section-loading.test.ts', 'test/fixtures/ceo-section-loading-l-report.md', 'test/fixtures/ceo-section-loading-q-report.md', 'test/fixtures/ceo-section-r-rejected-report.md', 'test/fixtures/ceo-section-s-trace-report.md', 'test/fixtures/ceo-section-u-report.md', 'test/fixtures/ceo-section-y-report.md', 'test/fixtures/ceo-section-aa-report.md', 'test/fixtures/sdk-stale-table-ad-v3.json', 'test/fixtures/sdk-ordering-ae.json', 'test/fixtures/sdk-columnar-af.json', 'test/fixtures/sdk-order-b-ag.json', 'test/fixtures/sdk-schedule-continuation-ah.json', 'test/fixtures/sdk-original-order-ai.json', 'test/fixtures/sdk-compact-sequence-aj.json',
'test/fixtures/ceo-fill-lifetime.json', 'plan-ceo-review/**', 'scripts/resolvers/sections.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/session-runner.ts', 'test/helpers/ceo-section-loading-fixture.ts', 'test/helpers/ceo-stale-fill-decision.ts', 'test/skill-e2e-plan-ceo-review-section-loading.test.ts', 'test/fixtures/ceo-section-loading-l-report.md', 'test/fixtures/ceo-section-loading-q-report.md', 'test/fixtures/ceo-section-r-rejected-report.md', 'test/fixtures/ceo-section-s-trace-report.md', 'test/fixtures/ceo-section-u-report.md', 'test/fixtures/ceo-section-y-report.md', 'test/fixtures/ceo-section-aa-report.md', 'test/fixtures/sdk-stale-table-ad-v3.json', 'test/fixtures/sdk-ordering-ae.json', 'test/fixtures/sdk-columnar-af.json', 'test/fixtures/sdk-order-b-ag.json', 'test/fixtures/sdk-schedule-continuation-ah.json', 'test/fixtures/sdk-original-order-ai.json', 'test/fixtures/sdk-compact-sequence-aj.json',
"test/fixtures/sdk-reported-coordination-ar.md", "test/fixtures/sdk-ordered-schedule-ar.md",
'scripts/resolvers/review-dashboard.ts', 'scripts/resolvers/plan-gates.ts', 'scripts/resolvers/spec-review.ts', 'scripts/resolvers/outside-voice-steps.ts', 'scripts/resolvers/review-scope.ts', 'scripts/resolvers/outside-voice.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/tasks-section.ts', 'test/helpers/agent-sdk-runner.ts', 'test/helpers/auq-native-capture.ts', 'test/helpers/auto-decision-state.ts', 'test/helpers/autoplan-artifact-digest.ts', 'test/helpers/autoplan-artifact-permission.ts', 'test/helpers/autoplan-artifact-recorder.ts', 'test/helpers/claude-pty-runner.ts', 'test/helpers/pty/**', 'test/helpers/dx-selected-navigation.ts', 'test/helpers/e2e-gate.ts', 'test/helpers/eng-cache-writer-decision.ts', 'test/helpers/hermetic-skill-runtime.ts', 'test/helpers/llm-judge.ts', 'test/helpers/native-auto-decide.ts', 'test/helpers/owned-claude-transcript.ts', 'test/helpers/plan-count-artifacts.ts', 'test/helpers/plan-count-file-permission.ts', 'test/helpers/plan-count-fixture.ts', 'test/helpers/plan-count-pending-exit.ts', 'test/helpers/plan-count-pending-question.ts', 'test/helpers/plan-count-transcript.ts', 'test/helpers/plan-floor-review.ts', 'test/helpers/plan-floor-target.ts', 'test/helpers/plan-scope-selection.ts', 'test/helpers/plan-seed-submission.ts', 'test/helpers/plan-skill-question-events.ts', 'test/helpers/plan-skill-question-hook-scope.ts', 'test/helpers/plan-skill-questions.ts', 'test/helpers/pty-screen.ts', 'test/helpers/pty-trust-dialog.ts', 'test/helpers/skill-census.ts'],
// Data-driven behavioral guard for the 'plan'/'prompt' carves (eng, design,
test/section-capture-native-tools.test.ts
+1 -1
View File
@@ -197,7 +197,7 @@ test('CEO section caller supplies the author scope instead of blanket recommenda
const caller = fs.readFileSync(path.join(import.meta.dir, 'skill-e2e-plan-ceo-review-section-loading.test.ts'), 'utf8');
expect(caller).toContain('decisionPolicy: CEO_SECTION_DECISION_POLICY');
expect(caller).toContain('validateCeoReviewCompletion(capture)');
expect(caller).toContain('expect(hasStaleFillRaceFinding(output)).toBe(true)');
expect(caller).toContain('expect(hasApprovedStaleFillDecision(output) || hasStaleFillRaceFinding(output)).toBe(true)');
expect(caller).toContain('timeout: LONG_SECTION_CAPTURE_MS');
expect(caller).toContain('CAPTURE_LONG_MS');
});
test/skill-e2e-plan-ceo-review-section-loading.test.ts
+5 -2
View File
@@ -37,6 +37,7 @@ import {
LONG_SECTION_CAPTURE_MS,
} from './helpers/auq-sdk-capture';
import { CEO_SECTION_CACHE_PLAN, CEO_SECTION_DECISION_POLICY, hasStaleFillRaceFinding } from './helpers/ceo-section-loading-fixture';
import { hasApprovedStaleFillDecision } from './helpers/ceo-stale-fill-decision';
import { createEvalCollector, finalizeEvalCollector, recordE2E } from './helpers/e2e-helpers';
const describeE2E = describeE2ETier('periodic');
@@ -111,6 +112,8 @@ function assertSectionLoadingReport(capture: Awaited<ReturnType<typeof captureSe
// A native capture must not invent an outside dispatch or claim coverage.
expect(hasDisabledOutsideReview(output)).toBe(true);
// Loading a section and producing a table alone must not hide an empty
// review: the complete fixture still contains a real ordering defect.
expect(hasStaleFillRaceFinding(output)).toBe(true);
// review: the complete fixture still contains a real ordering defect. The
// skill's own ledger and currentDecision records are the primary witness;
// a report that left its ledger row stale may still assert the finding.
expect(hasApprovedStaleFillDecision(output) || hasStaleFillRaceFinding(output)).toBe(true);
}
test/test-of-test-ratchet.test.ts
+1
View File
@@ -48,6 +48,7 @@ const BASELINE = [
'test/ceo-mode-prerequisite.test.ts',
'test/ceo-posture-packet.test.ts',
'test/ceo-section-loading-fixture.test.ts',
'test/ceo-stale-fill-decision.test.ts', // replays census reports against the paid CEO section-loading case's structured checker
'test/ceo-split-collection.test.ts',
'test/ceo-split-question-policy.test.ts',
'test/changed-files-union.test.ts',