Skip to content

docs(repo): resolve Tier-4 plan-readiness audit findings#124

Merged
Sawmonabo merged 8 commits into
developfrom
docs/tier-4-plan-readiness-audit
May 29, 2026
Merged

docs(repo): resolve Tier-4 plan-readiness audit findings#124
Sawmonabo merged 8 commits into
developfrom
docs/tier-4-plan-readiness-audit

Conversation

@Sawmonabo
Copy link
Copy Markdown
Owner

@Sawmonabo Sawmonabo commented May 28, 2026

Summary

NS-16 — Tier-4 plan-readiness audit of Plan-005 (Provider Driver Contract + Capabilities), Plan-006 (Session Event Taxonomy + Audit Log), and Plan-007 remainder (Local IPC + Daemon Control, Phases 4–7). Swaps the audited working copies into the corpus and lands the cross-cutting contract shapes the audit ratified. Plan-005 + Plan-006 stay approved; Plan-007 flips approvedreview for the Tier-4 remainder Phases 4–7 design reopen — the Phases 1–3 partial PR sequence (#16/#17/#19) is audit-evidence preserved in §Tier 1 Partial PR Sequence + §Progress Log + §Retroactive Audit Memo.

Audit runbook: docs/operations/plan-implementation-readiness-audit-runbook.md. This is the Tier-4 pass (Tier-3 = NS-15 merged in PR #118; G6 satisfied).

Decisions ratified (D1–D7 + G3 overage)

# Decision Disposition
D1 Status disposition Plan-005 + Plan-006 stay approved; Plan-007 flips approvedreview per D2
D2 Plan-007 status flip Tier-4 remainder Phases 4–7 reopen the design surface; Phases 1–3 partial PR sequence unaffected (preserved as audit-evidence)
D3 Spec-006 integrity protocol ADR-018 + RFC 8785 JCS canonicalizer (UTF-16 lex-sort scalar code units) for replay-key + content-hash determinism
D4 JSON-RPC state-refusal envelope -32603 InternalError envelope + reuse of transport.unavailable code for daemon-state-degraded refusals
D5 clipanion CLI version clipanion@4.0.0-rc.4 exact-RC pin (Yarn ecosystem precedent + v3 API-incompat avoidance); BL-134 tracks stable-v4 upgrade
D6 OS-keystore source @napi-rs/keyring v1.2.0 ratified (supersedes keytar; native binding sourced from npm preserving pnpm side-effects-cache:false per Plan-001 PR #3 precedent)
D7 Spec-027 daemon-side rows Rows 2/3/7a/7b/8 integrated into security-architecture.md (capability boundary + audit emitter wiring)
G3 overage Non-Tasks budget Plan-005 2.52× + Plan-007 1.53× ratified as audit-scope-justified — 100% audit-domain content, zero redesign (runbook §Failure Modes addresses recurring-across-tiers methodology overrun, not single-tier load-bearing content additions)

G5 ephemeral-ref scrub

~100 F-XXX-Y-ZZ ID prefixes + PR #N inline refs stripped from audit-introduced plan-body text per Tier-3 Plan-003 canonical precedent (PR #118). Plan-007 baseline-preserved zones (§Tier 1 Partial PR Sequence, §Progress Log, §Retroactive Audit Memo) retain 13 PR #N refs as legitimate audit-evidence; narrative content preserved verbatim with only ephemeral prefixes scrubbed.

Anchor + path corrections (pre-commit-hook-surfaced)

The audit-introducer subagents emitted citation-shape defects the docs-anchor-check hook (lychee) caught at commit time. Root-cause fixes applied in this PR (not deferred):

  • Plan-005 Required-ADRs — phantom ADR-004 citation with non-existent filename slug (004-event-streams-are-private-not-shared.md; real ADR-004 = SQLite/Postgres). Replaced with ADR-017 (Shared Event-Sourcing Scope) — the ADR the body's runtime_node.* driver-event emission actually invokes (line 271). Header + Preconditions + Done-Checklist all updated consistently.
  • Plan-005 §CP-005-6 — broken Plan-004 filename slug (004-run-state-machine-and-events.md). Corrected to canonical 004-queue-steer-pause-resume.md.
  • Plan-005 §CP-005-5 — two 4-up relative-path traversals (../../../../docs/architecture/contracts/api-payload-contracts.md). Corrected to 1-up (../architecture/contracts/api-payload-contracts.md).
  • Plan-006 §Phase 4 narrative — Spec-015 fragment slug (#idempotency-classes not in target). Corrected to canonical #idempotency-classes-and-recovery-behavior.
  • Plan-006 §Phase 4 narrative — forbidden .agents/tmp/research/... link target ((.) no-op markdown link to transient research path). Sentence rewritten per AGENTS.md Surface-Forward-Then-Delete anti-pattern (the load-bearing content is already surface-forwarded into this plan + the api-payload-contracts.md §Plan-006 doc-mirror).

File manifest

File Change
docs/plans/005-provider-driver-contract-and-capabilities.md Working-copy swap: 4 Phases of Tasks audited; capability-flag + idempotency_class + ProviderToolMetadata cross-cutting shapes ratified; ADR-017 added to Required-ADRs (replacing phantom ADR-004); CP-005-5 + CP-005-6 resolutions surface-forwarded
docs/plans/006-session-event-taxonomy-and-audit-log.md Working-copy swap: 4 Phases of Tasks audited; RFC 8785 JCS canonicalizer ratified; DaemonSigningKeySource + OsKeystoreSealedDaemonSigningKeySource (F-006-2-02 resolution) ratified; retention_class discriminator column added (F-006-3-02 Phase 3 Design B); session_snapshots compaction-cursor columns (F-006-4-01 Phase 4 Reading (a))
docs/plans/007-local-ipc-and-daemon-control.md Working-copy swap: Phases 4–7 Tier-4 audited (run.* / repo.* / artifact.* / settings.* / daemon.* namespace handlers + Spec-027 row 2/3/7a/7b/8 daemon integrations); status flips approvedreview per D2
docs/architecture/contracts/api-payload-contracts.md Plan-007 R1/R2 state-refusal envelope + Plan-006 additive method namespace shapes (event.readAfterCursor, event.readWindow, event.subscribe) per CP-006-4 / CP-007-N
docs/architecture/security-architecture.md Spec-027 daemon-side integration rows 2/3/7a/7b/8 (capability boundary + audit emitter wiring)
docs/specs/006-session-event-taxonomy-and-audit-log.md RFC 8785 JCS canonicalizer ratification; ADR-018 reference; audit-stub retention-class additive
docs/backlog.md BL-134 (clipanion stable-v4 upgrade tracking; V1.1; criterion-gated on upstream stable release)
docs/architecture/cross-plan-dependencies.md §6 fold (below)

Tag on merge: plan-readiness-audit-tier-4-complete.

§6 fold (cross-plan-dependencies.md)

  • NS-16 :::ready:::completed; PRs checkbox [x] tier-4.
  • NS-17 promoted blockedready per the audit-chain edge NS-16 → NS-17 (audit chain remains strictly serialized through NS-21).
  • Ready set after this PR: {NS-17}; blocked set: {NS-09, NS-10, NS-18..NS-21}.

Findings ledger — 97 findings, 17 critical-root-causes (5.7:1 collapse ratio)

Per-plan breakdown: Plan-005 = 49 findings (5 critical-roots) · Plan-006 = 15 findings (6 critical-roots) · Plan-007 remainder = 33 findings (7 critical-roots, minus overlap = ~17 distinct closures). Healthy finding-instance / root-cause collapse ratio (~5.7:1) — each consuming phase independently surfaced the same upstream gap from its own vantage, not over-flagging (same pattern as Tier-3 NS-15: 31 findings → 4 roots = 7.75:1).

Root-cause critical → disposition:

Critical Disposition
RFC 8785 JCS canonicalizer unratified D3 — ADR-018 + Spec-006 RFC 8785 ratification
JSON-RPC state-refusal envelope unratified D4 — -32603 InternalError + transport.unavailable reuse
clipanion CLI version pin underspecified D5 — exact-RC pin + BL-134
OS-keystore source library unratified D6 — @napi-rs/keyring v1.2.0
Spec-027 daemon-side rows ungoverned D7 — rows 2/3/7a/7b/8 in security-architecture.md
Plan-007 Tier-4 design surface contested D2 — status flip approvedreview
Plan-006 DaemonSigningKeySource + sealed Ed25519 source F-006-2-02 resolution (in-plan)
Plan-006 retention_class discriminator column F-006-3-02 Phase 3 Design B (in-plan)
Plan-006 session_snapshots compaction-cursor columns F-006-4-01 Phase 4 Reading (a) (in-plan)
Plan-005 InterventionType enum co-location CP-005-6 resolution (in-plan)
Plan-005 CapabilityDetails wrapper + providerFailureDetail CP-005-5 resolution (in-plan)
...6 additional critical-roots dispositioned per per-plan synthesis (full disposition matrix at .agents/tmp/research/plan-readiness-audit/REVIEW.md decision block)

Calibration + gates

B1–B6: B1 avg-critical/phase (Plan-005=1.25; Plan-006=1.5; Plan-007=1.75) in band; B2 total 97 (per-plan: 49/15/33 — Plan-005 over upper band, justified by 4-Phase × audit-density) · B3 Tasks:blocking (Plan-005=1.5:1; Plan-006=4.67:1) in band ✓ · B4 user-review ~60–90 min ✓ · B5 7 substantive advisor calls (range expanded vs Tier-3 baseline; Tier-4 contract-density justified) · B6 1 status-flip (Plan-007; 0–1) ✓

G1–G6: G1 skeleton additions-only ✓ · G2 all 17 root criticals dispositioned (full matrix in REVIEW.md) ✓ · G3 non-Tasks diff Plan-005 2.52×, Plan-006 1.42×, Plan-007 1.53× — Plan-005 + Plan-007 FAIL the literal <1.5× rule; ratified as audit-scope-justified per D-G3 overage (100% audit-domain content, zero redesign) · G4 all Tasks Steps → Spec coverage + Verifies-invariant ✓ · G5 0 ephemeral F-XXX-Y-ZZ / PR #N refs in audited plan bodies (Plan-007 §Tier 1 Partial PR Sequence + §Progress Log + §Retroactive Audit Memo preserve 13 PR refs as audit-evidence per baseline carryover; verified against plan-readiness-audit-tier-4-baseline tag) ✓ · G6 Tier-3 committed (NS-15 merged in PR #118) ✓

Test plan

  • pnpm install --frozen-lockfile provisioned the worktree; all pre-commit + commit-msg hooks green (gitleaks: no leaks; lychee: 603 links / 532 OK / 0 errors / 71 excluded; docs-corpus-checks: governance cite-walk; lint-staged: prettier; commitlint: passed).
  • 0 ephemeral F-XXX-Y-ZZ / PR #N tags in Plan-005 + Plan-006 + Plan-007 plan-bodies (Plan-007 baseline preserved zones retain 13 PR refs as legitimate audit-evidence per Tier-3 Plan-003 precedent).
  • Mermaid coherence: §6 ready/blocked sets consistent with edge set; NS-16 → completed, NS-17 promoted.
  • Anchor + path corrections verified — git show :path | grep confirms 0 remnants of ADR-004, 004-run-state-machine, 4-up traversal, #idempotency-classes standalone, or transient .agents/tmp/ link targets in the audited Plan-005 + Plan-006 surfaces.

Codex review resolution

Round 4 — 182ac84

Eleven findings (5×P1) resolved in 182ac84; all eleven review threads replied + resolved. Headline P1: F9daemon.start was modeled as an in-daemon IPC handler, which is impossible (a stopped daemon hosts no IPC server to receive the call). Removed DaemonStart* from the daemon-side surface (lifecycle schemas, IPC handlers, Tier-4 method-name table, client SDK list, renderer mutating-op gate, and the action:"start" union + shared DaemonLifecycleParams in api-payload-contracts.md); cold-boot is now the ai-sidekicks daemon start CLI process-spawn path (runtime-resolved daemon path + detached spawn + DaemonHelloAck wait, T-007r-3-4). This re-aligns Plan-007 to Spec-023:76's "spawned child process of the shell" framing — the plan had drifted from the spec, not the reverse. Other P1s: F11 (DaemonKeyStore real impl + composition-root injection at Plan-022 Tier 5 per CP-007-8, not R2 — no tier inversion), F1 (R1 registers only daemon.*+settings.*; the other five namespaces attach from their owning plans), F10 (banner read is daemon-availability-tolerant), F2 (evidence_pr: 124).

Round 5 — 34ee7f7

Seven findings (1×P1) resolved in 34ee7f7; all seven threads replied + resolved — 45/45 threads resolved, mergeStateStatus: CLEAN. Headline P1 R5-6: the round-2 stub_signature signed only the compacted-row payload projection, leaving the denormalized scalar columns (id, session_id, sequence, occurred_at, category, type, actor) — read by SQL filters and envelope reconstruction — unbound; an at-rest scalar edit could forge an actor/type while both stub_signature and the anchor over the frozen row_hash still verified. The verifier now adds a scalar-binding check (each scalar column byte-equals its payload-projection counterpart) → a new failureMode: 'stub_scalar_mismatch', an additive-MINOR enum extension 10→11 per ADR-018 §Decision #8. Other findings: R5-1 (DaemonStatusReadResult API mirror reconciled to the canonical processState shape), R5-2 (Spec-007 amended to disambiguate daemon start as a process-spawn capability, not a daemon.start IPC handler), R5-3 (separate DaemonStopRequest/DaemonRestartRequest mirror schemas carrying idleDrainDeadlineMs), R5-4 (Plan-005 DriverClient interface enumerates exactly the 7 client-facing methods, not the stale 11), R5-5 (Plan-005 storage summary lists all four Phase-2 SQLite tables), R5-7 (R2-T6 re-scoped to the emission-side securityCritical boundary R2 owns).

Round-4 deferrals, now resolved. The two round-4 "named but not chased" divergences are closed: (a) the api-payload-contracts.md DaemonStatusReadResult mirror is reconciled to the canonical processState shape (R5-1); (b) the Plan-007 CLI §Files inventory (one-vs-three daemon-lifecycle.ts) is fixed in 8631a1d, which also reconciled the broader R3 §Files block to its authoritative Tasks — added exit-codes.ts (T-3-3) + settings.ts (T-3-6), fixed the daemon-status.ts citation (T-3-3→T-3-5), and removed the session-*/meta.ts orphans (the CLI session surface is owned by Plan-001 Phase 5; --version/--help are clipanion Cli-builder built-ins). docs/specs/007-local-ipc-and-daemon-control.md:74 is now amended (R5-2) — a real defect in the approved spec's IPC-method listing, not an intentional carry-forward.

Remaining deferrals (each owned by its own pending audit). DaemonStart survives in the capability sense (the shell boots the daemon, realized by spawn) in three Tier-8 peer artifacts revisited by their own audits: docs/specs/023-desktop-shell-and-renderer.md:76,221, docs/plans/023-desktop-shell-and-renderer.md:62, docs/operations/local-daemon-runbook.md. Amending a peer plan/spec/runbook to restate something already correct in the capability sense is out of this PR's scope.

Refs: Plan-005, Plan-006, Plan-007, Spec-006, Spec-007, ADR-017, ADR-018, BL-134

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

NS-16 — Tier 4 plan-readiness audit of Plan-005 (Provider Driver
Contract + Capabilities), Plan-006 (Session Event Taxonomy + Audit
Log), and Plan-007 remainder (Local IPC + Daemon Control, Phases 4–7).
Swaps the audited working copies into the corpus and lands the
cross-cutting contract shapes the audit ratified:

- api-payload-contracts.md: Plan-007 R1/R2 envelope additions
  (state-refusal envelope under -32603 InternalError reuse of
  transport.unavailable code; daemon-state-degraded refusal path);
  Plan-006 additive method namespace shapes (event.readAfterCursor,
  event.readWindow, event.subscribe) per CP-006-4 / CP-007-N.
- security-architecture.md: Spec-027 daemon-side integration rows
  2/3/7a/7b/8 — capability boundary + audit emitter wiring for the
  daemon's namespace handlers (Plan-007 remainder).
- spec-006-session-event-taxonomy-and-audit-log.md: RFC 8785 JCS
  canonicalizer ratified (UTF-16 lex-sort scalar code units for
  replay-key + content-hash determinism); ADR-018 reference;
  audit-stub retention-class additive.
- backlog.md: BL-134 (clipanion stable-v4 upgrade tracking —
  criterion-gated on upstream stable release; CLI ships at
  clipanion@4.0.0-rc.4 exact-RC pin per Yarn-ecosystem precedent
  + v3 API-incompat avoidance).
- cross-plan-dependencies.md section 6: NS-16 → completed; NS-17
  promoted blocked → ready per the audit-chain edge NS-16 → NS-17
  (the §6 audit chain remains strictly serialized through NS-21
  per runbook §85–87, so NS-18..NS-21 stay blocked behind NS-17).

Plan-005 stays approved (D1); Plan-006 stays approved (D1); Plan-007
flips approved → review (D2 — Tier-4 remainder Phases 4–7 reopen the
design surface; Phases 1–3 partial PR sequence is audit-evidence
preserved in §Tier 1 Partial PR Sequence + §Progress Log + §Notes
+ §Retroactive Audit Memo). G3 non-Tasks budget overages (Plan-005
2.52×, Plan-007 1.53×) ratified as audit-scope-justified — 100%
audit-domain content, zero redesign (D-G3 overage). B1–B6 + G1, G2,
G4, G5, G6 pass; G3 overage user-ratified per REVIEW.md decision block.

G5 ephemeral-ref scrub: ~100 F-XXX-Y-ZZ ID prefixes + PR #N inline
refs stripped from audit-introduced plan-body text per Tier-3
Plan-003 canonical precedent. Plan-007 baseline preserved zones
(§Tier 1 Partial PR Sequence, §Progress Log, §Retroactive Audit Memo)
retain 13 PR refs as legitimate audit-evidence; narrative content
preserved verbatim with only ephemeral prefixes scrubbed. Plan-005
Required-ADRs corrected — phantom ADR-004 citation replaced with
ADR-017 (Shared Event-Sourcing Scope) that the body's `runtime_node.*`
driver-event emission actually invokes; broken Plan-004 filename slug
(`004-run-state-machine-and-events.md`) corrected to canonical
`004-queue-steer-pause-resume.md`; two 4-up relative-path traversals
corrected to 1-up (`../architecture/contracts/...`); Plan-006
Spec-015 anchor corrected from `#idempotency-classes` to the
canonical `#idempotency-classes-and-recovery-behavior`; transient
`.agents/tmp/...` link target removed per AGENTS.md anti-pattern.

Decisions ratified (D1–D7 + G3 overage; user-ratified 2026-05-28):

- D1: Plan-005 + Plan-006 + Plan-007 design soundness — approve all
- D2: Plan-007 status flip approved → review for Tier-4 remainder
  Phases 4–7 reopen; Phases 1–3 partial remain unaffected
- D3: ADR-018 + Spec-006 RFC 8785 JCS canonicalizer ratification
- D4: JSON-RPC -32603 InternalError state-refusal envelope + reuse
  of transport.unavailable code for daemon-state-degraded refusals
- D5: clipanion@4.0.0-rc.4 exact-RC pin (BL-134 tracks stable-v4
  upgrade; Yarn ecosystem precedent + v3 API-incompat avoidance)
- D6: @napi-rs/keyring v1.2.0 ratified (supersedes keytar; native
  binding sourced from npm preserving pnpm side-effects-cache:false
  per Plan-001 PR #3 precedent for native-binding CI traps)
- D7: Spec-027 daemon-side integration rows 2/3/7a/7b/8 integrated
  into security-architecture.md (capability boundary + audit emitter)

Refs: Plan-005, Plan-006, Plan-007, Spec-006, ADR-017, ADR-018, BL-134
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: df52b42b8e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread docs/plans/007-local-ipc-and-daemon-control.md Outdated
Comment thread docs/plans/005-provider-driver-contract-and-capabilities.md Outdated
Comment thread docs/architecture/contracts/api-payload-contracts.md
Comment thread docs/architecture/cross-plan-dependencies.md Outdated
Comment thread docs/plans/006-session-event-taxonomy-and-audit-log.md Outdated
Comment thread docs/plans/005-provider-driver-contract-and-capabilities.md Outdated
Comment thread docs/plans/005-provider-driver-contract-and-capabilities.md Outdated
Comment thread docs/plans/006-session-event-taxonomy-and-audit-log.md Outdated
Resolves the 8 ACTIONABLE findings Codex surfaced on the Tier-4
plan-readiness audit (PR #124). All fixes are corpus-doc edits;
no code or schema migrations land here (those ship under their
owning plans at code-execution time).

T1 — Plan-007 I-007-9 method-name regex updated to dotted-camelCase
(permits camelCase in subsequent segments per LSP/MCP precedent);
canonical regex source at api-payload-contracts.md updated and
downstream label-only sweep in Plan-003 / Plan-007 / Plan-008.

T2 — Plan-005 usage_quota → canonical usage_telemetry category.

T3 — api-payload-contracts.md ProviderDriver interface wired:
resumeSession → Promise<DriverResumeResult>, getCapabilities →
Promise<GetCapabilitiesResult> per the discriminated-union shapes.

T4 — sessions.daemon_signing_key (Postgres) → new local SQLite
daemon_signing_keys table per ADR-004 SQLite-local-state boundary;
canonical-schema doc, cross-plan-deps §6 (rows 26/42/43/135), and
Plan-006 narrative sweep.

T5 — anchor-before-compaction protocol per Spec-006 §Post-Compaction
Integrity; failureMode enum extended 7→9 (anchor_missing_for_
compacted_range, anchor_signature_invalid) per ADR-018 §Decision #8
MINOR-additive sanction; new AC #4; Plan-006 T3.2/T3.3/T4.1/I-006-3-03
rewrites + Audit Reconciliation entry; security-architecture.md
Verifier Rules add audit_stub carve-outs on chain + signature checks.

T6 — Plan-005 T2.3 ProviderRegistry capability gate scoped to
exclude applyIntervention (ADR-011 degraded-fallback must reach
driver).

T7 — Plan-005 T2.1 migration shape matches canonical
local-sqlite-schema (driver_capabilities per-flag + driver_tools
per-tool); T2.2 dropped findBySession; T2.4 split into per-flag
+ per-tool persistence; new driver_tools table added to canonical
schema.

T8 — Plan-006 Co-Tier Phase Ordering Precondition section + T1.4
Depends wording fix.

Refs: ADR-004, ADR-011, ADR-017, ADR-018, Plan-005, Plan-006,
Plan-007, Spec-006

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Sawmonabo
Copy link
Copy Markdown
Owner Author

@codex

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e6ae9466a2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread docs/specs/006-session-event-taxonomy-and-audit-log.md Outdated
Comment thread docs/plans/007-local-ipc-and-daemon-control.md Outdated
Comment thread docs/plans/006-session-event-taxonomy-and-audit-log.md Outdated
Comment thread docs/plans/007-local-ipc-and-daemon-control.md Outdated
Comment thread docs/plans/007-local-ipc-and-daemon-control.md Outdated
Comment thread docs/architecture/schemas/local-sqlite-schema.md
Comment thread docs/plans/007-local-ipc-and-daemon-control.md Outdated
Comment thread docs/plans/007-local-ipc-and-daemon-control.md Outdated
Comment thread docs/plans/005-provider-driver-contract-and-capabilities.md Outdated
Comment thread docs/plans/006-session-event-taxonomy-and-audit-log.md Outdated
Round-2 Codex review (chatgpt-codex-connector[bot]) on PR #124 surfaced one P1
and ten P2/P3 findings. All resolved in-PR as hardened contract fixes — no V1.1
deferral.

P1 — post-compaction stub authenticity (Spec-006 §Post-Compaction Integrity).
The round-1 anchor-before-compaction protocol proved the ORIGINAL pre-compaction
range existed but left the surviving audit-stub bytes unauthenticated. Added a
per-row `stub_signature` (Ed25519 over the exact canonical bytes stored in
`payload`) minted by the compactor and re-verified by the integrity verifier.
Sign-exact-bytes invariant: the compactor signs the identical byte string it
persists; the verifier checks `stub_signature` directly over the stored
`payload` bytes (strictly stronger than re-canonicalizing — catches any
byte-level edit, not only semantic — and avoids a JCS round-trip). `failureMode`
enum extended 9 -> 10 (`stub_signature_invalid`, additive-MINOR per ADR-018
Decision #8). Threat model: stub-edit, cross-row replay, signature-strip, and
both `retention_class`-flip directions all detected.

P2/P3 (Plan-006 / Plan-007 / Plan-005):
- keep stub payload: T3.2 REPLACES `payload` with the stub projection (column is
  NOT NULL), never nulls it — preserves the replay/renderer never-silent-omission
  contract.
- T2.7 signing-key create() returns the PUBLIC key only; the private key is
  reachable solely via the signer-local read() (no private-key leak).
- driver_contract_meta table (Plan-005): contractVersion hydrated from local
  state at cold start, not a driver round-trip.
- pending_anchor_uploads retry columns (attempt_count, last_attempt_at,
  last_error); session_events live-row partial index.
- Plan-007: idleDrainDeadlineMs uses .default(5000) alone (the trailing
  .optional() short-circuited the default to undefined on omit); securityCritical
  flag added to securityOverrides; R1 projects from the Tier-1 SupervisionHooks
  substrate (not the R3 consumer); session.stop/restart EXCLUDE the caller's own
  connection from the active-client refusal check.
- self-update CLI relocated R2 -> R3-PR-a (T-007r-2-6 -> T-007r-3-6a): a CLI
  command cannot compile in the R2 PR before R3 scaffolds apps/cli/; the reversal
  honors the ratification's own R3-PR-shipping intent, recorded with rationale
  rather than silently overwritten.

cross-plan-dependencies.md + local-sqlite-schema.md mirror the new
stub_signature / retention_class / driver_contract_meta ownership.

Refs: Plan-005, Plan-006, Plan-007, Spec-006, ADR-018

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Sawmonabo
Copy link
Copy Markdown
Owner Author

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 91ef9aaa67

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread docs/specs/006-session-event-taxonomy-and-audit-log.md Outdated
Comment thread docs/architecture/contracts/api-payload-contracts.md
Comment thread docs/architecture/contracts/api-payload-contracts.md
Comment thread docs/architecture/cross-plan-dependencies.md Outdated
Comment thread docs/plans/005-provider-driver-contract-and-capabilities.md Outdated
Comment thread docs/plans/007-local-ipc-and-daemon-control.md Outdated
Comment thread docs/architecture/schemas/local-sqlite-schema.md Outdated
Comment thread docs/architecture/schemas/local-sqlite-schema.md Outdated
Sawmonabo and others added 2 commits May 28, 2026 17:24
Round-3 external review surfaced 8 P2 findings (A-H) on the Tier-4 audit
diff; all resolved in-place as hardened fixes with zero deferral of in-scope
work.

- A (Spec-006:612): Merkle force-fire leaf basis corrected to row_hash leaves.
  canonical_bytes are discarded at compaction, so a verifier replaying the
  frozen row_hash could not reproduce a canonical_bytes-leaved root.
- B (api-payload-contracts:319): method-name regex examples realigned. V1
  tightens the leading segment to lowercase-only, so camelCase-root examples
  (textDocument.didOpen) are rejected; cite workspace.executeCommand (matches).
- C (api-payload-contracts:669): split ProviderToolMetadata into an ingress
  shape (z.input, idempotency_class optional) and NormalizedProviderToolMetadata
  (z.output, required post-.default). Only the normalized shape crosses the
  persistence / event boundary (NOT NULL driver_tools + capability_* payloads).
- D (cross-plan-deps:135): removed the reversed Plan-006 dependency clause; the
  dependency cycle is broken and the correct relationship stays on the next row.
- E (plans/005:269): narrowed the client driver.* JSON-RPC namespace 11 -> 7.
  The 4 session/run lifecycle ops (createSession/resumeSession/startRun/
  closeSession) are daemon-internal (orchestration-invoked), reversing the
  2026-05-27 ratified 11-method design with recorded rationale. The in-daemon
  10-op ProviderDriver interface is unchanged; only client exposure narrows.
- F (plans/007:958): renderer daemon.restart gated on a valid DaemonHelloAck
  mutating-op gate (I-007-19), NOT an admin-token check; path reconciled to
  Spec-027's canonical ./data/admin-token (the relay-admin token, not a
  local-IPC authority).
- G (sqlite + postgres schemas + Spec-006): anchor UNIQUE key extended to
  (session_id, node_id, start_sequence, end_sequence) on BOTH stores. A cadence
  anchor and a wider compaction-covering anchor share start_sequence and must
  coexist; "covering anchor exists" is a coverage query, not an exact-start
  match.
- H (local-sqlite-schema:45): retention_class column-domain CHECK added
  (ALTER-addable); co-presence with stub_signature is verifier-enforced since
  SQLite cannot ALTER-add a table-level multi-column CHECK.

Two consistency nits from the E/G ripple, plus one surfaced follow-up:
- E ripple: swapped the now-daemon-internal driver.createSession method-name
  example (plans/007:115) for the client-facing driver.listCapabilities.
- G ripple: de-staled the pending_anchor_uploads UNIQUE-key comment to the
  4-column key + identical-range dedup semantics.
- BL-135: surfaced the out-of-Tier-4 Plan-025 admin-token path divergence
  (./data/trust/relay-admin-token vs canonical ./data/admin-token) for
  resolution in Plan-025's own readiness audit (a Spec-027-level decision).

Refs: Plan-005, Plan-006, Plan-007, Spec-006, BL-135

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Plan-007's Phase R2 detail section carried a systematic row-number inversion
against the canonical Spec-027 §Required Behavior table: TLS-1.3 surfaces were
cited as row 2 and the first-run key ceremony as row 8, when row 2 is
"refuse-to-start without encryption", row 3 is "auto-generated strong secrets",
and row 8 is "TLS 1.3 minimum". Plan-007's own coverage table (lines 374-380)
and file-structure overview (lines 316-317) already cite correctly -- line 379
("8 -- TLS 1.3 minimum") directly contradicted line 591 ("row 8 ... generates
daemon master key"), which proves the defect. Likely cause: a Spec-027 row
renumbering whose Phase R2 citations were never realigned.

Corrected (8 references, no behavior change):
- TLS 1.3 listener / TlsSurface: row 2 -> row 8 (lines 590, 602, 971; plus
  the 969 "8 (TLS 1.3-only)" label).
- First-run key ceremony / admin-token output: row 8 -> row 3 (lines 591, 603,
  970; plus 588 firstRunKeysPolicy + adminTokenPath).
- nonLoopbackHost: row 3 -> row 4 (bind, per the row-4 loopback-bind default).
- 969 row-2 label corrected to "refuse-to-start without encryption".

tlsMode stays row 2 (the encryption-required posture row 2 governs; the TLS
version floor is the distinct row 8). Plan-005/006 reference no Spec-027 rows,
so the drift was Plan-007-only. Surfaced by Codex round-3 review on PR #124.

Refs: Plan-007, Spec-027

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Sawmonabo
Copy link
Copy Markdown
Owner Author

@codex

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 46eb65c10e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread docs/plans/007-local-ipc-and-daemon-control.md Outdated
Comment thread docs/plans/007-local-ipc-and-daemon-control.md Outdated
Comment thread docs/plans/007-local-ipc-and-daemon-control.md Outdated
Comment thread docs/plans/006-session-event-taxonomy-and-audit-log.md Outdated
Comment thread docs/architecture/cross-plan-dependencies.md Outdated
Comment thread docs/plans/005-provider-driver-contract-and-capabilities.md Outdated
Comment thread docs/architecture/security-architecture.md Outdated
Comment thread docs/plans/007-local-ipc-and-daemon-control.md Outdated
Comment thread docs/plans/007-local-ipc-and-daemon-control.md Outdated
Comment thread docs/plans/007-local-ipc-and-daemon-control.md Outdated
f9 (architecture): daemon.start is NOT an IPC method — a stopped daemon hosts no IPC
server to receive it. removed DaemonStart* from the daemon-side contract surface
(daemon-lifecycle.ts schemas, ipc/handlers, Tier-4 method-name table, client SDK list,
renderer mutating-op gate) and the action:"start" union + shared DaemonLifecycleParams
from api-payload-contracts.md. cold-boot is the `ai-sidekicks daemon start` CLI command
(runtime-resolved daemon path + detached child_process.spawn + DaemonHelloAck wait,
T-007r-3-4) — this re-aligns the plan to Spec-023:76's "spawned child process of the
shell" framing and reverses the prior 3-handler lifecycle design.

f11: DaemonKeyStore R2 ships the interface + InMemoryDaemonKeyStore stub + first-run
ceremony-against-stub only; composition-root injection + production-guard assertion +
real OsKeystoreSealedDaemonKeyStore land at Plan-022 Tier 5 per CP-007-8 (no tier
inversion — mirrors Plan-006 CP-006-1 PiiEncryptor boundary).

f1: Phase R1 registers only daemon.*+settings.*; the other five Tier-4 namespaces attach
from their owning plans per NS-26 (R1 test asserts the 2 R1-owned namespaces + the
I-007-6 duplicate-rejection mechanism, not all seven).

f3: nested clipanion command paths (daemon start/stop/restart/status); add daemon status.
f10: banner is daemon-availability-tolerant (daemon start surfaces the freshly-spawned
daemon's banner after DaemonHelloAck; an unreachable daemon degrades to a typed
"daemon unavailable" rather than failing the command).
f2: evidence_pr TBD → 124.
f4: DaemonSigningKeySource.generate() → .create(sessionId).
f5: split Plan-001 forward-declared integrity columns from Plan-006-owned additive
Tier-4 migrations per CP-006-6.
f6: canonicalize ^3.0.0 → exact pin 3.0.0 (canonical bytes feed every event
hash/signature; T2.3 golden vectors bind output to RFC 8785 §A).
f7: driver.* client-facing SDK surface is the 6 non-lifecycle verbs; the 4 session/run
lifecycle ops get no client-facing schema (daemon-internal).
f8: security-architecture schema-migration is two-wave (Plan-001 forward-declares 4
integrity columns; Plan-006 ships the additive Tier-4 columns + daemon_signing_keys +
pending_anchor_uploads tables).

Refs: Plan-005, Plan-006, Plan-007
@Sawmonabo
Copy link
Copy Markdown
Owner Author

@codex

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 182ac84b01

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread docs/plans/007-local-ipc-and-daemon-control.md
Comment thread docs/plans/007-local-ipc-and-daemon-control.md
Comment thread docs/plans/007-local-ipc-and-daemon-control.md
Comment thread docs/plans/005-provider-driver-contract-and-capabilities.md Outdated
Comment thread docs/plans/005-provider-driver-contract-and-capabilities.md Outdated
Comment thread docs/specs/006-session-event-taxonomy-and-audit-log.md
Comment thread docs/plans/007-local-ipc-and-daemon-control.md Outdated
Sawmonabo and others added 2 commits May 28, 2026 19:34
reconcile the api-payload contract mirror and the approved specs to the audited
plans (the contract/spec surface these plans directly implement), and close a P1
post-compaction audit-log forgeability gap. no improper deferral: the mirror and
specs are fixed at the root, not punted to their own audits.

- R5-1 (P2): api-payload DaemonStatusReadResult reconciled to the plan-007
  canonical (processState enum + protocolVersion + transportEndpoint + uptimeMs)
- R5-2 (P2): spec-007 disambiguates DaemonStart as the supervisor process-spawn
  capability (CLI/shell), not an IPC method -- coherent with its own Default
  Behavior / Example Flows; clarifying-amendment note added (status stays approved)
- R5-3 (P2): api-payload DaemonLifecycle split into separate DaemonStop /
  DaemonRestart request schemas carrying idleDrainDeadlineMs (default 5000) +
  {accepted:true}; refusal stays the lifecycle_conflict error envelope
- R5-4 (P2): plan-005 DriverClient enumerates the 6 non-lifecycle verbs +
  subscribeEvents (drops the stale "+ 8 more" implying the 11-method client)
- R5-5 (P3): plan-005 storage summary lists all four Phase-2 tables
  (+ driver_tools, driver_contract_meta); brittle line ref -> section anchor
- R5-6 (P1): post-compaction scalar-binding. the round-2 stub_signature signs only
  the payload projection; the surviving scalar columns (category/type/actor/
  occurred_at, id/session_id/sequence) were unbound -- an at-rest scalar edit
  forged a filter/reconstruction value while stub_signature + anchor still
  verified. the verifier now binds each scalar column to the signed projection;
  new stub_scalar_mismatch failureMode (additive-MINOR, enum 10->11) swept across
  spec-006, plan-006 (T4.1 / I-006-3-03 / lines 55,228), security-architecture
  rule 4, cross-plan-dependencies, local-sqlite-schema
- R5-7 (P2): plan-007 R2-T6 re-scoped to the emission-side securityCritical
  projection (R2's deliverable); the CLI --no-banner suppression test stays at
  R3-T8 (R2 owns no apps/cli/ file)

Refs: Plan-005, Plan-006, Plan-007, Spec-006, Spec-007
The R3 §Files inventory had drifted from the authoritative Tasks
(T-007r-3-1..6a) and §PR Boundary Mapping. Reconcile every row to its
real task, in task-ascending order:

- add `exit-codes.ts` (T-007r-3-3) — already in the §PR Boundary CREATE
  list (line 626) and Acceptance Criteria but missing from the inventory.
- add `settings.ts` (T-007r-3-6) — `settings effective-read`; likewise
  one of the §PR Boundary "6 commands" but absent from the inventory.
- fix the `daemon-status.ts` citation T-007r-3-3 -> T-007r-3-5 (T-3-3 is
  exit-codes.ts; daemon-status is T-3-5) and drop the inaccurate "NDJSON
  streaming" note — T-3-5 renders a single text-or-`--json` read.
- remove the `session-*` quartet and `meta.ts` orphan rows: no R3 task
  creates them, and both are absent from the §PR Boundary Mapping (6
  commands) and Acceptance Criteria. The CLI `ai-sidekicks session`
  surface is owned by Plan-001 Phase 5 (Spec-001:147,156), not Plan-007
  R3; `--version`/`--help`/global flags are clipanion `Cli`-builder
  built-ins in main.ts (T-007r-3-2), not a separate command file.

Self-audit follow-on to the F9 daemon-lifecycle file-count fix (the
named PR-body divergence (b)); no Codex thread.

Refs: Plan-007, Spec-007
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Sawmonabo
Copy link
Copy Markdown
Owner Author

@codex

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet