feat: batch processing demo with dashboard#66
Merged
Conversation
Signed-off-by: Pavel Anni <panni@redhat.com>
Signed-off-by: Pavel Anni <panni@redhat.com>
Add parseBatchResult to extract JSON arrays from LLM responses (handles both raw JSON and markdown-wrapped code blocks). Add formatCSV to sort candidates by score descending and write ranked CSV output with header. Includes unit tests for both parsing variants and CSV output. Signed-off-by: Pavel Anni <panni@redhat.com>
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Signed-off-by: Pavel Anni <panni@redhat.com>
Signed-off-by: Pavel Anni <panni@redhat.com>
Signed-off-by: Pavel Anni <panni@redhat.com>
Signed-off-by: Pavel Anni <panni@redhat.com>
Signed-off-by: Pavel Anni <panni@redhat.com>
Add scripts/seed-demo-data.sh to load generated JSON documents into a running document-service instance via its REST API. Supports loading hr, security, finance, or all demo scenarios. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Signed-off-by: Pavel Anni <panni@redhat.com>
Service mesh sidecars (Istio, SPIFFE CSI) inject containers that violate SCC UID range constraints. Demo agents use plain HTTP and don't need mesh mTLS. Signed-off-by: Pavel Anni <panni@redhat.com>
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
- Add standalone document-service (Go, stdlib-only) that loads seed
JSON files and serves GET /documents/{id} for the batch demo
- Add invoice cross-references to finance contract documents so the
agent can discover related invoices
- Update finance system prompt with document ID conventions
- Add deployment template and finance-batch.sh script for fan-out
of five contracts to five parallel analyst agents
- Update security data dates and protocolVersion to 1.0.0
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Pavel Anni <panni@redhat.com>
- Add document ID hints (DOC-VULN001, DOC-ASSET001) to security system prompt so the agent knows which documents to fetch - Add detailed output format template (SLA breaches, team summary) - Sync inline ConfigMap prompt with standalone system-prompt.txt - Add --llm-timeout 120 to security deployment for large reports - Add Route for external testing - Record finance and security demo results in demo-log-2026-05-12 Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Standalone Go web app (stdlib + embed.FS) that orchestrates batch demo scenarios from a browser: - Deploys agent pods via K8s API (in-cluster SA or kubeconfig) - Sends A2A tasks and tracks completion with live status - Shows per-agent memory, CPU, and token usage from K8s metrics API and pod log parsing - Clickable document IDs fetch content from document-service - Dark-themed UI with finance, security, and HR (placeholder) pages - RBAC manifests for ServiceAccount with minimal permissions Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
- Fix artifact extraction for a2a-go v2 Part format (no "kind" field) - Map TASK_STATE_COMPLETED to "Ready" in green - Add "View Report" links in contracts list when analysis completes - Add clickable document IDs to preview contract contents - Add column headers (Contract / Customer / Status) with left-aligned labels - Add document proxy endpoint for out-of-cluster access Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Clickable source documents (vuln scan, asset inventory), status indicators during analysis, and View Report link for the remediation plan output. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
- 10 parallel agents each screening 10 shuffled resumes against a job description via fan-out - Dedicated Report column in agent table (doesn't replace tokens) - Report viewer renders markdown tables as styled HTML, strips raw JSON resume blocks, shows document IDs reviewed - Update HR ConfigMap name to hr-screener-config - Shuffle resume assignment across batches for diverse results Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
The dashboard imports gopkg.in/yaml.v3 for kubeconfig parsing, so it needs its own go.mod for the Docker build context. Also optimize Dockerfile with separate dependency download layer. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR introduces a complete batch processing demonstration for DocsClaw featuring a deterministic CLI command and interactive web dashboard that orchestrate multiple agent workers across Kubernetes namespaces. It includes three scenario templates (HR resume screening, security vulnerability triage, finance invoice analysis) with purpose-built agent configurations, Dockerfiles, a document storage service, demo data generation, and end-to-end orchestration scripts. ChangesBatch CLI Command
Dashboard Web Application
Demo Agent Scenarios and Infrastructure
Documentation and Configuration Updates
Sequence Diagram(s)sequenceDiagram
actor User
participant Browser as Browser<br/>(Dashboard UI)
participant Dashboard as Dashboard<br/>Backend
participant K8s as Kubernetes API
participant Agent1 as Agent Pod 1
participant Agent2 as Agent Pod N
participant DocSvc as Document<br/>Service
User->>Browser: Click "Run Finance Scenario"
Browser->>Dashboard: POST /api/run/finance
Dashboard->>K8s: Create Deployments<br/>(5 agents)
K8s-->>Agent1: Pod 1 starting
K8s-->>Agent2: Pod N starting
Dashboard->>K8s: Poll pod readiness<br/>(in parallel)
Agent1-->>K8s: Ready ✓
Agent2-->>K8s: Ready ✓
Dashboard->>K8s: Get Route hostnames<br/>(for external access)
par Run tasks in parallel
Dashboard->>Agent1: POST /a2a<br/>(SendMessage: Doc 1)
Dashboard->>Agent2: POST /a2a<br/>(SendMessage: Doc 2)
Agent1->>DocSvc: GET /documents/{id}<br/>(fetch contract)
Agent2->>DocSvc: GET /documents/{id}<br/>(fetch contract)
Agent1-->>Dashboard: TaskID + state
Agent2-->>Dashboard: TaskID + state
end
loop Poll task completion
Dashboard->>Agent1: POST /a2a<br/>(GetTask: status)
Dashboard->>Agent2: POST /a2a<br/>(GetTask: status)
Agent1-->>Dashboard: Artifact JSON
Agent2-->>Dashboard: Artifact JSON
end
Dashboard->>K8s: Get pod metrics<br/>+ logs (token counts)
Dashboard-->>Browser: GET /api/status/finance<br/>(per-agent results,<br/>metrics, timing)
Browser->>Browser: Render agent table<br/>with scores & totals
User->>Browser: View Report
Browser->>Dashboard: GET /api/document/{id}
Dashboard->>DocSvc: Proxy request
DocSvc-->>Browser: Document content
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- Load CA cert from kubeconfig certificate-authority-data instead of InsecureSkipVerify; only skip TLS when kubeconfig explicitly sets insecure-skip-tls-verify - Add HTTP client timeouts (5min for A2A, 30s for document proxy) - Validate document ID format against ^[A-Za-z0-9][-A-Za-z0-9]*$ to prevent path traversal in document proxy - Escape dynamic values in innerHTML via escapeHtml() - Log json.Encode errors via writeJSON helper - Add seccompProfile: RuntimeDefault to dynamic agent deployments - Use strings.HasPrefix instead of manual slice comparison Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
This was referenced May 14, 2026
- Fix XSS in onclick attributes with escapeJsString() helper - Add 30s timeout to K8s HTTP clients (both in-cluster and kubeconfig) - Parallelize agent readiness checks (Phase 2) with WaitGroup - Handle fs.Sub error instead of discarding - Remove hardcoded namespace from demo manifests - Inline containsPrefix as strings.HasPrefix Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
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 (1)
demo/batch/document-service/configmap.yaml (1)
1-17:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
configmap.yamlis documentation-only, not an actual manifest.Line 1 through Line 17 define comments only. If users apply manifests directly,
document-seed-datais not created and document lookups can fail unless a separate manual seed step is run. Add a concrete ConfigMap manifest here (or move these instructions to README and rename this file to avoid deployment ambiguity).As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 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 `@demo/batch/document-service/configmap.yaml` around lines 1 - 17, Replace the comment-only configmap.yaml with a concrete Kubernetes ConfigMap manifest named document-seed-data containing the sample JSON files (hr-documents.json, finance-documents.json, security-documents.json) so the document-service can mount them at the expected path and load *.json files at runtime; alternatively move these usage instructions into a README and rename or remove configmap.yaml to avoid deployment ambiguity—ensure the manifest key names match the file names and the ConfigMap metadata.name is document-seed-data so mounts and oc create/apply commands work as documented.
🟡 Minor comments (6)
demo/batch/dashboard/static/style.css-17-17 (1)
17-17:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the two stylelint errors in this stylesheet.
font-family-name-quotesandproperty-no-deprecatedare both violated here.Proposed fix
- font-family: 'Red Hat Text', 'Inter', system-ui, sans-serif; + font-family: 'Red Hat Text', Inter, system-ui, sans-serif; ... - word-wrap: break-word; + overflow-wrap: break-word;Also applies to: 276-276
🤖 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 `@demo/batch/dashboard/static/style.css` at line 17, The font declaration violates stylelint rules: replace the single quotes around multi-word family names with double quotes and update the system-ui token to a modern, non-deprecated fallback stack; specifically change the font-family line in the stylesheet (the font-family declaration) to use double quotes for "Red Hat Text" and "Inter" only if needed and replace system-ui with a recommended stack like -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, "Helvetica Neue", Arial, sans-serif so the font-family declaration conforms to font-family-name-quotes and property-no-deprecated rules.scripts/README.md-34-36 (1)
34-36:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language identifier to the fenced code block.
This block triggers markdownlint
MD040.Proposed fix
- ``` + ```text Use the `requesting-code-review` skill to review this PR. ```🤖 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 `@scripts/README.md` around lines 34 - 36, Replace the opening fenced code block marker (```) that wraps the line "Use the `requesting-code-review` skill to review this PR." with a language-tagged fence (for example ```text) so the block includes a language identifier and avoids markdownlint MD040; keep the closing ``` unchanged.demo/batch/README.md-12-13 (1)
12-13:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFinance scenario description is inconsistent with the implemented 5-agent flow.
This README currently documents finance as a single-agent act, but the demo assets/scripts in this PR use a five-agent fan-out path. Please align the wording/commands so operators run the intended scenario.
Also applies to: 60-62
🤖 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 `@demo/batch/README.md` around lines 12 - 13, The README's "Finance invoice anomaly detection" entry currently describes a single-agent flow but the demo implements a five-agent fan-out; update the finance scenario text to describe the five-agent fan-out architecture (mention "5-agent fan-out" and the roles of the agents), and change any example commands/snippets so they invoke the multi-agent demo assets/scripts used by the PR (ensure the documented command names match the actual script names in the repo); also apply the same edits to the repeated text around lines 60-62 so both descriptions and commands consistently reflect the 5-agent flow.scripts/seed-demo-data.sh-22-24 (1)
22-24:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard option values before reading
$2.If
--scenarioor--urlis passed without a value, this path errors abruptly underset -uinstead of showing usage.Suggested fix
while [[ $# -gt 0 ]]; do case $1 in - --scenario) SCENARIO="$2"; shift 2 ;; - --url) BASE_URL="$2"; shift 2 ;; + --scenario) + [[ $# -lt 2 ]] && usage + SCENARIO="$2"; shift 2 + ;; + --url) + [[ $# -lt 2 ]] && usage + BASE_URL="$2"; shift 2 + ;; *) usage ;; esac done🤖 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 `@scripts/seed-demo-data.sh` around lines 22 - 24, The case branches that set SCENARIO and BASE_URL in the argument parsing code read $2 directly and will crash under set -u if the option is passed with no value; update the handling for the --scenario and --url branches (the case that assigns SCENARIO="$2" and BASE_URL="$2") to first verify a non-empty next argument exists and is not another option (e.g., test that "$2" is set and does not start with '-') before assigning and shifting; if the check fails, print the script usage/help and exit with a non-zero status so missing values are handled gracefully.docs/superpowers/plans/2026-05-09-batch-demo.md-901-901 (1)
901-901:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winProtocol version mismatch between plan and implementation.
The implementation plan specifies
"protocolVersion": "0.3.0"in the agent card examples (lines 901, 1010, 1097) and deployment manifests (line 1352), but the actual deployments in this PR use"protocolVersion": "1.0.0"(seedocs/demo/hr-analyst.yamlline 69). Update the plan to match the implemented version to avoid confusion.📝 Suggested fix
Update all occurrences of
"protocolVersion": "0.3.0"to"protocolVersion": "1.0.0"in the agent card JSON examples at lines 901, 1010, 1097, and 1352.Also applies to: 1010-1010, 1097-1097, 1352-1352
🤖 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 `@docs/superpowers/plans/2026-05-09-batch-demo.md` at line 901, Update all agent card JSON examples and deployment manifest entries that currently set "protocolVersion": "0.3.0" to "protocolVersion": "1.0.0" so the plan matches the actual deployment manifests (e.g., change every occurrence of the literal string "protocolVersion": "0.3.0" found in the agent card examples and manifests to "protocolVersion": "1.0.0"); ensure the updated values appear in the agent card JSON examples and any deployment manifest examples to keep them consistent with the implementation.scripts/generate-demo-data.py-36-43 (1)
36-43:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove constant f-strings to avoid lint failures (F541).
These lines use
f"..."without placeholders; Ruff flags them as errors.Also applies to: 463-463, 466-467, 511-511
🤖 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 `@scripts/generate-demo-data.py` around lines 36 - 43, Several print calls use f-strings even though they contain no placeholders (causing F541); remove the unnecessary leading "f" for those constant messages—e.g. change prints that currently use f" - Strong matches: 20", f" - Moderate matches: 30", f" - Weak matches: 30", f" - Poor matches: 20" (and the other constant f-strings flagged elsewhere) to plain string literals like " - Strong matches: 20"; leave f-strings that do include expressions (e.g., f"Generated demo data in {output_dir}/", f" HR documents: {len(hr_docs)} ...", f" Security documents: {len(security_docs)} ...", f" Finance documents: {len(finance_docs)} ...") unchanged.
🧹 Nitpick comments (2)
demo/batch/Dockerfile.finance (1)
1-10: ⚡ Quick winPin base images by digest to avoid supply-chain drift.
Line 1 and Line 9 use mutable tags, allowing rebuilds to silently pull different image manifests and introduce unexpected behavior or security changes. Pin both base images to immutable digests using the syntax
FROM golang:1.25-alpine@sha256:<digest>for reproducible builds.Suggested change
-FROM golang:1.25-alpine AS builder +FROM golang:1.25-alpine@sha256:<digest> AS builder @@ -FROM alpine:3.23 +FROM alpine:3.23@sha256:<digest>🤖 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 `@demo/batch/Dockerfile.finance` around lines 1 - 10, The Dockerfile uses mutable image tags in the two FROM lines (the golang:1.25-alpine builder stage and the alpine:3.23 runtime stage); replace each tag with an immutable digest form (e.g., FROM golang:1.25-alpine@sha256:<digest> and FROM alpine:3.23@sha256:<digest>) to pin the exact manifests, fetching the correct SHA256 digests from the official registries (or your internal registry) and updating those two FROM lines so builds are reproducible and resistant to supply-chain drift.demo/batch/dashboard/static/security.html (1)
67-71: ⚡ Quick winRemove unused variable declarations.
Lines 67, 68, and 71 declare variables (
_origRenderState,_renderStateOrig,origPollStatus) that are never used. The actual override logic at lines 75-80 doesn't reference them. Clean up these declarations to improve code clarity.♻️ Proposed cleanup
<script> - // Security-specific status updates. - const _origRenderState = window.renderState || renderState; - const _renderStateOrig = renderState; - - // Override renderState to add security-specific UI updates. - const origPollStatus = pollStatus; - - // Use a MutationObserver-free approach: hook into the polling cycle. + // Override renderState to add security-specific UI updates. (function() { const origRender = window.renderState;🤖 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 `@demo/batch/dashboard/static/security.html` around lines 67 - 71, Remove the unused variable declarations _origRenderState, _renderStateOrig, and origPollStatus which are declared from renderState and pollStatus but never referenced; update the file by deleting the three lines that set const _origRenderState = window.renderState || renderState;, const _renderStateOrig = renderState;, and const origPollStatus = pollStatus; so the override logic that follows uses only the actual renderState/pollStatus functions without dead variables.
🤖 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 `@demo/batch/dashboard/a2a.go`:
- Around line 142-163: GetTaskStatus currently unmarshals resp.Body blindly;
first check resp.StatusCode and Content-Type: if resp.StatusCode >= 500 return a
descriptive error including status code and a short snippet of the response
body, and if Content-Type does not contain "application/json" return an error
indicating non-JSON response with a body snippet; only then attempt
json.Unmarshal into the existing rpcResp struct and keep the existing
rpcResp.Error handling (references: GetTaskStatus, c.httpClient.Post call,
rpcResp struct and error handling) so transient HTML/5xx responses become
actionable instead of producing opaque parse errors.
- Around line 115-123: The code currently extracts taskID using extractField and
returns a TaskStatus even when taskID is empty; update the SendMessage response
handling so it fails fast: after taskID, check if taskID == "" (or
nil-equivalent) and return a descriptive error instead of a TaskStatus, e.g.
"missing task.id in SendMessage response"; keep extracting state and artifact
only when taskID is present. Ensure the error is returned from the function that
constructs the TaskStatus so callers don't receive empty IDs.
In `@demo/batch/dashboard/k8s.go`:
- Line 371: The readiness check currently uses
item.Status.ContainerStatuses[0].Ready which only inspects the first container
and misreports pods with sidecars; update the logic that sets the ready variable
(referencing item.Status.ContainerStatuses and ready) to ensure all container
statuses are Ready (e.g., require len(item.Status.ContainerStatuses) > 0 AND
every status in item.Status.ContainerStatuses has Ready == true) instead of only
checking index 0.
In `@demo/batch/dashboard/main.go`:
- Around line 256-258: The code unconditionally sets rt.phase = "done" with
s.mu.Lock()/s.mu.Unlock(), which misreports failures; change this to compute the
overall scenario phase before writing it under the lock (e.g., inspect agent
results/failure flags on the run/response collection used by this routine and
set overallPhase = "failed" if any agent failed, otherwise "done"), then acquire
s.mu and assign rt.phase = overallPhase and unlock; reference the
variables/functions rt.phase, s.mu.Lock()/s.mu.Unlock() and the agent result
collection used in this function when implementing the check.
In `@demo/batch/dashboard/static/app.js`:
- Around line 11-21: The fetch success path currently calls r.json() without
checking r.ok and starts startPolling(name) and startTimer() even for non-OK
responses; update the fetch handling in the POST block (and other similar blocks
at 50-66 and 174-189) to first check response.ok, attempt to parse JSON safely
(fall back to text if JSON parse fails), and if response.ok is false throw a
handled error that re-enables the button (btn.disabled = false; btn.textContent
= 'Run') and shows a useful message; only call startPolling(name) and
startTimer() after a confirmed OK response, and ensure polling errors are
surfaced to the UI (stop timer/polling and re-enable controls) rather than being
swallowed.
In `@demo/batch/data/hr-documents.json`:
- Around line 10-708: The fixture contains real-looking personal contact data
inside records where "sensitivity": "confidential" (see objects with fields id,
title, sensitivity, required_department and content), so replace all real
emails, phone numbers and uniquely identifying names in the "content" strings
with clearly synthetic placeholders (use reserved domain example.com for emails
and non-real phone placeholders like +1-000-000-0000), and optionally generic
names in "title" (e.g., "Resume - Person 001"); update every record where
sensitivity == "confidential" (match by id or the content field) and regenerate
the fixture so no real-looking PII remains while preserving structure and sample
diversity.
In `@demo/batch/document-service/main.go`:
- Around line 42-44: The loop that does for _, d := range batch { docs[d.ID] = d
} silently overwrites duplicates; modify the load so it fails fast on duplicate
document IDs by checking if d.ID already exists in the docs map before
assignment (e.g., if _, ok := docs[d.ID]; ok { return
error/panic/log.Fatalf(...) }) and surface a clear error including the duplicate
ID and source batch information; update the function that builds docs (the batch
load loop) to return an error up the call stack or exit immediately when a
duplicate is detected.
In `@demo/batch/hr/agent-card.json`:
- Line 6: The agent-card.json currently hardcodes "url": "http://hr-worker:8000"
which breaks when running multiple HR workers; change the manifest to use a
template variable instead of a fixed string (e.g., "url": "{{HR_WORKER_URL}}" or
"url": "http://hr-worker-{{WORKER_INDEX}}:8000") and update the
manifest/template generator code that produces the HR agent card to inject the
per-worker service hostname or index (from the generator's worker loop or env
var like HR_WORKER_HOST/WORKER_INDEX) so each parallel HR worker gets the
correct service URL.
In `@demo/batch/hr/deployment.yaml`:
- Around line 4-5: The ConfigMap in this file is misnamed and missing a
namespace, causing the Deployment hr-worker-WORKER_INDEX (which mounts
hr-worker-config) to fail; rename the ConfigMap from hr-screener-config to
hr-worker-config (or update the Deployment mount to use hr-screener-config) and
set metadata.namespace: demo-hr so the ConfigMap lives in the same namespace as
the Deployment; apply the same name/namespace fixes to the other ConfigMap
occurrence referenced around lines 137-139.
In `@demo/batch/security/deployment.yaml`:
- Around line 4-5: Add a consistent namespace to these resources and remove the
hardcoded cross-namespace service URL: set metadata.namespace to "demo-security"
for the ConfigMap named security-analyst-config (and the other resources
referenced at the same locations), and update any container/service references
from document-service.panni-docsclaw.svc to document-service.demo-security.svc
(or use a templated/variable like $(NAMESPACE) so the service DNS resolves
within the same namespace). Ensure all occurrences mentioned (around the other
ranges) are updated so deployments and container env vars point to the same
demo-security namespace.
In `@docs/superpowers/plans/2026-05-07-mcp-client-integration.md`:
- Around line 768-779: The Close method currently logs "disconnected from MCP
server" for both success and error branches which is misleading; update
Manager.Close to log a distinct error message when s.session.Close() returns an
error (e.g., use slog.Error or slog.Warn with message like "failed to disconnect
from MCP server" and include s.name and the error) and keep the success branch
logging "disconnected from MCP server" for clarity, ensuring you still set
lastErr = err when an error occurs so the method returns the last error
encountered.
- Around line 224-227: The subprocess is inheriting the entire parent
environment via cmd.Environ(), which can leak secrets; change the logic around
exec.Command/c.Command/c.Args so cmd.Env is set to a minimal, explicit list
built from c.Env (use envSlice(c.Env)) rather than appending cmd.Environ();
alternatively add and check an explicit boolean config like inherit_env before
copying cmd.Environ() and clearly document its security implications so only
explicitly allowed inheritance occurs.
In `@scripts/generate-demo-data.py`:
- Around line 320-321: The code uses datetime.now() in multiple places (e.g.,
when appending to vuln_content) which makes outputs non-deterministic despite
random.seed; replace direct datetime.now() calls with a single deterministically
computed date variable (e.g., deterministic_date) computed once using the seeded
RNG or passed-in CLI arg/ENV (for example: base_date = datetime(YYYY,MM,DD) +
timedelta(days=randint_from_seed)), and then use
deterministic_date.strftime('%Y-%m-%d') wherever datetime.now() is currently
used (references: vuln_content concatenation lines and the other occurrences
mentioned around lines 382, 411, 426-429) so generated JSON becomes
reproducible.
In `@scripts/pr-review.sh`:
- Around line 32-35: The PR metadata variables TITLE, BODY, AUTHOR and ADDITIONS
are printed directly and may contain terminal control/ANSI escape sequences; add
a sanitize step (e.g., a shell function sanitize() that
removes/control-characters and ANSI sequences such as regex replacing
[\x00-\x1F\x7F] and \x1B\[[0-9;]*[A-Za-z]) and run sanitize on each value after
extracting with jq (replace references to TITLE, BODY, AUTHOR, ADDITIONS with
sanitized versions); apply the same sanitization where those variables are used
later (the other occurrences noted around lines 75-78).
In `@scripts/seed-demo-data.sh`:
- Around line 50-55: urllib.request.urlopen(req) is being called without a
timeout which can hang indefinitely; change the call in the loop that posts
documents (the urlopen invocation where variables req, success and docs are
used) to pass a timeout (e.g. timeout=5 or read from an env var) and update the
exception handling to specifically catch socket.timeout/urllib.error.URLError
where appropriate and log a clear timeout warning for doc["id"] while still
incrementing/continuing as before.
---
Outside diff comments:
In `@demo/batch/document-service/configmap.yaml`:
- Around line 1-17: Replace the comment-only configmap.yaml with a concrete
Kubernetes ConfigMap manifest named document-seed-data containing the sample
JSON files (hr-documents.json, finance-documents.json, security-documents.json)
so the document-service can mount them at the expected path and load *.json
files at runtime; alternatively move these usage instructions into a README and
rename or remove configmap.yaml to avoid deployment ambiguity—ensure the
manifest key names match the file names and the ConfigMap metadata.name is
document-seed-data so mounts and oc create/apply commands work as documented.
---
Minor comments:
In `@demo/batch/dashboard/static/style.css`:
- Line 17: The font declaration violates stylelint rules: replace the single
quotes around multi-word family names with double quotes and update the
system-ui token to a modern, non-deprecated fallback stack; specifically change
the font-family line in the stylesheet (the font-family declaration) to use
double quotes for "Red Hat Text" and "Inter" only if needed and replace
system-ui with a recommended stack like -apple-system, BlinkMacSystemFont,
"Segoe UI", Roboto, "Helvetica Neue", Arial, sans-serif so the font-family
declaration conforms to font-family-name-quotes and property-no-deprecated
rules.
In `@demo/batch/README.md`:
- Around line 12-13: The README's "Finance invoice anomaly detection" entry
currently describes a single-agent flow but the demo implements a five-agent
fan-out; update the finance scenario text to describe the five-agent fan-out
architecture (mention "5-agent fan-out" and the roles of the agents), and change
any example commands/snippets so they invoke the multi-agent demo assets/scripts
used by the PR (ensure the documented command names match the actual script
names in the repo); also apply the same edits to the repeated text around lines
60-62 so both descriptions and commands consistently reflect the 5-agent flow.
In `@docs/superpowers/plans/2026-05-09-batch-demo.md`:
- Line 901: Update all agent card JSON examples and deployment manifest entries
that currently set "protocolVersion": "0.3.0" to "protocolVersion": "1.0.0" so
the plan matches the actual deployment manifests (e.g., change every occurrence
of the literal string "protocolVersion": "0.3.0" found in the agent card
examples and manifests to "protocolVersion": "1.0.0"); ensure the updated values
appear in the agent card JSON examples and any deployment manifest examples to
keep them consistent with the implementation.
In `@scripts/generate-demo-data.py`:
- Around line 36-43: Several print calls use f-strings even though they contain
no placeholders (causing F541); remove the unnecessary leading "f" for those
constant messages—e.g. change prints that currently use f" - Strong matches:
20", f" - Moderate matches: 30", f" - Weak matches: 30", f" - Poor
matches: 20" (and the other constant f-strings flagged elsewhere) to plain
string literals like " - Strong matches: 20"; leave f-strings that do include
expressions (e.g., f"Generated demo data in {output_dir}/", f" HR documents:
{len(hr_docs)} ...", f" Security documents: {len(security_docs)} ...", f"
Finance documents: {len(finance_docs)} ...") unchanged.
In `@scripts/README.md`:
- Around line 34-36: Replace the opening fenced code block marker (```) that
wraps the line "Use the `requesting-code-review` skill to review this PR." with
a language-tagged fence (for example ```text) so the block includes a language
identifier and avoids markdownlint MD040; keep the closing ``` unchanged.
In `@scripts/seed-demo-data.sh`:
- Around line 22-24: The case branches that set SCENARIO and BASE_URL in the
argument parsing code read $2 directly and will crash under set -u if the option
is passed with no value; update the handling for the --scenario and --url
branches (the case that assigns SCENARIO="$2" and BASE_URL="$2") to first verify
a non-empty next argument exists and is not another option (e.g., test that "$2"
is set and does not start with '-') before assigning and shifting; if the check
fails, print the script usage/help and exit with a non-zero status so missing
values are handled gracefully.
---
Nitpick comments:
In `@demo/batch/dashboard/static/security.html`:
- Around line 67-71: Remove the unused variable declarations _origRenderState,
_renderStateOrig, and origPollStatus which are declared from renderState and
pollStatus but never referenced; update the file by deleting the three lines
that set const _origRenderState = window.renderState || renderState;, const
_renderStateOrig = renderState;, and const origPollStatus = pollStatus; so the
override logic that follows uses only the actual renderState/pollStatus
functions without dead variables.
In `@demo/batch/Dockerfile.finance`:
- Around line 1-10: The Dockerfile uses mutable image tags in the two FROM lines
(the golang:1.25-alpine builder stage and the alpine:3.23 runtime stage);
replace each tag with an immutable digest form (e.g., FROM
golang:1.25-alpine@sha256:<digest> and FROM alpine:3.23@sha256:<digest>) to pin
the exact manifests, fetching the correct SHA256 digests from the official
registries (or your internal registry) and updating those two FROM lines so
builds are reproducible and resistant to supply-chain drift.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: f87e752b-7c27-4043-b0b5-6fe5c048501a
⛔ Files ignored due to path filters (2)
demo/batch/dashboard/go.sumis excluded by!**/*.sumdocs/demo/oc-adm-top-pod.pngis excluded by!**/*.png
📒 Files selected for processing (58)
.gitignoreREADME.mddemo/batch/Dockerfile.financedemo/batch/Dockerfile.hrdemo/batch/Dockerfile.securitydemo/batch/README.mddemo/batch/dashboard/Dockerfiledemo/batch/dashboard/README.mddemo/batch/dashboard/a2a.godemo/batch/dashboard/deployment.yamldemo/batch/dashboard/go.moddemo/batch/dashboard/k8s.godemo/batch/dashboard/main.godemo/batch/dashboard/scenarios.godemo/batch/dashboard/static/app.jsdemo/batch/dashboard/static/finance.htmldemo/batch/dashboard/static/hr.htmldemo/batch/dashboard/static/index.htmldemo/batch/dashboard/static/security.htmldemo/batch/dashboard/static/style.cssdemo/batch/data/finance-documents.jsondemo/batch/data/hr-documents.jsondemo/batch/data/security-documents.jsondemo/batch/demo-log-2026-05-12.mddemo/batch/document-service/Dockerfiledemo/batch/document-service/configmap.yamldemo/batch/document-service/deployment.yamldemo/batch/document-service/main.godemo/batch/finance/agent-card.jsondemo/batch/finance/agent-config.yamldemo/batch/finance/deployment-template.yamldemo/batch/finance/deployment.yamldemo/batch/finance/finance-batch.shdemo/batch/finance/system-prompt.txtdemo/batch/hr/agent-card.jsondemo/batch/hr/agent-config.yamldemo/batch/hr/deployment.yamldemo/batch/hr/system-prompt.txtdemo/batch/namespace-setup.yamldemo/batch/security/agent-card.jsondemo/batch/security/agent-config.yamldemo/batch/security/deployment.yamldemo/batch/security/system-prompt.txtdocs/demo/agent-serviceaccount.yamldocs/demo/executive-assistant.yamldocs/demo/hr-analyst.yamldocs/demo/skill-discovery-test.yamldocs/superpowers/plans/2026-05-07-mcp-client-integration.mddocs/superpowers/plans/2026-05-09-batch-demo.mddocs/superpowers/specs/2026-05-07-mcp-client-integration-design.mddocs/superpowers/specs/2026-05-09-batch-demo-design.mdinternal/cmd/batch.gointernal/cmd/batch_test.goscripts/README.mdscripts/demo-run.shscripts/generate-demo-data.pyscripts/pr-review.shscripts/seed-demo-data.sh
Comment on lines
+115
to
+123
| taskID, _ := extractField(taskData, "id") | ||
| state, _ := extractNestedField(taskData, "status", "state") | ||
| artifact := extractArtifactText(taskData) | ||
|
|
||
| return TaskStatus{ | ||
| ID: taskID, | ||
| State: state, | ||
| Artifact: artifact, | ||
| }, nil |
There was a problem hiding this comment.
Fail fast when task.id is missing in SendMessage response.
Returning success with an empty task ID pushes the real failure downstream and makes status polling unreliable.
Proposed fix
taskID, _ := extractField(taskData, "id")
state, _ := extractNestedField(taskData, "status", "state")
artifact := extractArtifactText(taskData)
+ if taskID == "" {
+ return TaskStatus{}, fmt.Errorf("A2A response missing task.id from %s", agentURL)
+ }
return TaskStatus{
ID: taskID,📝 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.
Suggested change
| taskID, _ := extractField(taskData, "id") | |
| state, _ := extractNestedField(taskData, "status", "state") | |
| artifact := extractArtifactText(taskData) | |
| return TaskStatus{ | |
| ID: taskID, | |
| State: state, | |
| Artifact: artifact, | |
| }, nil | |
| taskID, _ := extractField(taskData, "id") | |
| state, _ := extractNestedField(taskData, "status", "state") | |
| artifact := extractArtifactText(taskData) | |
| if taskID == "" { | |
| return TaskStatus{}, fmt.Errorf("A2A response missing task.id from %s", agentURL) | |
| } | |
| return TaskStatus{ | |
| ID: taskID, | |
| State: state, | |
| Artifact: artifact, | |
| }, nil |
🤖 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 `@demo/batch/dashboard/a2a.go` around lines 115 - 123, The code currently
extracts taskID using extractField and returns a TaskStatus even when taskID is
empty; update the SendMessage response handling so it fails fast: after taskID,
check if taskID == "" (or nil-equivalent) and return a descriptive error instead
of a TaskStatus, e.g. "missing task.id in SendMessage response"; keep extracting
state and artifact only when taskID is present. Ensure the error is returned
from the function that constructs the TaskStatus so callers don't receive empty
IDs.
Comment on lines
+11
to
+21
| fetch(`/api/run/${name}`, { method: 'POST' }) | ||
| .then(r => r.json()) | ||
| .then(() => { | ||
| startPolling(name); | ||
| startTimer(); | ||
| }) | ||
| .catch(err => { | ||
| btn.disabled = false; | ||
| btn.textContent = 'Run'; | ||
| alert('Failed to start: ' + err.message); | ||
| }); |
There was a problem hiding this comment.
HTTP error handling is fragile and can leave the dashboard in a bad state.
r.json() is called without checking r.ok, and polling errors are swallowed. If backend returns non-JSON error bodies (e.g., 409/500), UI can get stuck (disabled buttons/timer still running) with no actionable message.
Suggested fix pattern
+function parseJSONOrThrow(r) {
+ if (!r.ok) {
+ return r.text().then(t => { throw new Error(t || `HTTP ${r.status}`); });
+ }
+ return r.json();
+}
@@
- fetch(`/api/run/${name}`, { method: 'POST' })
- .then(r => r.json())
+ fetch(`/api/run/${name}`, { method: 'POST' })
+ .then(parseJSONOrThrow)
@@
- fetch(`/api/status/${name}`)
- .then(r => r.json())
+ fetch(`/api/status/${name}`)
+ .then(parseJSONOrThrow)
@@
- .catch(() => {});
+ .catch(() => {
+ stopPolling();
+ });
@@
- fetch(`/api/cleanup/${name}`, { method: 'POST' })
- .then(r => r.json())
+ fetch(`/api/cleanup/${name}`, { method: 'POST' })
+ .then(parseJSONOrThrow)
+ .catch(err => {
+ btn.disabled = false;
+ btn.textContent = 'Cleanup';
+ alert('Cleanup failed: ' + err.message);
+ });Also applies to: 50-66, 174-189
🤖 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 `@demo/batch/dashboard/static/app.js` around lines 11 - 21, The fetch success
path currently calls r.json() without checking r.ok and starts
startPolling(name) and startTimer() even for non-OK responses; update the fetch
handling in the POST block (and other similar blocks at 50-66 and 174-189) to
first check response.ok, attempt to parse JSON safely (fall back to text if JSON
parse fails), and if response.ok is false throw a handled error that re-enables
the button (btn.disabled = false; btn.textContent = 'Run') and shows a useful
message; only call startPolling(name) and startTimer() after a confirmed OK
response, and ensure polling errors are surfaced to the UI (stop timer/polling
and re-enable controls) rather than being swallowed.
Comment on lines
+224
to
+227
| cmd := exec.Command(c.Command, c.Args...) | ||
| if len(c.Env) > 0 { | ||
| cmd.Env = append(cmd.Environ(), envSlice(c.Env)...) | ||
| } |
There was a problem hiding this comment.
Security: Subprocess inherits parent environment variables.
Line 226 uses cmd.Environ() which copies the parent process's entire environment to the MCP server subprocess. This could unintentionally expose sensitive credentials, tokens, or internal configuration to the external MCP server process.
Consider starting with an empty environment and only passing explicitly configured variables, or document this as an intentional design decision if the inheritance is required.
🔒 Proposed fix to isolate subprocess environment
case "stdio":
cmd := exec.Command(c.Command, c.Args...)
if len(c.Env) > 0 {
- cmd.Env = append(cmd.Environ(), envSlice(c.Env)...)
+ cmd.Env = envSlice(c.Env)
}
return &mcp.CommandTransport{If parent environment inheritance is intentionally required, consider adding an explicit inherit_env: true config flag and documenting the security implications.
📝 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.
Suggested change
| cmd := exec.Command(c.Command, c.Args...) | |
| if len(c.Env) > 0 { | |
| cmd.Env = append(cmd.Environ(), envSlice(c.Env)...) | |
| } | |
| cmd := exec.Command(c.Command, c.Args...) | |
| if len(c.Env) > 0 { | |
| cmd.Env = envSlice(c.Env) | |
| } |
🤖 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 `@docs/superpowers/plans/2026-05-07-mcp-client-integration.md` around lines 224
- 227, The subprocess is inheriting the entire parent environment via
cmd.Environ(), which can leak secrets; change the logic around
exec.Command/c.Command/c.Args so cmd.Env is set to a minimal, explicit list
built from c.Env (use envSlice(c.Env)) rather than appending cmd.Environ();
alternatively add and check an explicit boolean config like inherit_env before
copying cmd.Environ() and clearly document its security implications so only
explicitly allowed inheritance occurs.
Comment on lines
+768
to
+779
| func (m *Manager) Close() error { | ||
| var lastErr error | ||
| for _, s := range m.servers { | ||
| if err := s.session.Close(); err != nil { | ||
| slog.Info("disconnected from MCP server", "name", s.name) | ||
| lastErr = err | ||
| } else { | ||
| slog.Info("disconnected from MCP server", "name", s.name) | ||
| } | ||
| } | ||
| return lastErr | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Clarify Close() error handling and logging.
The Close() method logs "disconnected from MCP server" in both the error and success paths (lines 772 and 775). If session.Close() returns an error, it typically means the disconnection failed or encountered issues, so logging "disconnected" in the error case could be misleading during debugging.
Consider either:
- Logging different messages for success vs. error cases
- Clarifying whether the error represents a failure to disconnect or just a non-critical cleanup issue
♻️ Proposed fix for clearer logging
func (m *Manager) Close() error {
var lastErr error
for _, s := range m.servers {
if err := s.session.Close(); err != nil {
- slog.Info("disconnected from MCP server", "name", s.name)
+ slog.Warn("error closing MCP server connection", "name", s.name, "error", err)
lastErr = err
} else {
slog.Info("disconnected from MCP server", "name", s.name)
}
}
return lastErr
}📝 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.
Suggested change
| func (m *Manager) Close() error { | |
| var lastErr error | |
| for _, s := range m.servers { | |
| if err := s.session.Close(); err != nil { | |
| slog.Info("disconnected from MCP server", "name", s.name) | |
| lastErr = err | |
| } else { | |
| slog.Info("disconnected from MCP server", "name", s.name) | |
| } | |
| } | |
| return lastErr | |
| } | |
| func (m *Manager) Close() error { | |
| var lastErr error | |
| for _, s := range m.servers { | |
| if err := s.session.Close(); err != nil { | |
| slog.Warn("error closing MCP server connection", "name", s.name, "error", err) | |
| lastErr = err | |
| } else { | |
| slog.Info("disconnected from MCP server", "name", s.name) | |
| } | |
| } | |
| return lastErr | |
| } |
🤖 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 `@docs/superpowers/plans/2026-05-07-mcp-client-integration.md` around lines 768
- 779, The Close method currently logs "disconnected from MCP server" for both
success and error branches which is misleading; update Manager.Close to log a
distinct error message when s.session.Close() returns an error (e.g., use
slog.Error or slog.Warn with message like "failed to disconnect from MCP server"
and include s.name and the error) and keep the success branch logging
"disconnected from MCP server" for clarity, ensuring you still set lastErr = err
when an error occurs so the method returns the last error encountered.
Comment on lines
+320
to
+321
| vuln_content += f"Scan Date: {datetime.now().strftime('%Y-%m-%d')}\n" | ||
| vuln_content += f"Total Findings: {len(vulns)}\n\n" |
There was a problem hiding this comment.
Data generation is not fully deterministic due to datetime.now() usage.
Even with random.seed(42), date fields change per run, so generated JSON is not reproducible.
Suggested fix
def main():
"""Generate all demo data files."""
random.seed(42)
+ base_now = datetime(2026, 3, 31, 12, 0, 0)
@@
- security_docs = generate_security_documents()
+ security_docs = generate_security_documents(base_now)
@@
-def generate_security_documents():
+def generate_security_documents(base_now):
@@
- vuln_content += f"Scan Date: {datetime.now().strftime('%Y-%m-%d')}\n"
+ vuln_content += f"Scan Date: {base_now.strftime('%Y-%m-%d')}\n"
@@
- vulns = generate_vulnerabilities()
+ vulns = generate_vulnerabilities(base_now)
@@
- assets = generate_asset_inventory()
+ assets = generate_asset_inventory(base_now)Also applies to: 382-382, 411-411, 426-429
🤖 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 `@scripts/generate-demo-data.py` around lines 320 - 321, The code uses
datetime.now() in multiple places (e.g., when appending to vuln_content) which
makes outputs non-deterministic despite random.seed; replace direct
datetime.now() calls with a single deterministically computed date variable
(e.g., deterministic_date) computed once using the seeded RNG or passed-in CLI
arg/ENV (for example: base_date = datetime(YYYY,MM,DD) +
timedelta(days=randint_from_seed)), and then use
deterministic_date.strftime('%Y-%m-%d') wherever datetime.now() is currently
used (references: vuln_content concatenation lines and the other occurrences
mentioned around lines 382, 411, 426-429) so generated JSON becomes
reproducible.
Comment on lines
+32
to
+35
| TITLE=$(echo "$PR_JSON" | jq -r '.title') | ||
| BODY=$(echo "$PR_JSON" | jq -r '.body // empty') | ||
| AUTHOR=$(echo "$PR_JSON" | jq -r '.author.login') | ||
| ADDITIONS=$(echo "$PR_JSON" | jq -r '.additions') |
There was a problem hiding this comment.
Sanitize PR metadata before printing to terminal.
TITLE/AUTHOR/BODY are untrusted input. Printing raw values can allow terminal escape-sequence injection.
Proposed hardening
+sanitize_for_terminal() {
+ LC_ALL=C tr -d '\000-\010\013\014\016-\037\177'
+}
+
if command -v jq &>/dev/null; then
- TITLE=$(echo "$PR_JSON" | jq -r '.title')
- BODY=$(echo "$PR_JSON" | jq -r '.body // empty')
- AUTHOR=$(echo "$PR_JSON" | jq -r '.author.login')
+ TITLE=$(echo "$PR_JSON" | jq -r '.title' | sanitize_for_terminal)
+ BODY=$(echo "$PR_JSON" | jq -r '.body // empty' | sanitize_for_terminal)
+ AUTHOR=$(echo "$PR_JSON" | jq -r '.author.login' | sanitize_for_terminal)Also applies to: 75-78
🤖 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 `@scripts/pr-review.sh` around lines 32 - 35, The PR metadata variables TITLE,
BODY, AUTHOR and ADDITIONS are printed directly and may contain terminal
control/ANSI escape sequences; add a sanitize step (e.g., a shell function
sanitize() that removes/control-characters and ANSI sequences such as regex
replacing [\x00-\x1F\x7F] and \x1B\[[0-9;]*[A-Za-z]) and run sanitize on each
value after extracting with jq (replace references to TITLE, BODY, AUTHOR,
ADDITIONS with sanitized versions); apply the same sanitization where those
variables are used later (the other occurrences noted around lines 75-78).
Comment on lines
+50
to
+55
| try: | ||
| urllib.request.urlopen(req) | ||
| success += 1 | ||
| except Exception as e: | ||
| print(f' WARN: {doc[\"id\"]}: {e}', file=sys.stderr) | ||
| print(f' Loaded {success}/{len(docs)} documents') |
There was a problem hiding this comment.
Add a timeout to each document POST to avoid indefinite hangs.
urllib.request.urlopen(req) has no timeout; a slow/unreachable service can block the script indefinitely and stall the demo flow.
Suggested fix
- urllib.request.urlopen(req)
+ urllib.request.urlopen(req, timeout=10)🤖 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 `@scripts/seed-demo-data.sh` around lines 50 - 55, urllib.request.urlopen(req)
is being called without a timeout which can hang indefinitely; change the call
in the loop that posts documents (the urlopen invocation where variables req,
success and docs are used) to pass a timeout (e.g. timeout=5 or read from an env
var) and update the exception handling to specifically catch
socket.timeout/urllib.error.URLError where appropriate and log a clear timeout
warning for doc["id"] while still incrementing/continuing as before.
- a2a.go: check HTTP status and Content-Type in GetTaskStatus before unmarshaling; add truncate helper for error messages - k8s.go: check all container statuses for readiness, not just the first (handles sidecar pods correctly) - main.go: set scenario phase to "failed" if any agent failed; inline strings.HasPrefix; handle fs.Sub error - app.js: check response.ok before parsing JSON in fetch calls - document-service: fail fast on duplicate document IDs during seed data loading - security.html: remove unused variable declarations - README.md: update finance description to reflect 5-agent fan-out Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
All three scenario pages (finance, security, HR) now render reports as HTML with styled tables, headings, and bold text instead of raw markdown. Removes duplicate renderMarkdown from hr.html. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Conference demo showcasing DocsClaw's lightweight agent runtime
through three business scenarios, orchestrated from a web dashboard.
invoices, flagging rate deviations and uncontracted charges
asset inventory and SLA deadlines to produce a prioritized
remediation plan
description with scored recommendations
Key components
seed data via
GET /documents/{id}files that deploys agents, tracks A2A task completion, and
displays live memory/CPU/token metrics
seed data for all three scenarios
Demo highlights
agents
resource metrics
Test plan
show reports with anomaly findings
breach prioritization
batches, scored reports
-kubeconfigSummary by CodeRabbit
New Features
docsclaw batchcommand for parallel document processing across multiple agentsDocumentation