fix: [ENG-2884] address PR #689 codex review
Five items from the latest codex review batch on PR #689.
- agent-process.ts: drop the dead-code `?? ''` fallback on
previousTexts. archived[] paths are invariant-guaranteed to have
all three fields (text + mtime + signals) populated together, so
the fallback was already unreachable — and if it ever fired, the
resulting `{[path]: undefined}` would violate the new PRUNE
schema's z.record(z.string(), z.number()) for previousMtimes. All
three lookups now agree: trust the invariant uniformly.
- search-knowledge-service.ts: extend refreshIndex docstring with a
maintainer note explaining that the explicit `state.buildingPromise
= undefined` after the await is defensive (acquireIndex's finally
block already nulls it). Also documents the benign double-build
inefficiency when a fresh acquireIndex() races into the gap between
await-resolve and clear — not a correctness issue, just non-obvious.
- SKILL.md §7: soften the merge bullet's path-attribute warning. The
prior "drop the trailing .html — the writer always appends it"
instruction was load-bearing before the path-doubling fix shipped;
now that the writer normalizes idempotently (both bare and
.html-suffixed forms resolve to the same on-disk file), the
instruction reduces to agent-facing cognitive load. Updated to
"either form works; the writer normalizes" so the agent doesn't
have to track which form scan emitted.
- dream.test.ts: add two regression tests for the DREAM_REMOVED_FLAGS
wire-up in dream.ts's run() body. (1) text mode: --timeout 30
rejects with this.error throwing before this.log runs; (2) json
mode: --timeout 30 --format json emits a JSON envelope with
success:false / data.status:'error' / data.error containing
'--timeout'. Pins the early-exit ordering so a future refactor
that drops the findRemovedFlagMessage(this.argv, ...) call fails
the suite instead of silently regressing to pre-removal state.
- schemas.ts: hoist TaskTypeSchema declaration above
TaskExecuteSchema (the schema previously sat ~100 lines below) and
replace TaskExecuteSchema.type's inline z.enum(...) with a
reference to TaskTypeSchema. The doc block at the original
declaration site claimed it was the single source of truth, but
the inline enum on TaskExecuteSchema.type silently violated that
— the only outlier; TaskCreatedSchema.type (now line 605) and
TaskCreateRequestSchema.type already referenced TaskTypeSchema.
Hoist needed to avoid a Zod TDZ at module load. No behavioral
change; schemas now share one literal enum source. N
Nguyễn Thuận Phát committed
e3f96e8c71777577648e2da488f5c058f25dbbd2
Parent: 8c7c1cc