Runtime Truth ratchet and Think-on-Echo model seam#15
Conversation
|
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: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (18)
WalkthroughThis PR enshrines “Runtime Truth”: doctrine and PR guidance, a ratchet script with baseline and npm lint integration, typed domain errors applied across ports/store/minds, Think memory GraphQL contract and design docs, Echo capability probe, MCP service refactors (injectable factories, followthrough/no‑op behavior), policies, and tests. ChangesTyped Error Classes and Usage Patterns
Runtime Truth Ratcheting and Code‑Limit Enforcement
Think‑on‑Echo GraphQL Contract and Data Model
MCP Services, Capture, Migration, and Policies
Tests, changelog, and packaging
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 138ee36c9b
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/method/backlog/up-next/CORE_think-echo-phase-2-runtime-roundtrip.md (1)
29-42:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix ordered-list numbering to satisfy markdownlint MD029.
Line 31 and Line 32 use
0.prefixes in a sequential ordered list, which violates MD029 (ol-prefix) and can fail lint gates.Suggested diff
-0. Confirm `docs/design/0068-think-memory-data-model/think-memory-data-model.md` +1. Confirm `docs/design/0068-think-memory-data-model/think-memory-data-model.md` is the source truth for the contract. -0. Revise `contracts/think-memory.graphql` from that model. -0. Run `npm run echo:probe -- --json` and require +2. Revise `contracts/think-memory.graphql` from that model. +3. Run `npm run echo:probe -- --json` and require `ready_enough_for_phase_2`. -1. Build a `CaptureThought` input through generated or minimally generated +4. Build a `CaptureThought` input through generated or minimally generated contract helpers. -2. Dispatch the canonical intent through Echo. -3. Receive admission evidence for the capture. -4. Build an exact `InspectThought` observation by entry id or coordinate. -5. Receive a `ReadingEnvelope` or equivalent Echo observation artifact. -6. Verify the reading posture is complete. -7. Decode the payload into a Think-owned `ThoughtEntry`. -8. Assert that raw text and capture metadata survived the round trip. +5. Dispatch the canonical intent through Echo. +6. Receive admission evidence for the capture. +7. Build an exact `InspectThought` observation by entry id or coordinate. +8. Receive a `ReadingEnvelope` or equivalent Echo observation artifact. +9. Verify the reading posture is complete. +10. Decode the payload into a Think-owned `ThoughtEntry`. +11. Assert that raw text and capture metadata survived the round trip.🤖 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/method/backlog/up-next/CORE_think-echo-phase-2-runtime-roundtrip.md` around lines 29 - 42, The ordered list uses mixed prefixes (`0.` then `1.`) and trips markdownlint MD029; update the entire ordered list so all items use a consistent ordered-list marker (e.g., change every numeric prefix to `1.`) for the steps that reference docs/design/0068-think-memory-data-model, contracts/think-memory.graphql, npm run echo:probe -- --json (ready_enough_for_phase_2), and the steps mentioning CaptureThought, InspectThought, ReadingEnvelope, and ThoughtEntry so the list marker style is uniform and the linter passes.
🤖 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.
Inline comments:
In `@contracts/think-memory.graphql`:
- Around line 37-43: The CaptureThoughtInput GraphQL input must be updated to
match the Phase 2 data model: replace the flat text: String! with a nested
content object containing text and digest fields (e.g., content.text and
content.digest) and add a causalRef object with runtime, coordinate, and witness
fields (causalRef.runtime, causalRef.coordinate, causalRef.witness); update or
extend the CaptureProvenanceInput referenced by CaptureThoughtInput as needed so
the schema aligns with the pinned model checklist and the names
CaptureThoughtInput, content.*, and causalRef.* are used exactly as specified.
- Around line 45-52: Rename and reshape the ThoughtEntry type to match the
pinned model by replacing entryId with thoughtId and changing the flat fields
into the Phase 2 structure: remove text: String!, add content: ThoughtContent
(containing text and digest), add causalRef: CausalRef (with
runtime/coordinate/witness), and include the metadata field; ensure the type
signature uses ThoughtEntry and references ThoughtContent, CausalRef, and
metadata exactly as in the spec so Phase 2 proofs can be implemented against
these fields.
- Line 41: The capturedAt field currently uses String! which loses temporal type
safety; change the GraphQL schema to use a proper datetime scalar (e.g.,
DateTime! or a custom scalar like DateTimeScalar) for the capturedAt field(s)
(refer to capturedAt in the ThinkMemory type and the other occurrence noted),
add/import the scalar definition in the schema, and update any
resolvers/mappers/validation that parse or serialize capturedAt (e.g., schema
resolver map or serialization helpers) to use the chosen DateTime scalar
implementation so values are validated and serialized consistently.
In `@docs/audit/2026-05-13_runtime-truth-code-standard.md`:
- Around line 369-371: The Markdown heading "### F8. Swift adapter is closer to
hexagonal shape, but app state is a composition hotspot" is split across two
lines and violates MD022; join the heading into a single line starting with
"###" followed by the full text and add a blank line immediately after the
heading to separate it from the following content so the heading renders
correctly.
In `@docs/BEARING.md`:
- Line 19: Add a blank line after each Markdown heading that's currently
directly followed by content — specifically after "### 1. Think-on-Echo Runtime
Proof" and the other headings noted — to satisfy MD022; update the heading
blocks (the "### 1. Think-on-Echo Runtime Proof" heading and the two other
headings mentioned in the comment) so there is an empty line between the heading
and the next paragraph or list.
In `@docs/design/0068-think-memory-data-model/think-memory-data-model.md`:
- Around line 727-736: Resolve the open decision about thoughtId semantics
before Phase 2 schema revision: decide whether thoughtId will be content-derived
(e.g., via contentDigest), capture-event-derived (e.g., via captureId), or a
composite of both, and then update the provisional GraphQL schema and uniqueness
constraints accordingly (or explicitly document the chosen temporary contract
for Phase 2). Update any references to thoughtId, contentDigest, captureId,
actorId, writerId, and the schema-generation rules so the ID semantics,
uniqueness rules, and how they map to GraphQL IDs are unambiguous; if you choose
a temporary approach, add a clear migration note in the doc indicating the final
strategy may change and how consumers should handle the interim contract.
- Around line 187-199: The Phase 2 minimal schema shows a "source" field but the
full model defines provenance: ThoughtProvenance; clarify whether "source" is a
flattened alias of provenance (e.g., string or enum extracted from
ThoughtProvenance) or a separate field by updating the doc to state the mapping
explicitly: either replace "source" with "provenance" in the Phase 2 list or
describe that "source" is a simplified representation of ThoughtProvenance (and
list which provenance properties are included, e.g., origin, channel) so
consumers of ThoughtEntry know how to translate between the full model
(provenance: ThoughtProvenance) and the Phase 2 minimum.
In `@docs/INFRASTRUCTURE_DOCTRINE.md`:
- Line 278: The docs list the non-existent ESLint rule "no-floating-promises"
which won't be enforced; update the rule name to the namespaced variant used
elsewhere by replacing "no-floating-promises" with
"@typescript-eslint/no-floating-promises" so the floating-promise detection is
actually provided by `@typescript-eslint` and matches the other
`@typescript-eslint/`... entries (search for the string "no-floating-promises" and
the surrounding rules block to apply the change).
In `@docs/method/backlog/bad-code/SURFACE_macos-appstate-composition-root.md`:
- Around line 23-24: The acceptance sentence is ambiguous; change the wording in
SURFACE_macos-appstate-composition-root.md so it reads as an explicit, testable
requirement for CaptureAppState.swift: require that CaptureAppState.swift must
be no more than 250 lines OR, if it exceeds 250 lines, must be split so each
resulting file implements exactly one primary responsibility (define "primary
responsibility" as a single class/struct/actor or a small cohesive set of
closely related functions that together implement one feature/concern). Update
the file to replace the current phrase with this explicit rule and the brief
definition to remove ambiguity for implementation and review.
In `@scripts/runtime-truth-ratchet.mjs`:
- Around line 59-64: The run function uses spawnSync without a timeout which can
hang; add a timeout option (e.g., 30000 ms) to the spawnSync call and handle
timeout failures explicitly: after calling spawnSync in run, detect a timeout
via result.error.code === 'ETIMEDOUT' or result.signal (or result.error) and
surface a ratchet-specific failure (throw or return an Error with a clear
message like "Runtime truth ratchet: command timed out") while respecting the
existing allowFailure behavior; update the spawnSync invocation that currently
uses { cwd: repoRoot, encoding: 'utf8', maxBuffer: ... } to include the timeout
and add the timeout handling logic in run.
- Around line 126-146: collectGenericThrowFindings misses throws that span
multiple lines because it scans one line at a time; change it to read each file
as a single string and run the genericThrowPattern as a global/multiline regex
over the whole file so matches can span line breaks. In the updated
collectGenericThrowFindings, iterate RegExp.exec(fileText) (or
fileText.matchAll) for each file, ensure genericThrowPattern uses [\s\S] or the
/s flag to allow dot to match newlines, and for each match compute line and
column by counting newlines up to match.index (e.g., line = fileText.slice(0,
match.index).split('\n').length, column = match.index - lastNewlineIndex + 1)
before pushing the frozen finding (keep fields category, file, kind, line,
column, text) so compareFindings still works.
In `@scripts/think-echo-capability-probe.mjs`:
- Around line 150-166: The temp-directory cleanup can be skipped if runCommand
or readGeneratedMarkers throws; update runGeneratorUnchecked to allocate tempDir
via mkdtempSync as before but wrap the command+marker reading and result
construction in a try block and move rmSync(tempDir, { force: true, recursive:
true }) into a finally block so cleanup always runs; ensure the function still
returns the frozen object when successful and rethrows or returns the original
error/result semantics if an exception occurs.
In `@test/ports/ports.test.js`:
- Around line 28-32: The predicate isPortError currently only checks instanceof,
portName, and methodName; extend it to also assert the canonical error shape by
verifying error.code and error.message match the PortNotImplementedError
contract (e.g., compare to PortNotImplementedError.prototype.code and
PortNotImplementedError.prototype.message or the class's known static values) so
tests fail on regressions to code/message; update the isPortError function to
include these additional equality checks.
In `@test/ports/think-echo-contract.test.js`:
- Around line 7-16: The test's brittle string matching uses the
requiredFragments array; replace it with AST-based checks by parsing the schema
(e.g., using graphql's parse/buildSchema) and asserting structure: that a
Mutation type contains captureThought with CaptureThoughtInput and the
`@wes_op/`@wes_footprint directives, that a Query type contains inspectThought
with mindId/entryId args returning ThoughtEntry, and that ThoughtEntry type
exists; locate the requiredFragments variable and update the assertion logic to
traverse the parsed DocumentNode (or GraphQLSchema) to verify type names, field
names, argument names/types, and directives instead of exact string fragments.
---
Outside diff comments:
In `@docs/method/backlog/up-next/CORE_think-echo-phase-2-runtime-roundtrip.md`:
- Around line 29-42: The ordered list uses mixed prefixes (`0.` then `1.`) and
trips markdownlint MD029; update the entire ordered list so all items use a
consistent ordered-list marker (e.g., change every numeric prefix to `1.`) for
the steps that reference docs/design/0068-think-memory-data-model,
contracts/think-memory.graphql, npm run echo:probe -- --json
(ready_enough_for_phase_2), and the steps mentioning CaptureThought,
InspectThought, ReadingEnvelope, and ThoughtEntry so the list marker style is
uniform and the linter passes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f377a97c-3238-4eaf-a94b-11050f201f48
📒 Files selected for processing (38)
.github/PULL_REQUEST_TEMPLATE.mdcontracts/think-memory.graphqldocs/BEARING.mddocs/INFRASTRUCTURE_DOCTRINE.mddocs/audit/2026-05-13_runtime-truth-code-standard.mddocs/audit/runtime-truth-ratchet-baseline.jsondocs/design/0067-think-echo-contract-proof/think-echo-contract-proof.mddocs/design/0068-think-memory-data-model/think-memory-data-model.mddocs/design/README.mddocs/method/backlog/asap/CORE_think-echo-contract-proof.mddocs/method/backlog/bad-code/CORE_boundary-codec-cutover.mddocs/method/backlog/bad-code/CORE_hexagonal-store-boundary.mddocs/method/backlog/bad-code/CORE_mcp-service-dependency-injection.mddocs/method/backlog/bad-code/CORE_runtime-domain-model-cutover.mddocs/method/backlog/bad-code/CORE_runtime-truth-standard-ratchet.mddocs/method/backlog/bad-code/CORE_think-echo-toolchain-capability-probe.mddocs/method/backlog/bad-code/SURFACE_browse-tui-strict-limits.mddocs/method/backlog/bad-code/SURFACE_macos-appstate-composition-root.mddocs/method/backlog/up-next/CORE_think-echo-phase-0-direction-charter.mddocs/method/backlog/up-next/CORE_think-echo-phase-1-app-contract.mddocs/method/backlog/up-next/CORE_think-echo-phase-2-runtime-roundtrip.mddocs/method/backlog/up-next/CORE_think-memory-data-model.mdpackage.jsonscripts/runtime-truth-ratchet.mjsscripts/think-echo-capability-probe.mjssrc/errors.jssrc/minds.jssrc/store/checkpoint-product-read.jssrc/store/checkpoint-state.jssrc/store/content-reader.jssrc/store/model.jssrc/store/ports.jstest/ports/content-reader.test.jstest/ports/minds.test.jstest/ports/model.test.jstest/ports/ports.test.jstest/ports/think-echo-contract.test.jstest/ports/think-echo-model-doc.test.js
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In `@docs/method/backlog/up-next/CORE_think-echo-phase-1-app-contract.md`:
- Around line 20-21: Replace the incorrect phrase "source truth" with the
correct "source of truth" wherever it appears in the document (the two
occurrences currently present as shown in the diff), e.g., update the contract
wording in CORE_think-echo-phase-1-app-contract.md by finding the literal string
"source truth" and changing it to "source of truth" so both occurrences read
grammatically and unambiguously.
In `@docs/method/backlog/up-next/CORE_think-echo-phase-2-runtime-roundtrip.md`:
- Around line 29-35: The ordered list in
CORE_think-echo-phase-2-runtime-roundtrip.md uses repeated "0." markers which
triggers MD029; update the four list items to use a proper incremental sequence
("1.", "2.", "3.", "4.") so the lint rule passes. Specifically change the lines
referencing docs/design/0068-think-memory-data-model/think-memory-data-model.md,
docs/design/0069-think-echo-integration-plan/think-echo-integration-plan.md,
contracts/think-memory.graphql, and the npm run echo:probe -- --json /
ready_enough_for_phase_2 instruction to be numbered 1–4 in order. Ensure
spacing/indentation is unchanged so only the numeric prefixes are corrected.
- Line 30: Replace the incorrect phrase "source truth" with the correct phrase
"source of truth" wherever it appears in the document (search for the exact
string "source truth" and normalize all occurrences to "source of truth"),
ensuring consistent wording across the file including other changed lines that
used the same phrase.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d4e86d7c-9593-4aaa-8ada-ce250866356f
📒 Files selected for processing (17)
CHANGELOG.mdcontracts/think-memory.graphqldocs/design/0067-think-echo-contract-proof/think-echo-contract-proof.mddocs/design/0069-think-echo-integration-plan/think-echo-integration-plan.mddocs/design/README.mddocs/method/backlog/asap/CORE_think-echo-contract-proof.mddocs/method/backlog/bad-code/CORE_large-mind-read-timeouts.mddocs/method/backlog/up-next/CORE_think-echo-integration-plan.mddocs/method/backlog/up-next/CORE_think-echo-phase-1-app-contract.mddocs/method/backlog/up-next/CORE_think-echo-phase-2-runtime-roundtrip.mdscripts/think-echo-capability-probe.mjssrc/cli/commands/capture.jssrc/mcp/service.jssrc/policies.jstest/ports/mcp-service.test.jstest/ports/think-echo-contract.test.jstest/ports/think-echo-integration-doc.test.js
💤 Files with no reviewable changes (1)
- src/policies.js
Review Cleanup Summary
Current gate state: all review threads resolved, CodeRabbit approved, CI |
|
@codex second opinion requested. Code Lawyer self-audit found one remaining wording consistency defect after
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a27ff4f95
ℹ️ 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".
Code Lawyer Activity Summary
Current known upstream gate after pushing |
Code Lawyer Gate UpdateLatest head:
|
Code Lawyer Activity Summary
Evidence from this pass:
Gate remains locked only under the strict Code Lawyer approval-count rule: current latest reviews show CodeRabbit |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1621502556
ℹ️ 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".
| const repoAlreadyExists = deps.hasGitRepo(repoDir); | ||
|
|
||
| await deps.ensureGitRepo(repoDir); | ||
| const entry = await deps.saveRawCapture(repoDir, thought, { |
There was a problem hiding this comment.
Reinstate timeout for MCP raw capture writes
The new captureThought flow now awaits deps.saveRawCapture(...) directly before any timeout wrapper, so a stalled WARP/local-store write can block the MCP request indefinitely. In the previous implementation this path was wrapped by capturePolicy (10s timeout) specifically to prevent hung store operations from hanging capture. This regression affects production cases where disk or graph IO stalls: callers no longer get a deferred/saved response and the request can hang forever.
Useful? React with 👍 / 👎.
Summary
Test plan