feat: doctrine alignment + ChittyAdvocate cross-model bootstrap#80
feat: doctrine alignment + ChittyAdvocate cross-model bootstrap#80chitcommit merged 4 commits intomainfrom
Conversation
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
chittyconnect | 3e76be2 | Mar 24 2026, 02:44 AM |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughAdds ChittyAdvocate design doc, a DB migration for doctrine lifecycle states, new public API endpoints for doctrine/bootstrap/articles, refactors context resolution to a synthetic-continuity model, introduces a Command UI with a Command store for health/autopilot/healing, and minor OAuth clientId sanitization and README badges. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Browser as Browser (Command Page)
participant Store as Command Store
participant API as Backend API
participant DB as Database
User->>Browser: Open /command
activate Browser
Browser->>Store: init()
activate Store
Store->>API: GET /api/connections (fetchAll)
API->>DB: SELECT connections
DB-->>API: rows
API-->>Store: connections array
Store->>API: POST /api/connections/:id/test (testOne) / POST /api/sweep (testAll)
API->>DB: update health state
DB-->>API: ack
API-->>Store: test results
Store->>Store: enqueueHeal / processHealQueue (exponential backoff)
Store->>API: healing attempts (retries)
API->>DB: persist updates
DB-->>API: ack
Store-->>Browser: emit state updates (connections, logs, insights)
deactivate Store
Browser->>Browser: render UI (connections, keys, logs)
deactivate Browser
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
Pull request overview
Adds doctrine-aligned context resolution + lifecycle updates in the worker, introduces public “ChittyAdvocate” bootstrap/seed endpoints, and ships a new “Command” UI for connection health/autopilot + key management.
Changes:
- ContextResolver v2 cascade (explicit → anchor → multi-signal → propose new) with stricter “never mint on DB failure” posture and coordination-need requirement for minting.
- Lifecycle alignment: supernova/fission now mark contexts as
retired(vsarchived) and a new migration attempts to widen the allowed lifecycle states. - New public bootstrap endpoints (
/api/v1/doctrine/seed,/api/v1/signal/*) and a new dashboard page (/command) with autopilot/healing + API key CRUD.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/stores/commandStore.js | New Zustand store backing “Command” UI (connections health, autopilot/heal queue, API key CRUD). |
| ui/src/pages/Command.jsx | New “Command” page UI (connections view, insights, autopilot settings, keys table, activity log). |
| ui/src/App.jsx | Adds sidebar nav + route for /command. |
| src/middleware/oauth-provider.js | Sanitizes OAuth userId construction to avoid : delimiter issues. |
| src/intelligence/context-resolver.js | Implements resolver v2 cascade + coordinationNeed requirement + multi-signal matching. |
| src/intelligence/context-intelligence.js | Uses retired status for supernova/fission source contexts. |
| src/intelligence/tests/context-resolver.test.js | Adds doctrine-related tests around resolver behavior. |
| src/index.js | Adds public doctrine seed + signal bootstrap/articles endpoints. |
| migrations/014_doctrine_lifecycle_states.sql | New migration attempting lifecycle status expansion via table rebuild. |
| docs/CHITTYADVOCATE_DESIGN.md | Design doc for ChittyAdvocate narrative/bootstrap distribution concept. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/intelligence/context-intelligence.js (1)
1036-1045:⚠️ Potential issue | 🔴 CriticalCHECK constraint violation:
'dissolved'is not an allowed status.The migration
014_doctrine_lifecycle_states.sqlenforces a CHECK constraint allowing only('fresh','active','dormant','stale','retired'), butdissolveSuspensionwritesstatus = 'dissolved'. This will cause a runtime constraint violation when this method executes.Either:
- Add
'dissolved'to the allowed states in the migration, or- Map dissolved suspensions to
'retired'like supernova/fissionOption 2: Map to 'retired' for consistency
async dissolveSuspension(suspensionChittyId, rollbackMetrics = true) { - // Archive the suspension + // Retire the suspension await this.db .prepare( ` - UPDATE context_entities SET status = 'dissolved' WHERE chitty_id = ? AND issuer = 'suspension' + UPDATE context_entities SET status = 'retired' WHERE chitty_id = ? AND issuer = 'suspension' `, ) .bind(suspensionChittyId) .run();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/intelligence/context-intelligence.js` around lines 1036 - 1045, The dissolveSuspension method currently writes status = 'dissolved', which violates the CHECK constraint from migration 014_doctrine_lifecycle_states.sql; update dissolveSuspension to set status = 'retired' instead (consistent with supernova/fission handling) by changing the UPDATE in the dissolveSuspension function to use 'retired' for issuer = 'suspension' and ensure any related logic/metrics rollbacks still refer to the same retirement semantics.src/intelligence/context-resolver.js (1)
436-489:⚠️ Potential issue | 🟠 MajorThe new justification is not durable across partial failures.
coordinationNeedonly reaches durable storage in the ledger payload, butmintChittyId()and both inserts happen first. IflogToLedger()fails, you keep a valid context with no recorded justification, and a retry can mint a second ChittyID for the same request. Please make this flow atomic/idempotent across mint + DB writes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/intelligence/context-resolver.js` around lines 436 - 489, The flow mints a ChittyID (mintChittyId) and inserts context_entities/context_dna before calling logToLedger, so if ledger logging fails you can be left with a context lacking coordinationNeed and a retry may mint a second chitty; fix by making the DB writes durable and idempotent: (1) include coordinationNeed as a column/field on context_entities (or a JSON metadata column) and perform the INSERTs inside a single DB transaction so coordinationNeed is persisted atomically with contextId/chittyId (use the same transaction for context_entities and context_dna inserts); (2) add a uniqueness check/constraint keyed on the anchors (projectPath, workspace, supportType, organization) and/or contextId so retries first query the DB for an existing context and reuse its chittyId instead of calling mintChittyId again; and (3) only call logToLedger after the transaction commits (so ledger is the last step) and make logToLedger retriable against the stored context (using contextId/chittyId) without reminting; update functions referenced: mintChittyId, logToLedger and the context_entities/context_dna insert logic to implement this transaction, uniqueness check, and durable coordinationNeed persistence.
🧹 Nitpick comments (3)
ui/src/pages/Command.jsx (2)
484-496: Consider adding user feedback after copying.The copy button calls
navigator.clipboard.writeText()but provides no visual confirmation. A brief "Copied!" state or toast would improve UX.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/Command.jsx` around lines 484 - 496, Add visual feedback when the Copy button succeeds or fails: after calling navigator.clipboard.writeText(newKeyValue) in the onClick handler of the Copy button, set a transient UI state (e.g., show "Copied!" text or a toast) and clear it after a short timeout; handle rejections by showing an error message. Use the existing clearNewKeyValue function for Dismiss behavior and add a local state variable (e.g., copied or copyError) in the component that the button's onClick updates so the UI near the Copy button displays the confirmation or error.
71-74: Consider addinginitandcleanupto the dependency array or adding a suppression comment.While Zustand store functions are typically stable and the empty dependency array works, this pattern may trigger ESLint exhaustive-deps warnings. Adding a comment clarifies the intentional omission.
Add suppression comment for clarity
useEffect(() => { init(); return cleanup; + // eslint-disable-next-line react-hooks/exhaustive-deps -- store functions are stable }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/Command.jsx` around lines 71 - 74, The useEffect call in Command.jsx currently calls init() and returns cleanup with an empty dependency array, which can trigger ESLint exhaustive-deps; either add init and cleanup to the dependency array of the useEffect or explicitly silence the rule with a focused suppression comment above the effect (e.g. // eslint-disable-next-line react-hooks/exhaustive-deps) and include a short comment explaining why init and cleanup are intentionally stable (Zustand store functions), referencing the useEffect, init, and cleanup symbols so reviewers understand the rationale.ui/src/stores/commandStore.js (1)
610-617: Potential for duplicate timers ifinit()is called multiple times.If
init()is invoked beforecleanup()(e.g., React strict mode double-mounting),startAutopilot()may create duplicate interval timers sinceinit()doesn't clear existing timers first.Clear existing timers in init
init: async () => { + get().cleanup(); // Clear any existing timers first await get().fetchAll(); await get().fetchApiKeys(); // Resume autopilot if it was on if (get().prefs.autopilotEnabled) { get().startAutopilot(); } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/stores/commandStore.js` around lines 610 - 617, init() can create duplicate autopilot intervals if called multiple times; before calling get().startAutopilot() check for and clear any existing timers by invoking the store's cleanup logic (e.g., call get().cleanup() or explicitly clearInterval/clearTimeout on the same timer IDs used by startAutopilot()) so startAutopilot() only creates a new interval when no existing timer is running; update init() to clear existing timers first (reference init(), startAutopilot(), and cleanup()).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@migrations/014_doctrine_lifecycle_states.sql`:
- Around line 45-51: The CHECK constraint in the migrations definition for the
lifecycle status omits the 'dissolved' state, causing runtime constraint
violations when dissolveSuspension() in src/intelligence/context-intelligence.js
sets status = 'dissolved'; update the SQL CHECK clause (the status TEXT DEFAULT
'fresh' CHECK (...) in the migration diff) to include 'dissolved' among the
allowed values, and then audit related places (dissolveSuspension(), doctrine
docs, and bootstrap payloads in src/index.js) to ensure 'dissolved' is listed
where lifecycle states are enumerated.
In `@src/intelligence/context-resolver.js`:
- Around line 424-432: The current guard "if (!coordinationNeed)" allows
whitespace or non-string truthy values; tighten it by validating that
coordinationNeed is a string with non-whitespace content (e.g., typeof
coordinationNeed === 'string' && coordinationNeed.trim().length > 0) and
otherwise throw the same Error; update the check around the coordinationNeed
parameter in the function (the existing if block referencing coordinationNeed)
to enforce this stricter validation.
- Around line 118-137: The catch block in ContextResolver's multi-signal step
swallows errors from findBestCandidate and falls through to the create_new path;
change the catch in the try/catch around this.findBestCandidate(hints) to
surface the failure by returning an error response (or rethrowing) instead of
just console.warn so callers do not proceed to mint; update the handler to
return a structure consistent with other error cases (e.g., action: "error" with
an error message/reason and resolution: "multi_signal_failure") or rethrow the
caught error from the method, and add a regression test that simulates
findBestCandidate throwing to assert the resolver returns an error (and does not
return a create_new proposal).
In `@src/middleware/oauth-provider.js`:
- Around line 129-130: The actor identity creation uses a lossy replacement
(safeClientId and actorId) that can map distinct oauthReqInfo.clientId values to
the same actor; change the normalization to a collision-free encoding (e.g.,
Base64 URL-safe or encodeURIComponent) of oauthReqInfo.clientId instead of
replacing ":" with "-" so actorId = `mcp-client-<encodedClientId>`; update the
safeClientId creation and any consumers of actorId (safeClientId, actorId) to
use the new encoding function to preserve uniqueness and avoid subject/grant
aliasing.
In `@ui/src/pages/Command.jsx`:
- Around line 538-546: The revoke button is passing a malformed key by doing
k.key.replace("...", ""); change it to pass the API key prefix returned by the
backend (use k.prefix) when invoking revokeApiKey (in Command.jsx where the
onClick calls revokeApiKey), and optionally fall back to k.key only if k.prefix
is undefined to avoid breaking older objects; update the invocation so
revokeApiKey receives the actual prefix string the DELETE endpoint expects.
In `@ui/src/stores/commandStore.js`:
- Around line 21-26: apiJson currently throws data.error which can be an object
and will stringify as [object Object]; update apiJson (which calls apiFetch and
res.json()) to extract a human-readable message when data.success is false by
checking if data.error is an object and using data.error.message (or combining
data.error.code and data.error.message) and fall back to a stringified JSON or a
default "API error" if neither is present; ensure the thrown Error includes that
extracted message so consumers get a useful error string.
---
Outside diff comments:
In `@src/intelligence/context-intelligence.js`:
- Around line 1036-1045: The dissolveSuspension method currently writes status =
'dissolved', which violates the CHECK constraint from migration
014_doctrine_lifecycle_states.sql; update dissolveSuspension to set status =
'retired' instead (consistent with supernova/fission handling) by changing the
UPDATE in the dissolveSuspension function to use 'retired' for issuer =
'suspension' and ensure any related logic/metrics rollbacks still refer to the
same retirement semantics.
In `@src/intelligence/context-resolver.js`:
- Around line 436-489: The flow mints a ChittyID (mintChittyId) and inserts
context_entities/context_dna before calling logToLedger, so if ledger logging
fails you can be left with a context lacking coordinationNeed and a retry may
mint a second chitty; fix by making the DB writes durable and idempotent: (1)
include coordinationNeed as a column/field on context_entities (or a JSON
metadata column) and perform the INSERTs inside a single DB transaction so
coordinationNeed is persisted atomically with contextId/chittyId (use the same
transaction for context_entities and context_dna inserts); (2) add a uniqueness
check/constraint keyed on the anchors (projectPath, workspace, supportType,
organization) and/or contextId so retries first query the DB for an existing
context and reuse its chittyId instead of calling mintChittyId again; and (3)
only call logToLedger after the transaction commits (so ledger is the last step)
and make logToLedger retriable against the stored context (using
contextId/chittyId) without reminting; update functions referenced:
mintChittyId, logToLedger and the context_entities/context_dna insert logic to
implement this transaction, uniqueness check, and durable coordinationNeed
persistence.
---
Nitpick comments:
In `@ui/src/pages/Command.jsx`:
- Around line 484-496: Add visual feedback when the Copy button succeeds or
fails: after calling navigator.clipboard.writeText(newKeyValue) in the onClick
handler of the Copy button, set a transient UI state (e.g., show "Copied!" text
or a toast) and clear it after a short timeout; handle rejections by showing an
error message. Use the existing clearNewKeyValue function for Dismiss behavior
and add a local state variable (e.g., copied or copyError) in the component that
the button's onClick updates so the UI near the Copy button displays the
confirmation or error.
- Around line 71-74: The useEffect call in Command.jsx currently calls init()
and returns cleanup with an empty dependency array, which can trigger ESLint
exhaustive-deps; either add init and cleanup to the dependency array of the
useEffect or explicitly silence the rule with a focused suppression comment
above the effect (e.g. // eslint-disable-next-line react-hooks/exhaustive-deps)
and include a short comment explaining why init and cleanup are intentionally
stable (Zustand store functions), referencing the useEffect, init, and cleanup
symbols so reviewers understand the rationale.
In `@ui/src/stores/commandStore.js`:
- Around line 610-617: init() can create duplicate autopilot intervals if called
multiple times; before calling get().startAutopilot() check for and clear any
existing timers by invoking the store's cleanup logic (e.g., call
get().cleanup() or explicitly clearInterval/clearTimeout on the same timer IDs
used by startAutopilot()) so startAutopilot() only creates a new interval when
no existing timer is running; update init() to clear existing timers first
(reference init(), startAutopilot(), and cleanup()).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7d060c43-9eeb-4218-996f-9350c0a111b5
📒 Files selected for processing (10)
docs/CHITTYADVOCATE_DESIGN.mdmigrations/014_doctrine_lifecycle_states.sqlsrc/index.jssrc/intelligence/__tests__/context-resolver.test.jssrc/intelligence/context-intelligence.jssrc/intelligence/context-resolver.jssrc/middleware/oauth-provider.jsui/src/App.jsxui/src/pages/Command.jsxui/src/stores/commandStore.js
…failure The OAuthProvider encodes authorization codes as userId:grantId:secret and validates by splitting on ":" expecting exactly 3 parts. The previous fix used a dash instead of a colon between the "mcp-client" prefix and the clientId, but the clientId itself could also contain colons (from dynamic client registration). Strip all colons from the clientId before use. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ng, API keys Adds a new Command page to the dashboard under Infrastructure with: - Connection health monitoring with needs-attention / healthy grouping - Autopilot mode with configurable sweep intervals and auto-heal - Self-healing queue with exponential backoff retry logic - Pattern learning (failure counts, co-failures, heal history) persisted to localStorage - Insights engine surfacing recurring failures, co-failure correlations, and unhealable services - API key CRUD (create, revoke, display) - Activity log with typed event badges - Zustand store (commandStore) with real API integration Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comprehensive doctrine alignment per chittycanon://doctrine/synthetic-continuity: Context Resolver v2.0: - 4-tier resolution cascade: explicit→anchor→multi-signal→create - DB failure returns error, never falls through to mint - Multi-signal scoring (project path, workspace, org, temporal, trust, experience) - createContext() requires coordinationNeed justification - 9 doctrine-compliance tests (260/260 total pass) Lifecycle alignment: - context-intelligence.js: supernova/fission set 'retired' not 'archived' - Migration 014: D1 CHECK constraint widened to fresh/active/dormant/stale/retired - State mapping: archived→retired, revoked→retired ChittyAdvocate (meta-bootstrap for cross-model alignment): - GET /api/v1/doctrine/seed — machine-readable governance bootstrap - GET /api/v1/signal/bootstrap — 3-prompt payload to hydrate any substrate - GET /api/v1/signal/articles — narrative archetype catalog - GET /api/v1/signal/article/:id — full article content (3 written) - Base class design: tunable per org/client for future use - Design doc: docs/CHITTYADVOCATE_DESIGN.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
e2beb38 to
0aa3804
Compare
… state, unused vars
- Multi-signal DB failure now returns error instead of silently falling
through to create_new (was a doctrine violation: DB failure must never
trigger minting)
- Add 'dissolved' to migration CHECK constraint (used by suspension
dissolve in context-intelligence.js, would cause runtime SQL error)
- Remove unused fetchApiKeys/addLog destructuring from Command.jsx
- Fix fetchApiKeys early return not resetting apiKeysLoading state
- Fix fragile k.key.replace("...","") in revoke — use prefix/split
- Reject blank coordinationNeed strings (whitespace-only)
- Add tests for blank coordinationNeed and multi-signal DB error
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
createContext()requirescoordinationNeedjustification. 9 doctrine-compliance tests added (260/260 pass).retirednotarchived. Migration 014 widens D1 CHECK constraint tofresh/active/dormant/stale/retired.GET /api/v1/doctrine/seed(machine-readable governance),GET /api/v1/signal/bootstrap(3-prompt payload to hydrate any AI substrate),GET /api/v1/signal/articles+/article/:id(narrative archetype articles). Base class design for future org/client tuning.Endpoints Added
GET /api/v1/doctrine/seedGET /api/v1/signal/bootstrapGET /api/v1/signal/articlesGET /api/v1/signal/article/:idTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Bug Fixes / Data
Tests