docs(mimir): land doc-only borrow subset (B4/B5 + C-MIMIR-1..4)#20
Conversation
Implements the doc-only ship-now subset of the Mimir vs code-oz template comparison (synthesis at docs/comparison/11-mimir/SYNTHESIS.md; Codex thread 019e12f0). Shipped borrows (acceptance criteria met): - B4 — docs/contracts/REVIEWER_MEMORY.md (file-based memory-hygiene rubric for M17-M20; four primitives: duplicate-check, evolved_from, contradicts, store-at-durable-points; no DB). - B5 — docs/contracts/CHECKLISTS.md (deterministic DEFINE-gate + PLAN-gate review checklists; advisory in v0.1, promotion to blocking requires events.jsonl evidence + rule-20 authority budget). Shipped project-level constraints: - C-MIMIR-1 — CLAUDE.md rule 1 sub-paragraph: MCP write-back fence. Gates are written only by orchestrator-owned approval primitives that call writeGate; external surfaces (MCP, hooks, future integrations) may submit advisory request files under .code-oz/state/runs/<runId>/requests/ but never bypass gate validation and never append events.jsonl directly. - C-MIMIR-2 — docs/references/provider-contract.md § "Telemetry visibility for runtime queues": any pre-execution gating wrapper around IAgentProvider.invoke must emit typed events into events.jsonl with the same visibility as rule-19 budget enforcement; event shape sketched for B3. - C-MIMIR-3 — docs/references/provider-contract.md § "HTTP adapter abstraction extraction discipline": a shared OpenAICompatProvider is extracted only after two concrete HTTP adapters pass independently with their own CODEX_REVIEW_PE<n>.md audit; strict-allowlist is additive only across subclasses; documented divergence axes for Together, Fireworks, OpenRouter, Groq, Ollama. - C-MIMIR-4 — CLAUDE.md rule 16 sub-paragraph: LLM-generated personas forbidden (deterministic template renderers acceptable later). Deferred per acceptance criteria (untouched in this PR): - B1 — read-only MCP-server adapter (gate: named external consumer, post-SHIP / W4+). - B2 — OpenAICompatProvider extraction (gate: PE-2 second adapter passes independently). - B3 — pre-execution rate-limit queue (gate: first 429 observed; telemetry shape pre-codified per C-MIMIR-2). M17 discoverability pointers added in REVIEW.md, REVIEW_PANEL.md, ROADMAP.md so the Reviewer Memory rubric is findable when M17 kickoff begins. SCIENTIST.md gains a sibling-discipline pointer to CHECKLISTS.md. Review trail (4 review rounds + 3 fix passes, per project rule 6): - R1 review (fix-first 7/10): 1 block-push (B5/SCIENTIST.md ref) + 6 fix-soon. Thread 019e1323. - R2 fix-pass closed all 7. Thread 019e1328. - R3 review (fix-first 7/10): 2 new fix-soon (B5 prose drift in SYNTHESIS, ROADMAP M17 pointer) + 1 nit. Thread 019e132f. - R4 fix-pass closed all 3. Thread 019e1432. - R5 review (fix-first 7/10): 2 residual partials on SYNTHESIS prose drift. Thread 019e1437. - R6 fix-pass: split B5 acceptance criteria into advisory/blocking phases; annotated ace cross-ref. Thread 019e143b. - R7 review (fix-first 7/10): 2 prose-consistency nits on Authority cost line and ace cross-ref accuracy. Thread 019e143c. - Final pass (Claude main): closed the two R7 nits inline; rule-6 4-round review cap reached; converged at score 7/10 with no block-borrow risks open. Scope: docs-only. No src/, tests/, package.json touches. 3108 tests pass, typecheck clean.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR establishes governance clarifications from code-oz's universal rules, conducts a complete structured adversarial audit of borrowing mechanics from Mimir with locked synthesis decisions, introduces two new contract frameworks (gate-readiness checklists and reviewer memory hygiene), and extends the provider contract with telemetry and HTTP adapter constraints derived from the borrow audit. ChangesGovernance Rules, Borrow Audit, and Contract Frameworks
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive comparison between code-oz and the Mimir framework, resulting in new project-level constraints and a locked borrow set. Key additions include advisory gate checklists, a memory-hygiene rubric, and updated provider contracts for abstraction extraction and telemetry. Feedback highlights the need to resolve directory path inconsistencies and remove machine-specific absolute paths from the documentation.
|
|
||
| 1. **File-based gate signals only.** Never parse LLM text output for pass/fail. Use `state/GATE_<PHASE>_PASSED.json` files validated by `src/state/gates.ts` schemas. (maestro lesson) | ||
|
|
||
| Gate writes are produced only by orchestrator-owned approval primitives that call `writeGate` (`src/state/gates.ts`, `src/state/run.ts`; e.g. `approveGate`, `approveReviewTaskGate`). MCP tool calls, hook invocations, and any future external integration surfaces cannot write gate files, canonical artifacts, or `events.jsonl` directly; only orchestrator-owned event-emission primitives append events. If a future external surface accepts write requests from clients, those requests are recorded as *advisory request files* under `.code-oz/state/runs/<runId>/requests/<request-id>.json` (same shape as `NEEDS_INTERVENTION.json`, a request the next phase preflight may consider) and never bypass gate validation; external consumers may read events and submit advisory requests, but they never own gate or event-log writes. Pinned 2026-05-10 from Mimir comparison (`docs/comparison/11-mimir/SYNTHESIS.md` § "C-MIMIR-1"). |
There was a problem hiding this comment.
The path .code-oz/state/runs/<runId>/requests/<request-id>.json is inconsistent with the established directory structure where runs/ is a top-level subdirectory of .code-oz/ (as seen in ROADMAP.md and REVIEW_PANEL.md). Based on the project's state model, this should likely be .code-oz/runs/<runId>/requests/<request-id>.json.
| Gate writes are produced only by orchestrator-owned approval primitives that call `writeGate` (`src/state/gates.ts`, `src/state/run.ts`; e.g. `approveGate`, `approveReviewTaskGate`). MCP tool calls, hook invocations, and any future external integration surfaces cannot write gate files, canonical artifacts, or `events.jsonl` directly; only orchestrator-owned event-emission primitives append events. If a future external surface accepts write requests from clients, those requests are recorded as *advisory request files* under `.code-oz/state/runs/<runId>/requests/<request-id>.json` (same shape as `NEEDS_INTERVENTION.json`, a request the next phase preflight may consider) and never bypass gate validation; external consumers may read events and submit advisory requests, but they never own gate or event-log writes. Pinned 2026-05-10 from Mimir comparison (`docs/comparison/11-mimir/SYNTHESIS.md` § "C-MIMIR-1"). | |
| Gate writes are produced only by orchestrator-owned approval primitives that call `writeGate` (`src/state/gates.ts`, `src/state/run.ts`; e.g. `approveGate`, `approveReviewTaskGate`). MCP tool calls, hook invocations, and any future external integration surfaces cannot write gate files, canonical artifacts, or `events.jsonl` directly; only orchestrator-owned event-emission primitives append events. If a future external surface accepts write requests from clients, those requests are recorded as *advisory request files* under `.code-oz/runs/<runId>/requests/<request-id>.json` (same shape as `NEEDS_INTERVENTION.json`, a request the next phase preflight may consider) and never bypass gate validation; external consumers may read events and submit advisory requests, but they never own gate or event-log writes. Pinned 2026-05-10 from Mimir comparison (`docs/comparison/11-mimir/SYNTHESIS.md` § "C-MIMIR-1"). |
|
|
||
| 2. **B2 (OpenAI-compat) inverts the timing.** Land *after* PE-2 brings the second concrete HTTP adapter, then extract `OpenAICompatProvider` from two passing adapters. Codex cited concrete divergence quirks per provider (Together, Fireworks, OpenRouter, Groq, Ollama) that would force the abstraction to diverge if extracted from xAI alone. | ||
|
|
||
| 3. **B3 (rate-limit queue) is an honest gap, not a production bug.** Defer until PE-2 or first real 429. Implement as a bounded per-`(provider, model)` FIFO wrapper around `IAgentProvider.invoke` with timeout/cancel + `events.jsonl` telemetry. Do not copy Mimir's hour-window singleton (`/Users/ozzy-mac/Projects/agents/templates/Mimir/src/orchestrator/rate-limit-queue.ts:21-26`). |
There was a problem hiding this comment.
Hardcoded absolute path detected. Documentation should avoid machine-specific absolute paths to ensure portability and maintainability for other contributors.
| 3. **B3 (rate-limit queue) is an honest gap, not a production bug.** Defer until PE-2 or first real 429. Implement as a bounded per-`(provider, model)` FIFO wrapper around `IAgentProvider.invoke` with timeout/cancel + `events.jsonl` telemetry. Do not copy Mimir's hour-window singleton (`/Users/ozzy-mac/Projects/agents/templates/Mimir/src/orchestrator/rate-limit-queue.ts:21-26`). | |
| 3. **B3 (rate-limit queue) is an honest gap, not a production bug.** Defer until PE-2 or first real 429. Implement as a bounded per-`(provider, model)` FIFO wrapper around `IAgentProvider.invoke` with timeout/cancel + `events.jsonl` telemetry. Do not copy Mimir's hour-window singleton (`Mimir/src/orchestrator/rate-limit-queue.ts:21-26`). |
| | D4 | Does `## Acceptance criteria` contain verifiable outcomes that can be checked later? | | ||
| | D5 | Does `## Explicit non-goals` exclude at least one plausible scope creep path? | | ||
| | D6 | Does `## Open questions` either name unresolved questions or explicitly say none are known at define time? | | ||
| | D7 | Does the run path match the locked classification contract: greenfield enters DEFINE, while brownfield or existing-code work enters AUDIT before PLAN (`CLAUDE.md` rule 14; `ROADMAP.md` "Locked decisions" 3)? | |
There was a problem hiding this comment.
The reference to ROADMAP.md is missing the docs/design/ directory prefix. This is inconsistent with the full-path referencing style used elsewhere in the project (e.g., line 42 of this file).
| | D7 | Does the run path match the locked classification contract: greenfield enters DEFINE, while brownfield or existing-code work enters AUDIT before PLAN (`CLAUDE.md` rule 14; `ROADMAP.md` "Locked decisions" 3)? | | |
| | D7 | Does the run path match the locked classification contract: greenfield enters DEFINE, while brownfield or existing-code work enters AUDIT before PLAN (`CLAUDE.md` rule 14; `docs/design/ROADMAP.md` "Locked decisions" 3)? | |
| ## Reference | ||
|
|
||
| - Source decision: `docs/comparison/11-mimir/SYNTHESIS.md` section "B5". | ||
| - Mimir source pattern: `/Users/ozzy-mac/Projects/agents/templates/Mimir/docs/agents/v2/00-ecko-preamble.md:11-18` and `:77-122`. |
There was a problem hiding this comment.
Hardcoded absolute path detected. Machine-specific paths in contract documentation hinder portability.
| - Mimir source pattern: `/Users/ozzy-mac/Projects/agents/templates/Mimir/docs/agents/v2/00-ecko-preamble.md:11-18` and `:77-122`. | |
| - Mimir source pattern: `Mimir/docs/agents/v2/00-ecko-preamble.md:11-18` and `:77-122`. |
| ## Reference | ||
|
|
||
| - Source decision: `docs/comparison/11-mimir/SYNTHESIS.md` section "B4". | ||
| - Borrowed pattern source: `/Users/ozzy-mac/Projects/agents/templates/Mimir/.agents/claudette-mimir-v3.yaml:28-51`. |
There was a problem hiding this comment.
Hardcoded absolute path detected. Documentation should use relative paths or placeholders instead of machine-specific absolute paths.
| - Borrowed pattern source: `/Users/ozzy-mac/Projects/agents/templates/Mimir/.agents/claudette-mimir-v3.yaml:28-51`. | |
| - Borrowed pattern source: `Mimir/.agents/claudette-mimir-v3.yaml:28-51`. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09638acbae
ℹ️ 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".
|
|
||
| 1. **File-based gate signals only.** Never parse LLM text output for pass/fail. Use `state/GATE_<PHASE>_PASSED.json` files validated by `src/state/gates.ts` schemas. (maestro lesson) | ||
|
|
||
| Gate writes are produced only by orchestrator-owned approval primitives that call `writeGate` (`src/state/gates.ts`, `src/state/run.ts`; e.g. `approveGate`, `approveReviewTaskGate`). MCP tool calls, hook invocations, and any future external integration surfaces cannot write gate files, canonical artifacts, or `events.jsonl` directly; only orchestrator-owned event-emission primitives append events. If a future external surface accepts write requests from clients, those requests are recorded as *advisory request files* under `.code-oz/state/runs/<runId>/requests/<request-id>.json` (same shape as `NEEDS_INTERVENTION.json`, a request the next phase preflight may consider) and never bypass gate validation; external consumers may read events and submit advisory requests, but they never own gate or event-log writes. Pinned 2026-05-10 from Mimir comparison (`docs/comparison/11-mimir/SYNTHESIS.md` § "C-MIMIR-1"). |
There was a problem hiding this comment.
Narrow gate-write claim to include intervention writers
The new rule text says gate writes are produced only by approval primitives (approveGate/approveReviewTaskGate), but the runtime also writes gate files through orchestrator-owned failure paths via writeNeedsInterventionGate (for example in src/providers/invoke.ts and phase handlers like src/phases/verify.ts). Keeping this statement as-is makes the architecture contract internally inconsistent and can mislead future work (especially MCP/write-back fencing) into treating valid intervention-gate writes as violations.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Doc-only update that lands the “ship-now” subset of the code-oz vs Mimir comparison: it records the locked synthesis and codifies a small set of discipline borrows as contracts/constraints (without implementing any runtime/code changes).
Changes:
- Add two new contracts: Reviewer Memory hygiene rubric (B4) and DEFINE/PLAN advisory checklists (B5).
- Codify two provider/runtime constraints in the provider contract (C-MIMIR-2..3) and pin two project-level fences in
CLAUDE.md(C-MIMIR-1..4 subset referenced in PR description). - Add discoverability pointers (ROADMAP + REVIEW + REVIEW_PANEL + SCIENTIST) and land the full Mimir comparison + Codex debate trail docs.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/references/provider-contract.md | Adds pre-codified constraints for future OpenAI-compat extraction timing and runtime-queue telemetry visibility. |
| docs/design/ROADMAP.md | Adds an M17 pointer to the new Reviewer Memory contract. |
| docs/contracts/SCIENTIST.md | Links CHECKLISTS as a sibling discipline for DEFINE/PLAN gate readiness. |
| docs/contracts/REVIEWER_MEMORY.md | New contract defining file-based Reviewer Memory lesson hygiene rubric (v0.1 rubric only). |
| docs/contracts/REVIEW.md | Adds cross-links/pointer to REVIEWER_MEMORY for M17 discoverability. |
| docs/contracts/REVIEW_PANEL.md | Adds cross-links/pointer to REVIEWER_MEMORY for panel-mode discoverability. |
| docs/contracts/CHECKLISTS.md | New contract defining deterministic DEFINE/PLAN advisory checklists + promotion gate criteria. |
| docs/comparison/11-mimir/SYNTHESIS.md | New locked synthesis document: verdict, borrows (B1–B5), constraints (C-MIMIR-1..4), and acceptance criteria. |
| docs/comparison/11-mimir/COMPARISON.md | New initial comparison writeup providing the basis for the synthesis and debate. |
| docs/comparison/11-mimir/CODEX_RESPONSE.md | Verbatim Codex response captured for audit trail. |
| docs/comparison/11-mimir/CODEX_BRIEFING.md | Codex briefing prompt used to drive the adversarial review/debate. |
| CLAUDE.md | Clarifies rule 1 with an MCP write-back fence and expands rule 16 with an explicit ban on LLM-generated personas. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| type BaseEvent = { | ||
| readonly version: 1 | ||
| readonly ts: string | ||
| readonly runId: string | ||
| readonly phase: PhaseName | ||
| readonly agent: string | ||
| } | ||
|
|
||
| // BaseEvent provides the standard events.jsonl envelope: version, ts, runId, phase, agent. | ||
| type RuntimeQueueEvent = BaseEvent & ( | ||
| | { readonly type: 'rate_limit_queued'; readonly provider: ProviderId; readonly model: string; readonly queueDepth: number } | ||
| | { readonly type: 'rate_limit_waited'; readonly provider: ProviderId; readonly model: string; readonly waitMs: number } | ||
| | { readonly type: 'rate_limit_dispatched'; readonly provider: ProviderId; readonly model: string; readonly queueDepth: number } | ||
| | { readonly type: 'rate_limit_dropped'; readonly provider: ProviderId; readonly model: string; readonly reason: string } |
|
|
||
| The dossier behind the discipline: `docs/research/05-scientist-and-open-questions-agent.md`. The maestro skill it operationalizes is `requirement-restate` (every gate restates inputs, claims, and falsifiers). | ||
|
|
||
| Related sibling discipline: [`CHECKLISTS.md`](./CHECKLISTS.md) captures static gate-readiness yes/no rubrics for SPEC.md and PLAN.md, while Scientist captures hypotheses and open questions. They are siblings, not competitors; both are advisory in v0.1. Pinned 2026-05-10 from `docs/comparison/11-mimir/SYNTHESIS.md` § "B5". |
R8 cross-family review caught that the SCIENTIST.md sibling-reference paragraph added in this branch incorrectly called Scientist sidecars "advisory in v0.1." Scientist sidecars are gate-relevant per CLAUDE.md rule 15 — missing, invalid, overdue, or blocking sidecars can block phase exit. The wrong phrasing would tell future operators Scientist is advisory when it is not. Fix: CHECKLISTS stays advisory in v0.1; Scientist remains governed by rule 15 and existing gate-preflight sidecar validation. Codex thread `019e1441`.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/contracts/REVIEWER_MEMORY.md (2)
135-136: ⚡ Quick winAvoid citing exact upstream line numbers.
Line 135 references the Mimir source with exact line numbers (
claudette-mimir-v3.yaml:28-51). Line numbers in external templates drift over time, making this reference brittle.Consider rephrasing to reference the concept without line numbers, e.g., "Borrowed pattern source: Mimir's Claudette agent frontmatter schema (graph memory relationships)."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/contracts/REVIEWER_MEMORY.md` around lines 135 - 136, Replace the exact-file-and-line citation in the "Borrowed pattern source:" entry (currently referencing `/Users/ozzy-mac/Projects/agents/templates/Mimir/.agents/claudette-mimir-v3.yaml:28-51`) with a non-line-numbered reference to the concept—e.g., "Mimir's Claudette agent frontmatter schema (graph memory relationships)"—and keep the "Borrowed pattern, not borrowed code." sentence; update the text that mentions the specific path/line so it no longer contains brittle line numbers.
67-80: ⚡ Quick winClarify M17 mechanization path for human-review criteria.
Lines 67-69 state that M17 human-review criteria are "intentionally not-yet-mechanical" and that "M17 must either keep them as human review criteria or replace them with different deterministic tests before using them in an automated writer."
This leaves the M17 implementation path ambiguous. Consider adding a note about what happens if M17 ships without mechanizing these criteria (e.g., "If M17 ships with human-review-only criteria, the writer surface must gate on human approval before storing lessons").
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/contracts/REVIEWER_MEMORY.md` around lines 67 - 80, The M17 section leaves the mechanization path ambiguous; update the M17 human-review criteria paragraph to explicitly state the runtime gating behavior if these criteria remain human-review-only (e.g., require human approval before the writer stores lessons), and reference the existing storage alternatives (HYPOTHESES.md and the run artifact) and the writer surface so implementers know to gate storage actions until human sign-off; ensure this note appears adjacent to the M17 criteria block and mentions that automated writers must either replace criteria with deterministic tests or require explicit human approval before writing lessons.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/contracts/REVIEWER_MEMORY.md`:
- Around line 135-136: Replace the exact-file-and-line citation in the "Borrowed
pattern source:" entry (currently referencing
`/Users/ozzy-mac/Projects/agents/templates/Mimir/.agents/claudette-mimir-v3.yaml:28-51`)
with a non-line-numbered reference to the concept—e.g., "Mimir's Claudette agent
frontmatter schema (graph memory relationships)"—and keep the "Borrowed pattern,
not borrowed code." sentence; update the text that mentions the specific
path/line so it no longer contains brittle line numbers.
- Around line 67-80: The M17 section leaves the mechanization path ambiguous;
update the M17 human-review criteria paragraph to explicitly state the runtime
gating behavior if these criteria remain human-review-only (e.g., require human
approval before the writer stores lessons), and reference the existing storage
alternatives (HYPOTHESES.md and the run artifact) and the writer surface so
implementers know to gate storage actions until human sign-off; ensure this note
appears adjacent to the M17 criteria block and mentions that automated writers
must either replace criteria with deterministic tests or require explicit human
approval before writing lessons.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fe3ee25b-2c5d-440f-bbfc-73f3d7463d20
📒 Files selected for processing (12)
CLAUDE.mddocs/comparison/11-mimir/CODEX_BRIEFING.mddocs/comparison/11-mimir/CODEX_RESPONSE.mddocs/comparison/11-mimir/COMPARISON.mddocs/comparison/11-mimir/SYNTHESIS.mddocs/contracts/CHECKLISTS.mddocs/contracts/REVIEW.mddocs/contracts/REVIEWER_MEMORY.mddocs/contracts/REVIEW_PANEL.mddocs/contracts/SCIENTIST.mddocs/design/ROADMAP.mddocs/references/provider-contract.md
…thority list Codex P2 finding on PR #20: the C-MIMIR-1 expansion to non-negotiable rule 1 claimed gate writes happen only via approval primitives. That was factually incomplete — orchestrator-owned intervention writers (writeNeedsInterventionGate, writePauseGate, writeStopGate, all routed through writeControlGate) also produce gate writes, invoked by src/phases/*.ts, src/providers/invoke.ts, and src/worktree/load-or-create-run-worktree.ts on refusal paths. The rule's intent stays unchanged: external surfaces (MCP tools, hook invocations) cannot write gate files, canonical artifacts, or events.jsonl; they may only submit advisory request files for the next phase preflight to consider. Only the writer set is broadened to match the actual code.
… clarity Closes three Codex/Gemini/Copilot review findings on PR #20: 1. Gemini (3 instances + same class in PR-added Mimir docs): replace hardcoded /Users/ozzy-mac/Projects/agents/templates/Mimir/ citations with the portable <templates-root>/Mimir/ placeholder across CODEX_RESPONSE.md, SYNTHESIS.md, CHECKLISTS.md, and REVIEWER_MEMORY.md. SYNTHESIS.md now documents the placeholder so the convention is portable across operator machines. 2. Copilot (PhaseName -> Phase): docs/references/provider-contract.md used PhaseName in a BaseEvent code snippet, but the codebase exports `Phase` (src/state/schemas.ts, src/config/schema.ts). Fixed. 3. Copilot self-contradiction in SCIENTIST.md: the sibling note about CHECKLISTS read ambiguously around "advisory in v0.1." Rewritten to make it explicit that the Scientist tail is gate-blocking under rule 15 with no v0.1 exception, and that CHECKLISTS alone is the advisory sibling. W3_LITE_SCOPE.md's hardcoded CWD path is pre-existing and out of scope for this PR.
Review-fix pass: 4 findings closed (2 commits, 5865fd5 + 71b1e37)Codex P2 — CLAUDE.md non-negotiable rule 1 writer-set inaccuracy (commit 5865fd5) Gemini — 3 hardcoded absolute paths (commit 71b1e37) Copilot — Copilot — SCIENTIST.md self-contradiction on advisory scope (commit 71b1e37) Ready for re-review. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71b1e37cbd
ℹ️ 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".
|
|
||
| **Authority cost.** Zero new authority axis at the gate boundary. New integration-boundary authority (rule 20). Operates strictly downstream of the file system; reads gate files / events / artifacts; writes nothing. | ||
|
|
||
| **Permanent fence (project-level constraint).** **No write-back to gates or canonical artifacts, ever.** If write-back is added in a future milestone, it must write *advisory request files* (same shape as `NEEDS_INTERVENTION.json` — a request the next phase preflight may consider, not authority that bypasses gate validation). The fence is added to rule 1 as a clarification: file-based gates are written by `writeGate` / `approveGate` only; MCP tool calls are not gate writes. |
There was a problem hiding this comment.
Expand C-MIMIR-1 gate-writer list beyond approval calls
This constraint says gate files are written only by writeGate/approveGate, but the orchestrator also writes control gates through non-approval primitives (writeNeedsInterventionGate via src/state/gates.ts) from runtime paths like src/providers/invoke.ts, src/phases/verify.ts, and src/worktree/load-or-create-run-worktree.ts. Keeping this narrower wording in the locked synthesis makes the authority model internally inconsistent and can cause future MCP/write-back enforcement work to misclassify legitimate orchestrator intervention writes as violations.
Useful? React with 👍 / 👎.
Summary
Implements the doc-only ship-now subset of the Mimir vs code-oz template comparison. The locked spec is in
docs/comparison/11-mimir/SYNTHESIS.md. Codex initial review thread:019e12f0.Verdict: code-oz is ahead on the discipline axes that matter to its category (15 axes); Mimir is ahead in its own category (memory-native MCP product) which is out-of-scope for code-oz by deliberate commitment.
What ships (acceptance criteria met)
Borrows:
docs/contracts/REVIEWER_MEMORY.md(file-based memory-hygiene rubric for M17-M20; four primitives: duplicate-check,evolved_from,contradicts, store-at-durable-points; per-repo files, no DB).docs/contracts/CHECKLISTS.md(deterministic DEFINE-gate + PLAN-gate review checklists; advisory in v0.1, promotion to blocking requiresevents.jsonlevidence + consumes a milestone's rule-20 authority budget).Project-level constraints:
CLAUDE.mdrule 1 MCP write-back fence (gate writes only via orchestrator-owned approval primitives; external surfaces submit advisory request files, never appendevents.jsonldirectly).docs/references/provider-contract.md§ "Telemetry visibility for runtime queues" (B3 pre-codification).docs/references/provider-contract.md§ "HTTP adapter abstraction extraction discipline" (extractOpenAICompatProvideronly after two passing adapters; divergence axes documented for Together / Fireworks / OpenRouter / Groq / Ollama).CLAUDE.mdrule 16 LLM-generated personas ban (deterministic template renderers acceptable later).Discoverability: M17 pointers added in
REVIEW.md,REVIEW_PANEL.md,ROADMAP.md.SCIENTIST.mdgains a sibling-discipline pointer toCHECKLISTS.md.What defers (acceptance gates not yet fired)
OpenAICompatProviderextraction. Gate: PE-2 second adapter passes independently. Slot: PE-2 follow-up commit.Review trail
7 rounds (4 review + 3 fix-pass) before final inline polish:
019e1323019e1328019e132f019e1432019e1437019e143b019e143cRule 6 cap (max 4 review rounds) reached. Score stable at 7/10 across R3/R5/R7 indicates convergence on prose-consistency nits with no block-borrow risks open. Closed the final two prose-consistency findings inline rather than burn another round on nit-level work.
Scope check
src/,tests/,package.json, orbun.locktouches.bun run typecheckclean.Test plan
bun testpasses against the rebased main (3108 tests baseline)bun run typecheckcleanREVIEW.md/REVIEW_PANEL.md/ROADMAP.mdcross-references🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation