fix(review): report status without changing local state

[kantorcodes]

Read review status without changing local state, and keep delivery history scoped to the current connection. Unavailable credentials report unavailable status without an implicit repair or migration.

Validation: 88 status, consent, and telemetry tests plus 76 policy and runtime compatibility tests pass. Lint and type checks pass. The deterministic report reconciles all 51,000 cases without lowered decisions.

[qodo-code-review]

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (2)

• release/2.2
• release/3.1

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Advanced

Run ID: b9f2d1a9-505a-4135-9c6e-a327074308e6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Make Cloud Review status reads passive and worker-aware

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Reads Cloud Review status through a passive, read-only SQLite snapshot.
• Unifies CLI and settings status with identity-bound, fresh worker readiness.
• Hardens loopback probes and documents unavailable storage behavior.

Diagram

graph TD
  CLI["Status CLI"] -->|"authenticated GET"| Probe["Loopback Probe"] --> Route["Daemon Route"] --> Workers["Worker Threads"]
  Probe --> Projection["Status Projection"] --> Snapshot["Passive Store"] --> DB[("Existing SQLite")]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use normal GuardStore with rollback

• ➕ Reuses existing initialization and credential handling directly
• ➕ Requires fewer new storage abstractions
• ➖ Initialization can mutate files, metadata, identities, and external key stores before rollback
• ➖ Database rollback cannot reverse vault migrations or permission repairs
• ➖ Violates the requirement that status reads remain observational

2. Build a standalone immutable repository

• ➕ Avoids subclassing GuardStore and private implementation dependencies
• ➕ Could expose a narrowly typed status-only data model
• ➖ Duplicates mature status queries and consent verification logic
• ➖ Increases parity risk between authorization and status interpretation
• ➖ Requires a broader refactor than this targeted fix

Recommendation: Keep the passive GuardStore adapter and shared projection. It preserves existing consent semantics while enforcing read-only SQLite and secret access; normal-store rollback cannot guarantee filesystem or keyring immutability, while a standalone repository would add substantial duplication and drift risk.

Files changed (21) +1280 / -102

Enhancement (2) +98 / -2

cloud_review_settings_route.pyAdd worker-aware Cloud Review GET handler +24/-2

Add worker-aware Cloud Review GET handler

• Observes daemon workers and returns the shared settings status with no-store caching headers.

src/codex_plugin_scanner/guard/daemon/cloud_review_settings_route.py

local_status_transport.pyIntroduce hardened local status transport +74/-0

Introduce hardened local status transport

• Adds allowlisted loopback HTTP reads with authentication, a one-second total deadline, a 64 KB response limit, and no proxy or redirect handling.

src/codex_plugin_scanner/guard/daemon/local_status_transport.py

Bug fix (10) +465 / -40

cloud_review_status_command.pyAdd pre-initialization passive status command +42/-0

Add pre-initialization passive status command

• Runs Cloud Review status directly against the Guard home before ordinary storage initialization. Converts missing, corrupt, or incompatible local state into a stable unavailable response.

src/codex_plugin_scanner/guard/cli/cloud_review_status_command.py

commands_dispatch_cloud_review.pyUse the shared Cloud Review status projection +8/-1

Use the shared Cloud Review status projection

• Replaces the consent-only CLI status response with shared connection, consent, delivery, and worker status fields.

src/codex_plugin_scanner/guard/cli/commands_dispatch_cloud_review.py

commands_router.pyRoute status before GuardStore initialization +7/-0

Route status before GuardStore initialization

• Intercepts 'cloud-review status' before the standard store and policy initialization path, preventing read-triggered setup or repair.

src/codex_plugin_scanner/guard/cli/commands_router.py

cloud_review_settings.pyShare status fields across CLI and settings +13/-35

Share status fields across CLI and settings

• Replaces duplicated settings projection logic with the common Cloud Review status function. Settings mutations supply a current identity-bound worker observation to the returned status.

src/codex_plugin_scanner/guard/daemon/cloud_review_settings.py

cloud_review_status_reader.pyRead worker evidence from an existing daemon +24/-0

Read worker evidence from an existing daemon

• Verifies the live daemon identity, loads its authentication token, and extracts worker evidence from the existing Cloud Review route without starting services.

src/codex_plugin_scanner/guard/daemon/cloud_review_status_reader.py

cloud_review_worker_status.pyObserve both Cloud Review workers without mutation +32/-0

Observe both Cloud Review workers without mutation

