feat: Self-Guardian subsystem + CI fix#30
Conversation
…CE loop, MissionState, docs - Tool calling surface with ToolRegistry + 4 tools (pause retry, draft update, schedule checkin, flag churn) wired to HITL manager with correct string tiers - SSEHub event-type filtering: Subscription.Types gates Broadcast fan-out; 3 new SSE endpoints (/events/mission, /events/hitl, /events/session) - Slack consolidation: extended SocketModeClient for interactive events; removed Mockoon dead code from slack.py - ACE loop completed: _send_feedback_signal() now calls curator.update() - MissionState: added prepared_brief, pending_decisions, last_updated_by in Python dataclass + SQL queries (DB already done in migration 004) - Updated AGENTS.md, README.md, and all 5 .opencode/context files - Fixed hitl_queue_test.go: NewHandler now takes 2 args
- pause_payment_retry: Stripe POST /v1/subscriptions/{id} with
collection_method=send_invoice; httpx-based, MOCK_MODE guard
- draft_investor_update: loads MissionState + chat_completion()
for bounded (300 token) investor email draft
- schedule_customer_checkin: Slack chat_postMessage via
SlackClient to configurable channel; mock when no SLACK_BOT_TOKEN
- flag_churn_risk: updates churn_risk_users field via
get_mission_state() + update_mission_state(); decision journal log
- generate_prepared_brief(): 2-sentence LLM summary triggered
after MissionState write when prepared_brief is empty
- update_mission_state() accepts generate_brief: bool = True param
…nability, dashboard - Agent authority manifest: 5 agents with role/permissions/tool allowlists/ escalation tiers/mission field ownership. Machine-readable Pydantic model. - Alert lineage: AlertLineage schema attached to every GuardianMessage. pattern_id + source_metrics + mission_context + raise_timeline_risk are deterministic (code-only). suggested_tool_ids from HITLManager. owner_agent from manifest. - MissionState explainability: 3 new fields (last_update_reason, last_changed_fields, active_agent_roles) across Python dataclass, SQL migration 005, and Go read model. - Curator write verification: StrategyDelta Pydantic model for structured audit trail. update_strategy_confidence() returns ConfidenceUpdateResult + StrategyDelta tuple. PostgreSQL audit with JSONL fallback. - Dashboard operating layer: 2 new Go handlers (APICommandAlertLineage, APICommandOperatingLayer) with HTMX partials. 5 new Go tests. - All 24 Go web tests pass. Python assertions all pass.
last_changed_fields and active_agent_roles are Python lists but the DB columns are JSONB. asyncpg requires json.dumps() before passing list values to JSONB columns. Added import json and serialization at the parameter boundary.
Add lightweight control plane with agent registry, policy engine, and audit logging. Implement prompt risk and output risk scanners for investor-facing workflows. Force HITL approve on all external-facing outputs. Extend MissionState with policy_state explainability. New modules: - src/control_plane/ — registry, policy engine, audit logger - src/risk/ — prompt risk scanner (9 rules), output risk scanner (9 rules) - src/schemas/control_plane.py — PolicyDecision, RiskScanResult, AgentRegistration, AuditEvent Pydantic v2 models - infrastructure/migrations/004_control_plane_audit.sql - tests/unit/test_control_plane.py — 25 tests Modified: - authority_manifest.py — added allowed_models, external_facing, data_classification to all 5 agents - mission_state.py — added policy_state field with full DB roundtrip - draft_investor_update.py — risk-gated: prompt scan -> LLM -> output scan -> audit log, always requires_approval: True - schemas/__init__.py — exported new control plane models Preserves Sarthi's guardian architecture, deterministic watchlists, and bounded LLM usage. Zero regressions (799 unit tests pass).
Add three Docker Compose profiles matching machine constraints: - dev: PostgreSQL + Redis only (~2GB, daily coding) - llmops: adds Jaeger + MinIO + Langfuse (~4-5GB, tracing/eval) - showcase: full stack Temporal + Redpanda + Qdrant + Langfuse (~9GB, demo only) Container names and volumes are profile-scoped to prevent collisions. All profiles share iterateswarm-net for compatible service discovery.
Add local-first CI with pre-push validation: - make ci-fast: vet + fmt + ruff + unit tests (pre-commit gate) - make ci-local: ci-fast + actionlint (pre-push gate) - Pre-push hook: blocks direct pushes to main, runs actionlint - GitHub Actions CI: typecheck -> lint -> unit-tests + workflow-lint - Integration test job exists as disabled placeholder Also fix pre-existing bugs discovered by local gates: - Fix orphaned except in hubspot.py (SyntaxError) - Fix missing try: block in quickbooks.py (SyntaxError) - Add missing lineage field in test_agentic_comprehensive.py - Fix broad "llm" substring match in test_qa_agent.py - Ignore env-var-dependent and Docker-dependent tests in ci-fast Config drift is now caught locally before push.
Add Self-Guardian internal monitoring subsystem (Phase 2): - AgentObservation, DeviationType, SelfGuardianAlert, SelfGuardianReport schemas - ObservationCollector: thread-safe in-memory buffer (10K cap) - SelfGuardianDetector: analyzes observations against AUTHORITY_MANIFEST - unauthorized_tool detection (critical) - external_facing_violation detection (warning) - failure pattern detection (info) - generate_report() aggregation - 25 unit tests, all passing CI fix: replace actions/setup-python@v5 with astral-sh/setup-uv@v5 - setup-python fails on Debian-slim containers (no Python 3.13 binaries) - setup-uv works on any Linux base, matches project's uv dependency
📝 WalkthroughWalkthroughThis PR introduces a Python AI control plane (registry, policy engine, audit logging), risk scanners, a global tool registry with four HITL tools, and a self-guardian deviation detector. It adds Slack SocketMode integration with an ACE feedback loop, extends MissionState with explainability fields, reworks the Go SSEHub with event-type filtering, adds new command-center endpoints/templates, updates documentation, CI workflow, git hooks, Makefile targets, and adds three Docker Compose local runtime profiles. ChangesCI, Git Hooks and Build Tooling
Architecture and Contributor Documentation
AI Control Plane, Risk Scanning and Tool Registry
Self-Guardian Deviation Monitoring
Slack SocketMode and ACE Feedback Loop
Mission-State Explainability and Go SSEHub Command Center
Local Docker Compose Runtime Profiles
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Handler
participant SSEHub
participant Postgres
Client->>Handler: GET /events/mission (tenant)
Handler->>SSEHub: Subscribe(tenantID, "mission-update")
SSEHub-->>Handler: Subscription{ID, Channel, Types}
Handler-->>Client: connected, heartbeat
Postgres->>Handler: mission_state row updated
Handler->>SSEHub: Broadcast(event type="mission-update")
SSEHub->>Handler: filtered payload via sub.Channel
Handler-->>Client: stream payload bytes
sequenceDiagram
participant SlackButton
participant PolicyEngine
participant ToolRegistry
participant ControlTool
participant AuditLogger
SlackButton->>PolicyEngine: evaluate(agent, requested_tool)
PolicyEngine-->>SlackButton: PolicyDecision
SlackButton->>ToolRegistry: get_tools_for_patterns(triggered_patterns)
ToolRegistry-->>SlackButton: candidate ToolDefs
SlackButton->>ControlTool: execute(tenant_id, ...)
ControlTool->>AuditLogger: log_event(AuditEvent)
AuditLogger-->>ControlTool: success/failure
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive agent control plane, including an agent registry, policy engine, and audit logging, alongside a Self-Guardian monitoring subsystem for tracking agent behavior. It also implements a centralized Tool Registry with HITL tier mapping, a Slack SocketMode integration with an ACE feedback loop, and an upgraded Go SSEHub with event-type filtering. While the architectural enhancements are solid, several critical runtime bugs and issues were identified in the review: indentation errors in the QuickBooks integration, incorrect asynchronous calls to the audit logger, blocking synchronous calls in the brief generator, database table name mismatches, and incorrect SSE event-type subscriptions in the Go handlers. Additionally, passing serialized JSON strings to asyncpg JSONB columns will fail, and the observation collector's buffer eviction can be optimized using a deque.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def _fetch_invoices() -> list: | ||
| """Single HTTP attempt — retried by ``retry_with_backoff``.""" | ||
| response = httpx.get(url, params=params, headers=headers, timeout=30.0) | ||
| response.raise_for_status() | ||
| data = response.json() | ||
| query_response = data.get("QueryResponse", {}) or {} |
There was a problem hiding this comment.
The indentation of invoices on line 101 is incorrect (8 spaces instead of 12), placing it outside the _fetch_invoices function scope. Since it references query_response which is local to _fetch_invoices, this will raise a NameError at runtime. Please indent line 101 and subsequent lines to be inside the function.
| "reason": "Output risk scan blocked draft", | ||
| }, | ||
| ) | ||
| _AUDIT.log(audit_event) |
There was a problem hiding this comment.
The AuditLogger class does not have a log method (only log_event which is asynchronous). Calling _AUDIT.log(audit_event) will raise an AttributeError at runtime. It should be awaited using await _AUDIT.log_event(audit_event).
| _AUDIT.log(audit_event) | |
| await _AUDIT.log_event(audit_event) |
| "output_scan": output_scan.model_dump(), | ||
| }, | ||
| ) | ||
| _AUDIT.log(audit_event) |
There was a problem hiding this comment.
The AuditLogger class does not have a log method. Calling _AUDIT.log(audit_event) will raise an AttributeError at runtime. It should be awaited using await _AUDIT.log_event(audit_event).
| _AUDIT.log(audit_event) | |
| await _AUDIT.log_event(audit_event) |
| if loop.is_running(): | ||
| import concurrent.futures | ||
| with concurrent.futures.ThreadPoolExecutor() as pool: | ||
| pool.submit(asyncio.run, _pg_write).result(timeout=5) |
There was a problem hiding this comment.
Passing the coroutine function _pg_write directly to asyncio.run will raise a ValueError because asyncio.run expects a coroutine object (the result of calling the function). Use a lambda to call the function: pool.submit(lambda: asyncio.run(_pg_write())).
| pool.submit(asyncio.run, _pg_write).result(timeout=5) | |
| pool.submit(lambda: asyncio.run(_pg_write())).result(timeout=5) |
| import asyncio | ||
|
|
||
| async def _pg_write() -> None: | ||
| conn = await asyncpg.connect() |
There was a problem hiding this comment.
Calling asyncpg.connect() without arguments will attempt to connect using default environment variables, which will fail if the database is on a non-standard port or requires specific credentials. Pass the configured database URL instead.
from src.config.database import get_database_url
conn = await asyncpg.connect(get_database_url("iterateswarm"))| // APICommandMissionEvents is an SSE endpoint for mission state updates (event type: "mission-update"). | ||
| func (h *Handler) APICommandMissionEvents(c *fiber.Ctx) error { | ||
| tenantID := c.Query("tenant_id", "default") | ||
| sub := h.sseHub.Subscribe(tenantID, "mission-update") |
There was a problem hiding this comment.
The endpoint subscribes to "mission-update", but the documented and expected event type is "mission". This mismatch will cause published mission events to be filtered out and never delivered to subscribers.
| sub := h.sseHub.Subscribe(tenantID, "mission-update") | |
| sub := h.sseHub.Subscribe(tenantID, "mission") |
| // APICommandHITLEvents is an SSE endpoint for HITL approval events (event type: "hitl-item"). | ||
| func (h *Handler) APICommandHITLEvents(c *fiber.Ctx) error { | ||
| tenantID := c.Query("tenant_id", "default") | ||
| sub := h.sseHub.Subscribe(tenantID, "hitl-item") |
There was a problem hiding this comment.
The endpoint subscribes to "hitl-item", but the documented and expected event type is "hitl". This mismatch will cause published HITL events to be filtered out and never delivered to subscribers.
| sub := h.sseHub.Subscribe(tenantID, "hitl-item") | |
| sub := h.sseHub.Subscribe(tenantID, "hitl") |
| // APICommandSessionEvents is an SSE endpoint for agent message events (event type: "agent-message"). | ||
| func (h *Handler) APICommandSessionEvents(c *fiber.Ctx) error { | ||
| tenantID := c.Query("tenant_id", "default") | ||
| sub := h.sseHub.Subscribe(tenantID, "agent-message") |
There was a problem hiding this comment.
The endpoint subscribes to "agent-message", but the documented and expected event type is "session". This mismatch will cause published session events to be filtered out and never delivered to subscribers.
| sub := h.sseHub.Subscribe(tenantID, "agent-message") | |
| sub := h.sseHub.Subscribe(tenantID, "session") |
| json.dumps( | ||
| event.policy_decision.model_dump() if event.policy_decision else None | ||
| ), | ||
| event.approval_state, | ||
| event.outcome, | ||
| json.dumps(event.details) if event.details else None, |
There was a problem hiding this comment.
Passing a JSON-serialized string (via json.dumps) to a JSONB column in asyncpg will raise a type mismatch error because asyncpg expects a Python dict or list for JSONB placeholders. Pass the dictionary directly instead of serializing it to a string.
| json.dumps( | |
| event.policy_decision.model_dump() if event.policy_decision else None | |
| ), | |
| event.approval_state, | |
| event.outcome, | |
| json.dumps(event.details) if event.details else None, | |
| event.policy_decision.model_dump() if event.policy_decision else None, | |
| event.approval_state, | |
| event.outcome, | |
| event.details, |
| def __init__(self) -> None: | ||
| self._observations: list[AgentObservation] = [] | ||
| self._lock = threading.Lock() | ||
|
|
||
| def record_observation(self, observation: AgentObservation) -> None: | ||
| """Record a single agent observation. | ||
|
|
||
| If the buffer is at capacity, the oldest entry is removed. | ||
| """ | ||
| with self._lock: | ||
| if len(self._observations) >= _MAX_BUFFER_SIZE: | ||
| self._observations.pop(0) | ||
| self._observations.append(observation) |
There was a problem hiding this comment.
Using list.pop(0) on a list of up to 10,000 elements is an collections.deque with maxlen=_MAX_BUFFER_SIZE is much more efficient ($O(1)$) and automatically handles eviction when capacity is reached.
def __init__(self) -> None:
from collections import deque
self._observations = deque(maxlen=_MAX_BUFFER_SIZE)
self._lock = threading.Lock()
def record_observation(self, observation: AgentObservation) -> None:
"""Record a single agent observation.
If the buffer is at capacity, the oldest entry is removed.
"""
with self._lock:
self._observations.append(observation)
log.debug(
"Observation recorded for agent=%s action=%s success=%s",
observation.agent_name,
observation.action,
observation.success,
)![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
.opencode/context/standards/coding-standards.md (1)
140-143: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the Python target version to 3.13.
This doc still says Python 3.11+, which conflicts with the repo’s declared Python 3.13 target.
Suggested fix
-- Python 3.11+ with type hints everywhere +- Python 3.13 with type hints everywhere🤖 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 @.opencode/context/standards/coding-standards.md around lines 140 - 143, The coding standards doc is outdated: update the Python target version from Python 3.11+ to Python 3.13 so it matches the repo’s declared baseline. Adjust the bullet in the standards section that currently mentions the Python version, keeping the rest of the guidance unchanged.Source: Coding guidelines
apps/core/internal/web/handler.go (1)
1582-1625: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMove
Unsubscribeinto the stream writer
defer h.sseHub.Unsubscribe(tenantID, sub.ID)runs when the handler returns, but the SSE body writer runs later, sosub.Channelgets closed before the loop can consume from it and the connection drops after the first frame. Apply the same fix toAPICommandMissionEvents,APICommandHITLEvents, andAPICommandSessionEvents.🤖 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 `@apps/core/internal/web/handler.go` around lines 1582 - 1625, The SSE handlers unsubscribe too early because the deferred Unsubscribe runs when the handler returns, before the stream writer finishes consuming from the subscription channel. Move the unsubscribe cleanup into the SetBodyStreamWriter closure in APICommandChatEvents, and apply the same change to APICommandMissionEvents, APICommandHITLEvents, and APICommandSessionEvents so sub.Channel stays open for the lifetime of the stream.apps/ai/src/integrations/quickbooks.py (1)
86-173: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winCritical:
query_responsereferenced out of scope — always raisesNameError, silently masked into mock data.
_fetch_invoices()(lines 95-100) computesquery_responseas a local variable inside the nested function, but the function is never called. Line 101 then referencesquery_responsedirectly, which is undefined at that scope — this will raiseNameErroron every execution of the real (non-mock) API path. Because the entire block is wrapped by the broadexcept Exception as e:at lines 171-173, thisNameErroris silently caught and the function always falls back to_MOCK_DATA— meaning the production QuickBooks integration is completely non-functional whenever credentials ARE configured, and this will go unnoticed since it masquerades as a normal "mock fallback."The docstring
"Single HTTP attempt — retried by retry_with_backoff"also implies a retry decorator was intended but is missing from the shown code.🐛 Proposed fix
def _fetch_invoices() -> list: """Single HTTP attempt — retried by ``retry_with_backoff``.""" response = httpx.get(url, params=params, headers=headers, timeout=30.0) response.raise_for_status() data = response.json() - query_response = data.get("QueryResponse", {}) or {} - invoices = query_response.get("Invoice", []) + return data.get("QueryResponse", {}) or {} + + query_response = _fetch_invoices() + invoices = query_response.get("Invoice", [])🤖 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 `@apps/ai/src/integrations/quickbooks.py` around lines 86 - 173, `query_response` is being read outside the scope of `_fetch_invoices()`, so the real QuickBooks path always throws `NameError` and falls back to `_MOCK_DATA`. Move the invoice parsing logic into `_fetch_invoices()` or return the parsed `QueryResponse` data from that helper, then call `_fetch_invoices()` from the main QuickBooks flow before using `invoices`. Keep the existing `logger.error` fallback behavior, but ensure the symbols `_fetch_invoices`, `query_response`, and `invoices` are wired so the non-mock path can complete successfully.apps/ai/src/agents/cofounder/curator.py (1)
291-341: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPass
prior_confidenceinto the strategy update
slack_buttons._send_feedback_signalcallsupdate_strategy_confidence(...)withoutprior_confidence, so eachStrategyDeltais written as if the strategy started at0.0. Thread the current confidence through this call (or resolve it inside the helper) so the audit trail reflects the real accumulated value.🤖 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 `@apps/ai/src/agents/cofounder/curator.py` around lines 291 - 341, update_strategy_confidence is currently building StrategyDelta from a default prior_confidence of 0.0, so the audit trail can be wrong when callers do not pass the current value. Update the call path from slack_buttons._send_feedback_signal to thread the real strategy confidence into update_strategy_confidence, or have update_strategy_confidence resolve the current confidence before constructing StrategyDelta. Make sure the StrategyDelta prior_confidence and new_confidence fields reflect the accumulated strategy state rather than a reset baseline.
🟡 Minor comments (8)
.github/workflows/ci.yml-78-88 (1)
78-88: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStale/misleading instructions in the placeholder job.
The comment at Line 82 suggests enabling via
if: github.event_name == 'push', while the echoed message at Line 88 says to enable by "removing'if: false'" — but noif: falsecondition actually exists on this job. Since there's noif:guard at all, the job currently runs on every workflow trigger (harmlessly printing echo statements). The instructions should be made consistent to avoid confusing future maintainers trying to enable real integration tests.✏️ Suggested fix
integration-tests: name: Integration Tests runs-on: ubuntu-latest needs: [unit-tests] - # Enable by adding: if: github.event_name == 'push' (or remove this comment) + if: false # Flip to true (or add a real condition) once Docker-based integration tests are ready steps: - uses: actions/checkout@v4 - name: Docker compose integration run: | echo "Integration tests require Docker and are disabled by default." - echo "Enable by removing 'if: false' from ci.yml" + echo "Enable by removing/adjusting the 'if:' condition above."🤖 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 @.github/workflows/ci.yml around lines 78 - 88, The placeholder guidance in the integration-tests job is inconsistent and misleading: the job currently has no if guard, yet the comments mention enabling it via github.event_name == 'push' and removing if: false. Update the messaging in the integration-tests job so the comment and echoed instructions match the actual behavior, and reference the integration-tests step in ci.yml to keep the placeholder enablement instructions accurate for future maintainers.Makefile-408-420 (1)
408-420: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign
ci-fastwith the CI unit-test scope
make ci-fastskips four extra tests that theunit-testsjob runs, so it can pass locally while CI still fails. Match the workflow ignore list or add a note explaining the intentional divergence.🤖 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 `@Makefile` around lines 408 - 420, The ci-fast target is not aligned with the unit-tests workflow because it omits several tests that CI still runs. Update the ci-fast recipe to match the same pytest ignore list used by the unit-tests job, or explicitly document the intentional difference in the Makefile target. Use the ci-fast and Python unit tests steps in the Makefile as the place to keep the scopes consistent.docker-compose.showcase.yml-68-72 (1)
68-72: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winUpdate the Temporal healthcheck
temporalio/auto-setup:latestno longer shipstctl, so this healthcheck will go unhealthy on newer image tags. Switch to thetemporalCLI or a gRPC probe; pinning the image is only a short-term stopgap.🤖 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 `@docker-compose.showcase.yml` around lines 68 - 72, The healthcheck for the Temporal service still relies on tctl inside temporalio/auto-setup:latest, which is no longer available in newer image tags. Update the healthcheck in the service definition to use the temporal CLI or a gRPC-based probe instead, and keep the change localized to the healthcheck stanza so the rest of the compose setup remains unchanged.README.md-7-7 (1)
7-7: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReplace the empty badge target.
(#)is a no-op link and triggers MD042; use a real anchor or drop the wrapper.Suggested fix
-[](#) +🤖 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 `@README.md` at line 7, The Tests badge in the README uses a no-op link target, which should be replaced or removed. Update the badge markup so the link points to a real anchor or relevant destination, or drop the link wrapper entirely; keep the change localized to the README badge entry.Source: Linters/SAST tools
.opencode/context/domain/chat-flow.md-142-147 (1)
142-147: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the SSE domain count.
This section says “Three SSE event domains” but lists four (
chat,mission,hitl,session).Suggested fix
-**Three SSE event domains** (2026-06-28): +**Four SSE event domains** (2026-06-28):🤖 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 @.opencode/context/domain/chat-flow.md around lines 142 - 147, Update the SSE domain summary in the chat-flow docs so the count matches the listed domains: the section currently says “Three SSE event domains” but the bullets enumerate four domains (`chat`, `mission`, `hitl`, `session`). Adjust the heading text and keep the existing domain names and endpoint references intact in this documentation block..opencode/context/templates/architecture-overview.md-39-41 (1)
39-41: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCall out the blocked HITL tier here.
The ToolRegistry docs now use four routing outcomes (
auto,review,approve,blocked), so “3-tier routing” is stale.Suggested fix
-| **HITL Manager** | `src/hitl/` | 3-tier routing (auto/review/approve), guardrail-aware route_extended, confidence scoring | +| **HITL Manager** | `src/hitl/` | 4-tier routing (auto/review/approve/blocked), guardrail-aware route_extended, confidence scoring |🤖 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 @.opencode/context/templates/architecture-overview.md around lines 39 - 41, The HITL Manager description is outdated because it still says “3-tier routing” while the current routing outcomes include blocked. Update the architecture overview entry for the HITL Manager in the template so it explicitly lists the blocked tier alongside auto, review, and approve, and keep the wording aligned with the route_extended and confidence scoring behavior in src/hitl/..opencode/context/adr/001-sarthi-v4-architecture-evolution.md-195-200 (1)
195-200: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the Slack button count.
The text says five actions, but the list only names four. Either update the count or add the missing action.
Suggested fix
- - Slack buttons: routes five button actions: `acknowledge`, `dispute`, `show_breakdown`, `log_decision`. + - Slack buttons: routes four button actions: `acknowledge`, `dispute`, `show_breakdown`, `log_decision`.🤖 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 @.opencode/context/adr/001-sarthi-v4-architecture-evolution.md around lines 195 - 200, The Slack button action summary is inconsistent because the count says five actions while only four are listed. Update the wording in this ADR snippet to match the actual set of actions, or add the missing action name if there are truly five; check the `slack_buttons.py` action list and the `open_decision_modal`/`_send_feedback_signal()` flow so the documented count matches the implemented buttons.apps/ai/src/agents/tools/flag_churn_risk.py-37-39 (1)
37-39: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSubstring membership causes incorrect dedup.
segment_id not in existingperforms a substring check against the comma-joined string, not a segment-level membership test. E.g. withexisting = "seg10,seg2"andsegment_id = "seg1","seg1" in "seg10"is truthy, so a genuinely new segment is skipped. Split on commas and compare exact tokens.🐛 Proposed fix
- existing = state.churn_risk_users or "" - if segment_id not in existing: - state.churn_risk_users = (existing + "," + segment_id).strip(",") + existing = state.churn_risk_users or "" + segments = [s for s in existing.split(",") if s] + if segment_id not in segments: + segments.append(segment_id) + state.churn_risk_users = ",".join(segments) state.last_updated_by = "flag_churn_risk_tool" await update_mission_state(state)🤖 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 `@apps/ai/src/agents/tools/flag_churn_risk.py` around lines 37 - 39, The dedup check in flag_churn_risk.py is using substring matching on state.churn_risk_users, which can wrongly skip a new segment when its id appears inside another id. Update the logic in the churn risk update flow to parse the comma-separated state.churn_risk_users value into exact segment tokens, compare segment_id against those tokens, and only append when it is not already present. Keep the existing state.churn_risk_users field and the append behavior, but base the membership test on exact item equality rather than a raw string contains check.
🧹 Nitpick comments (11)
.github/workflows/ci.yml (1)
22-29: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTypecheck job never fails.
Both
go vet(Line 24) andmypy(Line 29) are piped through|| true, so the "Type Check" job always succeeds regardless of errors found. Givenunit-tests(Line 54) depends only ontypecheckandlintsucceeding, this gate provides no actual enforcement. If this is intentional for a soft rollout, consider at least surfacing failures via job summary/annotations so regressions aren't silently swallowed indefinitely.🤖 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 @.github/workflows/ci.yml around lines 22 - 29, The Type Check job is swallowing failures in the Go vet and Python mypy steps, so the job never blocks on type errors. Update the workflow steps in the Type Check job to remove the unconditional success behavior, or otherwise explicitly capture and propagate the exit status from go vet and mypy while still filtering gen/go output. Use the existing Go vet and Python mypy step names to locate the commands, and if you keep a soft rollout, add a visible failure signal such as job summary or annotations instead of `|| true`.apps/ai/src/integrations/slack_buttons.py (1)
102-130: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRedundant exception tuple and silent failure swallow the entire ACE loop.
ImportErroris a subclass ofException, soexcept (ImportError, Exception)is equivalent toexcept Exception. More importantly, all three ACE steps (score_from_button,update_strategy_confidence,curator.update) discard every error with a barepass. If Reflector/Curator/Graphiti wiring silently regresses, the feedback loop becomes a no-op with zero signal in logs. Consider logging at least a warning per failed step.♻️ Suggested change (Step 1 shown; apply to all three)
- except (ImportError, Exception): - pass + except Exception: + logger.warning("ACE step 1 (reflector scoring) failed", exc_info=True)🤖 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 `@apps/ai/src/integrations/slack_buttons.py` around lines 102 - 130, The ACE loop in slack_buttons.py is swallowing failures in the step handlers, and the repeated except (ImportError, Exception) is redundant because ImportError is already covered by Exception. Update the three try/except blocks around score_from_button, update_strategy_confidence, and Curator.update to catch the appropriate exception set once and emit a warning or error with step-specific context instead of passing silently, so failures are observable without breaking the flow.apps/ai/src/self_guardian/detector.py (2)
68-150: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winOnly the first matched deviation is ever reported per observation.
An observation that is simultaneously unauthorized (critical) and failed (info) will only surface the
UNAUTHORIZED_TOOLalert — the failure is silently dropped, andgenerate_report's deviation counts will not reflect it either. Consider accumulating all applicable deviations (or at minimum documenting this first-match priority explicitly) so downstream consumers aren't misled about severity/counts.🤖 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 `@apps/ai/src/self_guardian/detector.py` around lines 68 - 150, The detector in SelfGuardian only returns the first matching alert from the check chain, so later deviations like failed observations are dropped and never counted. Update the alert logic in the detector method that evaluates observation checks to accumulate all applicable deviations for a single observation, or explicitly document and encode the priority if first-match behavior is intended. Make sure the report generation path that consumes these alerts still sees every detected deviation, including UNAUTHORIZED_TOOL and STATE_CORRUPTION, rather than only the first one returned.
126-148: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
STATE_CORRUPTIONis a semantic mismatch for generic failure detection.Any unsuccessful observation is tagged
STATE_CORRUPTION, but that deviation type implies actual corrupted internal state, not a transient operational failure (e.g., a CRM timeout). This can mislead operators reading Self-Guardian alerts/reports about the real nature of the issue.🤖 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 `@apps/ai/src/self_guardian/detector.py` around lines 126 - 148, The failed-observation branch in `detector.py` is using `DeviationType.STATE_CORRUPTION` for any unsuccessful observation, which is semantically too strong for generic execution failures. Update the `SelfGuardianAlert` creation in the failed-observation check to use the deviation type that represents operational/action failure rather than corrupted state, and keep the existing message/description wording aligned with that classification. Use the `observation.success` check and the `SelfGuardianAlert` constructor in this block to locate and adjust the mapping.apps/ai/src/self_guardian/monitor.py (1)
26-44: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse
collections.deque(maxlen=...)instead oflist.pop(0)for O(1) eviction.Once the buffer is full, every new observation triggers an O(n)
pop(0)shift of up to 10,000 elements. Adeque(maxlen=_MAX_BUFFER_SIZE)evicts the oldest entry automatically in O(1) and removes the manual capacity check.♻️ Proposed refactor
+from collections import deque + class ObservationCollector: def __init__(self) -> None: - self._observations: list[AgentObservation] = [] + self._observations: deque[AgentObservation] = deque(maxlen=_MAX_BUFFER_SIZE) self._lock = threading.Lock() def record_observation(self, observation: AgentObservation) -> None: with self._lock: - if len(self._observations) >= _MAX_BUFFER_SIZE: - self._observations.pop(0) self._observations.append(observation)🤖 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 `@apps/ai/src/self_guardian/monitor.py` around lines 26 - 44, The observation buffer in the monitor is doing O(n) eviction with list.pop(0) in AgentObservationMonitor.record_observation, which should be replaced with an O(1) bounded deque. Update AgentObservationMonitor.__init__ to initialize _observations as collections.deque(maxlen=_MAX_BUFFER_SIZE), remove the manual length check and pop(0) logic in record_observation, and keep the append/logging behavior unchanged.docker-compose.llmops.yml (2)
90-95: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winHardcoded secrets in committed compose file.
NEXTAUTH_SECRET,SALT, and the DB password embedded inDATABASE_URLare hardcoded plaintext. Checkov also flags this (CKV_SECRET_4). Even for local/dev tooling, prefer sourcing these from a.envfile via${VAR}interpolation so no secret-like value is committed, even a placeholder one.🔒 Suggested fix
environment: - DATABASE_URL: postgresql://iterateswarm:iterateswarm@postgres:5432/iterateswarm - NEXTAUTH_SECRET: dev-secret-do-not-use-in-prod + DATABASE_URL: postgresql://${POSTGRES_USER:-iterateswarm}:${POSTGRES_PASSWORD:-iterateswarm}`@postgres`:5432/${POSTGRES_DB:-iterateswarm} + NEXTAUTH_SECRET: ${LANGFUSE_NEXTAUTH_SECRET} NEXTAUTH_URL: http://localhost:4000 - SALT: dev-salt-do-not-use-in-prod + SALT: ${LANGFUSE_SALT}Based on learnings, "Use feature branches, conventional commits, never commit to
main, keep secrets in.env, and regenerate sqlc code after schema changes."Also applies to: 114-119
🤖 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 `@docker-compose.llmops.yml` around lines 90 - 95, The compose service environment block contains committed secret-like values; update the configuration in the Docker Compose service definition to source `NEXTAUTH_SECRET`, `SALT`, and the password portion of `DATABASE_URL` from environment variables instead of hardcoded plaintext. Use `${VAR}` interpolation in the existing environment section so the values come from a local `.env` file or runtime environment, and keep the change consistent anywhere the same compose service variables are repeated.Sources: Learnings, Linters/SAST tools
5-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffDuplicated postgres/redis stack across profiles blocks running tiers together.
docker-compose.dev.yml,docker-compose.llmops.yml, anddocker-compose.showcase.ymleach fully redefinepostgres/rediswith identical host ports (5433,6379) and the sameiterateswarm-netnetwork name. If a developer starts more than one profile at once (e.g., dev + llmops to incrementally add services, as the "adds" framing in the PR description implies), the host port bindings will collide.Consider using Compose's file-merging (
docker compose -f docker-compose.dev.yml -f docker-compose.llmops.yml up) orextends, so llmops/showcase only declare their additional services and reuse the base postgres/redis definitions.Also applies to: 27-39
🤖 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 `@docker-compose.llmops.yml` around lines 5 - 22, The postgres/redis stack is being duplicated across profile compose files, which causes host port and network conflicts when profiles are run together. Update the llmops compose definitions so they reuse the base `postgres` and `redis` services instead of redeclaring them, using Compose file merging or `extends`, and keep `docker-compose.llmops.yml` focused on only the additional llmops services. Ensure the shared service names and bindings remain defined in one place so `postgres` and `redis` can be combined safely with dev/showcase.apps/core/internal/web/templates/partials/command_operating_layer.html (1)
58-61: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
ActiveAgentRoleslikely renders raw JSON text instead of individual role badges.Per the
handler.gocontext snippet,ActiveAgentRolesis populated fromactive_agent_roles::text— a serialized JSON array cast to a string (e.g.["Finance specialist","Ops specialist"]). Rendering it directly in a single pill (Line 60) would show the raw JSON syntax to founders instead of one badge per role, unlike the multi-badge pattern used forSuggestedActionsin the siblingcommand_alert_lineage.htmltemplate.♻️ Proposed fix (requires handler to pass a slice)
- {{if .ActiveAgentRoles}} - <div class="flex flex-wrap gap-1.5 mt-1 ml-5.5"> - <span class="text-[10px] px-2 py-0.5 rounded-full bg-blue-900/30 text-blue-400">{{.ActiveAgentRoles}}</span> - </div> + {{if .ActiveAgentRoles}} + <div class="flex flex-wrap gap-1.5 mt-1 ml-5.5"> + {{range .ActiveAgentRoles}} + <span class="text-[10px] px-2 py-0.5 rounded-full bg-blue-900/30 text-blue-400">{{.}}</span> + {{end}} + </div>🤖 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 `@apps/core/internal/web/templates/partials/command_operating_layer.html` around lines 58 - 61, ActiveAgentRoles is being rendered as a single raw JSON string instead of separate role badges. Update the data flow so the handler parses active_agent_roles into a slice before passing it to command_operating_layer.html, then change the template to iterate over .ActiveAgentRoles like the multi-badge pattern used for SuggestedActions in command_alert_lineage.html. Keep the existing ActiveAgentRoles symbol as the entry point for the fix and replace the single pill rendering with one badge per role.apps/core/internal/web/handler.go (1)
1627-1763: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated SSE streaming boilerplate into a shared helper.
APICommandMissionEvents,APICommandHITLEvents, andAPICommandSessionEventsare near-identical copies ofAPICommandChatEvents, differing only in the event-type filter and theconnectedmessage text. Consolidating into one helper reduces the maintenance surface (e.g., the unsubscribe fix above only needs to land once).♻️ Suggested helper
func (h *Handler) streamSSE(c *fiber.Ctx, eventType, connectedText string) error { tenantID := c.Query("tenant_id", "default") sub := h.sseHub.Subscribe(tenantID, eventType) c.Set("Content-Type", "text/event-stream") c.Set("Cache-Control", "no-cache") c.Set("Connection", "keep-alive") done := c.Context().Done() c.Context().SetBodyStreamWriter(func(w *bufio.Writer) { defer h.sseHub.Unsubscribe(tenantID, sub.ID) defer func() { recover() }() fmt.Fprintf(w, "event: connected\ndata: {\"status\":\"connected\",\"text\":%q}\n\n", connectedText) w.Flush() heartbeat := time.NewTicker(30 * time.Second) defer heartbeat.Stop() for { select { case <-heartbeat.C: if _, err := fmt.Fprintf(w, "event: heartbeat\ndata: {}\n\n"); err != nil { return } w.Flush() case msgBytes, ok := <-sub.Channel: if !ok { return } if _, err := fmt.Fprintf(w, "%s", msgBytes); err != nil { return } w.Flush() case <-done: return } } }) return nil }Each handler then becomes e.g.
return h.streamSSE(c, "mission-update", "Connected to mission events").🤖 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 `@apps/core/internal/web/handler.go` around lines 1627 - 1763, The SSE handlers APICommandMissionEvents, APICommandHITLEvents, and APICommandSessionEvents duplicate the same streaming setup and should be refactored into a shared helper like streamSSE to reduce maintenance overhead. Move the common tenant lookup, subscription, headers, body stream writer, heartbeat loop, and unsubscribe handling into the helper, parameterized by eventType and connected message text, then have each handler call it with its specific values.apps/ai/src/schemas/control_plane.py (1)
56-80: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
approval_statelacksLiteraltyping unlike siblingescalation_tier.
AuditEvent.approval_stateis documented as "auto"/"review"/"approve"/"blocked" but typedstr | None, while the equivalentescalation_tierfield onAgentRegistrationcorrectly usesLiteral[...]. This is a minor consistency/validation gap in the audit trail schema.♻️ Proposed fix
- approval_state: str | None = None + approval_state: Literal["auto", "review", "approve", "blocked"] | None = None🤖 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 `@apps/ai/src/schemas/control_plane.py` around lines 56 - 80, Update AuditEvent in control_plane.py so approval_state uses a Literal type matching the documented values ("auto", "review", "approve", "blocked") instead of a plain str | None, keeping it consistent with the sibling escalation_tier field on AgentRegistration. Adjust the type annotation and any related schema expectations in AuditEvent so the model enforces the allowed states while still allowing None if intended.apps/ai/tests/unit/test_qa_agent.py (1)
104-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNarrowed LLM-detection checks may miss real LLM integrations.
Restricting to
"from langchain"/"import llm"/"openai"/"langchain"substrings avoids the docstring false-positive, but now misses other LLM SDK imports (e.g.anthropic,from src.llm import ...without the literal "import llm" phrasing, or a helper call likecall_model()). This weakens the test's actual guarantee that the QA graph stub has no LLM integration. Also, line 106 ("from langchain") is redundant with line 109 ("langchain"), which already subsumes it.♻️ Proposed fix
- # Check for actual LLM integration (imports/calls), not substring "llm" - # which matches the docstring and internal config.llm module. - assert "from langchain" not in content.lower() - assert "import llm" not in content.lower() - assert "openai" not in content.lower() - assert "langchain" not in content.lower() + # Check for actual LLM integration (imports/calls), not the bare + # substring "llm" which also matches the docstring/config.llm module. + import re + assert not re.search(r"^\s*(from|import)\s+\S*llm\S*", content, re.MULTILINE | re.IGNORECASE) + assert not re.search(r"^\s*(from|import)\s+(openai|anthropic|langchain)\b", content, re.MULTILINE | re.IGNORECASE)As per path instructions for
apps/ai/tests/**/*.py: "Run and maintain the Python test suite underapps/ai/tests/."🤖 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 `@apps/ai/tests/unit/test_qa_agent.py` around lines 104 - 109, The LLM-detection assertion in test_qa_agent.py is too narrow and can miss real integrations, so broaden the check in the test around the QA graph stub to detect more SDK/import patterns and LLM helper calls rather than only the current literal substrings. Update the assertions near the existing content checks to cover additional common providers and wrapper calls used by the codebase, and remove the redundant "from langchain" check since the broader "langchain" match already covers it. Keep the test focused on verifying that the stubbed graph has no actual LLM integration by using symbols like the QA graph/test content scan in this test module.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 @.github/workflows/ci.yml:
- Around line 14-88: The workflow jobs are using the default GITHUB_TOKEN
permissions and `actions/checkout` is persisting credentials, which is broader
access than needed. Add an explicit least-privilege job-level `permissions`
block for each job in `ci.yml`, keep `workflow-lint` overridden with
`pull-requests: write` for `reviewdog/action-actionlint`, and set
`persist-credentials: false` on every `actions/checkout@v4` step in the
`typecheck`, `lint`, `unit-tests`, `workflow-lint`, and `integration-tests`
jobs.
In `@apps/ai/src/agents/cofounder/curator.py`:
- Around line 436-444: The _file_audit_write fallback currently writes audit
records to a hardcoded /tmp/strategy_audit.jsonl path, which is insecure and not
durable. Update _file_audit_write to resolve the audit file location from a
configurable environment variable instead of a fixed /tmp value, and default it
to a restricted-permission directory suitable for compliance logs. Ensure the
path is created securely with appropriate permissions before appending, and
consider adding basic concurrency-safe write handling so multiple writers do not
corrupt the audit trail.
In `@apps/ai/src/agents/tools/draft_investor_update.py`:
- Line 97: The audit logging in draft_investor_update is calling the wrong API
and not awaiting the async write. Update both audit paths in the
draft_investor_update flow to use AuditLogger.log_event(...) instead of
_AUDIT.log(...), and ensure each call is awaited so the coroutine actually runs.
Check the blocked-path and success-path audit records around the existing
audit_event handling so both are recorded correctly.
In `@apps/ai/src/control_plane/audit.py`:
- Around line 20-24: The audit logger interface is mismatched: callers use
_AUDIT.log(...) but AuditLogger only exposes log_event(...), which will fail at
runtime. Update the audit path to call await log_event(...) consistently, or add
a log() shim on AuditLogger that forwards to log_event so existing call sites
continue to work. Use the AuditLogger class and its log_event method to locate
the change and keep the async behavior intact.
In `@apps/ai/src/control_plane/policy.py`:
- Around line 74-91: The degraded-health branch in policy evaluation is setting
blocked_reason too early, which hides tool permission failures and incorrectly
clears allowed models. Update the decision logic in the policy function around
the degraded-health handling so “degraded” only triggers requires_human_approval
and does not populate blocked_reason unless there is an actual block, then let
the tool check record tool_not_allowed when applicable. Also adjust the
allowed_models assignment so it only becomes empty when a real blocked_reason is
present, keeping degraded cases aligned with the PolicyDecision.blocked_reason
contract.
- Around line 21-99: PolicyEngine.evaluate currently ignores
agent.escalation_tier, so agents marked review or approve can still pass without
human involvement. Update evaluate in PolicyEngine to incorporate
escalation_tier alongside the existing health, classification, external-facing,
and tool checks: treat blocked as a hard deny, set requires_human_approval for
review and approve tiers as intended, and ensure the final PolicyDecision
reflects the tier even when no other rule triggers.
In `@apps/ai/src/integrations/slack_client.py`:
- Around line 42-67: The Slack Socket Mode setup is using async patterns with a
synchronous SocketModeClient. Update the listener in slack_client.py around
_process and the connection flow so connect() is called without await, and
replace both req.ack() calls with
client.send_socket_mode_response(SocketModeResponse(envelope_id=req.envelope_id))
using the same client instance. If you want to keep async behavior, switch to
the aiohttp Socket Mode client instead of this sync client.
In `@apps/ai/src/risk/output_risk.py`:
- Line 45: The OC009 logic in output_risk.py is inverted because
_MISSING_APPROVAL is included in _ALL_OUTPUT_PATTERNS and then also checked
later in the disclaimer branch. Remove _MISSING_APPROVAL from
_ALL_OUTPUT_PATTERNS so the scan in the output risk evaluation only covers real
risk patterns, and keep OC009 emission solely in the has_draft_disclaimer check
inside the output risk function that builds the risk flags.
In `@apps/ai/src/schemas/control_plane.py`:
- Around line 39-53: Update the schema types in control_plane.py so
PolicyDecision.data_classification and AgentRegistration.data_classification use
a Literal union of the fixed classification values instead of plain str. Keep
the allowed values aligned with PolicyEngine and _RESTRICTED_CLASSIFICATIONS so
invalid typos are rejected at validation time. Make the change in the
PolicyDecision and AgentRegistration model definitions, and ensure any related
docstrings/types still reflect the same fixed set.
In `@apps/ai/src/self_guardian/detector.py`:
- Around line 72-77: The logging in detector.py is still exposing sensitive
observation data verbatim. Update the unauthorized-tool handling in the
self_guardian detector path so any use of observation fields like action,
target_entity, and error_message is redacted or truncated before being written
into the warning log, description, or suggested_action. Keep the existing logic
in the detector/observation flow, but sanitize those fields at the point where
they are formatted or copied so internal or credential-like values are not
emitted.
In `@apps/ai/src/session/brief_generator.py`:
- Around line 29-38: The synchronous chat_completion call inside
generate_prepared_brief is blocking the event loop and has no timeout. Move the
brief generation work off the event loop (for example by wrapping
chat_completion in an async executor/helper) and add a bounded timeout so
update_mission_state is not stalled by a slow or hanging LLM request. Keep the
fix localized to generate_prepared_brief and the chat_completion invocation so
the shared mission-state write path stays responsive.
- Around line 18-49: In generate_prepared_brief, the MissionState write is using
a stale full object and can overwrite unrelated concurrent changes through
update_mission_state’s upsert behavior. Change this flow so only the
brief-related fields are persisted here, or make the save conditional/versioned
to detect concurrent updates. Keep the fix localized to generate_prepared_brief
and the update_mission_state path that performs the ON CONFLICT DO UPDATE write.
In `@apps/ai/src/session/mission_state.py`:
- Around line 182-198: update_mission_state is overwriting existing
explainability data with None whenever generate_brief is true and callers omit
update_reason/changed_fields. In update_mission_state, only assign
state.last_update_reason and state.last_changed_fields when those arguments are
explicitly provided, and otherwise preserve any existing values already on the
MissionState instance from get_mission_state or caller-set fields. Keep the
behavior localized to update_mission_state so plain await
update_mission_state(state) does not clear audit-trail history.
- Around line 210-217: Add `policy_state` support consistently in the mission
state persistence path: either migrate the `mission_states` table to include the
missing column or remove it from `get_mission_state` and `update_mission_state`
so they no longer read/write it. Check the `mission_state.py` query builders
around `get_mission_state`/`update_mission_state`, since the current
INSERT/SELECT logic references `policy_state` even though the schema drift
migration only covers the other fields. Make the schema and the two session
methods match so Postgres no longer throws a missing-column error.
- Around line 277-283: The mission state save/query path in mission_state.py now
references pending_decisions and policy_state, but the session schema migrations
never add those columns, so the insert/select will fail. Update the session
layer migration set, including 001_session_layer.sql and any later migration
definitions, to add the missing mission_states columns in the same order/types
expected by MissionStateRepository’s save/load logic so the database schema
matches the fields used in mission_state.py.
- Around line 168-175: The MissionState read path is returning raw serialized
strings for structured fields instead of the declared list/dict types. In the
MissionState construction logic, deserialize pending_decisions,
last_changed_fields, active_agent_roles, and policy_state before passing them
into MissionState, or apply a JSON codec at the connection level so the
dataclass receives decoded values. Use the MissionState read/row-mapping code to
locate the fix.
In `@docker-compose.showcase.yml`:
- Around line 131-146: Update the qdrant service in docker-compose.showcase.yml
so the persistent volume is mounted to Qdrant’s actual storage path instead of
/var/lib/qdrant. Use the qdrant service definition and its volumes entry to
point sarthi-showcase-qdrant-data at /qdrant/storage, keeping the rest of the
service configuration unchanged.
---
Outside diff comments:
In @.opencode/context/standards/coding-standards.md:
- Around line 140-143: The coding standards doc is outdated: update the Python
target version from Python 3.11+ to Python 3.13 so it matches the repo’s
declared baseline. Adjust the bullet in the standards section that currently
mentions the Python version, keeping the rest of the guidance unchanged.
In `@apps/ai/src/agents/cofounder/curator.py`:
- Around line 291-341: update_strategy_confidence is currently building
StrategyDelta from a default prior_confidence of 0.0, so the audit trail can be
wrong when callers do not pass the current value. Update the call path from
slack_buttons._send_feedback_signal to thread the real strategy confidence into
update_strategy_confidence, or have update_strategy_confidence resolve the
current confidence before constructing StrategyDelta. Make sure the
StrategyDelta prior_confidence and new_confidence fields reflect the accumulated
strategy state rather than a reset baseline.
In `@apps/ai/src/integrations/quickbooks.py`:
- Around line 86-173: `query_response` is being read outside the scope of
`_fetch_invoices()`, so the real QuickBooks path always throws `NameError` and
falls back to `_MOCK_DATA`. Move the invoice parsing logic into
`_fetch_invoices()` or return the parsed `QueryResponse` data from that helper,
then call `_fetch_invoices()` from the main QuickBooks flow before using
`invoices`. Keep the existing `logger.error` fallback behavior, but ensure the
symbols `_fetch_invoices`, `query_response`, and `invoices` are wired so the
non-mock path can complete successfully.
In `@apps/core/internal/web/handler.go`:
- Around line 1582-1625: The SSE handlers unsubscribe too early because the
deferred Unsubscribe runs when the handler returns, before the stream writer
finishes consuming from the subscription channel. Move the unsubscribe cleanup
into the SetBodyStreamWriter closure in APICommandChatEvents, and apply the same
change to APICommandMissionEvents, APICommandHITLEvents, and
APICommandSessionEvents so sub.Channel stays open for the lifetime of the
stream.
---
Minor comments:
In @.github/workflows/ci.yml:
- Around line 78-88: The placeholder guidance in the integration-tests job is
inconsistent and misleading: the job currently has no if guard, yet the comments
mention enabling it via github.event_name == 'push' and removing if: false.
Update the messaging in the integration-tests job so the comment and echoed
instructions match the actual behavior, and reference the integration-tests step
in ci.yml to keep the placeholder enablement instructions accurate for future
maintainers.
In @.opencode/context/adr/001-sarthi-v4-architecture-evolution.md:
- Around line 195-200: The Slack button action summary is inconsistent because
the count says five actions while only four are listed. Update the wording in
this ADR snippet to match the actual set of actions, or add the missing action
name if there are truly five; check the `slack_buttons.py` action list and the
`open_decision_modal`/`_send_feedback_signal()` flow so the documented count
matches the implemented buttons.
In @.opencode/context/domain/chat-flow.md:
- Around line 142-147: Update the SSE domain summary in the chat-flow docs so
the count matches the listed domains: the section currently says “Three SSE
event domains” but the bullets enumerate four domains (`chat`, `mission`,
`hitl`, `session`). Adjust the heading text and keep the existing domain names
and endpoint references intact in this documentation block.
In @.opencode/context/templates/architecture-overview.md:
- Around line 39-41: The HITL Manager description is outdated because it still
says “3-tier routing” while the current routing outcomes include blocked. Update
the architecture overview entry for the HITL Manager in the template so it
explicitly lists the blocked tier alongside auto, review, and approve, and keep
the wording aligned with the route_extended and confidence scoring behavior in
src/hitl/.
In `@apps/ai/src/agents/tools/flag_churn_risk.py`:
- Around line 37-39: The dedup check in flag_churn_risk.py is using substring
matching on state.churn_risk_users, which can wrongly skip a new segment when
its id appears inside another id. Update the logic in the churn risk update flow
to parse the comma-separated state.churn_risk_users value into exact segment
tokens, compare segment_id against those tokens, and only append when it is not
already present. Keep the existing state.churn_risk_users field and the append
behavior, but base the membership test on exact item equality rather than a raw
string contains check.
In `@docker-compose.showcase.yml`:
- Around line 68-72: The healthcheck for the Temporal service still relies on
tctl inside temporalio/auto-setup:latest, which is no longer available in newer
image tags. Update the healthcheck in the service definition to use the temporal
CLI or a gRPC-based probe instead, and keep the change localized to the
healthcheck stanza so the rest of the compose setup remains unchanged.
In `@Makefile`:
- Around line 408-420: The ci-fast target is not aligned with the unit-tests
workflow because it omits several tests that CI still runs. Update the ci-fast
recipe to match the same pytest ignore list used by the unit-tests job, or
explicitly document the intentional difference in the Makefile target. Use the
ci-fast and Python unit tests steps in the Makefile as the place to keep the
scopes consistent.
In `@README.md`:
- Line 7: The Tests badge in the README uses a no-op link target, which should
be replaced or removed. Update the badge markup so the link points to a real
anchor or relevant destination, or drop the link wrapper entirely; keep the
change localized to the README badge entry.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 22-29: The Type Check job is swallowing failures in the Go vet and
Python mypy steps, so the job never blocks on type errors. Update the workflow
steps in the Type Check job to remove the unconditional success behavior, or
otherwise explicitly capture and propagate the exit status from go vet and mypy
while still filtering gen/go output. Use the existing Go vet and Python mypy
step names to locate the commands, and if you keep a soft rollout, add a visible
failure signal such as job summary or annotations instead of `|| true`.
In `@apps/ai/src/integrations/slack_buttons.py`:
- Around line 102-130: The ACE loop in slack_buttons.py is swallowing failures
in the step handlers, and the repeated except (ImportError, Exception) is
redundant because ImportError is already covered by Exception. Update the three
try/except blocks around score_from_button, update_strategy_confidence, and
Curator.update to catch the appropriate exception set once and emit a warning or
error with step-specific context instead of passing silently, so failures are
observable without breaking the flow.
In `@apps/ai/src/schemas/control_plane.py`:
- Around line 56-80: Update AuditEvent in control_plane.py so approval_state
uses a Literal type matching the documented values ("auto", "review", "approve",
"blocked") instead of a plain str | None, keeping it consistent with the sibling
escalation_tier field on AgentRegistration. Adjust the type annotation and any
related schema expectations in AuditEvent so the model enforces the allowed
states while still allowing None if intended.
In `@apps/ai/src/self_guardian/detector.py`:
- Around line 68-150: The detector in SelfGuardian only returns the first
matching alert from the check chain, so later deviations like failed
observations are dropped and never counted. Update the alert logic in the
detector method that evaluates observation checks to accumulate all applicable
deviations for a single observation, or explicitly document and encode the
priority if first-match behavior is intended. Make sure the report generation
path that consumes these alerts still sees every detected deviation, including
UNAUTHORIZED_TOOL and STATE_CORRUPTION, rather than only the first one returned.
- Around line 126-148: The failed-observation branch in `detector.py` is using
`DeviationType.STATE_CORRUPTION` for any unsuccessful observation, which is
semantically too strong for generic execution failures. Update the
`SelfGuardianAlert` creation in the failed-observation check to use the
deviation type that represents operational/action failure rather than corrupted
state, and keep the existing message/description wording aligned with that
classification. Use the `observation.success` check and the `SelfGuardianAlert`
constructor in this block to locate and adjust the mapping.
In `@apps/ai/src/self_guardian/monitor.py`:
- Around line 26-44: The observation buffer in the monitor is doing O(n)
eviction with list.pop(0) in AgentObservationMonitor.record_observation, which
should be replaced with an O(1) bounded deque. Update
AgentObservationMonitor.__init__ to initialize _observations as
collections.deque(maxlen=_MAX_BUFFER_SIZE), remove the manual length check and
pop(0) logic in record_observation, and keep the append/logging behavior
unchanged.
In `@apps/ai/tests/unit/test_qa_agent.py`:
- Around line 104-109: The LLM-detection assertion in test_qa_agent.py is too
narrow and can miss real integrations, so broaden the check in the test around
the QA graph stub to detect more SDK/import patterns and LLM helper calls rather
than only the current literal substrings. Update the assertions near the
existing content checks to cover additional common providers and wrapper calls
used by the codebase, and remove the redundant "from langchain" check since the
broader "langchain" match already covers it. Keep the test focused on verifying
that the stubbed graph has no actual LLM integration by using symbols like the
QA graph/test content scan in this test module.
In `@apps/core/internal/web/handler.go`:
- Around line 1627-1763: The SSE handlers APICommandMissionEvents,
APICommandHITLEvents, and APICommandSessionEvents duplicate the same streaming
setup and should be refactored into a shared helper like streamSSE to reduce
maintenance overhead. Move the common tenant lookup, subscription, headers, body
stream writer, heartbeat loop, and unsubscribe handling into the helper,
parameterized by eventType and connected message text, then have each handler
call it with its specific values.
In `@apps/core/internal/web/templates/partials/command_operating_layer.html`:
- Around line 58-61: ActiveAgentRoles is being rendered as a single raw JSON
string instead of separate role badges. Update the data flow so the handler
parses active_agent_roles into a slice before passing it to
command_operating_layer.html, then change the template to iterate over
.ActiveAgentRoles like the multi-badge pattern used for SuggestedActions in
command_alert_lineage.html. Keep the existing ActiveAgentRoles symbol as the
entry point for the fix and replace the single pill rendering with one badge per
role.
In `@docker-compose.llmops.yml`:
- Around line 90-95: The compose service environment block contains committed
secret-like values; update the configuration in the Docker Compose service
definition to source `NEXTAUTH_SECRET`, `SALT`, and the password portion of
`DATABASE_URL` from environment variables instead of hardcoded plaintext. Use
`${VAR}` interpolation in the existing environment section so the values come
from a local `.env` file or runtime environment, and keep the change consistent
anywhere the same compose service variables are repeated.
- Around line 5-22: The postgres/redis stack is being duplicated across profile
compose files, which causes host port and network conflicts when profiles are
run together. Update the llmops compose definitions so they reuse the base
`postgres` and `redis` services instead of redeclaring them, using Compose file
merging or `extends`, and keep `docker-compose.llmops.yml` focused on only the
additional llmops services. Ensure the shared service names and bindings remain
defined in one place so `postgres` and `redis` can be combined safely with
dev/showcase.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7d9c4481-ce7b-45ff-a5f1-acba67ac5be7
⛔ Files ignored due to path filters (1)
apps/ai/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (56)
.githooks/pre-push.github/workflows/ci.yml.gitignore.opencode/context/adr/001-sarthi-v4-architecture-evolution.md.opencode/context/domain/architecture.md.opencode/context/domain/chat-flow.md.opencode/context/standards/coding-standards.md.opencode/context/templates/architecture-overview.mdAGENTS.mdMakefileREADME.mdapps/ai/infrastructure/migrations/004_control_plane_audit.sqlapps/ai/pyproject.tomlapps/ai/src/agents/authority_manifest.pyapps/ai/src/agents/cofounder/curator.pyapps/ai/src/agents/tools/__init__.pyapps/ai/src/agents/tools/draft_investor_update.pyapps/ai/src/agents/tools/flag_churn_risk.pyapps/ai/src/agents/tools/pause_payment_retry.pyapps/ai/src/agents/tools/schedule_customer_checkin.pyapps/ai/src/control_plane/__init__.pyapps/ai/src/control_plane/audit.pyapps/ai/src/control_plane/policy.pyapps/ai/src/control_plane/registry.pyapps/ai/src/hitl/manager.pyapps/ai/src/integrations/hubspot.pyapps/ai/src/integrations/quickbooks.pyapps/ai/src/integrations/slack.pyapps/ai/src/integrations/slack_buttons.pyapps/ai/src/integrations/slack_client.pyapps/ai/src/risk/__init__.pyapps/ai/src/risk/output_risk.pyapps/ai/src/risk/prompt_risk.pyapps/ai/src/schemas/__init__.pyapps/ai/src/schemas/control_plane.pyapps/ai/src/schemas/guardian.pyapps/ai/src/schemas/self_guardian.pyapps/ai/src/self_guardian/__init__.pyapps/ai/src/self_guardian/detector.pyapps/ai/src/self_guardian/monitor.pyapps/ai/src/session/brief_generator.pyapps/ai/src/session/mission_state.pyapps/ai/tests/unit/test_agentic_comprehensive.pyapps/ai/tests/unit/test_control_plane.pyapps/ai/tests/unit/test_qa_agent.pyapps/ai/tests/unit/test_self_guardian.pyapps/core/hitl_queue_test.goapps/core/internal/db/migrations/005_mission_state_explainability.sqlapps/core/internal/web/command_center_test.goapps/core/internal/web/handler.goapps/core/internal/web/sse_hub.goapps/core/internal/web/templates/partials/command_alert_lineage.htmlapps/core/internal/web/templates/partials/command_operating_layer.htmldocker-compose.dev.ymldocker-compose.llmops.ymldocker-compose.showcase.yml
💤 Files with no reviewable changes (1)
- apps/ai/src/integrations/hubspot.py
| typecheck: | ||
| name: Type Check | ||
| runs-on: ubuntu-latest | ||
| defaults: | ||
| run: | ||
| working-directory: apps/core | ||
| steps: | ||
| - uses: actions/checkout@v6 | ||
|
|
||
| - name: Setup Go | ||
| uses: actions/setup-go@v5 | ||
| - uses: actions/checkout@v4 | ||
| - uses: actions/setup-go@v5 | ||
| with: | ||
| go-version: ${{ env.GO_VERSION }} | ||
| cache-dependency-path: apps/core/go.sum | ||
|
|
||
| - name: Check formatting | ||
| go-version: "1.24" | ||
| - name: Go vet | ||
| run: | | ||
| unformatted=$(gofmt -l .) | ||
| if [ -n "$unformatted" ]; then | ||
| echo "Unformatted files:" | ||
| echo "$unformatted" | ||
| exit 1 | ||
| fi | ||
|
|
||
| - name: Vet | ||
| run: go vet ./... | ||
|
|
||
| - name: Build | ||
| run: go build ./... | ||
|
|
||
| - name: Test (unit – no Docker) | ||
| run: go test ./internal/web/... ./internal/agents/... ./internal/workflow/... -v -count=1 -timeout=60s | ||
| cd apps/core && go vet ./... 2>&1 | grep -v 'gen/go' || true | ||
| - name: Set up uv | ||
| uses: astral-sh/setup-uv@v5 | ||
| - name: Python mypy | ||
| run: | | ||
| cd apps/ai && uv sync --group dev && uv run mypy src/ --ignore-missing-imports || true | ||
|
|
||
| python: | ||
| name: Python – Lint & Unit Tests | ||
| lint: | ||
| name: Lint & Format | ||
| runs-on: ubuntu-latest | ||
| defaults: | ||
| run: | ||
| working-directory: apps/ai | ||
| steps: | ||
| - uses: actions/checkout@v6 | ||
|
|
||
| - name: Setup Python | ||
| uses: actions/setup-python@v6 | ||
| - uses: actions/checkout@v4 | ||
| - uses: actions/setup-go@v5 | ||
| with: | ||
| python-version-file: ${{ env.PYTHON_VERSION_FILE }} | ||
| go-version: "1.24" | ||
| - name: Go fmt | ||
| run: | | ||
| cd apps/core && test -z "$(go fmt ./...)" || (echo "Go files not formatted:"; go fmt ./...; exit 1) | ||
| - name: Set up uv | ||
| uses: astral-sh/setup-uv@v5 | ||
| - name: Ruff check | ||
| run: | | ||
| cd apps/ai && uv run ruff check src/ || echo "ruff not configured, skipping" | ||
| - name: Ruff format | ||
| run: | | ||
| cd apps/ai && uv run ruff format --check src/ || echo "ruff format not configured, skipping" | ||
|
|
||
| - name: Install uv | ||
| uses: astral-sh/setup-uv@v6 | ||
| unit-tests: | ||
| name: Unit Tests | ||
| runs-on: ubuntu-latest | ||
| needs: [typecheck, lint] | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: actions/setup-go@v5 | ||
| with: | ||
| enable-cache: true | ||
| cache-dependency-glob: "apps/ai/uv.lock" | ||
|
|
||
| - name: Sync dependencies | ||
| run: uv sync --locked --all-extras --dev | ||
| go-version: "1.24" | ||
| - name: Go test | ||
| run: | | ||
| cd apps/core && go test ./... -count=1 -timeout=5m | ||
| - name: Set up uv | ||
| uses: astral-sh/setup-uv@v5 | ||
| - name: Python unit tests | ||
| run: | | ||
| cd apps/ai && uv run pytest tests/unit/ -q --timeout=60 --ignore=tests/unit/test_guardian_schemas.py --ignore=tests/unit/test_curator_graphiti.py -x | ||
|
|
||
| - name: Lint with ruff | ||
| run: uv run ruff check src/ tests/ | ||
| workflow-lint: | ||
| name: Workflow Lint | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: reviewdog/action-actionlint@v1 | ||
| with: | ||
| github_token: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
||
| - name: Run unit tests | ||
| run: uv run pytest tests/unit/ -v --timeout=60 -x -q | ||
| integration-tests: | ||
| name: Integration Tests | ||
| runs-on: ubuntu-latest | ||
| needs: [unit-tests] | ||
| # Enable by adding: if: github.event_name == 'push' (or remove this comment) | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Docker compose integration | ||
| run: | | ||
| echo "Integration tests require Docker and are disabled by default." | ||
| echo "Enable by removing 'if: false' from ci.yml" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Add an explicit least-privilege permissions block and disable credential persistence.
None of the five jobs declare a permissions key, so the GITHUB_TOKEN falls back to the repo/org default (which can be permissive read/write). Combined with actions/checkout steps (Lines 18, 35, 56, 73, 84) that leave persist-credentials: true (the default), any dependency pulled in by uv sync, pip, ruff, or pytest in these jobs gets access to a token that can potentially write to the repo. None of these jobs need write access or a persisted git credential.
🔒 Proposed fix
+permissions:
+ contents: read
+
jobs:
typecheck:
name: Type Check
runs-on: ubuntu-latest
steps:
- - uses: actions/checkout@v4
+ - uses: actions/checkout@v4
+ with:
+ persist-credentials: falseApply the same persist-credentials: false to the checkout steps in lint, unit-tests, workflow-lint, and integration-tests. Give workflow-lint pull-requests: write (needed by reviewdog to post inline comments) as a job-level override.
You should therefore make sure that the GITHUB_TOKEN is granted the minimum required permissions. It's good security practice to set the default permission for the GITHUB_TOKEN to read access only for repository contents. the persisted token is doing nothing useful, and it is potentially a problem. Any third party action or package in that job now has access to your token. Depending on your permissions, that could be an elevated token that can open pull requests, write to your repo, or worse.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 18-18: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 35-35: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 56-56: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 73-73: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 84-84: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 14-29: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[warning] 31-49: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[warning] 51-67: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[warning] 69-76: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
🤖 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 @.github/workflows/ci.yml around lines 14 - 88, The workflow jobs are using
the default GITHUB_TOKEN permissions and `actions/checkout` is persisting
credentials, which is broader access than needed. Add an explicit
least-privilege job-level `permissions` block for each job in `ci.yml`, keep
`workflow-lint` overridden with `pull-requests: write` for
`reviewdog/action-actionlint`, and set `persist-credentials: false` on every
`actions/checkout@v4` step in the `typecheck`, `lint`, `unit-tests`,
`workflow-lint`, and `integration-tests` jobs.
Source: Linters/SAST tools
| def _file_audit_write(delta: StrategyDelta) -> None: | ||
| """Fallback: append StrategyDelta as JSON line to /tmp/strategy_audit.jsonl.""" | ||
| import os | ||
|
|
||
| audit_path = "/tmp/strategy_audit.jsonl" | ||
| line = delta.model_dump_json() + "\n" | ||
| with open(audit_path, "a") as f: | ||
| f.write(line) | ||
| log.info(f"[Audit] Wrote strategy delta to {audit_path}: {delta.domain}/{delta.strategy_key}") No newline at end of file |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Hardcoded /tmp path for audit log (CWE-377).
Writing audit records to a fixed, world-writable /tmp/strategy_audit.jsonl is insecure and unreliable for a compliance-relevant audit trail — the file can be tampered with by other local processes/users, may be cleared on reboot/container restart, and has no locking against concurrent writers. Make the path configurable via env var and write to a directory with restricted permissions.
🛡️ Proposed fix
def _file_audit_write(delta: StrategyDelta) -> None:
"""Fallback: append StrategyDelta as JSON line to /tmp/strategy_audit.jsonl."""
import os
- audit_path = "/tmp/strategy_audit.jsonl"
+ audit_path = os.getenv("STRATEGY_AUDIT_LOG_PATH", "/var/log/sarthi/strategy_audit.jsonl")
+ os.makedirs(os.path.dirname(audit_path), exist_ok=True)
line = delta.model_dump_json() + "\n"
with open(audit_path, "a") as f:
f.write(line)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _file_audit_write(delta: StrategyDelta) -> None: | |
| """Fallback: append StrategyDelta as JSON line to /tmp/strategy_audit.jsonl.""" | |
| import os | |
| audit_path = "/tmp/strategy_audit.jsonl" | |
| line = delta.model_dump_json() + "\n" | |
| with open(audit_path, "a") as f: | |
| f.write(line) | |
| log.info(f"[Audit] Wrote strategy delta to {audit_path}: {delta.domain}/{delta.strategy_key}") | |
| def _file_audit_write(delta: StrategyDelta) -> None: | |
| """Fallback: append StrategyDelta as JSON line to /tmp/strategy_audit.jsonl.""" | |
| import os | |
| audit_path = os.getenv("STRATEGY_AUDIT_LOG_PATH", "/var/log/sarthi/strategy_audit.jsonl") | |
| os.makedirs(os.path.dirname(audit_path), exist_ok=True) | |
| line = delta.model_dump_json() + "\n" | |
| with open(audit_path, "a") as f: | |
| f.write(line) | |
| log.info(f"[Audit] Wrote strategy delta to {audit_path}: {delta.domain}/{delta.strategy_key}") |
🧰 Tools
🪛 ast-grep (0.44.1)
[info] 439-439: Do not hardcode temporary file or directory names
Context: "/tmp/strategy_audit.jsonl"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
[warning] 441-441: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(audit_path, "a")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🤖 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 `@apps/ai/src/agents/cofounder/curator.py` around lines 436 - 444, The
_file_audit_write fallback currently writes audit records to a hardcoded
/tmp/strategy_audit.jsonl path, which is insecure and not durable. Update
_file_audit_write to resolve the audit file location from a configurable
environment variable instead of a fixed /tmp value, and default it to a
restricted-permission directory suitable for compliance logs. Ensure the path is
created securely with appropriate permissions before appending, and consider
adding basic concurrency-safe write handling so multiple writers do not corrupt
the audit trail.
| "reason": "Output risk scan blocked draft", | ||
| }, | ||
| ) | ||
| _AUDIT.log(audit_event) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Audit calls are broken: wrong method name and not awaited.
AuditLogger defines only async def log_event(...), so _AUDIT.log(audit_event) raises AttributeError. Even after renaming, log_event is a coroutine and must be awaited, otherwise the audit write silently never runs (unawaited-coroutine warning). Both blocked-path (Line 97) and success-path (Line 120) audit records would be lost.
🐛 Proposed fix
- _AUDIT.log(audit_event)
+ await _AUDIT.log_event(audit_event)Also applies to: 120-120
🤖 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 `@apps/ai/src/agents/tools/draft_investor_update.py` at line 97, The audit
logging in draft_investor_update is calling the wrong API and not awaiting the
async write. Update both audit paths in the draft_investor_update flow to use
AuditLogger.log_event(...) instead of _AUDIT.log(...), and ensure each call is
awaited so the coroutine actually runs. Check the blocked-path and success-path
audit records around the existing audit_event handling so both are recorded
correctly.
| async def log_event( | ||
| self, | ||
| event: AuditEvent, | ||
| conn: asyncpg.Connection | None = None, | ||
| ) -> bool: |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all call sites of the audit logger to confirm the method name used.
rg -nP '_AUDIT\.\w+\(' apps/ai/src
rg -nP 'def\s+(log|log_event)\b' apps/ai/src/control_plane/audit.pyRepository: Aparnap2/Track_Guard
Length of output: 353
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## audit.py\n'
sed -n '1,220p' apps/ai/src/control_plane/audit.py
printf '\n## draft_investor_update.py\n'
sed -n '1,220p' apps/ai/src/agents/tools/draft_investor_update.py
printf '\n## audit logger references\n'
rg -nP '\b_AUDIT\.(log|log_event)\b|class\s+AuditLogger\b|def\s+(log|log_event)\b' apps/ai/srcRepository: Aparnap2/Track_Guard
Length of output: 7194
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## file list\n'
git ls-files apps/ai/src/control_plane/audit.py apps/ai/src/agents/tools/draft_investor_update.py
printf '\n## audit.py outline\n'
ast-grep outline apps/ai/src/control_plane/audit.py --view expanded
printf '\n## draft_investor_update.py outline\n'
ast-grep outline apps/ai/src/agents/tools/draft_investor_update.py --view expandedRepository: Aparnap2/Track_Guard
Length of output: 763
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for path in [
Path('apps/ai/src/control_plane/audit.py'),
Path('apps/ai/src/agents/tools/draft_investor_update.py'),
]:
print(f'\n## {path}')
text = path.read_text()
for needle in ['class AuditLogger', 'def log_event', 'def log(', '__getattr__', '_AUDIT =', 'AuditLogger(']:
if needle in text:
print(f'contains: {needle}')
PYRepository: Aparnap2/Track_Guard
Length of output: 345
Expose log() or update callers
_AUDIT.log(...) is called, but AuditLogger only defines log_event(...). That will raise AttributeError when the audit path runs; either add a log() shim or switch the call sites to await log_event(...).
🤖 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 `@apps/ai/src/control_plane/audit.py` around lines 20 - 24, The audit logger
interface is mismatched: callers use _AUDIT.log(...) but AuditLogger only
exposes log_event(...), which will fail at runtime. Update the audit path to
call await log_event(...) consistently, or add a log() shim on AuditLogger that
forwards to log_event so existing call sites continue to work. Use the
AuditLogger class and its log_event method to locate the change and keep the
async behavior intact.
| def evaluate( | ||
| self, | ||
| agent: AgentRegistration, | ||
| requested_tool: str | None = None, | ||
| ) -> PolicyDecision: | ||
| """Evaluate whether an agent action is permitted. | ||
|
|
||
| Args: | ||
| agent: The registered agent attempting the action. | ||
| requested_tool: The specific tool being requested, if any. | ||
|
|
||
| Returns: | ||
| PolicyDecision with approval state and constraints. | ||
| """ | ||
| approved_tools: list[str] = [] | ||
| requires_human_approval = False | ||
| blocked_reason: str | None = None | ||
|
|
||
| # 1. Health check — unhealthy agents are blocked | ||
| if agent.health_status == "unhealthy": | ||
| blocked_reason = "agent_health_unhealthy" | ||
| log.warning( | ||
| "Policy block: agent %s is unhealthy", | ||
| agent.agent_name, | ||
| ) | ||
| return PolicyDecision( | ||
| data_classification=agent.data_classification, | ||
| allowed_model_classes=[], | ||
| requires_human_approval=True, | ||
| blocked_reason=blocked_reason, | ||
| approved_tools=[], | ||
| ) | ||
|
|
||
| # 2. Data classification check — restricted data blocks | ||
| if agent.data_classification.lower() in _RESTRICTED_CLASSIFICATIONS: | ||
| blocked_reason = "data_classification_restricted" | ||
| log.warning( | ||
| "Policy block: agent %s has restricted classification: %s", | ||
| agent.agent_name, | ||
| agent.data_classification, | ||
| ) | ||
| return PolicyDecision( | ||
| data_classification=agent.data_classification, | ||
| allowed_model_classes=[], | ||
| requires_human_approval=True, | ||
| blocked_reason=blocked_reason, | ||
| approved_tools=[], | ||
| ) | ||
|
|
||
| # 3. External-facing check — forces human approval | ||
| if agent.external_facing: | ||
| requires_human_approval = True | ||
|
|
||
| # 4. Degraded health — forces review but not full block | ||
| if agent.health_status == "degraded": | ||
| requires_human_approval = True | ||
| blocked_reason = "agent_health_degraded" | ||
|
|
||
| # 5. Tool permission check | ||
| if requested_tool is not None: | ||
| if requested_tool in agent.allowed_tools: | ||
| approved_tools.append(requested_tool) | ||
| else: | ||
| if blocked_reason is None: | ||
| blocked_reason = f"tool_not_allowed: {requested_tool}" | ||
| requires_human_approval = True | ||
|
|
||
| # 6. Determine allowed model classes | ||
| allowed_models = agent.allowed_models | ||
| if blocked_reason is not None: | ||
| allowed_models = [] | ||
|
|
||
| return PolicyDecision( | ||
| data_classification=agent.data_classification, | ||
| allowed_model_classes=allowed_models, | ||
| requires_human_approval=requires_human_approval, | ||
| blocked_reason=blocked_reason, | ||
| approved_tools=approved_tools, | ||
| ) No newline at end of file |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
git ls-files | rg 'apps/ai/src/control_plane/(policy|authority_manifest|.*registration.*|.*authority.*)\.py$|apps/ai/src/.*/.*\.py$'
printf '\n--- search escalation_tier ---\n'
rg -n "escalation_tier|PolicyEngine\.evaluate|class AgentRegistration|class AgentAuthority|requires_human_approval" apps/ai/src -g '*.py'
printf '\n--- outline policy.py ---\n'
ast-grep outline apps/ai/src/control_plane/policy.py --view expanded
printf '\n--- outline authority_manifest.py ---\n'
ast-grep outline apps/ai/src/control_plane/authority_manifest.py --view expandedRepository: Aparnap2/Track_Guard
Length of output: 12900
🏁 Script executed:
set -euo pipefail
printf '\n--- outline apps/ai/src/schemas/control_plane.py ---\n'
ast-grep outline apps/ai/src/schemas/control_plane.py --view expanded
printf '\n--- outline apps/ai/src/agents/authority_manifest.py ---\n'
ast-grep outline apps/ai/src/agents/authority_manifest.py --view expanded
printf '\n--- outline apps/ai/src/control_plane/registry.py ---\n'
ast-grep outline apps/ai/src/control_plane/registry.py --view expanded
printf '\n--- policy.py lines 1-140 ---\n'
cat -n apps/ai/src/control_plane/policy.py | sed -n '1,140p'
printf '\n--- control_plane.py lines 1-160 ---\n'
cat -n apps/ai/src/schemas/control_plane.py | sed -n '1,160p'
printf '\n--- authority_manifest.py lines 1-180 ---\n'
cat -n apps/ai/src/agents/authority_manifest.py | sed -n '1,180p'
printf '\n--- registry.py lines 1-120 ---\n'
cat -n apps/ai/src/control_plane/registry.py | sed -n '1,120p'Repository: Aparnap2/Track_Guard
Length of output: 20628
🏁 Script executed:
set -euo pipefail
printf '\n--- global escalation_tier references ---\n'
rg -n "escalation_tier" apps/ai/src -g '*.py'
printf '\n--- approval_policy.py outline and relevant lines ---\n'
ast-grep outline apps/ai/src/orchestrators/approval_policy.py --view expanded
cat -n apps/ai/src/orchestrators/approval_policy.py | sed -n '1,220p'Repository: Aparnap2/Track_Guard
Length of output: 3187
🏁 Script executed:
set -euo pipefail
printf '\n--- PolicyEngine call sites ---\n'
rg -n "PolicyEngine\(|evaluate\(" apps/ai/src -g '*.py'
printf '\n--- approval_state / requires_human_approval uses ---\n'
rg -n "requires_human_approval|approval_state|blocked_reason" apps/ai/src -g '*.py'Repository: Aparnap2/Track_Guard
Length of output: 2902
Honor agent.escalation_tier in PolicyEngine.evaluate
evaluate() only uses health, classification, external-facing, and tool checks. Agents registered with escalation_tier="review" or "approve" can still be auto-approved here when those other rules don’t trigger, so the declared HITL tier is ignored. Fold the tier into this decision and treat blocked as a hard block.
🤖 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 `@apps/ai/src/control_plane/policy.py` around lines 21 - 99,
PolicyEngine.evaluate currently ignores agent.escalation_tier, so agents marked
review or approve can still pass without human involvement. Update evaluate in
PolicyEngine to incorporate escalation_tier alongside the existing health,
classification, external-facing, and tool checks: treat blocked as a hard deny,
set requires_human_approval for review and approve tiers as intended, and ensure
the final PolicyDecision reflects the tier even when no other rule triggers.
| prepared_brief=row["prepared_brief"], | ||
| pending_decisions=row["pending_decisions"], | ||
| last_updated_by=row["last_updated_by"], | ||
| last_update_reason=row["last_update_reason"], | ||
| last_changed_fields=row["last_changed_fields"], | ||
| active_agent_roles=row["active_agent_roles"], | ||
| policy_state=row["policy_state"], | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
p = Path("apps/ai/src/session/mission_state.py")
print(p.exists(), p)
if p.exists():
lines = p.read_text().splitlines()
for start in (1, 1):
pass
PYRepository: Aparnap2/Track_Guard
Length of output: 200
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== mission_state.py outline =="
ast-grep outline apps/ai/src/session/mission_state.py --view expanded || true
echo
echo "== relevant slices =="
nl -ba apps/ai/src/session/mission_state.py | sed -n '1,260p'
echo
echo "== search for JSON field usage and tests =="
rg -n "pending_decisions|last_changed_fields|active_agent_roles|policy_state|prepared_brief|burn_alert|records_update_reason_and_policy_state" apps/ai -SRepository: Aparnap2/Track_Guard
Length of output: 2241
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
path = Path("apps/ai/src/session/mission_state.py")
text = path.read_text().splitlines()
for start, end in [(1, 260)]:
for i, line in enumerate(text[start-1:end], start):
print(f"{i:4d}: {line}")
PY
echo
echo "== tests mentioning mission state =="
python3 - <<'PY'
from pathlib import Path
import re
for path in Path("apps/ai").rglob("*test*.py"):
txt = path.read_text()
if "mission_state" in txt or "burn_alert" in txt or "last_changed_fields" in txt or "policy_state" in txt:
print(f"\n--- {path} ---")
for i, line in enumerate(txt.splitlines(), 1):
if re.search(r"mission_state|burn_alert|last_changed_fields|policy_state|pending_decisions|active_agent_roles", line):
print(f"{i:4d}: {line}")
PYRepository: Aparnap2/Track_Guard
Length of output: 22769
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
for path in Path("apps/ai").rglob("*.sql"):
txt = path.read_text(errors="ignore")
if "mission_states" in txt:
print(f"\n--- {path} ---")
for i, line in enumerate(txt.splitlines(), 1):
if "mission_states" in line or "pending_decisions" in line or "last_changed_fields" in line or "active_agent_roles" in line or "policy_state" in line:
print(f"{i:4d}: {line}")
PYRepository: Aparnap2/Track_Guard
Length of output: 2381
🌐 Web query:
asyncpg jsonb default decoding Python objects or raw text documentation
💡 Result:
By default, asyncpg does not automatically decode PostgreSQL json and jsonb types into Python objects [1][2]. Instead, it returns them as raw strings (text) [3][4][2]. To have asyncpg automatically decode these types into Python objects, you must explicitly register a custom type codec using the connection's set_type_codec method [1][5][2]. This is typically done by utilizing the standard library's json module [3][1]. Example configuration: import json import asyncpg async def setup_json_codec(conn): await conn.set_type_codec( 'json', encoder=json.dumps, decoder=json.loads, schema='pg_catalog') await conn.set_type_codec( 'jsonb', encoder=json.dumps, decoder=json.loads, schema='pg_catalog') Once configured, asyncpg will automatically handle the conversion between Python dictionaries/lists and PostgreSQL JSON/JSONB types during query execution [1]. If you are using a connection pool, you can use the init parameter of create_pool to apply this codec to all connections in the pool [2].
Citations:
- 1: https://magicstack.github.io/asyncpg/devel/usage.html
- 2: json_agg and json_build_object raw queries return string type rather than json MagicStack/asyncpg#432
- 3: https://github.com/MagicStack/asyncpg/blob/master/docs/usage.rst
- 4: In Record complex data coming as string MagicStack/asyncpg#560
- 5: https://stackoverflow.com/questions/79727814/pythonasyncpgpostgresql-returns-jsonb-column-as-a-string-instead-of-an-object
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== mission_states schema slice =="
python3 - <<'PY'
from pathlib import Path
path = Path("apps/ai/src/session/001_session_layer.sql")
text = path.read_text().splitlines()
for i, line in enumerate(text, 1):
if 12 <= i <= 55:
print(f"{i:4d}: {line}")
PY
echo
echo "== search for JSON codecs / asyncpg setup =="
python3 - <<'PY'
from pathlib import Path
for path in Path("apps/ai").rglob("*.py"):
txt = path.read_text(errors="ignore")
if "set_type_codec" in txt or "asyncpg.connect(" in txt or "create_pool(" in txt:
print(f"\n--- {path} ---")
for i, line in enumerate(txt.splitlines(), 1):
if "set_type_codec" in line or "asyncpg.connect(" in line or "create_pool(" in line:
print(f"{i:4d}: {line}")
PYRepository: Aparnap2/Track_Guard
Length of output: 3082
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== search for the four fields in schema/migrations =="
python3 - <<'PY'
from pathlib import Path
patterns = ["pending_decisions", "last_changed_fields", "active_agent_roles", "policy_state"]
for path in Path("apps/ai").rglob("*"):
if path.suffix not in {".sql", ".py", ".md", ".yml", ".yaml"}:
continue
try:
txt = path.read_text(errors="ignore")
except Exception:
continue
if any(p in txt for p in patterns):
print(f"\n--- {path} ---")
for i, line in enumerate(txt.splitlines(), 1):
if any(p in line for p in patterns):
print(f"{i:4d}: {line}")
PYRepository: Aparnap2/Track_Guard
Length of output: 2648
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
path = Path("apps/ai/src/session/mission_state.py")
text = path.read_text().splitlines()
for start, end in [(260, 300)]:
for i in range(start, min(end, len(text)) + 1):
print(f"{i:4d}: {text[i-1]}")
PYRepository: Aparnap2/Track_Guard
Length of output: 1975
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
paths = [
Path("apps/ai/tests/integration/test_mission_state.py"),
Path("apps/ai/src/session/mission_state.py"),
]
for path in paths:
print(f"\n== {path} ==")
text = path.read_text().splitlines()
for i, line in enumerate(text, 1):
if path.name == "test_mission_state.py":
if 80 <= i <= 240:
print(f"{i:4d}: {line}")
else:
if 106 <= i <= 300:
print(f"{i:4d}: {line}")
PYRepository: Aparnap2/Track_Guard
Length of output: 16936
Deserialize the structured MissionState fields on read.
pending_decisions, last_changed_fields, active_agent_roles, and policy_state are passed straight into MissionState here, so callers get raw string values instead of the declared list/dict types. Decode them (or register a JSON codec on the connection) before returning the dataclass.
🤖 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 `@apps/ai/src/session/mission_state.py` around lines 168 - 175, The
MissionState read path is returning raw serialized strings for structured fields
instead of the declared list/dict types. In the MissionState construction logic,
deserialize pending_decisions, last_changed_fields, active_agent_roles, and
policy_state before passing them into MissionState, or apply a JSON codec at the
connection level so the dataclass receives decoded values. Use the MissionState
read/row-mapping code to locate the fix.
| async def update_mission_state(state: MissionState, generate_brief: bool = True, update_reason: str | None = None, changed_fields: list[str] | None = None) -> bool: | ||
| """Update MissionState in database atomically. | ||
|
|
||
| Per PRD Section 11: Updated atomically. | ||
|
|
||
| Args: | ||
| state: MissionState to persist | ||
| generate_brief: Whether to auto-generate prepared_brief if missing | ||
| update_reason: Why this write happened | ||
| changed_fields: Which fields were modified in this update | ||
|
|
||
| Returns: | ||
| True if successful, False otherwise | ||
| """ | ||
| if generate_brief: | ||
| state.last_update_reason = update_reason | ||
| state.last_changed_fields = changed_fields |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
update_mission_state silently nulls out explainability fields on most calls.
When generate_brief=True (the default) and a caller doesn't pass update_reason/changed_fields, Lines 196-198 unconditionally overwrite state.last_update_reason/state.last_changed_fields with None before persisting — even if state was loaded via get_mission_state and already carried real audit-trail values, or if the caller set them directly on state before calling this function. Any plain await update_mission_state(state) call will wipe existing explainability history in the DB.
🐛 Proposed fix
- if generate_brief:
- state.last_update_reason = update_reason
- state.last_changed_fields = changed_fields
+ if update_reason is not None:
+ state.last_update_reason = update_reason
+ if changed_fields is not None:
+ state.last_changed_fields = changed_fields📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async def update_mission_state(state: MissionState, generate_brief: bool = True, update_reason: str | None = None, changed_fields: list[str] | None = None) -> bool: | |
| """Update MissionState in database atomically. | |
| Per PRD Section 11: Updated atomically. | |
| Args: | |
| state: MissionState to persist | |
| generate_brief: Whether to auto-generate prepared_brief if missing | |
| update_reason: Why this write happened | |
| changed_fields: Which fields were modified in this update | |
| Returns: | |
| True if successful, False otherwise | |
| """ | |
| if generate_brief: | |
| state.last_update_reason = update_reason | |
| state.last_changed_fields = changed_fields | |
| async def update_mission_state(state: MissionState, generate_brief: bool = True, update_reason: str | None = None, changed_fields: list[str] | None = None) -> bool: | |
| """Update MissionState in database atomically. | |
| Per PRD Section 11: Updated atomically. | |
| Args: | |
| state: MissionState to persist | |
| generate_brief: Whether to auto-generate prepared_brief if missing | |
| update_reason: Why this write happened | |
| changed_fields: Which fields were modified in this update | |
| Returns: | |
| True if successful, False otherwise | |
| """ | |
| if update_reason is not None: | |
| state.last_update_reason = update_reason | |
| if changed_fields is not None: | |
| state.last_changed_fields = changed_fields |
🤖 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 `@apps/ai/src/session/mission_state.py` around lines 182 - 198,
update_mission_state is overwriting existing explainability data with None
whenever generate_brief is true and callers omit update_reason/changed_fields.
In update_mission_state, only assign state.last_update_reason and
state.last_changed_fields when those arguments are explicitly provided, and
otherwise preserve any existing values already on the MissionState instance from
get_mission_state or caller-set fields. Keep the behavior localized to
update_mission_state so plain await update_mission_state(state) does not clear
audit-trail history.
| guardrail_blocking, investor_facing_alert, created_at, | ||
| prepared_brief, pending_decisions, last_updated_by, | ||
| last_update_reason, last_changed_fields, active_agent_roles, | ||
| policy_state | ||
| ) | ||
| VALUES ($1, NOW(), $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, | ||
| $14, $15, $16, $17, $18, $19, $20, $21, $22, $23, $24, $25, NOW()) | ||
| $14, $15, $16, $17, $18, $19, $20, $21, $22, $23, $24, $25, NOW(), | ||
| $26, $27, $28, $29, $30, $31, $32) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for prior migrations defining these columns on mission_states
rg -n "prepared_brief|pending_decisions|last_updated_by|policy_state" --type sql
fd -e sql . apps/core/internal/db/migrations apps/ai/infrastructure/migrations 2>/dev/null | sortRepository: Aparnap2/Track_Guard
Length of output: 158
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== mission_state.py locations =="
fd -a 'mission_state.py' .
echo
echo "== migration files mentioning mission_states or relevant columns =="
rg -n --hidden --glob '*.sql' --glob '*.py' --glob '*.md' \
'mission_states|prepared_brief|pending_decisions|last_updated_by|policy_state|last_update_reason|last_changed_fields|active_agent_roles' \
apps 2>/dev/null || true
echo
echo "== candidate migration directories =="
fd -a -e sql . apps 2>/dev/null | sed -n '1,200p'Repository: Aparnap2/Track_Guard
Length of output: 13976
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in \
apps/ai/src/session/001_session_layer.sql \
apps/core/internal/db/migrations/004_resolve_mission_state_drift.sql \
apps/ai/src/session/mission_state.py
do
echo "== $f =="
wc -l "$f"
echo
done
echo "== 001_session_layer.sql (first 180 lines) =="
sed -n '1,180p' apps/ai/src/session/001_session_layer.sql
echo
echo "== 004_resolve_mission_state_drift.sql =="
cat -n apps/core/internal/db/migrations/004_resolve_mission_state_drift.sql
echo
echo "== mission_state.py relevant slices =="
sed -n '80,310p' apps/ai/src/session/mission_state.pyRepository: Aparnap2/Track_Guard
Length of output: 16336
Add policy_state to mission_states or stop persisting it here. 004_resolve_mission_state_drift.sql already covers prepared_brief, pending_decisions, and last_updated_by, but policy_state is still absent from the table. Both get_mission_state and update_mission_state read/write that column, so calls will hit a Postgres column error and fall back to the broad exception handlers. apps/ai/src/session/mission_state.py
🤖 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 `@apps/ai/src/session/mission_state.py` around lines 210 - 217, Add
`policy_state` support consistently in the mission state persistence path:
either migrate the `mission_states` table to include the missing column or
remove it from `get_mission_state` and `update_mission_state` so they no longer
read/write it. Check the `mission_state.py` query builders around
`get_mission_state`/`update_mission_state`, since the current INSERT/SELECT
logic references `policy_state` even though the schema drift migration only
covers the other fields. Make the schema and the two session methods match so
Postgres no longer throws a missing-column error.
| state.prepared_brief, | ||
| state.pending_decisions, | ||
| state.last_updated_by, | ||
| state.last_update_reason, | ||
| json.dumps(state.last_changed_fields) if state.last_changed_fields else '[]', | ||
| json.dumps(state.active_agent_roles) if state.active_agent_roles else '[]', | ||
| json.dumps(state.policy_state) if state.policy_state else None, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file and nearby symbols first.
ast-grep outline apps/ai/src/session/mission_state.py --view expanded
printf '\n--- mission_state.py slice ---\n'
sed -n '240,320p' apps/ai/src/session/mission_state.py
printf '\n--- search for policy_state / pending_decisions / asyncpg usage ---\n'
rg -n "pending_decisions|policy_state|asyncpg|json.dumps" apps/ai/src/session apps/ai/tests apps/ai/src -g '!**/__pycache__/**'
printf '\n--- locate test_mission_state_policy_state_field ---\n'
rg -n "test_mission_state_policy_state_field|PolicyDecision" apps/ai -g '!**/__pycache__/**'Repository: Aparnap2/Track_Guard
Length of output: 19355
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- mission_state.py top/model definitions ---\n'
sed -n '1,120p' apps/ai/src/session/mission_state.py
printf '\n--- test_control_plane relevant section ---\n'
sed -n '340,390p' apps/ai/tests/unit/test_control_plane.py
printf '\n--- PolicyDecision schema ---\n'
sed -n '1,140p' apps/ai/src/schemas/control_plane.py
printf '\n--- search for custom json/jsonb codecs on asyncpg connections ---\n'
rg -n "set_type_codec|jsonb|json" apps/ai/src apps/ai/tests -g '!**/__pycache__/**'Repository: Aparnap2/Track_Guard
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- mission_states table migration ---\n'
rg -n "mission_states|pending_decisions|policy_state" apps/ai/infrastructure apps/ai/src -g '*.sql' -g '*.py'
printf '\n--- trust_battery_db write path ---\n'
sed -n '1,140p' apps/ai/src/services/trust_battery_db.py
printf '\n--- mission_state update helper around parameters ---\n'
sed -n '220,290p' apps/ai/src/session/mission_state.py
printf '\n--- state writers assigning policy_state ---\n'
rg -n "policy_state\s*=" apps/ai/src apps/ai/tests -g '!**/__pycache__/**'Repository: Aparnap2/Track_Guard
Length of output: 14289
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- mission_states schema ---\n'
sed -n '1,140p' apps/ai/src/session/001_session_layer.sql
printf '\n--- look for jsonb columns in other mission_state-related migrations ---\n'
rg -n "pending_decisions|policy_state|jsonb|JSONB" apps/ai/infrastructure apps/ai/src/session -g '*.sql' -g '*.py'Repository: Aparnap2/Track_Guard
Length of output: 5590
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- all mission_states DDL / ALTERs ---\n'
rg -n "mission_states\s*\(|ALTER TABLE mission_states|ADD COLUMN.*pending_decisions|ADD COLUMN.*policy_state" apps/ai -g '*.sql' -g '*.py'
printf '\n--- inspect any later migration files in session directory ---\n'
fd -t f . apps/ai/src/session apps/ai/infrastructure/migrations apps/ai/infrastructure | sortRepository: Aparnap2/Track_Guard
Length of output: 3328
Add the missing mission_states columns
apps/ai/src/session/mission_state.py writes pending_decisions and policy_state, but apps/ai/src/session/001_session_layer.sql and the later migrations never add those columns. The insert/select will fail with column ... does not exist, so the save path returns False and drops the update.
🧰 Tools
🪛 ast-grep (0.44.1)
[info] 280-280: use jsonify instead of json.dumps for JSON output
Context: json.dumps(state.last_changed_fields)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 281-281: use jsonify instead of json.dumps for JSON output
Context: json.dumps(state.active_agent_roles)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 282-282: use jsonify instead of json.dumps for JSON output
Context: json.dumps(state.policy_state)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🤖 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 `@apps/ai/src/session/mission_state.py` around lines 277 - 283, The mission
state save/query path in mission_state.py now references pending_decisions and
policy_state, but the session schema migrations never add those columns, so the
insert/select will fail. Update the session layer migration set, including
001_session_layer.sql and any later migration definitions, to add the missing
mission_states columns in the same order/types expected by
MissionStateRepository’s save/load logic so the database schema matches the
fields used in mission_state.py.
| qdrant: | ||
| image: qdrant/qdrant:latest | ||
| container_name: sarthi-showcase-qdrant | ||
| ports: | ||
| - "6333:6333" # REST API | ||
| - "6334:6334" # gRPC | ||
| volumes: | ||
| - sarthi-showcase-qdrant-data:/var/lib/qdrant | ||
| networks: | ||
| - iterateswarm-net | ||
| healthcheck: | ||
| test: ["CMD-SHELL", "bash -c 'echo > /dev/tcp/localhost/6333'"] | ||
| interval: 10s | ||
| timeout: 5s | ||
| retries: 5 | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '125,150p' docker-compose.showcase.ymlRepository: Aparnap2/Track_Guard
Length of output: 870
🌐 Web query:
Qdrant default storage path /qdrant/storage documentation
💡 Result:
In the Qdrant container environment, the default internal directory where data is stored is /qdrant/storage [1][2]. When running Qdrant locally outside of a container, the default storage path is./storage, which is relative to the current working directory from which the application is executed [3][4][5]. Key details regarding storage paths: * Container Deployments: For persistent storage in Docker, you must map a host directory or volume to the internal /qdrant/storage path [1][2]. If you do not mount this directory, all data will be lost when the container is removed [6]. * Configuration: The storage path can be customized via the configuration file (typically config/config.yaml) under the storage_path setting [3][4], or by using the QDRANTSTORAGESTORAGE_PATH environment variable [5][1]. * Snapshots: Similarly, the default internal path for snapshots is /qdrant/snapshots [1]. This can also be configured via the snapshots_path setting in the configuration file [3][4]. Users are advised to ensure that the volume mapping in their Docker or Kubernetes configuration correctly targets the internal /qdrant/storage directory to ensure data persistence [1][2].
Citations:
- 1: https://qdrant-qdrant-18.mintlify.app/deployment/docker
- 2: https://theneuralbase.com/qdrant/learn/advanced/docker-compose-with-persistence/
- 3: https://github.com/qdrant/qdrant/blob/master/config/config.yaml
- 4: https://qdrant.tech/documentation/guides/configuration/
- 5: https://qdrant-qdrant-18.mintlify.app/deployment/configuration
- 6: https://github.com/qdrant/qdrant/blob/master/docs/DEVELOPMENT.md
Mount Qdrant data at /qdrant/storage
Qdrant persists data in /qdrant/storage inside the container, so mounting the volume to /var/lib/qdrant leaves the real storage directory unbacked and data will be lost when the container is recreated.
🐛 Proposed fix
volumes:
- - sarthi-showcase-qdrant-data:/var/lib/qdrant
+ - sarthi-showcase-qdrant-data:/qdrant/storage📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| qdrant: | |
| image: qdrant/qdrant:latest | |
| container_name: sarthi-showcase-qdrant | |
| ports: | |
| - "6333:6333" # REST API | |
| - "6334:6334" # gRPC | |
| volumes: | |
| - sarthi-showcase-qdrant-data:/var/lib/qdrant | |
| networks: | |
| - iterateswarm-net | |
| healthcheck: | |
| test: ["CMD-SHELL", "bash -c 'echo > /dev/tcp/localhost/6333'"] | |
| interval: 10s | |
| timeout: 5s | |
| retries: 5 | |
| qdrant: | |
| image: qdrant/qdrant:latest | |
| container_name: sarthi-showcase-qdrant | |
| ports: | |
| - "6333:6333" # REST API | |
| - "6334:6334" # gRPC | |
| volumes: | |
| - sarthi-showcase-qdrant-data:/qdrant/storage | |
| networks: | |
| - iterateswarm-net | |
| healthcheck: | |
| test: ["CMD-SHELL", "bash -c 'echo > /dev/tcp/localhost/6333'"] | |
| interval: 10s | |
| timeout: 5s | |
| retries: 5 |
🤖 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 `@docker-compose.showcase.yml` around lines 131 - 146, Update the qdrant
service in docker-compose.showcase.yml so the persistent volume is mounted to
Qdrant’s actual storage path instead of /var/lib/qdrant. Use the qdrant service
definition and its volumes entry to point sarthi-showcase-qdrant-data at
/qdrant/storage, keeping the rest of the service configuration unchanged.
1 participant
Summary
Internal monitoring subsystem that observes agent behavior, detects deviations from declared authority, and generates compliance reports.
Changes
Self-Guardian Subsystem (Phase 2)
AgentObservation,DeviationType(6 values),SelfGuardianAlert,SelfGuardianReportAUTHORITY_MANIFEST:unauthorized_tool— tool not in agent'sallowed_tool_ids(critical)external_facing_violation— agent marked external-facing accessing internal data (warning)CI Fix
actions/setup-python@v5withastral-sh/setup-uv@v5across all jobsact -P ubuntu-latest=node:20-slimSummary by CodeRabbit
New Features
Bug Fixes