Skip to content

fix: enforce bundled agent and Loop ownership - #326

Merged
pedronauck merged 5 commits into
mainfrom
fix-321-322
Aug 6, 2026
Merged

pedronauck merged 5 commits into
mainfrom
fix-321-322

Conversation

@pedronauck

@pedronauck pedronauck commented Aug 6, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Fresh Compozy homes now start with the bundled dev-cycle extension enabled, while later boots and updates preserve an operator's explicit enabled or disabled choice.
  • Loop action runs keep one daemon-owned worker, and sessions that require Compozy tools fail before provider startup when hosted MCP cannot satisfy that contract.
  • Loop cancellation and resumed-session ledgers now converge without requiring a daemon restart or producing conflicting forensic projections.

Closes #321.
Closes #322.

Root causes

  • InstallLocalManaged appended its disabled default after caller options, so the bundled dev-cycle installer could not opt into enabled=true.
  • Coordinator observation and task-role activation did not reject loop-action worker runs even after the dedicated executor had claimed them.
  • Session startup treated missing or disabled hosted MCP as a recoverable condition even when the effective agent or lineage policy required concrete tools.
  • Loop cancel delivery only attempted the active task/session path synchronously; no boot or scheduler sweep retried durable requested/delivering state.
  • Resume kept the stopped-session ledger.jsonl projection while new events accumulated, so the next stop could reject different content as an existing ledger conflict.
  • The task catalog's infinite query inherited an interval refetch that was not part of its manual-refresh contract.

Changes

  • Bundled extension lifecycle: apply the generic disabled default before caller options, opt only dev-cycle into fresh-home enablement, and retain stored state on reinstall/update.
  • Loop worker ownership: fence Loop workers out of coordinator bootstrap and ordinary task-role activation while keeping the dedicated loop-action executor authoritative.
  • Hosted native tools: return ErrHostedMCPUnavailable before provider startup when effective tools/toolsets require hosted MCP but the runtime mode, provider, launcher, or launch cannot provide it.
  • Durable cancellation: persist cancellation delivery state, deliver idempotent prompt cancellation, advance to draining after acknowledgment, and retry pending delivery from daemon boot and scheduler sweeps.
  • Session ledger lifecycle: discard the stopped projection before resume, restore it after failed provider startup or failed clear rollback, and rematerialize the complete history on the next stop.
  • Web: remove hidden interval polling from the task catalog while preserving manual reload and mutation-driven refresh behavior.
  • Docs and QA: document the new extension, tool, ownership, cancellation, and ledger contracts; add the Loop-worker ownership charter/scenario and update the affected living QA scenarios.

Release Notes

🐛 Bug Fixes

  • Enable the bundled dev-cycle extension on fresh Compozy homes without overriding later operator choices.
  • Prevent coordinators and task-role sessions from competing with the dedicated Loop action worker, and fail required-tool sessions before startup when hosted MCP is unavailable.
  • Retry Loop cancellation delivery durably and replace stopped-session ledger projections safely across resume.

QA

Final Status: PASS for the #321/#322 acceptance contract; all nine changed or adjacent product scenarios reached a terminal pass.

Coverage: Five personas exercised fresh extension lifecycle, real hosted Compozy tools, exclusive Loop-worker ownership, cancellation through draining to canceled, stopped-session continuation, and task-catalog refresh behavior.

Issues: 0 (Blocks-Completion: 0, Data-Loss: 0, Trust-Damage: 0, Friction: 0, Cosmetic: 0).

Blocked / needs human: None for the changed behavior. The separate broad business playbook remained blocked by missing local psql and incomplete channel handoffs; those rows were not used as evidence for #321 or #322.

Full report: docs/qa/reports/2026-08-06-loop-agent-ownership.md

Test plan

  • make gate-full — full monorepo verification, 20,778 Go tests with -race, Bun lint/typecheck/test, web build, Go lint/build, codegen, installer check, and package boundaries.
  • go test -race ./internal/session -count=1 — 769 session tests.
  • bunx turbo run typecheck test build --filter=./packages/site — 305 site tests and 2,012 generated static pages.
  • Strict real-scenario evidence audit — PASS, 0 blockers, 0 warnings.
  • Isolated QA teardown — clean: true, 0 surviving processes.

Web / Docs Impact

  • The task catalog no longer polls on an interval; manual reload and mutation invalidations remain the refresh contract. No route, payload, or generated client changed.
  • Public docs now state the fresh dev-cycle exception, hosted-MCP fail-closed behavior, exclusive Loop-worker owner, durable cancel retry, and stopped-session ledger replacement.
  • No OpenAPI, CLI verb, config key, environment variable, site navigation, CI workflow, example, or release-tooling contract changed.

