diff --git a/.claude/commands/pr-shepherd.md b/.claude/commands/pr-shepherd.md new file mode 100644 index 00000000..c6dc87a1 --- /dev/null +++ b/.claude/commands/pr-shepherd.md @@ -0,0 +1,303 @@ +--- +description: Full PR lifecycle — review, fix findings, address comments, quality gate, push, CI fix loop, merge +disable-model-invocation: true +allowed-tools: Bash(gh pr view:*), Bash(gh pr diff:*), Bash(gh pr comment:*), Bash(gh pr merge:*), Bash(gh pr checks:*), Bash(gh pr edit:*), Bash(gh pr list:*), Bash(gh pr checkout:*), Bash(gh api:*), Bash(gh repo view:*), Bash(gh run view:*), Bash(gh run watch:*), Bash(git diff:*), Bash(git log:*), Bash(git fetch:*), Bash(git checkout:*), Bash(git status:*), Bash(git branch:*), Bash(git add:*), Bash(git commit:*), Bash(git push:*), Bash(git merge:*), Bash(git rebase:*), Bash(cargo fmt:*), Bash(cargo clippy:*), Bash(cargo test:*), Bash(cargo check:*), Read, Edit, Write, Grep, Glob, Agent +argument-hint: " [--fix] [--merge] [--review-only]" +--- + +# PR Shepherd + +Full PR lifecycle: review → fix → quality gate → push → CI → merge. + +Parse `$ARGUMENTS`: +- Extract PR number from bare number or `https://github.com/owner/repo/pull/123` URL. +- Flags: `--fix` (auto-fix without asking), `--merge` (merge when CI green), `--review-only` (stop after review, don't fix). +- If no PR number, detect from current branch: `gh pr list --head $(git branch --show-current) --json number --jq '.[0].number'` +- If still nothing, stop and ask the user. + +--- + +## Phase 1: Situational Awareness + +Gather everything in parallel: + +**PR metadata:** +``` +gh pr view {number} --json number,title,body,author,baseRefName,headRefName,headRefOid,state,isDraft,mergeable,mergeStateStatus,files,additions,deletions,labels,reviewRequests +``` + +**Diff:** +``` +gh pr diff {number} +gh pr diff {number} --name-only +``` + +**CI status:** +``` +gh pr checks {number} --json name,status,conclusion,detailsUrl +``` + +**Review comments (human + bot):** +``` +gh api --paginate repos/{owner}/{repo}/pulls/{number}/comments +gh api --paginate repos/{owner}/{repo}/pulls/{number}/reviews +``` + +Resolve `{owner}/{repo}`: +``` +gh repo view --json owner,name --jq '"\(.owner.login)/\(.name)"' +``` + +Save `headRefOid` — needed for posting line comments later. + +**Assess the situation and print a status card:** + +``` +PR #{number}: {title} +Author: {author} Base: {base} ← {head} +Size: +{additions} -{deletions} across {file_count} files +CI: {PASS|FAIL|PENDING|NONE} Mergeable: {yes|no|conflict} +Reviews: {N approved, N changes_requested, N comments-only, N bot-only} +Unresolved comments: {N} +Draft: {yes|no} +``` + +**Decide the mode** based on situation: +- **Has unresolved review comments** → Phase 2a (address comments first, then review remaining) +- **No reviews yet / bot-only reviews** → Phase 2b (full deep review) +- **CI failing, no review issues** → Phase 4 (jump to CI fix) +- **Everything green + approved** → Phase 6 (ready to merge) + +--- + +## Phase 2a: Address Existing Review Comments + +For each unresolved review comment or review with CHANGES_REQUESTED: + +1. **Read the referenced code** at the file and line mentioned. Never assess without reading. +2. **Classify each comment:** + - ✅ **Valid & unresolved** — needs a code fix + - ✅ **Already fixed** — a later commit addressed it + - ❌ **False positive** — explain why the code is correct + - 🔧 **Nit** — optional improvement, not blocking + +3. **Deduplicate** — bots (Copilot, Gemini) often post the same finding. Group by actual issue. + +Present a table: + +| # | Source | File:Line | Issue | Status | Planned Fix | +|---|--------|-----------|-------|--------|-------------| + +Wait for user confirmation (unless `--fix` flag set), then proceed to Phase 3. + +--- + +## Phase 2b: Deep Review (6 Lenses) + +Read EVERY changed file in full (not just diff hunks). For PRs touching >20 files, prioritize: service logic > handlers > types > tests > docs. Batch reads in parallel via Agent tool. + +### IronClaw-specific checks (always) +- No `.unwrap()` or `.expect()` in production code +- Prefer `crate::` for cross-module imports (`super::` OK in tests/intra-module) +- Error types use `thiserror` +- If persistence touched, both backends updated (postgres.rs AND libsql/) +- New tools implement `Tool` trait correctly and registered +- External tool output passes through safety layer +- Tool parameters redacted before logging/SSE +- No byte-index slicing on external strings +- Case-insensitive comparisons where needed + +### Correctness +Off-by-one, wrong operators, inverted conditions, unreachable code, type confusion, error propagation, broken invariants, TOCTOU races. + +### Edge cases & failure handling +Empty/None/zero-length input, external service failures, integer boundaries, malformed/adversarial input, partial failure handling. + +### Security (assume adversarial actors) +Auth/authz bypass, IDOR, injection (SQL/command/log/header), data leakage in logs/errors/API responses, resource exhaustion, replay/race conditions. + +### Test coverage +New public functions tested? Error paths tested? Edge cases covered? Existing tests still valid? + +### Architecture +Follows existing patterns? Unnecessary abstractions? Duplicated logic? Clean module dependencies? + +**Present findings as a table:** + +| # | Severity | Category | File:Line | Finding | Suggested Fix | +|---|----------|----------|-----------|---------|---------------| + +Severity: Critical > High > Medium > Low > Nit + +If `--review-only` flag is set, post findings as GitHub comments (see Phase 2c) and STOP. + +Otherwise, ask which findings to fix (default: all Critical + High + Medium). Then proceed to Phase 3. + +--- + +## Phase 2c: Post Review Comments on GitHub + +For each finding the user approved (or all Critical/High/Medium if `--fix`): + +**Line-specific findings** — post as PR review comments: +``` +gh api repos/{owner}/{repo}/pulls/{number}/comments \ + -f body="**{Severity}**: {finding}\n\n{explanation}\n\n**Suggested fix:** {suggestion}" \ + -f path="{file}" \ + -f commit_id="{headRefOid}" \ + -F line={line} \ + -f side="RIGHT" +``` + +**Cross-cutting/architectural findings** — post as regular PR comment: +``` +gh pr comment {number} --body "..." +``` + +--- + +## Phase 3: Fix + +Checkout the PR branch if not already on it (handles fork PRs automatically): +``` +gh pr checkout {number} +``` + +**Implement fixes** for: +1. All approved review comment fixes (from Phase 2a) +2. All approved review findings (from Phase 2b) + +Follow IronClaw conventions: +- `thiserror` for errors +- `crate::` imports +- No `.unwrap()` in production +- Both DB backends if persistence touched +- Regression test for every bug fix (enforced by commit-msg hook; bypass only with `[skip-regression-check]` if genuinely not feasible) + +After all fixes implemented, proceed to Phase 4. + +--- + +## Phase 4: Quality Gate + +Run the full IronClaw shipping checklist: + +```bash +cargo fmt +``` + +```bash +cargo clippy --all --benches --tests --examples --all-features +``` + +```bash +cargo test --lib +``` + +If persistence changes are present, also verify feature isolation: +```bash +cargo check --no-default-features --features libsql +cargo check --all-features +``` + +**If any step fails:** fix the issue and re-run. Do NOT proceed past a failing step. Loop up to 3 times per step. If still failing after 3 attempts, report the failure and stop. + +--- + +## Phase 5: Commit & Push + +Stage changed files by name (never `git add -A` — it can include unintended files): +```bash +git add path/to/changed/file1 path/to/changed/file2 +git commit -m "{message}" +``` + +Commit message format: +- For review fixes: `fix: address review findings on PR #{number}` +- For comment responses: `fix: address review comments on PR #{number}` +- For CI fixes: `fix: resolve CI failures on PR #{number}` +- Include specifics in the body (which findings/comments were addressed) + +Push: +```bash +git push origin {headRefName} +``` + +**Reply to addressed review comments on GitHub.** For each comment that was fixed, reply with the commit SHA and a brief description of what was done. For false positives, reply explaining why no change was needed. + +--- + +## Phase 6: CI Monitor & Fix Loop + +Wait briefly for CI to start, then poll (do NOT use `--watch` as it can hang indefinitely): +``` +gh pr checks {number} --json name,status,conclusion +``` + +Re-check every 30 seconds, up to 10 minutes. If still pending after 10 minutes, report status and ask the user whether to keep waiting. + +**If CI passes** → proceed to Phase 7. + +**If CI fails** (up to 3 fix attempts): + +1. Identify the failing check: + ``` + gh run view {run_id} --log-failed + ``` + If `--log-failed` shows nothing useful: + ``` + gh run view {run_id} --log | tail -100 + ``` + +2. Diagnose and fix the failure. +3. Re-run Phase 4 (quality gate). +4. Commit and push (Phase 5). +5. Go back to top of Phase 6. + +**After 3 failed CI fix attempts:** Report what's failing and why, then stop. Don't keep looping. + +--- + +## Phase 7: Merge Decision + +Print final status: +``` +PR #{number}: {title} +CI: ✅ PASS +Reviews: {summary} +Findings fixed: {N} +Comments addressed: {N} +Commits added: {N} +``` + +**Auto-merge conditions** (if `--merge` flag or user confirms): +- CI is passing +- No unresolved CHANGES_REQUESTED reviews +- PR is not draft +- PR is mergeable (no conflicts) + +If all conditions met, ask the user for merge strategy: + +"CI is green. Merge this PR? [squash/rebase/merge/no]" + +Then execute: +``` +gh pr merge {number} --{strategy} --delete-branch +``` + +If any condition NOT met, report what's blocking and let the user decide. + +--- + +## Rules + +- **Read before judging.** Never comment on code you haven't read in full. Verify line numbers. +- **Be specific.** "Line 42 returns 404 but should return 400 because X" not "this might have issues." +- **Fix the pattern, not just the instance.** When fixing a bug, grep for the same pattern across `src/`. +- **Respect the commit-msg hook.** Bug fixes need regression tests. Use `[skip-regression-check]` only if genuinely not feasible. +- **Don't over-fix.** Only change what was flagged. Don't refactor surrounding code or add improvements beyond the review scope. +- **Credit original authors.** If taking over someone else's PR, credit them in commits and comments. +- **No secrets in comments.** Never include customer data, credentials, or PII in GitHub comments. +- **Distinguish certainty.** "This IS a bug" vs "This COULD be a bug if X." Be honest. +- **Round up severity when uncertain.** Cheaper to dismiss a false alarm than miss a real bug. +- **Parallel where possible.** Use Agent tool for parallel file reads on large PRs. Batch `gh api` calls. diff --git a/.claude/rules/database.md b/.claude/rules/database.md new file mode 100644 index 00000000..07accf07 --- /dev/null +++ b/.claude/rules/database.md @@ -0,0 +1,63 @@ +--- +paths: + - "src/db/**" + - "src/history/**" + - "migrations/**" +--- +# Database Rules + +Dual-backend persistence: PostgreSQL + libSQL/Turso. **All new persistence features must support both backends.** + +See `src/db/CLAUDE.md` for full schema, dialect differences, and libSQL limitations. + +## Adding a New Operation + +1. Decide which sub-trait it belongs to (`ConversationStore`, `JobStore`, `SandboxStore`, `RoutineStore`, `ToolFailureStore`, `SettingsStore`, `WorkspaceStore`) or create a new one +2. Add the async method signature to that sub-trait in `src/db/mod.rs` +3. Implement in `src/db/postgres.rs` (delegate to `Store`/`Repository`) +4. Implement in `src/db/libsql/.rs` (use `self.connect().await?` per operation) +5. Add migration if needed: + - PostgreSQL: new `migrations/VN__description.sql` + - libSQL: add `CREATE TABLE IF NOT EXISTS` to `libsql_migrations.rs` +6. Test feature isolation: + ```bash + cargo check # postgres (default) + cargo check --no-default-features --features libsql # libsql only + cargo check --all-features # both + ``` + +## SQL Dialect Translation Checklist + +When writing SQL for both backends, translate these types: + +| PostgreSQL | libSQL | +|-----------|--------| +| `UUID` | `TEXT` | +| `TIMESTAMPTZ` | `TEXT` (ISO-8601, write with `fmt_ts()`, read with `get_ts()`) | +| `JSONB` | `TEXT` (JSON string) | +| `BOOLEAN` | `INTEGER` (0/1 -- use `get_i64(row, idx) != 0` to read) | +| `NUMERIC` | `TEXT` (preserves `rust_decimal` precision) | +| `TEXT[]` | `TEXT` (JSON-encoded array) | +| `VECTOR` | `BLOB` (flexible dimensions; vector index dropped, brute-force search fallback) | +| `jsonb_set(col, '{key}', val)` | `json_patch(col, '{"key": val}')` -- replaces top-level keys entirely, cannot do partial nested updates | +| `DEFAULT NOW()` | `DEFAULT (datetime('now'))` | +| `tsvector` + `ts_rank_cd` | FTS5 virtual table + sync triggers | + +## Schema Translation Beyond DDL + +Don't just translate `CREATE TABLE`. Also check: +- **Indexes** -- diff `CREATE INDEX` statements between backends +- **Seed data** -- check for `INSERT INTO` in migrations (e.g., `leak_detection_patterns`) +- **Triggers** -- PostgreSQL functions vs SQLite triggers (no stored procs in SQLite) + +## Transaction Safety + +Multi-step operations (INSERT+INSERT, UPDATE+DELETE, read-modify-write) MUST be wrapped in a transaction. Ask: "If this crashes between step N and N+1, is the database consistent?" If not, wrap in a transaction. Applies to both backends. + +## libSQL Connection Model + +`LibSqlBackend::connect()` creates a fresh connection per operation with `PRAGMA busy_timeout = 5000`. This is intentional -- no pool exists. Never hold connections open across `await` points. Satellite stores (`LibSqlSecretsStore`, `LibSqlWasmToolStore`) receive `Arc` via `shared_db()` and call `.connect()` themselves -- never pass a live `Connection`. + +## Fix the Pattern, Not the Instance + +When fixing a bug in one backend's SQL, always grep for the same pattern in the other. A fix to `postgres.rs` that doesn't also fix `libsql/jobs.rs` is half a fix. Same applies to satellite stores. diff --git a/.claude/rules/review-discipline.md b/.claude/rules/review-discipline.md new file mode 100644 index 00000000..74ace30a --- /dev/null +++ b/.claude/rules/review-discipline.md @@ -0,0 +1,48 @@ +--- +paths: + - "src/**/*.rs" +--- +# Review & Fix Discipline + +Hard-won lessons from code review -- follow these when fixing bugs or addressing review feedback. + +**Fix the pattern, not just the instance:** When a reviewer flags a bug (e.g., TOCTOU race in INSERT + SELECT-back), search the entire codebase for all instances of that same pattern. A fix in `SecretsStore::create()` that doesn't also fix `WasmToolStore::store()` is half a fix. + +**Propagate architectural fixes to satellite types:** If a core type changes its concurrency model (e.g., `LibSqlBackend` switches to connection-per-operation), every type that was handed a resource from the old model must also be updated. Grep for the old type across the codebase. + +**Schema translation is more than DDL:** When translating a database schema between backends (PostgreSQL to libSQL, etc.), check for: +- **Indexes** -- diff `CREATE INDEX` statements between the two schemas +- **Seed data** -- check for `INSERT INTO` in migrations (e.g., `leak_detection_patterns`) +- **Semantic differences** -- document where SQL functions behave differently (e.g., `json_patch` vs `jsonb_set`) + +**Feature flag testing:** When adding feature-gated code, test compilation with each feature in isolation: +```bash +cargo check # default features +cargo check --no-default-features --features libsql # libsql only +cargo check --all-features # all features +``` + +**Regression test with every fix:** Every bug fix must include a test that would have caught the bug. Add a `#[test]` or `#[tokio::test]` that reproduces the original failure. Exempt: changes limited to `src/channels/web/static/` or `.md` files. Use `[skip-regression-check]` in commit message or PR label if genuinely not feasible. The `commit-msg` hook and CI workflow enforce this automatically. + +**Zero clippy warnings policy:** Fix ALL clippy warnings before committing, including pre-existing ones in files you didn't change. Never leave warnings behind. + +**Transaction safety:** Multi-step database operations (INSERT+INSERT, UPDATE+DELETE, read-then-write) MUST be wrapped in a transaction. Never assume sequential calls are atomic. This applies to both postgres and libsql backends. + +**UTF-8 string safety:** Never use byte-index slicing (`&s[..n]`) on user-supplied or external strings -- it panics on multi-byte characters. Use `is_char_boundary()` or `char_indices()`. Grep for `[..` in changed files. + +**Case-insensitive comparisons:** When comparing user-supplied strings (file paths, media types, extension names), normalize to lowercase with `.to_ascii_lowercase()`. Path comparisons must be case-insensitive on macOS/Windows. + +**Decorator/wrapper trait delegation:** When adding a new method to `LlmProvider` (or any trait with decorator wrappers), update ALL wrapper types to delegate. Grep for `impl LlmProvider for` to find all implementations. Test through the full provider chain. + +**Sensitive data in logs & events:** Tool parameters and outputs MUST be redacted before logging or broadcasting via SSE/WebSocket. Use `redact_params()` before any `tracing::info!`, `JobEvent`, or SSE emission that includes tool call data. + +**Test temporary files:** Use the `tempfile` crate. Never hardcode `/tmp/...` paths. + +**Trust boundaries in multi-process architecture:** Data from worker containers is untrusted. The orchestrator MUST validate: tool domain, nesting depth (server-side tracking), and parameter sensitivity. + +**Mechanical verification before committing:** +- `cargo clippy --all --benches --tests --examples --all-features` -- zero warnings +- `grep -rnE '\.unwrap\(|\.expect\(' ` -- no panics in production +- `grep -rn 'super::' ` -- prefer `crate::` for cross-module imports (`super::` OK in tests/intra-module) +- If you fixed a pattern bug, `grep` for other instances across `src/` +- Run `scripts/pre-commit-safety.sh` to catch UTF-8, case-sensitivity, hardcoded /tmp, and logging issues diff --git a/.claude/rules/safety-and-sandbox.md b/.claude/rules/safety-and-sandbox.md new file mode 100644 index 00000000..50e1135e --- /dev/null +++ b/.claude/rules/safety-and-sandbox.md @@ -0,0 +1,34 @@ +--- +paths: + - "src/safety/**" + - "src/sandbox/**" + - "src/secrets/**" + - "src/tools/wasm/**" +--- +# Safety Layer & Sandbox Rules + +## Safety Layer + +All external tool output passes through `SafetyLayer`: +1. **Sanitizer** - Detects injection patterns, escapes dangerous content +2. **Validator** - Checks length, encoding, forbidden patterns +3. **Policy** - Rules with severity (Critical/High/Medium/Low) and actions (Block/Warn/Review/Sanitize) +4. **Leak Detector** - Scans for 15+ secret patterns at two points: tool output before LLM, and LLM responses before user + +Tool outputs are wrapped in `` XML before reaching the LLM. + +## Shell Environment Scrubbing + +The shell tool scrubs sensitive env vars before executing commands. The sanitizer detects command injection patterns (chained commands, subshells, path traversal). + +## Sandbox Policies + +| Policy | Filesystem | Network | +|--------|-----------|---------| +| ReadOnly | Read-only workspace | Allowlisted domains | +| WorkspaceWrite | Read-write workspace | Allowlisted domains | +| FullAccess | Full filesystem | Unrestricted | + +## Zero-Exposure Credential Model + +Secrets are stored encrypted on the host and injected into HTTP requests by the proxy at transit time. Container processes never see raw credential values. diff --git a/.claude/rules/skills.md b/.claude/rules/skills.md new file mode 100644 index 00000000..ded26de9 --- /dev/null +++ b/.claude/rules/skills.md @@ -0,0 +1,56 @@ +--- +paths: + - "src/skills/**" + - "skills/**" +--- +# Skills System + +SKILL.md files extend the agent's prompt with domain-specific instructions. Each skill is a YAML frontmatter block (metadata, activation criteria, required tools) followed by a markdown body injected into the LLM context. + +## Trust Model + +| Trust Level | Source | Tool Access | +|-------------|--------|-------------| +| **Trusted** | User-placed in `~/.ironclaw/skills/` or workspace `skills/` | All tools available to the agent | +| **Installed** | Downloaded from ClawHub registry (`~/.ironclaw/installed_skills/`) | Read-only tools only (no shell, file write, HTTP) | + +## SKILL.md Format + +```yaml +--- +name: my-skill +version: 0.1.0 +description: Does something useful +activation: + patterns: + - "deploy to.*production" + keywords: + - "deployment" + exclude_keywords: + - "rollback" + tags: + - "devops" + max_context_tokens: 2000 +metadata: + openclaw: + requires: + bins: [docker, kubectl] + env: [KUBECONFIG] +--- + +# Skill instructions here... +``` + +## Selection Pipeline + +1. **Gating** -- Check binary/env/config requirements; skip skills whose prerequisites are missing +2. **Scoring** -- Deterministic scoring: keywords (10/5 pts, cap 30) + patterns (20 pts, cap 40) + tags (3 pts, cap 15). `exclude_keywords` veto (score = 0 if any present) +3. **Budget** -- Select top-scoring skills within `SKILLS_MAX_TOKENS` prompt budget +4. **Attenuation** -- Minimum trust across active skills determines tool ceiling; installed skills lose dangerous tools + +## Skill Tools + +- `skill_list` -- List all discovered skills with trust level and status +- `skill_search` -- Search ClawHub registry for available skills +- `skill_install` -- Download and install a skill from ClawHub +- `skill_remove` -- Remove an installed skill diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md new file mode 100644 index 00000000..3d50b3ea --- /dev/null +++ b/.claude/rules/testing.md @@ -0,0 +1,25 @@ +--- +paths: + - "src/**/*.rs" + - "tests/**" +--- +# Testing Rules + +## Test Tiers + +| Tier | Command | External deps | +|------|---------|---------------| +| Unit | `cargo test` | None | +| Integration | `cargo test --features integration` | Running PostgreSQL | +| Live | `cargo test --features integration -- --ignored` | PostgreSQL + LLM API keys | + +Run `bash scripts/check-boundaries.sh` to verify test tier gating. + +## Key Patterns + +- Unit tests in `mod tests {}` at the bottom of each file +- Async tests with `#[tokio::test]` +- No mocks, prefer real implementations or stubs +- Use `tempfile` crate for test directories, never hardcode `/tmp/` +- Regression test with every bug fix (enforced by commit-msg hook) +- Integration tests (`--test workspace_integration`) require PostgreSQL; skipped if DB is unreachable diff --git a/.claude/rules/tools.md b/.claude/rules/tools.md new file mode 100644 index 00000000..a35d9e23 --- /dev/null +++ b/.claude/rules/tools.md @@ -0,0 +1,39 @@ +--- +paths: + - "src/tools/**" + - "tools-src/**" +--- +# Tool Architecture + +**Keep tool-specific logic out of the main agent codebase.** The main agent provides generic infrastructure; tools are self-contained units that declare requirements through `.capabilities.json` sidecar files (in dev mode: `tools-src//-tool.capabilities.json`). + +Tools can be WASM (sandboxed, credential-injected, single binary) or MCP servers (ecosystem, any language, no sandbox). Both are first-class via `ironclaw tool install`. + +See `src/tools/README.md` for full architecture, adding new tools, auth JSON examples, and WASM vs MCP decision guide. + +## Tool Implementation Pattern + +```rust +#[async_trait] +impl Tool for MyTool { + fn name(&self) -> &str { "my_tool" } + fn description(&self) -> &str { "Does something useful" } + fn parameters_schema(&self) -> serde_json::Value { + serde_json::json!({ + "type": "object", + "properties": { + "param": { "type": "string", "description": "A parameter" } + }, + "required": ["param"] + }) + } + async fn execute(&self, params: serde_json::Value, ctx: &JobContext) + -> Result + { + let start = std::time::Instant::now(); + // ... do work ... + Ok(ToolOutput::text("result", start.elapsed())) + } + fn requires_sanitization(&self) -> bool { true } // External data +} +``` diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml new file mode 100644 index 00000000..86d1bb2f --- /dev/null +++ b/.github/workflows/claude-review.yml @@ -0,0 +1,99 @@ +name: Claude Code Review + +on: + pull_request: + types: [opened, labeled] + +permissions: + contents: read + pull-requests: write + issues: write + id-token: write + +concurrency: + group: claude-review-${{ github.event.pull_request.number || github.run_id }} + cancel-in-progress: true + +jobs: + review: + name: Claude Code Review + if: contains(github.event.pull_request.labels.*.name, 'staging-promotion') + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + with: + fetch-depth: 0 + + - name: Run Claude Code review + uses: anthropics/claude-code-action@v1 + with: + anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} + claude_args: "--max-turns 50 --model claude-haiku-4-5-20251001 --allowedTools 'Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr list:*),Bash(gh issue view:*),Bash(gh issue list:*),Bash(gh search:*),Bash(git blame:*),Bash(git log:*),Bash(git diff:*)'" + prompt: | + Code review this pull request. Follow these steps precisely: + + 1. Use a Haiku agent to find relevant CLAUDE.md files: the root CLAUDE.md + and any CLAUDE.md files in directories whose files this PR modifies. + + 2. Use a Haiku agent to summarize the PR change (use `gh pr diff`). + + 3. Launch 4 parallel agents to review the change independently. Each agent should + read the PR diff with `gh pr diff` and the full source files for changed + code, then return a list of issues found: + + Agent 1 — Security & Safety + Check for: command injection, path traversal, SSRF, XSS, auth bypass, + secrets in logs, .unwrap()/.expect() in production code (not tests), + race conditions, TOCTOU, unsafe blocks, panics in async, unbounded allocations. + + Agent 2 — Architecture & Patterns + Check for: extensible design (traits/enums over nested conditionals), + clean abstractions, proper error types (thiserror), CLAUDE.md compliance, + type-driven design over stringly-typed code, DRY violations. + + Agent 3 — Bug Scan + Shallow diff-only scan for obvious bugs: logic errors, off-by-one, + missing error handling, division by zero, incorrect return values. + Ignore nitpicks and likely false positives. Do NOT read extra context + beyond the diff — focus only on the changes. + + Agent 4 — Performance & Production + Check for: blocking in async, N+1 queries, unbounded loops, missing + timeouts, resource leaks (file handles, connections), large allocations + in hot paths. + + 4. For each issue found, launch a parallel Haiku agent to: + a. Assign a severity: + - CRITICAL: security vulns, panics in prod (.unwrap/.expect), data exfiltration, race conditions + - HIGH: logic bugs, missing error handling, breaking API/schema changes + - MEDIUM: missing tests, unnecessary complexity, performance issues + - LOW: documentation gaps, naming suggestions + b. Score confidence 0-100 (give this rubric verbatim): + 0: False positive, doesn't stand up to scrutiny, or pre-existing issue. + 25: Might be real, but may be false positive. Stylistic issues not in CLAUDE.md. + 50: Real issue but nitpick or rare in practice. Not very important. + 75: Verified real issue, will be hit in practice. Directly impacts functionality + or explicitly mentioned in CLAUDE.md. + 100: Certain, confirmed, will happen frequently. Evidence directly confirms. + + 5. Post a single comment on the PR using `gh pr comment` with this format. + If no issues were found, post "No issues found." instead: + + ### Code review + + Found N issues: + + 1. [SEVERITY:CONFIDENCE] + + + + Example: [CRITICAL:92] `.unwrap()` can panic in production when config is missing + + You MUST use the full git SHA in links (not HEAD or branch name). + Provide 1 line of context before and after each linked range. + + Notes: + - Use `gh` for all GitHub interactions, not web fetch + - Do NOT check build signal or attempt to build/test the code + - Ignore pre-existing issues not introduced by this PR + - Ignore issues a linter/compiler would catch (formatting, imports, types) diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 3dc95a2d..fea70b87 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -1,5 +1,6 @@ name: E2E Tests on: + workflow_call: schedule: - cron: "0 6 * * 1" # Weekly Monday 6 AM UTC workflow_dispatch: diff --git a/.github/workflows/staging-ci.yml b/.github/workflows/staging-ci.yml new file mode 100644 index 00000000..9e887436 --- /dev/null +++ b/.github/workflows/staging-ci.yml @@ -0,0 +1,473 @@ +name: Staging CI (Batched) + +on: + schedule: + - cron: "0 * * * *" # Every 60 minutes + workflow_dispatch: + inputs: + force: + description: "Force run even if no new commits" + type: boolean + default: false + skip_claude_gate: + description: "Skip Claude review gate (bypass blocking findings)" + type: boolean + default: false + +permissions: + contents: write + issues: write + pull-requests: write + checks: read + +concurrency: + group: staging-ci + cancel-in-progress: false # Let running suites finish + +jobs: + # ── Check for new commits ────────────────────────────────────── + check-changes: + name: Check for new commits + runs-on: ubuntu-latest + outputs: + has_changes: ${{ steps.check.outputs.has_changes }} + current_head: ${{ steps.check.outputs.current_head }} + diff_range: ${{ steps.check.outputs.diff_range }} + steps: + - uses: actions/checkout@v6 + with: + ref: staging + fetch-depth: 0 + fetch-tags: true + + - name: Check for changes since last tested + id: check + env: + FORCE_RUN: ${{ inputs.force }} + run: | + CURRENT_HEAD=$(git rev-parse HEAD) + echo "current_head=${CURRENT_HEAD}" >> "$GITHUB_OUTPUT" + + if git rev-parse staging-tested >/dev/null 2>&1; then + LAST_TESTED=$(git rev-parse staging-tested) + else + LAST_TESTED="" + fi + + DIFF_RANGE="" + if [ -n "$LAST_TESTED" ] && [ "$LAST_TESTED" = "$CURRENT_HEAD" ]; then + echo "No new commits since last tested (${CURRENT_HEAD})" + HAS_CHANGES=false + else + HAS_CHANGES=true + if [ -n "$LAST_TESTED" ]; then + COMMIT_COUNT=$(git rev-list --count "${LAST_TESTED}..HEAD") + echo "Found ${COMMIT_COUNT} new commit(s) since last tested" + DIFF_RANGE="${LAST_TESTED}..${CURRENT_HEAD}" + else + git fetch origin main + MERGE_BASE=$(git merge-base origin/main HEAD) + echo "First run -- reviewing from merge-base ${MERGE_BASE}" + DIFF_RANGE="${MERGE_BASE}..${CURRENT_HEAD}" + fi + fi + + # Force override from workflow_dispatch + if [ "$FORCE_RUN" = "true" ]; then + echo "Force run requested" + HAS_CHANGES=true + if [ -z "$DIFF_RANGE" ]; then + DIFF_RANGE="${CURRENT_HEAD}..${CURRENT_HEAD}" + fi + fi + + echo "has_changes=${HAS_CHANGES}" >> "$GITHUB_OUTPUT" + echo "diff_range=${DIFF_RANGE}" >> "$GITHUB_OUTPUT" + + # ── Run full test suite ────────────────────────────────────────── + tests: + name: Test Suite + needs: check-changes + if: needs.check-changes.outputs.has_changes == 'true' + uses: ./.github/workflows/test.yml + + # ── Run E2E browser tests ──────────────────────────────────────── + e2e: + name: E2E Browser Tests + needs: check-changes + if: needs.check-changes.outputs.has_changes == 'true' + uses: ./.github/workflows/e2e.yml + + # ── Create promotion PR (triggers claude-review.yml on the PR) ── + create-promotion-pr: + name: Create Promotion PR + needs: check-changes + if: needs.check-changes.outputs.has_changes == 'true' + runs-on: ubuntu-latest + outputs: + pr_number: ${{ steps.create-pr.outputs.pr_number }} + promotion_branch: ${{ steps.branch.outputs.branch }} + steps: + - uses: actions/checkout@v6 + with: + ref: staging + fetch-depth: 0 + + - name: Generate GitHub App token + id: app-token + if: ${{ secrets.GH_RELEASES_MANAGER_APP_ID != '' }} + uses: actions/create-github-app-token@v2 + with: + app-id: ${{ secrets.GH_RELEASES_MANAGER_APP_ID }} + private-key: ${{ secrets.GH_RELEASES_MANAGER_APP_PRIVATE_KEY }} + + - name: Set token + id: token + run: | + if [ -n "${{ steps.app-token.outputs.token }}" ]; then + echo "token=${{ steps.app-token.outputs.token }}" >> "$GITHUB_OUTPUT" + else + echo "token=${{ github.token }}" >> "$GITHUB_OUTPUT" + fi + + - name: Check if staging is ahead of main + id: ahead-check + env: + GH_TOKEN: ${{ steps.token.outputs.token }} + run: | + git fetch origin main + AHEAD=$(git rev-list --count origin/main..origin/staging) + echo "commits_ahead=${AHEAD}" >> "$GITHUB_OUTPUT" + if [ "$AHEAD" -eq 0 ]; then + echo "Staging is not ahead of main. Nothing to promote." + else + echo "Staging is ${AHEAD} commits ahead of main." + fi + + - name: Create promotion branch + id: branch + if: steps.ahead-check.outputs.commits_ahead != '0' + run: | + SHORT_SHA=$(echo "${{ needs.check-changes.outputs.current_head }}" | cut -c1-8) + BRANCH="staging-promote/${SHORT_SHA}-${{ github.run_id }}" + git checkout -b "$BRANCH" + git push origin "$BRANCH" + echo "branch=${BRANCH}" >> "$GITHUB_OUTPUT" + echo "Created promotion branch: ${BRANCH}" + + - name: Find base branch + id: find-base + if: steps.ahead-check.outputs.commits_ahead != '0' + env: + GH_TOKEN: ${{ steps.token.outputs.token }} + run: | + # Find the newest open promotion PR with a staging-promote/* head branch + LATEST=$(gh pr list --label staging-promotion --state open \ + --json headRefName,createdAt \ + --jq '[.[] | select(.headRefName | startswith("staging-promote/"))] | sort_by(.createdAt) | last | .headRefName // empty') + if [ -n "$LATEST" ]; then + echo "base=${LATEST}" >> "$GITHUB_OUTPUT" + echo "Chaining onto existing promotion branch: ${LATEST}" + else + echo "base=main" >> "$GITHUB_OUTPUT" + echo "No existing promotion PR — targeting main" + fi + + - name: Create promotion PR + id: create-pr + if: steps.ahead-check.outputs.commits_ahead != '0' + env: + GH_TOKEN: ${{ steps.token.outputs.token }} + run: | + RANGE="${{ needs.check-changes.outputs.diff_range }}" + TIMESTAMP=$(date -u +"%Y-%m-%d %H:%M UTC") + BRANCH="${{ steps.branch.outputs.branch }}" + BASE="${{ steps.find-base.outputs.base }}" + + PR_URL=$(gh pr create \ + --base "$BASE" \ + --head "$BRANCH" \ + --title "chore: promote staging to main (${TIMESTAMP})" \ + --body "## Auto-promotion from staging CI + + **Batch range:** \`${RANGE}\` + **Promotion branch:** \`${BRANCH}\` + **Base:** \`${BASE}\` + **Triggered by:** Staging CI batch at ${TIMESTAMP} + + Waiting for gates: + - Tests: pending + - E2E: pending + - Claude Code review: pending (will post comments on this PR) + + --- + *Auto-created by staging-ci workflow*" \ + --label "staging-promotion") + + PR_NUM=$(echo "$PR_URL" | grep -oE '[0-9]+$') + echo "pr_number=${PR_NUM}" >> "$GITHUB_OUTPUT" + echo "Created promotion PR #${PR_NUM}" + + # ── Gate: wait for review, process findings, merge or block ───── + gate: + name: Staging Gate + needs: [check-changes, tests, e2e, create-promotion-pr] + if: > + always() && + needs.check-changes.outputs.has_changes == 'true' && + needs.tests.result == 'success' && + needs.e2e.result == 'success' && + needs.create-promotion-pr.result == 'success' + runs-on: ubuntu-latest + timeout-minutes: 25 + outputs: + gate_passed: ${{ steps.evaluate.outputs.passed }} + steps: + - uses: actions/checkout@v6 + with: + ref: staging + fetch-depth: 1 + + - name: Generate GitHub App token + id: app-token + if: ${{ secrets.GH_RELEASES_MANAGER_APP_ID != '' }} + uses: actions/create-github-app-token@v2 + with: + app-id: ${{ secrets.GH_RELEASES_MANAGER_APP_ID }} + private-key: ${{ secrets.GH_RELEASES_MANAGER_APP_PRIVATE_KEY }} + + - name: Set token + id: token + run: | + if [ -n "${{ steps.app-token.outputs.token }}" ]; then + echo "token=${{ steps.app-token.outputs.token }}" >> "$GITHUB_OUTPUT" + else + echo "token=${{ github.token }}" >> "$GITHUB_OUTPUT" + fi + + - name: Wait for Claude review job + env: + GH_TOKEN: ${{ steps.token.outputs.token }} + PR_NUMBER: ${{ needs.create-promotion-pr.outputs.pr_number }} + REPO: ${{ github.repository }} + run: | + if [ -z "$PR_NUMBER" ]; then + echo "No PR number — skipping wait" + exit 0 + fi + + PR_SHA=$(gh pr view "$PR_NUMBER" --json headRefOid --jq '.headRefOid' || echo "") + if [ -z "$PR_SHA" ]; then + echo "::warning::Could not get PR head SHA" + exit 0 + fi + + echo "Polling for Claude Code Review job on PR #${PR_NUMBER} (SHA: ${PR_SHA})..." + TIMEOUT=1200 # 20 minutes + ELAPSED=0 + INTERVAL=30 + + while [ "$ELAPSED" -lt "$TIMEOUT" ]; do + STATUS=$(gh api "repos/${REPO}/commits/${PR_SHA}/check-runs" \ + --jq '[.check_runs[] | select(.name == "Claude Code Review") | .conclusion // .status] | first // "pending"' 2>/dev/null || echo "pending") + + if [ "$STATUS" = "success" ] || [ "$STATUS" = "failure" ] || [ "$STATUS" = "cancelled" ]; then + echo "Claude review job completed with status: ${STATUS} (${ELAPSED}s)" + exit 0 + fi + + echo "Claude review status: ${STATUS} (${ELAPSED}s elapsed)" + sleep "$INTERVAL" + ELAPSED=$((ELAPSED + INTERVAL)) + done + + echo "::warning::Claude review job not completed after ${TIMEOUT}s" + + - name: Process Claude review comments and create issues + id: process-findings + env: + GH_TOKEN: ${{ steps.token.outputs.token }} + PR_NUMBER: ${{ needs.create-promotion-pr.outputs.pr_number }} + REPO: ${{ github.repository }} + run: | + HAS_BLOCKING=false + ISSUES_CREATED=0 + + if [ -z "$PR_NUMBER" ]; then + echo "No PR — skipping finding processing" + echo "has_blocking=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + + # Check for "No issues found" first (clean pass) + NO_ISSUES=$(gh api "repos/${REPO}/issues/${PR_NUMBER}/comments" \ + --jq '[.[] | select(.user.login == "claude[bot]") | select(.body | test("No issues found"))] | length' 2>/dev/null || echo "0") + if [ "$NO_ISSUES" -gt 0 ]; then + echo "Claude review found no issues — gate passes" + echo "has_blocking=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + + # Get the last Claude comment that contains findings + JQ_FILTER='[.[] | select(.user.login == "claude[bot]") | select(.body | test("Found [0-9]+ issue"))] | last' + BODY=$(gh api "repos/${REPO}/issues/${PR_NUMBER}/comments" \ + --jq "${JQ_FILTER} | .body // empty" 2>/dev/null || echo "") + COMMENT_URL=$(gh api "repos/${REPO}/issues/${PR_NUMBER}/comments" \ + --jq "${JQ_FILTER} | .html_url // empty" 2>/dev/null || echo "") + + if [ -z "$BODY" ]; then + echo "::warning::No Claude review comment found for PR #${PR_NUMBER} — treating as blocking" + echo "has_blocking=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + + # Parse [SEVERITY:CONFIDENCE] tags from each numbered finding + # Matrix: CRITICAL always→issue, ≥80→block. HIGH ≥50→issue. MEDIUM ≥80→issue. LOW ≥80→issue. + # Use process substitution so variables propagate to parent shell + while read -r line; do + TAG=$(echo "$line" | grep -oE '^\[(CRITICAL|HIGH|MEDIUM|LOW):[0-9]+\]') + SEVERITY=$(echo "$TAG" | sed 's/\[\(.*\):\(.*\)\]/\1/') + CONFIDENCE=$(echo "$TAG" | sed 's/\[\(.*\):\(.*\)\]/\2/') + DESC=$(echo "$line" | sed "s/\[${SEVERITY}:${CONFIDENCE}\] *//" | head -1) + + echo "Found: [${SEVERITY}:${CONFIDENCE}] ${DESC}" + + # Check if blocking (CRITICAL ≥80) + if [ "$SEVERITY" = "CRITICAL" ] && [ "$CONFIDENCE" -ge 80 ]; then + HAS_BLOCKING=true + fi + + # Determine if this should create an issue + CREATE_ISSUE=false + case "$SEVERITY" in + CRITICAL) CREATE_ISSUE=true ;; + HIGH) [ "$CONFIDENCE" -ge 50 ] && CREATE_ISSUE=true ;; + MEDIUM) [ "$CONFIDENCE" -ge 80 ] && CREATE_ISSUE=true ;; + LOW) [ "$CONFIDENCE" -ge 80 ] && CREATE_ISSUE=true ;; + esac + + if [ "$CREATE_ISSUE" = "true" ]; then + case "$SEVERITY" in + CRITICAL) LABELS="bug,risk: high,staging-ci-review" ;; + HIGH) LABELS="bug,risk: medium,staging-ci-review" ;; + MEDIUM) LABELS="risk: medium,staging-ci-review" ;; + LOW) LABELS="risk: low,staging-ci-review" ;; + esac + + TITLE=$(echo "$DESC" | cut -c1-80) + { + echo "## [${SEVERITY}:${CONFIDENCE}] Issue Found by Staging CI Review" + echo "" + echo "**Severity:** ${SEVERITY}" + echo "**Confidence:** ${CONFIDENCE}/100" + echo "**PR comment:** ${COMMENT_URL}" + echo "" + echo "### Description" + echo "$DESC" + echo "" + echo "---" + echo "*Auto-created by staging-ci Claude Code review*" + } > /tmp/issue-body.md + + if gh issue create \ + --title "[${SEVERITY}] ${TITLE}" \ + --body-file /tmp/issue-body.md \ + --label "${LABELS}"; then + ISSUES_CREATED=$((ISSUES_CREATED + 1)) + else + echo "::warning::Failed to create issue for ${SEVERITY} finding" + fi + fi + done < <(echo "$BODY" | grep -oE '\[(CRITICAL|HIGH|MEDIUM|LOW):[0-9]+\].*') + + echo "Created ${ISSUES_CREATED} issues" + echo "has_blocking=${HAS_BLOCKING}" >> "$GITHUB_OUTPUT" + + - name: Evaluate gate + id: evaluate + env: + PR_NUMBER: ${{ needs.create-promotion-pr.outputs.pr_number }} + SKIP_GATE: ${{ inputs.skip_claude_gate }} + HAS_BLOCKING: ${{ steps.process-findings.outputs.has_blocking }} + run: | + SKIP_INPUT="$SKIP_GATE" + + if [ "$HAS_BLOCKING" = "true" ]; then + echo "::warning::Claude review found blocking issues (CRITICAL ≥80 confidence)" + if [ "$SKIP_INPUT" = "true" ]; then + echo "::warning::Gate overridden by skip_claude_gate workflow input" + echo "passed=true" >> "$GITHUB_OUTPUT" + else + echo "::error::Blocking promotion due to CRITICAL findings (≥80 confidence)" + echo "::error::PR #${PR_NUMBER} left open with review comments" + echo "passed=false" >> "$GITHUB_OUTPUT" + exit 1 + fi + else + echo "No blocking findings. Gate passed." + echo "passed=true" >> "$GITHUB_OUTPUT" + fi + + - name: Merge promotion PR + id: merge + if: steps.evaluate.outputs.passed == 'true' + env: + GH_TOKEN: ${{ steps.token.outputs.token }} + PR_NUMBER: ${{ needs.create-promotion-pr.outputs.pr_number }} + run: | + if [ -n "$PR_NUMBER" ]; then + echo "Merging promotion PR #${PR_NUMBER}" + # Do NOT use --delete-branch: deleting a promotion branch closes + # any chained PRs that use it as their base (verified in ironclaw-ci-test). + # Stale promotion branches are cleaned up separately. + gh pr merge "$PR_NUMBER" --merge + echo "merged=true" >> "$GITHUB_OUTPUT" + fi + + # ── Update tested tag (always, so next batch covers only new commits) ── + update-tag: + name: Update staging-tested tag + needs: [check-changes, tests, e2e, create-promotion-pr, gate] + if: > + always() && + needs.check-changes.outputs.has_changes == 'true' && + needs.tests.result == 'success' && + needs.e2e.result == 'success' && + needs.create-promotion-pr.result == 'success' + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + with: + ref: staging + fetch-depth: 1 + + - name: Update staging-tested tag + run: | + git tag -f staging-tested "${{ needs.check-changes.outputs.current_head }}" + git push origin staging-tested --force + echo "Updated staging-tested tag to ${{ needs.check-changes.outputs.current_head }}" + + # ── Report ─────────────────────────────────────────────────────── + report: + name: Staging CI Summary + needs: [check-changes, tests, e2e, create-promotion-pr, gate, update-tag] + if: always() && needs.check-changes.outputs.has_changes == 'true' + runs-on: ubuntu-latest + steps: + - name: Summary + run: | + echo "## Staging CI Batch Results" >> "$GITHUB_STEP_SUMMARY" + echo "" >> "$GITHUB_STEP_SUMMARY" + echo "| Check | Result |" >> "$GITHUB_STEP_SUMMARY" + echo "|-------|--------|" >> "$GITHUB_STEP_SUMMARY" + echo "| Tests | ${{ needs.tests.result }} |" >> "$GITHUB_STEP_SUMMARY" + echo "| E2E | ${{ needs.e2e.result }} |" >> "$GITHUB_STEP_SUMMARY" + echo "| Promotion PR | ${{ needs.create-promotion-pr.result }} |" >> "$GITHUB_STEP_SUMMARY" + echo "| Gate | ${{ needs.gate.result }} |" >> "$GITHUB_STEP_SUMMARY" + echo "| Tag Updated | ${{ needs.update-tag.result }} |" >> "$GITHUB_STEP_SUMMARY" + echo "" >> "$GITHUB_STEP_SUMMARY" + echo "Range: ${{ needs.check-changes.outputs.diff_range }}" >> "$GITHUB_STEP_SUMMARY" + PR_NUM="${{ needs.create-promotion-pr.outputs.pr_number }}" + if [ -n "$PR_NUM" ]; then + echo "Promotion PR: #${PR_NUM}" >> "$GITHUB_STEP_SUMMARY" + fi diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8f0fd2bb..efa28648 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1,5 +1,6 @@ name: Run Tests on: + workflow_call: pull_request: push: branches: @@ -38,6 +39,9 @@ jobs: telegram-tests: name: Telegram Channel Tests + if: > + github.event_name == 'push' || + (github.event_name == 'pull_request' && github.base_ref != 'staging') runs-on: ubuntu-latest steps: - name: Checkout repository @@ -50,6 +54,9 @@ jobs: windows-build: name: Windows Build (${{ matrix.name }}) + if: > + github.event_name == 'push' || + (github.event_name == 'pull_request' && github.base_ref != 'staging') runs-on: windows-latest strategy: fail-fast: false @@ -74,6 +81,9 @@ jobs: wasm-wit-compat: name: WASM WIT Compatibility + if: > + github.event_name == 'push' || + (github.event_name == 'pull_request' && github.base_ref != 'staging') runs-on: ubuntu-latest steps: - name: Checkout repository @@ -94,6 +104,9 @@ jobs: docker-build: name: Docker Build + if: > + github.event_name == 'push' || + (github.event_name == 'pull_request' && github.base_ref != 'staging') runs-on: ubuntu-latest steps: - name: Checkout repository @@ -123,12 +136,22 @@ jobs: needs: [tests, telegram-tests, wasm-wit-compat, docker-build, windows-build, version-check] steps: - run: | - if [[ "${{ needs.tests.result }}" != "success" || "${{ needs.telegram-tests.result }}" != "success" || "${{ needs.wasm-wit-compat.result }}" != "success" || "${{ needs.docker-build.result }}" != "success" || "${{ needs.windows-build.result }}" != "success" ]]; then - echo "One or more jobs failed" - exit 1 - fi - # version-check only runs on PRs, so skip/success are both acceptable - if [[ "${{ needs.version-check.result }}" == "failure" ]]; then - echo "Version bump check failed" + # Unit tests must always pass + if [[ "${{ needs.tests.result }}" != "success" ]]; then + echo "Unit tests failed" exit 1 fi + # Gated jobs: must pass on promotion PRs / push, skipped on developer PRs + for job in telegram-tests wasm-wit-compat docker-build windows-build version-check; do + case "$job" in + telegram-tests) result="${{ needs.telegram-tests.result }}" ;; + wasm-wit-compat) result="${{ needs.wasm-wit-compat.result }}" ;; + docker-build) result="${{ needs.docker-build.result }}" ;; + windows-build) result="${{ needs.windows-build.result }}" ;; + version-check) result="${{ needs.version-check.result }}" ;; + esac + if [[ "$result" == "failure" || "$result" == "cancelled" ]]; then + echo "$job failed" + exit 1 + fi + done diff --git a/CLAUDE.md b/CLAUDE.md index a77575ea..411af57a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -647,7 +647,7 @@ Key test patterns: ## Current Limitations / TODOs -1. **libSQL CLI connection crash** - `ironclaw tool setup` and `ironclaw secret set` crash with "invalid connection string" error on libSQL deployments. Blocks all interactive setup flows. Workaround: manually encrypt secrets via Python. See `src/db/mod.rs` `create_secrets_store()` doc comment. Related: #655 (libSQL backend gaps). +1. ~~**libSQL CLI connection crash**~~ - **Fixed in commit d8dcc34.** Root cause was inline compile-time `#[cfg]` blocks in CLI subcommands always choosing postgres when both features were compiled. The fix moved backend dispatch to `db::create_secrets_store()` with explicit pattern matching. Defensively improved `connect_from_config()` and `create_secrets_store()` to use explicit `Postgres =>` patterns instead of wildcards, catching misconfiguration in postgres-only builds more clearly. 2. **Domain-specific tools** - `marketplace.rs`, `restaurant.rs`, `taskrabbit.rs`, `ecommerce.rs` return placeholder responses; need real API integrations 3. **Integration tests** - Need testcontainers setup for PostgreSQL 4. **MCP stdio transport** - Only HTTP transport implemented diff --git a/rust_out b/rust_out new file mode 100755 index 00000000..2e504bca Binary files /dev/null and b/rust_out differ diff --git a/src/cli/mcp.rs b/src/cli/mcp.rs index 7e32b470..bc89e617 100644 --- a/src/cli/mcp.rs +++ b/src/cli/mcp.rs @@ -626,12 +626,7 @@ async fn save_servers( } } -/// Initialize and return the secrets store. /// Get the secrets store for MCP authentication operations. -/// -/// **Known Issue:** This function crashes with "invalid connection string" on libSQL deployments -/// when called from `mcp auth` subcommands. See `src/db/mod.rs` `create_secrets_store()` -/// documentation for details and workaround. async fn get_secrets_store() -> anyhow::Result> { let config = Config::from_env().await?; diff --git a/src/cli/tool.rs b/src/cli/tool.rs index 19f8f4d3..752f4263 100644 --- a/src/cli/tool.rs +++ b/src/cli/tool.rs @@ -551,10 +551,6 @@ fn validate_tool_name(name: &str) -> anyhow::Result<()> { } /// Initialize the secrets store from environment config. -/// -/// **Known Issue:** This function crashes with "invalid connection string" on libSQL deployments -/// when called from `tool setup` or `secret set` subcommands. See `src/db/mod.rs` `create_secrets_store()` -/// documentation for details and workaround. async fn init_secrets_store() -> anyhow::Result> { let config = Config::from_env().await?; let master_key = config.secrets.master_key().ok_or_else(|| { diff --git a/src/db/mod.rs b/src/db/mod.rs index 08475d1b..0462361a 100644 --- a/src/db/mod.rs +++ b/src/db/mod.rs @@ -77,7 +77,7 @@ pub async fn connect_from_config( Ok(Arc::new(backend)) } #[cfg(feature = "postgres")] - _ => { + crate::config::DatabaseBackend::Postgres => { let pg = postgres::PgBackend::new(config) .await .map_err(|e| DatabaseError::Pool(e.to_string()))?; @@ -85,9 +85,16 @@ pub async fn connect_from_config( Ok(Arc::new(pg)) } #[cfg(not(feature = "postgres"))] - _ => Err(DatabaseError::Pool( - "No database backend available. Enable 'postgres' or 'libsql' feature.".to_string(), + crate::config::DatabaseBackend::Postgres => Err(DatabaseError::Pool( + "No postgres backend available. Rebuild with --features postgres.".to_string(), )), + // Catches LibSql in postgres-only builds (libsql arm compiled out) + #[allow(unreachable_patterns)] + _ => Err(DatabaseError::Pool(format!( + "Database backend {:?} not available in this build. \ + Set DATABASE_BACKEND to a compiled-in backend, or rebuild with the matching feature flag.", + config.backend + ))), } } @@ -96,24 +103,6 @@ pub async fn connect_from_config( /// This is the shared factory for CLI commands and other call sites that need /// a `SecretsStore` without going through the full `AppBuilder`. Mirrors the /// pattern of [`connect_from_config`] but returns a secrets-specific store. -/// -/// ## Known Issue: libSQL CLI Connection Crash -/// -/// Running `ironclaw tool setup` or `ironclaw secret set` crashes with an -/// "invalid connection string" error when DATABASE_BACKEND=libsql. This blocks -/// all interactive setup flows on libSQL deployments (the default for hosted agents). -/// -/// **Workaround:** Manually read the master key from `/proc/PID/environ`, encrypt -/// secrets with AES-256-GCM via Python ctypes, and write directly to the secrets table. -/// -/// **Related Issue:** #655 (libSQL backend gaps) -/// -/// **Root Cause:** Unclear; the local libSQL database opens fine for the main process, -/// but CLI subcommands fail when calling `LibSqlBackend::new_local(path)`. Likely -/// relates to path resolution, file permissions, or WAL mode conflicts in concurrent -/// connection scenarios. -/// -/// **TODO:** Debug why libSQL connection fails in CLI context while working in main.rs. pub async fn create_secrets_store( config: &crate::config::DatabaseConfig, crypto: Arc, @@ -148,7 +137,7 @@ pub async fn create_secrets_store( ))) } #[cfg(feature = "postgres")] - _ => { + crate::config::DatabaseBackend::Postgres => { let pg = postgres::PgBackend::new(config) .await .map_err(|e| DatabaseError::Pool(e.to_string()))?; @@ -160,10 +149,16 @@ pub async fn create_secrets_store( ))) } #[cfg(not(feature = "postgres"))] - _ => Err(DatabaseError::Pool( - "No database backend available for secrets. Enable 'postgres' or 'libsql' feature." - .to_string(), + crate::config::DatabaseBackend::Postgres => Err(DatabaseError::Pool( + "No postgres backend available. Rebuild with --features postgres.".to_string(), )), + // Catches LibSql in postgres-only builds (libsql arm compiled out) + #[allow(unreachable_patterns)] + _ => Err(DatabaseError::Pool(format!( + "Database backend {:?} not available in this build. \ + Set DATABASE_BACKEND to a compiled-in backend, or rebuild with the matching feature flag.", + config.backend + ))), } } diff --git a/src/llm/CLAUDE.md b/src/llm/CLAUDE.md index a1eb72be..d1b9eea2 100644 --- a/src/llm/CLAUDE.md +++ b/src/llm/CLAUDE.md @@ -19,6 +19,7 @@ Multi-provider LLM integration with circuit breaker, retry, failover, and respon | `rig_adapter.rs` | Adapter bridging rig-core `CompletionModel` → `LlmProvider`; used by OpenAI, Anthropic, Ollama, Tinfoil | | `smart_routing.rs` | `SmartRoutingProvider` — 13-dimension complexity scorer routes cheap vs primary model | | `recording.rs` | `RecordingLlm` — trace capture for E2E replay testing (`IRONCLAW_RECORD_TRACE`) | +| `bedrock.rs` | AWS Bedrock provider via native Converse API (feature-gated: `--features bedrock`) | ## Provider Selection @@ -32,6 +33,18 @@ Set via `LLM_BACKEND` env var: | `ollama` | Ollama local | `OLLAMA_BASE_URL` | | `openai_compatible` | Any OpenAI-compatible endpoint | `LLM_BASE_URL`, `LLM_API_KEY`, `LLM_MODEL` | | `tinfoil` | Tinfoil TEE inference | `TINFOIL_API_KEY`, `TINFOIL_MODEL` | +| `bedrock` | AWS Bedrock (requires `--features bedrock`) | `BEDROCK_REGION`, `BEDROCK_MODEL`, `AWS_PROFILE` | + +## AWS Bedrock Provider + +Uses the native Converse API via `aws-sdk-bedrockruntime` (`bedrock.rs`). Requires `--features bedrock` at build time — not in default features due to heavy AWS SDK dependencies. + +**Auth:** Standard AWS credential chain — IAM credentials (`AWS_ACCESS_KEY_ID`/`AWS_SECRET_ACCESS_KEY`), SSO profiles (`AWS_PROFILE`), or instance roles. The SDK resolves auth automatically from the environment. + +**Config:** +- `BEDROCK_REGION` — AWS region (default: `us-east-1`) +- `BEDROCK_MODEL` — Required model ID (e.g., `anthropic.claude-opus-4-6-v1`) +- `BEDROCK_CROSS_REGION` — Optional cross-region inference prefix (`us`, `eu`, `apac`, `global`) ## NEAR AI Provider Gotchas