Closes review asks from cross-review of C4:
- Jobs ask #2: team-building.en.md still referenced the bash+fetch
pattern for goal creation. Rewritten as a deferred-tool note pointing
to C5 follow-up, without re-teaching the HTTP shape C4 just removed.
- Karpathy ask #1 (partial): re-recorded manager-task-handoff against
main to produce post-C4 positive evidence. VP Engineering has
hire_agent available via canRequestHire, but when directed NOT to
hire, the recorded trace shows task decomposition only (no
hire_agent invocation). 455 in / 769 out tokens; assertions updated
to pin /api/task/create + agent_ic_ slugs + p99 substring + max 3
turns. The `non-ceo-dm-reply` re-record is deferred to the follow-up
ticket.
- Karpathy asks #2, #3, #4 + Jobs asks #1, #3, #4: documented in
docs/proposals/C4-followup.md with concrete completion criteria.
Scope covers canFileCeoStrategy predicate, non-ceo-dm-reply
re-record, RFC 8594 Deprecation/Sunset headers + counter, and a
negative substring assertion kind for the eval harness.
Eval still green at 8/8. C4 main batch (e2c839b..8e99e01) stands
unchanged; this commit is pure follow-up groundwork.
Append a "Phase 2 considerations" section capturing the two
unresolved architecture debates Linus and Jobs surfaced in T1
review:
- Linus: drop the status->phase TRIGGER in favour of explicit
dual-write at every call site. Deferred to Phase 2 with explicit
rationale (existing two-layer guard + 5-commit blast radius).
- Jobs: flip reads from legacy columns to phase / task_leases.
Deferred until B5 eval harness can baseline agent behaviour.
Also records the four entry criteria for Phase 2 and the four gates
for Phase 3 drop-old-columns, so the next iteration has a concrete
checklist rather than a re-argument.
Address T2 review asks from Karpathy (request_changes) + Jobs
(approve with minor asks) without touching published commits:
- scripts/codemod/README.md (new): records remove-appdatabase-passthrough.mjs
usage, the completeness proof for METHOD_TO_MODULE (typecheck-clean +
524 tests green at 118e3d8 = no orphaned call sites), and the sequential
revert recipe for C1 (3 -> 2 -> 1, not independent because 118e3d8
bundled vault simplification).
- CONTRIBUTING.md (new): pass-through ban + codemod usage + sequential
revert recipe + file-size ceiling + C6 wave cap. Whitelisted in
.gitignore alongside the existing doc exceptions.
- docs/proposals/C6.2-error-taxonomy-phase2.md: wave 2 scope boundary
names renderer-adjacent / api-server / browser / automation / skills
explicitly and excludes src/main/database/** + src/main/worker/runtime/**
(B2 / M10 high-churn zones). Wave 2 and wave 3 each capped at 30
catch sites so C6 cannot expand without bound.
- src/main/agent-runtime/artifact-projector.ts, runtime-bridge.ts:
N-scan TODOs now carry a death date ("TODO(M-followup by 2026-05-20)").
If the date passes without decomposition, a follow-up session MUST
revisit or justify the exception.
Karpathy T2 asks #1 and #2 (split part 3 / "independent revert" language)
are addressed in CONTRIBUTING + codemod README as documentation, not
history rewrites, because 118e3d8 is already published.
Typecheck clean. No structural code changes; pure docs + comment edits.
- backup.ts: log.warn when `git rev-parse HEAD` fails so sandbox/CI runs
with no git show diagnosably instead of silently emitting gitSha:null
(Karpathy review ask).
- backup.ts: expand comment on VACUUM INTO to pin the WAL-mode invariant
— future readers must not "helpfully" add a wal_checkpoint call
(Karpathy review ask).
- C6.2 proposal: add hard coordination gate before wave 3 because
commit 0db6e42 already demonstrated the race hazard when two agents
touch scripts/ / connector-definitions/ in the same window
(Linus review ask).
Partitions the remaining catch sites into three waves (worker +
orchestration, IPC surfaces, database + credentials) so each wave is
one coherent review and can land in parallel with the B2 migration
precondition work that depends on classified DB catches.
Non-goals are explicit: no observable behaviour change, no renderer-
side coverage, no Result<T, AppError> envelope. The proposal exists
so the error taxonomy reaches the 95%+ coverage a lint rule needs.
- harness.test.ts: add in-memory self-test layer (pass + fail + purity), guard
fixtures dir with existsSync so absent dir yields empty list without
crashing the runner. Empty-fixture pnpm test:eval now carries real signal.
- replay-client.ts: remove dead if (!turn) branch; cursor bounds-check above
already guarantees the element. Use non-null assertion with a comment.
- assertion-judge.ts: drop unnecessary (turn as AssistantTurn | { content:
unknown[] }) cast. turn.role === assertion.role already narrows to
UserTurn | AssistantTurn and both expose .content.
- _BLOCKED-BY-CONSTRAINT.md: correct electron-builder note. files: is a
whitelist, so tests/eval is automatically excluded and no new rule is
required.
Document the follow-up tier on top of the B5 assertion harness: thin
single-call rubric judge, additive RubricDimension/RubricVerdict types,
cost cap ($2/run) with loud-skip behavior, per-fixture rubric cap (8
dimensions), baseline acceptance workflow, and explicit trigger
conditions (10+ fixtures / 2 incidents / structural-miss / 50-run zero
catch) — do not build speculatively.
Roundtable outcome: 2-1 to defer tonight. Linus — don't bolt a second
LLM to an empty harness. Jobs — single signal first. Karpathy — fixture
accumulation is what will tell us which rubric dimensions matter; the
schema stays extensible so we can add rubric-per-fixture without
breaking B5.
ZacharyZhang-NY