Compozy Impact Audit

  • Native tools: No compozy__* ID, descriptor, schema, digest, risk flag, or toolset changed. Session availability now fails closed when the effective agent/lineage policy requires tools and hosted MCP cannot project them. Real QA invoked compozy__tool_info, compozy__skill_view, and task tools through the hosted seam.
  • Extensibility and hooks: Fresh bundled dev-cycle installs are enabled; generic local and marketplace installs remain disabled by default, and stored extension state survives restart/update. Extension agents, Loops, and tools were verified across disable → restart → enable. Hooks, bridge SDKs, registries, MCP sidecar contracts, and config keys/defaults are unchanged.
  • Workspace data isolation: Pending cancellation discovery is global for scheduler efficiency, but every command carries the stored workspace_id and the Loop service revalidates run ownership before transition. Ledger discard/rematerialization resolves the immutable session/workspace owner. No new cross-workspace read or write surface was added.
  • Official Compozy skill: No impact after checking skills/compozy/: tool IDs, CLI paths, hook events, capability names, and agent-facing operating steps are unchanged; the public runtime docs own these lifecycle guarantees.

Summary by CodeRabbit

  • New Features

    • Added automatic retry and recovery for pending cooperative cancellations.
    • Added clearer handling when required hosted tools are unavailable.
    • Improved session ledger recovery during resume and startup failures.
    • Bundled dev-cycle extensions are now enabled after installation.
    • Improved preservation of provider-reported bridge status during updates.
  • Bug Fixes

    • Prevented loop worker runs from being activated or bootstrapped as regular tasks.
    • Corrected event payload mapping validation.
    • Removed task-list polling to provide up-to-date results on demand.
    • Improved tool approval request tracking and prompt failure handling.

Enable the bundled dev-cycle extension on fresh homes while preserving explicit state. Keep Loop workers under one executor, fail closed when required hosted tools are unavailable, reconcile cancellations durably, and replace stopped-session ledger projections across resume.
@vercel

vercel Bot commented Aug 6, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
compozy-site Ready Ready Preview Aug 6, 2026 1:41pm

Request Review

@coderabbitai

coderabbitai Bot commented Aug 6, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

The PR adds deferred Loop cancellation reconciliation across storage, services, scheduling, and daemon boot. It also updates session ledger recovery, hosted MCP validation, Loop-worker guards, bundled extension installation, approval metadata, bridge state handling, event payload handling, prompt fixtures, database opening, and task-list polling.

Changes

Loop cancellation reconciliation

Layer / File(s) Summary
Cancellation contracts and reconciliation
internal/loop/cancel_control.go, internal/loop/service_cancellation*.go, internal/loop/service_test.go
Cooperative cancellation now defers delivery. The Loop service retries pending deliveries, advances state, activates results, and aggregates errors.
Pending cancellation persistence
internal/store/globaldb/global_db_loop_cancel_pending.go, internal/store/globaldb/global_db_loop_test.go
The database lists bounded, workspace-scoped pending cancellations and hydrates associated sessions.
Scheduler and daemon backstop
internal/scheduler/*, internal/daemon/boot_components.go, internal/daemon/loop_cancellation_reconcile.go, internal/daemon/scheduler_*
The scheduler records reconciliation counts and errors. Daemon boot and runtime wiring perform bounded recovery sweeps.

Session recovery and hosted MCP validation

Layer / File(s) Summary
Resume and cleanup ledger recovery
internal/session/ledger.go, internal/session/manager_resume.go, internal/session/manager_clear_family_cleanup.go, internal/session/*ledger_test.go, internal/session/manager_clear_test.go
Resume and clear recovery discard materialized ledgers and rematerialize stopped-session ledgers after failed operations or restored metadata.
Hosted MCP requirement validation
internal/session/manager.go, internal/session/manager_start_mcp.go, internal/session/manager_test.go
Session startup returns ErrHostedMCPUnavailable when restricted tools require unavailable hosted MCP support.
Session lifecycle test support
internal/session/*_test.go, internal/session/manager_prompt_*.go, internal/session/manager_prompt_contract_test.go
Tests use hosted MCP harnesses. Prompt failure persistence and output-channel closure order are updated.

Loop-worker and extension runtime guards

Layer / File(s) Summary
Loop-worker execution guards
internal/coordinator/*, internal/daemon/coordinator_runtime_test.go, internal/daemon/task_role_*
Coordinator bootstrap and task-role activation skip runs associated with a Loop run.
Enabled bundled extension installation
extensions/dev-cycle/*, internal/extension/*, internal/daemon/daemon_test.go
Bundled dev-cycle installations persist as enabled. Caller-provided options can override the managed default.

Runtime and persistence consistency

Layer / File(s) Summary
Runtime identifiers and prompt handling
internal/acp/*, internal/daemon/tool_approval_*, internal/loop/start_binding.go, internal/loop/watch_events_ledger.go, internal/testutil/acpmock/*
Approval requests share one exported request identifier. Loop mappings use the configured payload key. Valid Loop output contracts are removed from test prompt text.
Bridge operational state preservation
internal/bridges/resource_projection.go, internal/store/globaldb/global_db_bridge.go, internal/store/globaldb/queries/bridge_core.sql, internal/store/globaldb/sqlcgen/bridge_core.sql.go, internal/store/globaldb/global_db_bridges_test.go
Bridge replacement preserves newer provider-owned status, degradation details, and timestamps for unchanged enabled runtimes.
Task query and database opening behavior
web/src/systems/tasks/lib/query-options.ts, internal/store/globaldb/global_db_task_test.go, internal/store/sessiondb/read_only_pool.go, internal/store/sessiondb/session_db_integration_test.go
The task catalog query no longer uses default polling. Shared read-only database opening is not canceled by the initiating caller context.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant LoopService
  participant GlobalDB
  participant Scheduler
  participant Daemon
  participant Session
  LoopService->>GlobalDB: persist deferred cancellation
  Scheduler->>Daemon: run cancellation backstop
  Daemon->>GlobalDB: list pending cancellations
  Daemon->>LoopService: reconcile pending cancellations
  LoopService->>Session: retry cancellation delivery
  LoopService-->>Scheduler: reconciled count and errors
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning Several changes appear unrelated to issues #321 and #322, including bridge state preservation, ACP metadata, prompt handling, task polling, and read-only pool behavior. Split unrelated bridge, ACP, prompt, task-catalog, fixture, and session-database changes into separate pull requests or link issues that justify them.
Docstring Coverage ⚠️ Warning Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies bundled extension lifecycle and Loop ownership changes, which are major parts of the pull request.
Linked Issues check ✅ Passed The changes address durable Loop cancellation reconciliation, ledger recovery, Loop-worker ownership, and fail-closed hosted MCP requirements from issues #321 and #322.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-321-322

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 6, 2026 •

Copy link
Copy Markdown
Contributor

React Doctor found no new issues. 🎉

Reviewed by React Doctor for commit 36b0e8a.

@greptile-apps

greptile-apps Bot commented Aug 6, 2026 •

Copy link
Copy Markdown

Greptile Summary

The PR strengthens ownership and recovery contracts for bundled extensions, Loop workers, hosted tools, cancellation delivery, and resumed-session ledgers.

  • Enables the bundled dev-cycle extension for fresh homes while preserving later operator choices.
  • Prevents ordinary coordinator and task-role paths from claiming Loop-owned workers.
  • Fails required-tool sessions early when hosted MCP is unavailable.
  • Persists and retries cooperative cancellation delivery.
  • Replaces stopped-session ledger projections safely across resume.
  • Removes periodic task-catalog polling while retaining explicit refresh paths.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains within the eligible follow-up review scope.

No blocking failure remains.

Important Files Changed

Filename Overview
internal/loop/service_cancellation_reconcile.go Adds durable, retryable reconciliation for requested and delivering Loop cancellations.
internal/store/globaldb/global_db_loop_cancel_pending.go Adds persisted discovery and hydration of pending cancellation deliveries.
internal/session/manager_resume.go Discards stopped ledger projections before resume and restores them if provider startup fails.
internal/session/ledger.go Adds ledger discard and rematerialization operations supporting resumed-session continuity.
internal/session/manager_start_mcp.go Enforces hosted-MCP availability before starting sessions whose effective policy requires tools.
internal/daemon/task_role_runtime.go Fences Loop-owned action workers from ordinary task-role activation.
internal/extension/install_managed.go Applies generic disabled defaults before caller options so bundled installation can opt into fresh-home enablement.
web/src/systems/tasks/lib/query-options.ts Removes interval refetching from the task catalog while preserving explicit refresh behavior.

Sequence Diagram

sequenceDiagram
    participant Operator
    participant LoopService as Loop service
    participant Store as Global store
    participant Reconciler as Boot/Scheduler reconciler
    participant Session as Loop-owned session
    Operator->>LoopService: Request cooperative cancellation
    LoopService->>Store: Persist requested state
    LoopService->>Session: Attempt cancellation delivery
    alt Delivery acknowledged
        LoopService->>Store: Advance to draining
    else Delivery unavailable or fails
        Reconciler->>Store: Read requested/delivering cancellations
        Reconciler->>Session: Retry cancellation delivery
        Reconciler->>Store: Advance to draining
    end
Loading

Reviews (5): Last reviewed commit: "fix: stabilize terminal session reads" | Re-trigger Greptile

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (2)
internal/scheduler/scheduler_test.go (1)

1025-1054: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for a failing cancellation backstop.

fakeLoopCancellationTaskSource.ReconcilePendingCancellations always returns a nil error. RunOnce wraps a backstop error as "scheduler: Loop cancellation backstop: %w" and continues the rest of the cycle instead of aborting it. That resilience behavior is untested.

Add a subtest where the fake returns a partial count and an error. Assert that RunOnce returns the wrapped error with ErrorContains, that result.ReconciledCancellations keeps the partial count, and that the cycle still loads snapshots.

As per path instructions: "Focus on critical paths: workflow execution, state management, error handling".

🤖 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 `@internal/scheduler/scheduler_test.go` around lines 1025 - 1054, Add a subtest
alongside TestRunOnceRunsLoopCancellationBackstop that configures
fakeLoopCancellationTaskSource.ReconcilePendingCancellations to return a partial
reconciled count and an error. Assert RunOnce returns an error containing
“scheduler: Loop cancellation backstop:”, preserves the partial count in
result.ReconciledCancellations, and still verifies the cycle loads snapshots
after the backstop failure.

Source: Path instructions

internal/daemon/loop_cancellation_reconcile.go (1)

29-57: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Add a comment for the fail-open reconciliation path.

state.logger is assigned in bootConfig before bootComponents runs, so the dereference after the missing-capability and actor-derivation checks is safe. The reconciliation failure path still defers to the scheduler retry; add a short comment to avoid future callers changing it to fail boot.

🤖 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 `@internal/daemon/loop_cancellation_reconcile.go` around lines 29 - 57, Add a
brief comment in reconcileBootLoopCancellations before or around the
ReconcilePendingCancellations error handling documenting that reconciliation is
intentionally fail-open and failures defer to the scheduler retry rather than
failing boot. Keep the existing state.logger usage and return behavior
unchanged.
🤖 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 `@internal/daemon/scheduler_runtime.go`:
- Around line 63-68: Ensure the failure branch in the Loops
cancellation-reconciler type assertion shuts down the already-created and
configured scheduler session waker before returning the unsupported-service
error. Reuse the same cleanup behavior as later scheduler construction failures,
while leaving the successful reconciliation path unchanged.

In `@internal/daemon/task_role_runtime.go`:
- Around line 269-271: Update starvationActivation to immediately return the
inactive result when run.IsLoopWorker() is true, matching the existing guard in
the related activation path. Ensure loop-worker runs cannot proceed to
startStarvationSession or create a normal task-role session, while preserving
existing behavior for non-loop workers.

In `@internal/session/ledger_test.go`:
- Around line 208-213: Strengthen the rematerialized-ledger assertions in the
failed-resume test by reading the ledger content after the existence check and
verifying it contains the original prompt and an EventTypeSessionStopped event.
Keep the existing stopped metadata assertion, but ensure the test validates
restored history rather than file presence alone.

In `@internal/store/globaldb/global_db_loop_cancel_pending.go`:
- Around line 66-78: Update the run-level query in the pending-cancellation flow
to derive cancel_reason, cancel_actor_kind, and cancel_actor_id from one
correlated control row rather than applying independent MIN() aggregates. Select
the control with the earliest non-null cancel_requested_at while preserving the
existing run grouping and cancellation-state behavior, so
PendingCancellation.RequestedBy and its reason remain from the same cancellation
event.

In `@internal/store/globaldb/global_db_loop_test.go`:
- Around line 246-251: Strengthen the cross-workspace fixture in the relevant
global DB loop test by requesting cancellation for foreignRun after creating it,
so it satisfies the pending-cancellation filter. Update the exact
workspace-owned run delivery assertions to verify the command for run.ID
contains only session-cancel-owned and the command for foreignRun.ID contains
only session-cancel-other, ensuring workspace isolation is actually tested.

---

Nitpick comments:
In `@internal/daemon/loop_cancellation_reconcile.go`:
- Around line 29-57: Add a brief comment in reconcileBootLoopCancellations
before or around the ReconcilePendingCancellations error handling documenting
that reconciliation is intentionally fail-open and failures defer to the
scheduler retry rather than failing boot. Keep the existing state.logger usage
and return behavior unchanged.

In `@internal/scheduler/scheduler_test.go`:
- Around line 1025-1054: Add a subtest alongside
TestRunOnceRunsLoopCancellationBackstop that configures
fakeLoopCancellationTaskSource.ReconcilePendingCancellations to return a partial
reconciled count and an error. Assert RunOnce returns an error containing
“scheduler: Loop cancellation backstop:”, preserves the partial count in
result.ReconciledCancellations, and still verifies the cycle loads snapshots
after the backstop failure.
🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f86f773f-ab68-4697-8173-f80b52aee3e8

📥 Commits

Reviewing files that changed from the base of the PR and between a00a9df and 91b5985.

⛔ Files ignored due to path filters (22)
  • docs/_memory/glossary.md is excluded by !**/*.md
  • docs/qa/charters/CH-loop-worker-exclusive-dispatch.md is excluded by !**/*.md
  • docs/qa/reports/2026-08-06-loop-agent-ownership.md is excluded by !**/*.md
  • docs/qa/scenarios/ET-052.md is excluded by !**/*.md
  • docs/qa/scenarios/ET-compozy-native-tool-invocation.md is excluded by !**/*.md
  • docs/qa/scenarios/ET-managed-session-skill-loading.md is excluded by !**/*.md
  • docs/qa/scenarios/LP-016.md is excluded by !**/*.md
  • docs/qa/scenarios/LP-cancel-vs-kill.md is excluded by !**/*.md
  • docs/qa/scenarios/RT-018.md is excluded by !**/*.md
  • docs/qa/scenarios/TA-002.md is excluded by !**/*.md
  • docs/qa/scenarios/TA-loop-worker-exclusive-dispatch.md is excluded by !**/*.md
  • docs/qa/scenarios/TA-task-role-session-activation.md is excluded by !**/*.md
  • packages/site/content/docs/agents/definitions.mdx is excluded by !**/*.mdx
  • packages/site/content/docs/autonomy/task-runs-and-leases.mdx is excluded by !**/*.mdx
  • packages/site/content/docs/configuration/agent-md.mdx is excluded by !**/*.mdx
  • packages/site/content/docs/configuration/file-locations.mdx is excluded by !**/*.mdx
  • packages/site/content/docs/loops/catalog.mdx is excluded by !**/*.mdx
  • packages/site/content/docs/loops/index.mdx is excluded by !**/*.mdx
  • packages/site/content/docs/loops/running.mdx is excluded by !**/*.mdx
  • packages/site/content/docs/memory/dream.mdx is excluded by !**/*.mdx
  • packages/site/content/docs/sessions/resume.mdx is excluded by !**/*.mdx
  • packages/site/content/docs/tools/index.mdx is excluded by !**/*.mdx
📒 Files selected for processing (41)
  • extensions/dev-cycle/embed_test.go
  • extensions/dev-cycle/install.go
  • internal/coordinator/coordinator.go
  • internal/coordinator/coordinator_test.go
  • internal/daemon/boot_components.go
  • internal/daemon/coordinator_runtime_test.go
  • internal/daemon/daemon_test.go
  • internal/daemon/loop_cancellation_reconcile.go
  • internal/daemon/scheduler_loop_cancellation.go
  • internal/daemon/scheduler_runtime.go
  • internal/daemon/scheduler_task_source_types.go
  • internal/daemon/task_role_runtime.go
  • internal/daemon/task_role_runtime_test.go
  • internal/extension/install_managed.go
  • internal/extension/install_managed_test.go
  • internal/loop/cancel_control.go
  • internal/loop/service_cancellation.go
  • internal/loop/service_cancellation_reconcile.go
  • internal/loop/service_test.go
  • internal/loop/start_binding.go
  • internal/loop/watch_events_ledger.go
  • internal/scheduler/scheduler.go
  • internal/scheduler/scheduler_test.go
  • internal/scheduler/types.go
  • internal/session/ledger.go
  • internal/session/ledger_test.go
  • internal/session/manager.go
  • internal/session/manager_clear_family_cleanup.go
  • internal/session/manager_clear_test.go
  • internal/session/manager_lineage_test.go
  • internal/session/manager_prompt_contract_test.go
  • internal/session/manager_resume.go
  • internal/session/manager_start_mcp.go
  • internal/session/manager_test.go
  • internal/session/manager_test_support_test.go
  • internal/session/soul_test.go
  • internal/session/spawn_test.go
  • internal/store/globaldb/global_db_loop_cancel_pending.go
  • internal/store/globaldb/global_db_loop_test.go
  • internal/store/globaldb/global_db_task_test.go
  • web/src/systems/tasks/lib/query-options.ts
💤 Files with no reviewable changes (1)
  • web/src/systems/tasks/lib/query-options.ts

Comment thread internal/daemon/scheduler_runtime.go
Comment thread internal/daemon/task_role_runtime.go
Comment thread internal/session/ledger_test.go Outdated
Comment thread internal/store/globaldb/global_db_loop_cancel_pending.go Outdated
Comment thread internal/store/globaldb/global_db_loop_test.go

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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 `@internal/daemon/task_role_sessions.go`:
- Around line 181-183: Move the run.IsLoopWorker() early return in
activateForStarvation to immediately after the existing context validation and
before resolveStarvationAgent, while retaining the later guard as defense in
depth. Add a test using a failing or counting resolver to verify Loop-worker
runs skip agent resolution and return without an error.

In `@internal/scheduler/scheduler_test.go`:
- Around line 1082-1086: Update the RunOnce assertion in the scheduler test to
check err is non-nil before calling errors.Is or err.Error(). Keep the wrapped
backstopErr and message validations after that guard so an unexpected nil error
produces a normal test failure instead of a panic.
🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a1fc5f39-3278-4850-96e2-891e3e4f0eb2

📥 Commits

Reviewing files that changed from the base of the PR and between 91b5985 and c5d58c8.

📒 Files selected for processing (8)
  • internal/daemon/loop_cancellation_reconcile.go
  • internal/daemon/scheduler_runtime.go
  • internal/daemon/task_role_runtime_test.go
  • internal/daemon/task_role_sessions.go
  • internal/scheduler/scheduler_test.go
  • internal/session/ledger_test.go
  • internal/store/globaldb/global_db_loop_cancel_pending.go
  • internal/store/globaldb/global_db_loop_test.go
🚧 Files skipped from review as they are similar to previous changes (5)
  • internal/session/ledger_test.go
  • internal/daemon/loop_cancellation_reconcile.go
  • internal/daemon/scheduler_runtime.go
  • internal/store/globaldb/global_db_loop_cancel_pending.go
  • internal/store/globaldb/global_db_loop_test.go

Comment thread internal/daemon/task_role_sessions.go
Comment thread internal/scheduler/scheduler_test.go
coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 6, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
internal/testutil/acpmock/fixture_test.go (1)

990-999: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Cover the contract-preservation branches.

This case covers only successful stripping. stripTrailingLoopOutputContract also preserves prompts when the marker is absent or the suffix is not a JSON object. Add table cases for malformed JSON, a valid non-object value, and marker-like user text if that input is supported.

As per path instructions, tests must “Ensure tests verify behavior outcomes, not just function calls.”

🤖 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 `@internal/testutil/acpmock/fixture_test.go` around lines 990 - 999, Add
table-driven cases for stripTrailingLoopOutputContract covering malformed JSON
after the marker, a valid JSON value that is not an object, and marker-like user
text when supported. Assert each result preserves the original prompt, while
retaining the existing successful-stripping case and verifying observable output
rather than merely invoking the function.

Source: Path instructions

🤖 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 `@internal/daemon/tool_approval_bridge.go`:
- Around line 123-132: Update toolApprovalCallID and the approval request
construction so calls without a reliable call identity receive a newly
generated, per-call approval request ID rather than reusable metadata values.
Use that generated ID consistently for both PermissionRequestIDMetaKey and
ToolCallId, and add coverage proving repeated fallback approvals within the same
session receive distinct IDs and cannot affect each other.

---

Nitpick comments:
In `@internal/testutil/acpmock/fixture_test.go`:
- Around line 990-999: Add table-driven cases for
stripTrailingLoopOutputContract covering malformed JSON after the marker, a
valid JSON value that is not an object, and marker-like user text when
supported. Assert each result preserves the original prompt, while retaining the
existing successful-stripping case and verifying observable output rather than
merely invoking the function.
🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 26a2d755-8b0a-434b-a820-ab571c33a3df

📥 Commits

Reviewing files that changed from the base of the PR and between 3dd32c4 and 65920f7.

⛔ Files ignored due to path filters (3)
  • web/e2e/__tests__/marketplace.spec.ts is excluded by !web/e2e/**
  • web/e2e/__tests__/settings.spec.ts is excluded by !web/e2e/**
  • web/e2e/__tests__/triggers-hardening.spec.ts is excluded by !web/e2e/**
📒 Files selected for processing (15)
  • internal/acp/permission.go
  • internal/acp/permission_helpers.go
  • internal/bridges/resource_projection.go
  • internal/daemon/tool_approval_bridge.go
  • internal/daemon/tool_approval_bridge_test.go
  • internal/daemon/tool_approval_grants_integration_test.go
  • internal/session/manager_prompt_contract_test.go
  • internal/session/manager_prompt_pump.go
  • internal/session/manager_prompt_runtime_loop.go
  • internal/store/globaldb/global_db_bridge.go
  • internal/store/globaldb/global_db_bridges_test.go
  • internal/store/globaldb/queries/bridge_core.sql
  • internal/store/globaldb/sqlcgen/bridge_core.sql.go
  • internal/testutil/acpmock/fixture_prompt_strip.go
  • internal/testutil/acpmock/fixture_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • internal/session/manager_prompt_contract_test.go

Comment on lines +123 to +132
requestID := toolApprovalCallID(call, view)

response, err := sessions.RequestPermission(
approvalCtx,
sessionID,
acp.RequestPermissionRequest{
Meta: map[string]any{acp.PermissionRequestIDMetaKey: requestID},
SessionId: acpsdk.SessionId(sessionID),
ToolCall: acpsdk.ToolCallUpdate{
ToolCallId: acpsdk.ToolCallId(toolApprovalCallID(call, view)),
ToolCallId: acpsdk.ToolCallId(requestID),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 6 'toolApprovalCallID|RequestToolApproval|ToolCallID|CorrelationID' --glob '*.go' . || true
rg -n -C 8 '\bCallRequest\s*\{|\bToolCallID\s*:' --glob '*.go' . || true

Repository: compozy/compozy

Length of output: 50372


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '--- locating tool_approval_bridge.go ---\n'
fd -a 'tool_approval_bridge\.go$' . || true

printf '\n--- relevant file outline ---\n'
ast-grep outline internal/daemon/tool_approval_bridge.go || true

printf '\n--- relevant file lines ---\n'
sed -n '1,260p' internal/daemon/tool_approval_bridge.go

printf '\n--- focused definitions and approvals ---\n'
rg -n -C 4 'func .*RequestToolApproval|func .*ApplyToolApproval|func .*toolApprovalCallID|type CallRequest|ToolCallID|CorrelationID|ToolCall update|Approval.*Pending|pending request|decisions|decision' internal/daemon internal/tools internal -g '*.go' | sed -n '1,240p'

Repository: compozy/compozy

Length of output: 244


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- locating tool_approval_bridge.go ---'
fd -a 'tool_approval_bridge\.go$' . || true

echo
echo '--- relevant file outline ---'
ast-grep outline internal/daemon/tool_approval_bridge.go || true

echo
echo '--- relevant file lines ---'
sed -n '1,260p' internal/daemon/tool_approval_bridge.go

echo
echo '--- focused definitions and approvals ---'
rg -n -C 4 'func .*RequestToolApproval|func .*ApplyToolApproval|func .*toolApprovalCallID|type CallRequest|ToolCallID|CorrelationID|ToolCall update|Approval.*Pending|pending request|decisions|decision' internal/daemon internal/tools internal -g '*.go' | sed -n '1,260p'

Repository: compozy/compozy

Length of output: 27757


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- CallRequest definition/usages ---'
rg -n -C 5 'type CallRequest|ToolCallID\s+|CorrelationID\s+|ApprovalToken' internal -g '*.go' | sed -n '1,320p'

echo
echo '--- approval bridge tests mentioning rejection/fallback ---'
fd -a 'native_tool_approval*|tool_approval_bridge*' internal | sed -n '1,120p'
if [ -f internal/daemon/native_tool_approval_test.go ]; then
  sed -n '1,280p' internal/daemon/native_tool_approval_test.go
else
  echo 'internal/daemon/native_tool_approval_test.go not found'
fi

echo
echo '--- approval contracts definitions ---'
rg -n -C 8 'type ApprovalBridge|Method RequestToolApproval|ApplyToolApproval|ToolCallID|CorrelationID' internal/tools internal -g '*.go' | sed -n '1,260p'

Repository: compozy/compozy

Length of output: 41631


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- tool_approval_bridge_test.go ---'
sed -n '1,420p' internal/daemon/tool_approval_bridge_test.go

echo
echo '--- approval bridge implementation and bridge invocation ---'
sed -n '260,460p' internal/tools/tool.go
sed -n '130,240p' internal/tools/dispatch.go
rg -n -C 5 'RequestToolApproval|approve-all|ApprovalAvailable|permission|ApprovalRequired|ApprovalUnreachable|ApprovalTimedOut|ApprovalCanceled' internal/daemon -g '*.go' | sed -n '1,320p'

Repository: compozy/compozy

Length of output: 49667


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- approval bridge tests continuation ---'
sed -n '420,660p' internal/daemon/tool_approval_bridge_test.go

echo
echo '--- CallRequest identity fields in non-test production tools internal ---'
python3 - <<'PY'
from pathlib import Path
import re
needle = re.compile(r'CallRequest\{')
for p in sorted(Path('internal').rglob('*.go')):
    if '/testdata/' in str(p) or p.suffix != '.go':
        continue
    text = p.read_text()
    if text.startswith('package ') and (text.startswith('package test') or text.startswith('package mocks')):
        continue
    ms = [m for m in needle.finditer(text) if not text[m.start()-200:m.start()].strip().startswith('//') and not text[m.start()-200:m.start()].strip().startswith('/*')]
    if not ms:
        continue
    print(f'FILE {p}')
    for m in ms:
        line = text.count('\n', 0, m.start()) + 1
        print(f'  line {line}')
        block = text[m.start():]
        brace = 0
        for i, ch in enumerate(block):
            if ch == '{':
                brace += 1
            elif ch == '}':
                brace -= 1
                if brace == 0:
                    body = block[:i+1]
                    break
        else:
            body = block
        for field in ['ToolCallID', 'CorrelationID', 'SessionID', 'ApprovalToken', 'ToolID']:
            hits = [(x.strip()+':', f'{field}:') for x in body.split('\n') if (field+':') in x or field + ',' in x]
            print(f'    {field}:', ' '.join(hits[:5]))

echo
echo '--- production calls to approval bridge ---'
rg -n -C 8 'bridge\.RequestToolApproval|RequestToolApproval\(' internal -g '*.go' --glob '!*_test.go' | sed -n '1,260p'

Repository: compozy/compozy

Length of output: 8078


🏁 Script executed:

#!/bin/bash
set -euo pipefail
export LC_ALL=C

echo '--- approved-tool-call callers (non-test) ---'
rg -n -C 8 'ApprovedToolCall|approved-tool-call|approve-tool-call|ApproveToolCall|ApprovalRequired' internal -g '*.go' --glob '!*_test.go' | sed -n '1,260p'

echo
echo '--- production Read/Approval bridge wiring references'
rg -n -C 5 'approvalbridge|ApprovalBridge|ApprovedToolCall|toolApprovals|ApproveToolCall|ToolApprovalsSet|ApprovalRequired|approvalprobe' internal -g '*.go' --glob '!*_test.go' | sed -n '1,320p'

echo
echo '--- tool_approval_bridge_grants.go relevant sections'
sed -n '1,320p' internal/daemon/tool_approval_bridge_grants.go

Repository: compozy/compozy

Length of output: 44077


Generate a per-call approval request ID when no call identity is available.

toolApprovalCallID falls back to non-unique metadata values, and that value is sent as both request_id and ToolCallId. A second approval from the same caller can map to the first pending request, so the rejection path and “allow once” grants can apply to the wrong call. Use a generated approval request ID for these fallbacks, and cover same-session reuse in the fallback case.

🤖 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 `@internal/daemon/tool_approval_bridge.go` around lines 123 - 132, Update
toolApprovalCallID and the approval request construction so calls without a
reliable call identity receive a newly generated, per-call approval request ID
rather than reusable metadata values. Use that generated ID consistently for
both PermissionRequestIDMetaKey and ToolCallId, and add coverage proving
repeated fallback approvals within the same session receive distinct IDs and
cannot affect each other.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@internal/store/sessiondb/read_only_pool.go`:
- Around line 213-215: Update the opening flow around p.open and
context.WithoutCancel so the detached opener always has an explicit bounded
timeout, including NewReadOnlyPool’s default OpenSessionDBReadOnly path.
Preserve any earlier caller deadline when it is shorter, and ensure the timeout
context is released after p.open and p.finishOpening complete.
🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 81fb10a5-fd83-4269-82b9-cbabbd347faa

📥 Commits

Reviewing files that changed from the base of the PR and between 65920f7 and 36b0e8a.

⛔ Files ignored due to path filters (2)
  • web/e2e/__tests__/os-shell.spec.ts is excluded by !web/e2e/**
  • web/e2e/__tests__/session-hardening.spec.ts is excluded by !web/e2e/**
📒 Files selected for processing (2)
  • internal/store/sessiondb/read_only_pool.go
  • internal/store/sessiondb/session_db_integration_test.go

Comment on lines +213 to +215
openingCtx := context.WithoutCancel(ctx)
recorder, err := p.open(openingCtx, key.owner, key.path)
p.finishOpening(openingCtx, key, opening, recorder, err)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

ast-grep outline internal/store/sessiondb/read_only_pool.go --items all
rg -n -C 5 'func \(p \*ReadOnlyPool\) (finishOpening|Close)|func openSessionDBReadOnlyWithRetry|context\.WithoutCancel|context\.WithDeadline|context\.WithTimeout|Deadline\(' internal/store/sessiondb

Repository: compozy/compozy

Length of output: 19641


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== read_only_pool.go relevant sections =="
sed -n '1,130p' internal/store/sessiondb/read_only_pool.go
sed -n '156,260p' internal/store/sessiondb/read_only_pool.go
sed -n '258,330p' internal/store/sessiondb/read_only_pool.go

echo "== read_only_pool shutdown =="
sed -n '1,220p' internal/store/sessiondb/read_only_pool_shutdown.go

echo "== read_only.go open helper/retry =="
sed -n '1,240p' internal/store/sessiondb/read_only.go

echo "== all WithTimeout/WithDeadline/Deadline/context.WithoutCancel in files under internal/store/sessiondb =="
rg -n -C 4 'context\.WithoutCancel|context\.WithTimeout|context\.WithDeadline|Deadline\(|deadline|timeout|deadline=|ttl=' internal/store/sessiondb --glob '*.go'

Repository: compozy/compozy

Length of output: 32667


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== OpenSessionDBReadOnly callers that set ReadOnlyPool Open option =="
rg -n -C 8 'OpenSessionDBReadOnly(\(|\s*|,|ReadOnlyPoolConfig|WithReadOnlyOpenRetry|--Open' internal --glob '*.go' | sed -n '1,240p'

echo "== NewReadOnlyPool Open config assignments =="
rg -n -C 6 'NewReadOnlyPool\(|ReadOnlyPoolConfig|Open:' internal --glob '*.go' | sed -n '1,260p'

echo "== read_only.go context usage and retry delay =="
sed -n '220,290p' internal/store/sessiondb/read_only.go

echo "== SQL context propagation around read-only opener path =="
rg -n -C 3 'MustContext|ExecContext|QueryRowContext|QueryContext|PrepareContext|Context\(\)$' internal/store/sessiondb/sqlcgen internal/store/sessiondb -g '*.go' | sed -n '1,220p'

Repository: compozy/compozy

Length of output: 361


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== OpenSessionDBReadOnly callers from internal/ =="
rg -n -C 8 'OpenSessionDBReadOnly|WithReadOnlyOpenRetry|NewReadOnlyPool\(|ReadOnlyPoolConfig|Open:' internal --glob '*.go' | sed -n '1,340p'

echo "== read_only.go retry delay implementation =="
sed -n '220,290p' internal/store/sessiondb/read_only.go

echo "== read_only_pool.go constructor and config default path =="
sed -n '127,155p' internal/store/sessiondb/read_only_pool.go

Repository: compozy/compozy

Length of output: 30221


Bounding the detached opener is needed for NewReadOnlyPool defaults.

context.WithoutCancel(ctx) removes the caller deadline and leaves p.open without an owner deadline. The built-in opener is OpenSessionDBReadOnly, which relies only on retry/backoff and SQLite busy handling and has no explicit total timeout. A blocked DB open while an entry is reserved can hold the pool indefinitely. Add an explicit timeout when the caller deadline is removed, or require injectable timeouts for the opener.

🤖 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 `@internal/store/sessiondb/read_only_pool.go` around lines 213 - 215, Update
the opening flow around p.open and context.WithoutCancel so the detached opener
always has an explicit bounded timeout, including NewReadOnlyPool’s default
OpenSessionDBReadOnly path. Preserve any earlier caller deadline when it is
shorter, and ensure the timeout context is released after p.open and
p.finishOpening complete.

Source: Coding guidelines

@pedronauck
pedronauck merged commit f213aa2 into main Aug 6, 2026
19 checks passed
@pedronauck
pedronauck deleted the fix-321-322 branch August 6, 2026 13:45

This branch was successfully deployed

1 active deployment
Preview — 36b0e8a8 Deployed Aug 6, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant