fix: resolve 8 Codex P2 findings on PR #101 - #103
Merged
Merged
Conversation
Contributor
|
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)
📝 WalkthroughWalkthroughThis PR adds session ownership guards to prevent stale leaders from interfering with active sessions and execution tokens to prevent stale leaders from incorrectly completing work. The database schema adds composite foreign keys and reclaim tracking. The leader loop integrates session-scoped heartbeats, intent reclamation, and token-gated completion. New tests validate leadership and intent lifecycle guards independently. ChangesSession Ownership and Execution Token Guards
Sequence DiagramsequenceDiagram
participant LeaderA as Leader Session A
participant Lease as cc_node_leases
participant Intent as cc_intents
participant LeaderB as Leader Session B
participant Exec as executor(intent)
LeaderA->>Lease: claim leadership (sessionId_A)
Lease-->>LeaderA: lease granted
Note over LeaderA: process restarts
LeaderB->>Lease: claim leadership (sessionId_B, same nodeId)
Lease-->>LeaderB: lease granted (newer session)
LeaderA->>Lease: heartbeat (sessionId_A)
Lease-->>LeaderA: null (session mismatch)
LeaderB->>Intent: claim intent, dispatch
Intent-->>LeaderB: token_1
LeaderB->>Exec: call with background heartbeat (sessionId_B)
Exec-->>LeaderB: completes with result
LeaderA->>Intent: complete with stale token
Intent-->>LeaderA: null (wrong sessionId in context)
LeaderB->>Intent: complete with token_1
Intent-->>LeaderB: done (token matches)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ 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 |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
chittycommand | 053b158 | Jun 04 2026, 02:08 AM |
|
|
To use Codex here, create a Codex account and connect to github. |
chitcommit
enabled auto-merge (squash)
June 3, 2026 19:34
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1258ac13d4
ℹ️ 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".
6 tasks
Codex P2 PR#101 findings 1 and 4. - F4: cc_intents now references cc_plans via composite FK (plan_id, goal_id) -> cc_plans(id, goal_id) so an intent's goal_id MUST match its plan's goal_id. Adds UNIQUE(id, goal_id) on cc_plans to back the composite reference. - F1: adds cc_intents.reclaim_count for stuck-intent bookkeeping surfaced by reclaimStuckIntents() in the next commit. Validated on disposable Neon branch br-delicate-mode-akkgde73 off project cool-bar-13270800: matching (plan_id, goal_id) insert succeeds; mismatched goal_id insert is rejected with "violates foreign key constraint cc_intents_plan_goal_cc_plans_fk". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codex P2 PR#101 findings 1 and 6.
- F1: adds reclaimStuckIntents(maxRunningSeconds) — atomically resets
intents whose status='claimed'|'running' updated_at is older than the
threshold back to 'pending', increments reclaim_count, and clears
dispatched_task_id + error_message. Idempotent; returns rowcount.
cc_intents has no claimed_by/claimed_at; staleness is measured via
updated_at, which dispatch and (next-commit) heartbeat paths bump.
- F6: completeIntent now requires status='running'; failIntent allows
('claimed','running') but never overwrites a terminal state. Prevents
parallel cancellation paths from being silently clobbered.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codex P2 PR#101 findings 2, 3, and 5. - F5: heartbeat() now matches WHERE session_id IS NOT DISTINCT FROM $sessionId. A restarted process with the same nodeId cannot extend a lease that already belongs to a newer leader. - F2: releaseLeadership() applies the same session-ownership guard. - F3: runLeaderLoop wraps each executor.execute(intent) in a setInterval heartbeat ticker at lease/2 cadence (min 1s). The interval is cleared in finally. Ticker uses the same session token so stale executors cannot keep a foreign lease alive. - Also: calls reclaimStuckIntents(leaseSeconds * 2) once per tick before claiming new work (wires up F1 from the previous commit). - Updates tests/daemon/leader.spec.ts to pass sessionId on heartbeat and release so it remains green under the new ownership guard. Schema column is session_id; the original task spec said "session_token". Used the real column. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…llback Codex P2 PR#101 findings 7 and 8. - F8: ContextEnv now prefers CHITTY_CONNECT_TOKEN — the binding already used by src/lib/cron.ts, src/routes/bridge/credentials.ts, and the rest of the worker. Legacy CHITTYCONNECT_TOKEN is kept as a back-compat fallback via resolveConnectToken(). - F7: getEcosystemAwareness now falls back to the AGENT_CONNECT service binding when the HTTPS path has no URL/token or the upstream fetch fails — matching the persist/recall fallback pattern already in place. Avoids hard failure when the worker only has the binding. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- tests/meta/intent-lifecycle.spec.ts — exercises F1 (reclaim round- trip, reclaim_count increment, fresh-running rows untouched) and F6 (completeIntent rejects non-running; failIntent cannot overwrite a terminal state). - tests/daemon/leader-session.spec.ts — exercises F2 + F5 by letting an old session's lease expire, having a new session reclaim, then proving the old session can neither heartbeat nor release. Both tests follow the existing pattern in tests/daemon/leader.spec.ts: real Neon, skip with `describe.skipIf(!DATABASE_URL)` so CI without a Neon branch URL doesn't fail. Realistic ChittyOS-shaped IDs only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chitcommit
force-pushed
the
fix/codex-p2-comments
branch
from
June 3, 2026 22:46
1258ac1 to
5ce549f
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
|
To use Codex here, create a Codex account and connect to github. |
Codex P1-A — the hand-written migrations/0003_codex_p2_fixes.sql was not listed in migrations/meta/_journal.json. drizzle-kit migrate iterates the journal entries, so the file would have been silently skipped at deploy time: reclaim_count column missing, composite FK uninstalled, reclaimStuckIntents() crashing at runtime on the production schema. Ran drizzle-kit generate against the updated schema in src/db/schema.ts to produce migrations/0003_foamy_king_cobra.sql plus the matching migrations/meta/0003_snapshot.json and the journal entry. The generated SQL preserves every change from the hand-written file: reclaim_count column, cc_plans UNIQUE(id, goal_id), drop of the old plan_id FK, and the new composite cc_intents_plan_goal_cc_plans_fk. Validated end-to-end on Neon branch br-soft-moon-aki26rfn off cool-bar-13270800: after applying 0002 + 0003 the schema reflects the new column and composite FK, mismatched (plan_id, goal_id) inserts are rejected with the expected constraint name.
…de fresh runs Codex P1-B — daemon/loop.ts:199 had a race where a leader L1 that loses its lease mid-executor() could still return and call completeIntent / failIntent for the intent that a fresher leader L2 has already reclaimed and is currently re-driving. The stale call would flip L2's running execution to 'done' (or 'failed') before L2 finishes. Use dispatched_task_id as a per-execution token. markIntentDispatched sets it and reclaimStuckIntents clears it, so it's monotonic within an execution attempt. completeIntent and failIntent gain an optional expectedDispatchedTaskId argument; when provided, the WHERE clause also requires dispatched_task_id = token. UPDATE 0 rows means a fresher leader has taken over — we log the stale event and return null. daemon/loop.ts captures the token returned from markIntentDispatched (itself null when status is no longer 'claimed') and passes it through both the success path and the catch branch. Failure before dispatch keeps the original unguarded behaviour. Tests/meta/intent-lifecycle.spec.ts gains a stale-token scenario that seeds an intent running under token-T2, confirms a completeIntent / failIntent call with token-T1 is rejected (status unchanged), and that the matching token-T2 call succeeds. Validated against Neon branch br-soft-moon-aki26rfn (cool-bar-13270800): the gated UPDATE returns 0 rows when the token mismatches and flips to 'done' when it matches. npm run typecheck passes.
…composite FK Codex P2-A — before the new composite FK existed, cc_intents.plan_id and cc_intents.goal_id had independent FKs, so a row could have plan_id pointing at goal_X while goal_id was goal_Y. Adding the composite FK cc_intents_plan_goal_cc_plans_fk validates every existing row; any such mismatched legacy row aborts the migration. Insert a preflight UPDATE inside 0003_foamy_king_cobra.sql, run after the cc_plans UNIQUE constraint is added and before the composite FK is added, that rewrites cc_intents.goal_id to match its plan's goal_id for any row where they disagree. The plan's goal_id is the authoritative source — the composite FK enforces exactly this invariant going forward. Validated on Neon branch br-soft-moon-aki26rfn (cool-bar-13270800): seeded a row with mismatched goal_id, ran 0003 in full, confirmed the preflight UPDATE healed the row (goal_id flipped from goal_B to plan->goal_A) and the composite FK installed cleanly. Post-migration, a fresh mismatched insert is rejected with the new constraint name.
|
|
To use Codex here, create a Codex account and connect to github. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 053b15871a
ℹ️ 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".
| await completeIntent(env, intent.id); | ||
| intentsProcessed += 1; | ||
| log('intent_completed', { intentId: intent.id, dispatchedTaskId: result.dispatchedTaskId }); | ||
| const dispatched = await markIntentDispatched(env, intent.id, result.dispatchedTaskId); |
There was a problem hiding this comment.
When a leader loses its lease while still inside executor(intent) before markIntentDispatched() runs, this patch's new reclaim path can reset that stale claimed row to pending, and the next leader can claim it back to claimed; the old executor then reaches this line and the status-only dispatch update can succeed against the new leader's claim, completing (or, in the catch path, failing) work that another executor is currently driving. Fresh evidence beyond the prior stale-completion issue is that reclaimStuckIntents() now reclaims claimed rows too, but there is still no per-claim token checked here before writing dispatched_task_id.
Useful? React with 👍 / 👎.
chitcommit
added a commit
that referenced
this pull request
Jun 4, 2026
…egister the Roux columns Hand-written 0004_roux_privilege_space.sql was on disk but never registered in migrations/meta/_journal.json, so drizzle-kit migrate would silently skip it. Regenerated via `drizzle-kit generate` after rebasing onto the merged PR #103 baseline; auto-named 0004_premium_toad_men.sql now carries the same 4 ALTER COLUMN + 4 CREATE INDEX statements (cc_intents/cc_disputes privilege+space + composite indexes) and is properly journaled with a matching meta/0004_snapshot.json. Validated on disposable Neon branch br-green-mode-akxyxiz5 of project cool-bar-13270800: 0002 + 0003 + 0004 sequence applied cleanly; all 4 columns and 4 indexes verified via information_schema and pg_indexes. Rebase also dropped 5 commits (f4cb131, 7027daa, 117d77d, 001e157, 5ce549f) that were the pre-squash form of merged PR #103. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chitcommit
added a commit
that referenced
this pull request
Jun 4, 2026
* feat(db): add Roux privilege+space columns to cc_intents and cc_disputes
Migration 0004 adds privilege ∈ {privileged,pii,hoa_evidentiary,public} and
space ∈ {business,legalink} as first-class text columns on cc_intents and
cc_disputes, with composite indexes on (axis, status). CHECK constraints are
deferred — the Roux spec URI is STATUS:PENDING and enum is enforced in the
app layer.
@canon: chittycanon://gov/governance#classification-axes STATUS:PENDING
Validated on Neon branch br-broad-mud-ak4k790p of project cool-bar-13270800:
ADD COLUMN + CREATE INDEX succeed on top of the post-0003 schema; defaults
applied to existing rows.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(meta): thread Roux privilege+space through Intent + createIntent + claimNextIntent
- Intent interface gains privilege/space (typed unions, sourced from canon).
- CreateIntentInput accepts optional privilege/space; INSERT defaults to
public/business when caller omits.
- claimNextIntent grows null-passthrough filters for privilege, space, and
priority_lte so a single prepared statement covers the autonomous-bucket
and human-channel paths.
- rowToIntent maps the new columns.
- sovereignty.ts: add optional privilege field to IntentForSovereignty for
audit-trail persistence. decide() is intentionally NOT modified — privilege
is orthogonal to the trust-tier sensitivity axis the gate consumes.
@canon: chittycanon://gov/governance#classification-axes STATUS:PENDING
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(routes): ChittyTriage queue routes (list/claim/claim-next/complete)
GET /api/triage — list pending intents, filter by privilege/space
POST /api/triage/:id/claim — atomic claim-by-ID, 409 if not pending
POST /api/triage/claim-next — bucket-ordered claim for autonomous agents
POST /api/triage/:id/complete — terminal transition (done|failed) with the
same status guards used in meta/intent.ts
Mounted after /api/tasks; covered by the existing global /api/* authMiddleware.
@canon: chittycanon://gov/governance#classification-axes STATUS:PENDING
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(mcp): add 4 ChittyTriage tools to the MCP surface
- triage_list_intents({privilege?, space?, limit?})
- triage_claim_intent({id})
- triage_claim_next({privilege?, space?, priority_lte?})
- triage_complete_intent({id, outcome, error?})
Each tool wraps the matching /api/triage route's semantics so Claude Code
sessions can drive the queue without HTTP plumbing. Validation reuses the
ratified privilege/space enums from meta/intent.ts.
@canon: chittycanon://gov/governance#classification-axes STATUS:PENDING
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(dispute-sync): Roux derive + Notion suppression gate + backfill warn
- deriveRouxFromType(disputeType) maps dispute_type → ratified Roux defaults:
'legal' ⇒ {privilege:'privileged', space:'legalink'}
'insurance' ⇒ {privilege:'pii', space:'business'}
others ⇒ {privilege:'public', space:'business'}
- linkDisputeToNotion now gates: if effective privilege ∈ {privileged, pii}
OR space = 'legalink', the function logs the suppression and returns false
before touching Notion. Effective values resolve as explicit > derived.
- pushUnlinkedDisputesToNotion now SELECTs privilege/space and emits a
per-row warn when the row sits on the migration defaults — surfaces
legacy untagged disputes for explicit triage (Q2=(a) pass-through-warn).
- linkDisputeToNotion is now exported for direct integration testing.
@canon: chittycanon://gov/governance#classification-axes STATUS:PENDING
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(adr-001): append Roux/Triage carry-through delta
Records the four ratified decisions from chittycanon-code-cardinal:
Q1 — privilege+space land as first-class columns on cc_intents AND
cc_disputes; CHECK deferred until Roux spec is CERTIFIED.
Q2 — pass-through-with-warn on legacy rows (column defaults).
Q3 — both specific-by-ID and bucket-ordered claim modes.
Q4 — privilege is orthogonal to sensitivity; decide() is untouched.
Also documents the Notion mirror gate and explicit > derived resolution.
@canon: chittycanon://gov/governance#classification-axes STATUS:PENDING
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test: real-Neon coverage for ChittyTriage + Roux dispute-sync gate
tests/routes/triage-roux.spec.ts
- createIntent privilege+space round-trip
- claimNextIntent honors bucket filter (won't grab a higher-priority intent
in the wrong bucket)
- second atomic claim of the same intent returns 0 rows (the 409 source)
- bucket-scoped claim still respects priority within the bucket
tests/lib/dispute-sync-roux.spec.ts
- deriveRouxFromType: legal/insurance/property/vendor + unknown safe default
- linkDisputeToNotion suppresses on explicit privilege=privileged
- linkDisputeToNotion suppresses on explicit space=legalink
- linkDisputeToNotion suppresses 'legal' via derived defaults
- explicit (public, business) override beats derived (privileged, legalink)
for a 'legal' dispute_type — gate passes (Notion call then no-ops in test)
Skip pattern mirrors tests/meta/intent-lifecycle.spec.ts — disabled without
DATABASE_URL or with SKIP_INTEGRATION=1.
@canon: chittycanon://gov/governance#classification-axes STATUS:PENDING
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(tests): bump MCP tool count to 54 + guard dispute-sync-roux neon init
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(migration): regenerate 0004 via drizzle-kit so journal/snapshot register the Roux columns
Hand-written 0004_roux_privilege_space.sql was on disk but never registered
in migrations/meta/_journal.json, so drizzle-kit migrate would silently skip
it. Regenerated via `drizzle-kit generate` after rebasing onto the merged
PR #103 baseline; auto-named 0004_premium_toad_men.sql now carries the
same 4 ALTER COLUMN + 4 CREATE INDEX statements (cc_intents/cc_disputes
privilege+space + composite indexes) and is properly journaled with a
matching meta/0004_snapshot.json.
Validated on disposable Neon branch br-green-mode-akxyxiz5 of project
cool-bar-13270800: 0002 + 0003 + 0004 sequence applied cleanly; all 4
columns and 4 indexes verified via information_schema and pg_indexes.
Rebase also dropped 5 commits (f4cb131, 7027daa, 117d77d, 001e157,
5ce549f) that were the pre-squash form of merged PR #103.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(dispute-sync): normalize dispute_type + re-derive on default backfill
Finding 6 (P1): deriveRouxFromType only matched exact strings, so free-text
dispute_type values like "Legal" or "legal dispute" fell through to the
default public/business mapping and leaked into the Notion business bucket.
Lowercase + trim, then route any string containing "legal" to
privileged/legalink and any containing "insurance" to pii/business. This is
intentionally over-broad on the privileged side as a fail-safe.
Finding 1 (P1): pushUnlinkedDisputesToNotion treated rows that landed on the
column defaults (public/business) as explicit values, so a row with
dispute_type='legal' but no explicit privilege/space was passed through the
gate. Now, when both axes sit on defaults, re-derive from dispute_type for
the gate decision (DB row is untouched — retag is a separate concern).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(triage): reject invalid claim-next filters with 400
Finding 2 (P2): claim-next called parsePrivilege/parseSpace and treated null
as "no filter", so an unrecognized value (e.g. "pi" instead of "pii") would
silently fall through and claim the highest-priority pending intent from any
bucket — including privileged/legalink. Mirror the GET /api/triage pattern:
when a filter is provided but doesn't parse, return 400 with the valid enum.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(triage): exclude future scheduled_for from direct claim
Finding 5 (P2): list + claim-next correctly exclude future-scheduled intents,
but the direct claim path (POST /api/triage/:id/claim and the
triage_claim_intent MCP tool) only gated on status='pending'. A client with
the ID could short-circuit the schedule and claim tomorrow's intent today.
Add scheduled_for guard to both paths and distinguish 409 with the
scheduled_for value in the response for client UX.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(intent): allow completeIntent/failIntent from claimed or running
Finding 4 (P2): the triage routes expose claim (pending→claimed) but no
explicit claimed→running transition, so an autonomous agent that did its
work and called complete always got 409. Extend completeIntent's WHERE to
accept status IN ('claimed', 'running'), matching failIntent's existing
guard. The P1-B execution-token gate still rejects stale completions when a
token is supplied — so concurrent leaders are still protected.
Update error messages in the HTTP and MCP wrappers to reflect the relaxed
guard. Add three lifecycle tests: claimed→done, claimed→failed, and the
token-gate rejection on a claimed (not running) intent.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(auth): require chittytriage:write scope on triage routes
Mounting /api/triage under the generic authMiddleware let any valid
ChittyAuth user token list, claim, and complete privileged/legalink
intents. Add requireTriageScope middleware that runs after authMiddleware
and rejects callers without chittytriage:write, chittytriage:admin,
admin, or wildcard scope.
Also exports hasTriageScope() for reuse by the MCP dispatcher (next
commit) since that path runs inside JSON-RPC, not Hono middleware.
fixes codex-p2 PR#104 P1 — elevated scope on triage routes
* fix(mcp): scope-gate triage tools, filter from tools/list when unscoped
The 4 triage_* MCP tools were reachable by any caller accepted by
mcpAuthMiddleware with the broad 'mcp' scope or the legacy KV shared
service token. That lets generic MCP integrations enumerate and mutate
privileged/legalink orchestration intents.
Two layers:
- tools/list now filters triage_* tools out of the catalog unless the
caller has chittytriage:write (don't advertise what they can't call).
- tools/call enforces the same scope as a defense-in-depth gate so a
caller that knows the tool name can't bypass the list filter.
Uses hasTriageScope() helper exported from src/middleware/auth.ts.
fixes codex-p2 PR#104 P1 — elevated scope on triage MCP tools
* test(intent): pin reclaim → re-claim → stale-token-A race rejection
Codex flagged a possible race where a stale client A could complete a
re-claimed intent owned by client B with token A. After audit, the
existing token gate from PR#103 P1-B already prevents this:
meta/intent.ts:332-337,367-373 — completeIntent/failIntent UPDATE
predicate is `status IN ('claimed','running') AND dispatched_task_id
= $expectedDispatchedTaskId`, covering BOTH claimed and running.
meta/intent.ts:402 — reclaimStuckIntents sets dispatched_task_id =
NULL when resetting to pending, so any subsequent re-claim writes a
fresh token and the stale A token can never match.
Add a regression test that drives the exact race the reviewer
described (claim with tokenA → updated_at past reclaim window →
reclaimStuckIntents → re-claim with tokenB → tokenA tries complete +
fail) and asserts the row stays B's. No production-code change.
fixes codex-p2 PR#104 P2 — pin reclaim claim-token race
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves the 8 P2 findings Codex raised on #101.
Findings → resolution
meta/intent.ts:255runningreclaimStuckIntents(maxRunningSeconds); called once per leader tick fromdaemon/loop.tsbefore claim. Addscc_intents.reclaim_countcolumn.daemon/leader.ts:166releaseLeadershipmatches onlynode_id— a stale process can release a newer leader's leasesession_id IS NOT DISTINCT FROM $sessionId.daemon/loop.ts:203executor.execute(intent)— slow executors can let the lease lapsesetIntervalticker atleaseSeconds/2(min 1s) wraps the execute span; cleared infinally; uses current session.src/db/schema.ts:456cc_intents.plan_idandcc_intents.goal_idhad independent FKs, allowingintent.goal_id ≠ plan.goal_idUNIQUE(id, goal_id)oncc_plans; replacesplan_idFK with composite(plan_id, goal_id) → cc_plans(id, goal_id) ON DELETE CASCADE.daemon/leader.ts:140heartbeatmatches onlynode_id— a restarted process clobbers a newer leadersession_id IS NOT DISTINCT FROM $sessionId.meta/intent.ts:287completeIntentoverwrites any state, includingfailed/blocked_human/doneWHERE status = 'running';failIntentmirrors withWHERE status IN ('claimed','running'). Returns null when guarded.meta/context.ts:131getEcosystemAwarenesshad noAGENT_CONNECTfallbackfallback()helper (service binding) when the HTTPS path is unavailable.meta/context.ts:63CHITTY_CONNECT_TOKENbindingContextEnvnow readsCHITTY_CONNECT_TOKEN(canonical, used insrc/lib/cron.ts,src/routes/bridge/*) with legacyCHITTYCONNECT_TOKENas fallback.Schema validation (F4)
Validated on disposable Neon branch
br-delicate-mode-akkgde73of projectcool-bar-13270800BEFORE committing:UNIQUE(id, goal_id), add composite FK, addreclaim_countcolumn.(plan_id, goal_id)insert succeeded.goal_idinsert failed with:insert or update on table "cc_intents" violates foreign key constraint "cc_intents_plan_goal_cc_plans_fk"✓Tests
tests/meta/intent-lifecycle.spec.ts— F1 reclaim round-trip (incl.reclaim_countbump, fresh-running rows untouched) + F6 state guards.tests/daemon/leader-session.spec.ts— F2 + F5: old session cannot heartbeat or release after a new session of the same node reclaims.tests/daemon/leader.spec.tsupdated to passsessionIdonheartbeat/releaseLeadershipcalls.All tests follow the existing real-Neon pattern: skip cleanly when
DATABASE_URLis absent.Deviations from spec (call-outs for the next architect)
session_token; the real schema column issession_id. Usedsession_id.claimed_by/claimed_atin the reclaim path;cc_intentshas neither. Staleness measured viaupdated_at(bumped by dispatch + heartbeat) anddispatched_task_idis cleared instead.src/middleware/auth.tshad noCHITTYCONNECT_TOKENreference; the canonical binding in this worker isCHITTY_CONNECT_TOKEN(seesrc/lib/cron.ts:378). Used that.Verification
npm run typecheck— clean.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Database