• Reports decision and upload worker liveness under a non-blocking lifecycle lock. Observations include source, connection identity hash, and timestamp.

src/codex_plugin_scanner/guard/daemon/cloud_review_worker_status.py

server.pyServe worker-aware status through the route handler +2/-2

Serve worker-aware status through the route handler

• Delegates the existing '/v1/cloud-review' GET route to the new handler so responses include live worker evidence.

src/codex_plugin_scanner/guard/daemon/server.py

passive_status_store.pyAdd an immutable Cloud Review storage adapter +187/-0

Add an immutable Cloud Review storage adapter

• Opens one query-only SQLite snapshot while bypassing normal GuardStore initialization. Reads existing OAuth and integrity secrets without migration, promotion, repair, permission changes, or identity creation.

src/codex_plugin_scanner/guard/passive_status_store.py

cloud_review_status.pyCentralize Cloud Review status projection +146/-0

Centralize Cloud Review status projection

• Combines connection, consent, outbox, recovery, synchronization, and worker evidence into one shared response. Delivery readiness requires matching identity, matching source, both workers, and an observation no older than five seconds.

src/codex_plugin_scanner/guard/runtime/cloud_review_status.py

exact_cloud_review.pyMake consent status verification optionally read-only +4/-2

Make consent status verification optionally read-only

• Adds a read-only verification mode that reports connection-binding drift without persisting revocation state or events.

src/codex_plugin_scanner/guard/runtime/exact_cloud_review.py

Refactor (1) +11 / -57

client.pyReuse bounded local status transport for health probes +11/-57

Reuse bounded local status transport for health probes

• Moves authenticated health-detail reads onto the shared loopback-only, deadline-bounded transport implementation.

src/codex_plugin_scanner/guard/daemon/client.py

Tests (7) +654 / -3

decision-diff-report.jsonRefresh command decision report hashes +3/-2

Refresh command decision report hashes

• Regenerates expected source hashes for the changed command routing and status behavior while retaining existing decisions.

tests/fixtures/guard-command-corpus/decision-diff-report.json

test_cloud_review_status_cli_entry.pyVerify the public status entry point remains passive +145/-0

Verify the public status entry point remains passive

• Covers absent, corrupt, legacy, and symlinked databases plus missing identity and OAuth metadata. Also verifies signer backend preservation and unchanged valid storage.

tests/test_cloud_review_status_cli_entry.py

test_cloud_review_status_parity.pyVerify CLI and settings status parity +138/-0

Verify CLI and settings status parity

• Checks shared fields across disconnected, connected, enabled, and recovery states. Confirms identity drift and status reads do not revoke consent or mutate persisted state.

tests/test_cloud_review_status_parity.py

test_cloud_review_status_readiness.pyTest identity-bound worker readiness +108/-0

Test identity-bound worker readiness

• Exercises both-worker requirements, malformed evidence, five-second freshness, source and binding mismatches, and non-blocking lifecycle observation.

tests/test_cloud_review_status_readiness.py

test_cloud_review_status_storage.pyTest immutable storage and secret handling +188/-0

Test immutable storage and secret handling

• Uses database and filesystem snapshots to prove status reads do not repair OAuth metadata, migrate vaults, create identities, or promote integrity keys. It also validates private, hash-bound credential requirements and snapshot isolation.

tests/test_cloud_review_status_storage.py

test_cloud_review_status_transport.pyTest hardened authenticated loopback probes +71/-0

Test hardened authenticated loopback probes

• Verifies token delivery only to approved local routes and rejects redirects, oversized bodies, non-object JSON, and unapproved paths despite proxy environment settings.

tests/test_cloud_review_status_transport.py

test_policy_proof_prerequisites.pyRequire the documented incomplete-proof exit code +1/-1

Require the documented incomplete-proof exit code

• Tightens the missing frontend dependency assertion to require exit status 2 rather than any nonzero result.

tests/test_policy_proof_prerequisites.py

Documentation (1) +52 / -0

cloud-review-status.mdDocument passive Cloud Review status semantics +52/-0

Document passive Cloud Review status semantics

• Defines shared CLI and settings fields, tri-state delivery readiness, freshness and identity requirements, bounded daemon probes, and non-mutating behavior for unavailable storage and signers.

docs/guard/cloud-review-status.md

[greptile-apps]

Confidence Score: 4/5

The behavioral status changes appear sound, but the explicit prohibition on legacy-format support must be satisfied before merging.

Findings

1.  Legacy Keys Remain Supported ▶

Summary

This PR introduces a shared, read-only Cloud Review status projection for the public CLI and authenticated daemon route, adds bounded loopback worker-status probes, and prevents status reads from initializing or repairing local storage.

• Routes cloud-review status before ordinary GuardStore initialization.
• Reads Cloud Review state through a query-only SQLite snapshot and existing credential material.
• Requires fresh, source- and connection-bound observations of both delivery workers before reporting readiness.
• Consolidates bounded authenticated loopback HTTP transport for health and Cloud Review status.
• Adds parity, storage-mutation, readiness, transport, and CLI-entry regression coverage.
• One repository-rule violation remains: the passive reader newly supports a legacy raw vault-key format.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  CLI[cloud-review status CLI] --> Probe[Authenticated loopback status probe]
  Probe --> Daemon[Existing Guard daemon]
  Daemon --> Workers[Observe decision and upload workers]
  CLI --> Snapshot[Read-only SQLite snapshot]
  Daemon --> Snapshot
  Snapshot --> Identity[Validate OAuth and consent identity]
  Workers --> Projection[Shared status projection]
  Identity --> Projection
  Projection --> Ready{Both workers fresh and running?}
  Ready -->|Yes| Available[delivery_ready = true]
  Ready -->|Known prerequisite absent| NotReady[delivery_ready = false]
  Ready -->|Observation unavailable| Unknown[delivery_ready = null]

Loading

_{Reviews (1) · Last reviewed commit: "fix(review): share passive connection an..."}

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0) 🎨 UX issues (0) 📜 Skill insights (0)

1. Status reads can migrate local storage ✓ Resolved 🐞 Bug ≡ Correctness

Description

run_guard_command dispatches the special status handler only after resolve_guard_home(...),
whose implicit-path resolver can transactionally migrate legacy state despite the handler itself
avoiding GuardStore construction. With no explicit home, when the legacy home contains state and
the canonical home does not—including corrupt SQLite counted as existing state—resolution can create
staging directories, copy and prune or rewrite SQLite data, remove an existing destination, and move
storage before reaching the passive reader.

Code

src/codex_plugin_scanner/guard/cli/commands_router.py[R194-195]

+    if args.guard_command == "cloud-review" and getattr(args, "cloud_review_command", None) == "status":
+        from .cloud_review_status_command import run_cloud_review_status_command

Evidence

The status branch in run_guard_command is downstream of resolve_guard_home, so avoiding
GuardStore construction does not avoid side effects from home resolution. Without an override, the
resolver checks for legacy state—including corrupt SQLite—and, when the canonical location is empty,
invokes migration code that creates staging storage, copies and prunes SQLite state, removes an
existing destination, and moves the migrated directory into place before the status handler runs.

src/codex_plugin_scanner/guard/cli/commands_router.py[157-199]
src/codex_plugin_scanner/guard/config.py[459-483]
src/codex_plugin_scanner/guard/config.py[1379-1417]
src/codex_plugin_scanner/guard/config.py[1424-1428]
src/codex_plugin_scanner/guard/config.py[459-484]
src/codex_plugin_scanner/guard/config.py[1379-1389]
src/codex_plugin_scanner/guard/config.py[1466-1516]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The public `cloud-review status` route is intended to be passive, but it is dispatched only after default Guard-home resolution, which can migrate legacy storage into the canonical home.
## Fix Focus Areas
- src/codex_plugin_scanner/guard/cli/commands_router.py[157-199]
- src/codex_plugin_scanner/guard/config.py[459-484]
## Recommended Fix
Detect and route `cloud-review status` before calling the migration-capable Guard-home resolver. Resolve an explicit `--guard-home` override normally; for the default path, use a non-mutating lookup that selects the canonical location directly and, if legacy lookup is supported, inspects the existing legacy location in place without migration. Return unavailable for legacy or incomplete storage when it cannot be read passively rather than creating directories, copying state, or moving storage during the status request.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Legacy vault keys remain supported ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

read_existing_vault_text recognizes a raw 32-byte key and converts it in memory to the current
Fernet encoding. test_status_reads_raw_existing_vault_key_without_upgrading_it explicitly
exercises this compatibility branch, so status remains dependent on the obsolete vault format.

Code

src/codex_plugin_scanner/guard/passive_status_store.py[R183-184]

+        if len(key) == 32:
+            key = base64.urlsafe_b64encode(key)

Evidence

Rule 2757193 forbids retaining runtime branches and tests solely for legacy behavior. The new reader
converts a raw 32-byte key for continued use, while the new test names that key legacy_key and
verifies that status still reads it without upgrading storage.

Rule 2757193: Remove and forbid use of legacy feature flags and code paths
src/codex_plugin_scanner/guard/passive_status_store.py[182-185]
tests/test_cloud_review_status_storage.py[113-140]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The passive status reader introduces a compatibility branch for raw legacy vault keys, and a new test preserves that legacy code path.
## Fix Focus Areas
- src/codex_plugin_scanner/guard/passive_status_store.py[182-185]
- tests/test_cloud_review_status_storage.py[113-140]
## Recommended Fix
Remove the raw 32-byte key conversion from `read_existing_vault_text` so only the current vault-key format is accepted. Replace the compatibility test with an assertion that legacy key storage reports status as unavailable without mutating or migrating storage.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Daemon server exceeds the file limit 📘 Rule violation ⚙ Maintainability

Description

The modified server.py remains 8,839 physical lines long, with at least 1,000 independently
verified non-blank indented lines. Changing the Cloud Review route in this oversized module leaves
later maintainers navigating far beyond the mandated module boundary for server behavior.

Code

src/codex_plugin_scanner/guard/daemon/server.py[R2319-2321]

+            from .cloud_review_settings_route import handle_cloud_review_status
-            self._write_json(cloud_review_settings_status(store), extra_headers={"Cache-Control": "no-store"})
+            handle_cloud_review_status(self._daemon_server(), self._write_json)

Evidence

Rule 2757183 limits every changed source file to 500 non-blank lines. The PR modifies the Cloud
Review route at lines 2319-2321 in a file that reaches 8,839 lines, and repository inspection
confirms more than 1,000 non-blank indented lines alone.

Rule 2757183: Limit source file length to 500 non-blank lines or less
src/codex_plugin_scanner/guard/daemon/server.py[1-8839]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The changed daemon server contains more than 500 non-blank lines and therefore violates the source-file size limit.
## Fix Focus Areas
- src/codex_plugin_scanner/guard/daemon/server.py[2318-2321]
## Recommended Fix
Continue extracting route dispatch and related server responsibilities into focused modules, then reduce `server.py` to no more than 500 non-blank lines while preserving the Cloud Review handler delegation.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Profiles show foreign quarantined events ✓ Resolved 🐞 Bug ≡ Correctness

Description

_project_cloud_review_status() passes no identity filter to review_event_outbox_status() when
the requested source has no binding, then returns its quarantined_depth as isolated_events. The
outbox diagnostics query has no source predicate in that no-workspace case, so a disconnected named
profile can display quarantined events belonging to another profile.

Code

src/codex_plugin_scanner/guard/runtime/cloud_review_status.py[85]

+        "isolated_events": outbox.get("quarantined_depth", 0),

Evidence

The new projection calls the outbox status reader without binding values and directly publishes
quarantined_depth. The underlying ready-depth query is source-filtered, but its diagnostics query
only adds source filtering when workspace_id is non-null; therefore the new field is cross-source
for disconnected profiles.

src/codex_plugin_scanner/guard/runtime/cloud_review_status.py[44-55]
src/codex_plugin_scanner/guard/runtime/cloud_review_status.py[83-85]
src/codex_plugin_scanner/guard/store_review_event_outbox.py[330-390]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
Issue description
`isolated_events` is derived from an unscoped quarantined-event diagnostic when the selected profile has no connection binding, allowing another OAuth source's event count to appear in this profile's status.
Fix Focus Areas
- src/codex_plugin_scanner/guard/runtime/cloud_review_status.py[48-85]
- src/codex_plugin_scanner/guard/store_review_event_outbox.py[361-390]
Recommended Fix
Ensure the quarantined diagnostics used for a no-binding status read are filtered by `self._guard_source`, or report zero isolated events until a complete binding exists. Add coverage with quarantined rows for one source and a disconnected status read for another source.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[gitar-bot]

Code Review ✅ Approved

Fixes review status reporting to avoid modifying local state and scopes delivery history to the current connection. Unavailable credentials now report unavailable status without implicit repair or migration. All 88 status, consent, and telemetry tests plus 76 policy and runtime compatibility tests pass with no issues found.

Review coverage

Rules No rules evaluated

Functional validation Not enabled · Set up

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source}