Skip to content

chore: promote staging to staging-promote/f441d788-23825523544 (2026-04-01 06:31 UTC) - #1857

Merged
henrypark133 merged 20 commits into
staging-promote/f441d788-23825523544from
staging-promote/684a9d30-23835295614
Apr 1, 2026
Merged

henrypark133 merged 20 commits into
staging-promote/f441d788-23825523544from
staging-promote/684a9d30-23835295614

Conversation

@ironclaw-ci

@ironclaw-ci ironclaw-ci Bot commented Apr 1, 2026 •

Copy link
Copy Markdown
Contributor

Auto-promotion from staging CI

Batch range: f441d788b3838dd17e650b288cc6c11a9c4c907d..684a9d30ea7602c43f5787c37e3bbdb60c498d5e
Promotion branch: staging-promote/684a9d30-23835295614
Base: staging-promote/f441d788-23825523544
Triggered by: Staging CI batch at 2026-04-01 06:31 UTC

Commits in this batch (1):

Current commits in this promotion (1)

Current base: staging-promote/f441d788-23825523544
Current head: staging-promote/684a9d30-23835295614
Current range: origin/staging-promote/f441d788-23825523544..origin/staging-promote/684a9d30-23835295614

Auto-updated by staging promotion metadata workflow

Waiting for gates:

  • Tests: pending
  • E2E: pending
  • Claude Code review: pending (will post comments on this PR)

Auto-created by staging-ci workflow

…on calls (#1752)

* fix(gemini): preserve and echo thoughtSignature for Gemini 3.x function calls

Gemini 3.x models return a `thoughtSignature` field alongside
`functionCall` parts and require it to be echoed back when replaying
conversation history. Without this, all tool-calling requests fail
with HTTP 400 "Function call is missing a thought_signature".

Changes:
- Add `thought_signature: Option<String>` to `ToolCall` struct
- Capture `thoughtSignature` from Gemini response in `from_gemini_response()`
- Echo it back on `functionCall` parts in `to_gemini_request()`
- Add 4 tests covering roundtrip, capture, and omission

Fixes #1510

* fix(gemini): address review feedback — remove .unwrap(), document DB round-trip limitation

M1: Replace `part.as_object_mut().unwrap().insert(...)` with
`if let Some(obj)` pattern per CLAUDE.md no-unwrap policy.

M2: Add comments in thread_ops.rs and session.rs documenting that
thought_signature is lost on DB round-trip. The synthetic fallback
in ensure_thought_signatures() covers this at request time.

* refactor(gemini): move thought_signature from ToolCall to provider-local storage

Instead of adding a Gemini-specific `thought_signature` field to the
shared `ToolCall` struct (which required `thought_signature: None` in
18 files across every provider and consumer), store captured thought
signatures in a `HashMap<String, String>` on `GeminiOauthProvider`
keyed by tool-call ID.

- `from_gemini_response()` returns captured signatures as a third tuple
  element; `complete_with_tools()` stores them on the provider instance
- `to_gemini_request()` accepts the signatures map and injects real
  signatures before `ensure_thought_signatures()` fills synthetic gaps
- Zero changes outside `gemini_oauth.rs` except removing the reverted
  `thought_signature: None` lines

The `ensure_thought_signatures()` fallback continues to work for
history entries loaded from DB (where real signatures aren't available).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(gemini): prune thought_signatures map to prevent unbounded growth

Address review feedback from Copilot:
- Prune stale entries after each response by retaining only signatures
  for tool-call IDs present in the conversation history or just-received
  response. This prevents unbounded map growth and O(n) clone overhead.
- Fix inaccurate test comment to reflect the actual condition.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: ilblackdragon@gmail.com <ilblackdragon@gmail.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added scope: llm LLM integration size: L 200-499 changed lines risk: low Changes to docs, tests, or low-risk modules contributor: core 20+ merged PRs labels Apr 1, 2026
@claude

claude Bot commented Apr 1, 2026

Copy link
Copy Markdown

Code review

Found 8 issues:

  1. [HIGH:92] std::sync::Mutex in read-heavy workload—should use tokio::sync::Mutex or RwLock instead. This lock is read in 3 places (count_tokens, complete, complete_with_tools) and written in 1, but Mutex blocks the executor. RwLock allows concurrent readers and is already used in similar patterns elsewhere (e.g., active_model: RwLock<String>).

https://github.com/anthropics/claude-code/blob/684a9d30/src/llm/gemini_oauth.rs#L925-L927

  1. [HIGH:85] HashMap cloned on every request despite being mostly read-only. Lines 1074-1078, 2087-2090, 2116-2120 all .clone() the entire map before making requests, triggering heap allocation even when empty. Consider using Arc<RwLock<HashMap>> to share references instead.

https://github.com/anthropics/claude-code/blob/684a9d30/src/llm/gemini_oauth.rs#L2086-L2090

  1. [HIGH:75] .unwrap_or_else(|e| e.into_inner()) on poisoned mutex violates CLAUDE.md rule "No .unwrap() or .expect() in production code." This pattern silently ignores mutex poisoning without proper error context. Should map to LlmError::Internal { reason: ... } instead.

https://github.com/anthropics/claude-code/blob/684a9d30/src/llm/gemini_oauth.rs#L1075-L1077

  1. [HIGH:90] Potential race condition in concurrent tool_complete() calls. The double-lock pattern (read before request, write after response) with retain() based on per-request live_ids means concurrent requests with different message sets could prune signatures that are live in another in-flight request's conversation.

https://github.com/anthropics/claude-code/blob/684a9d30/src/llm/gemini_oauth.rs#L2142-L2150

  1. [MEDIUM:85] Tuple return type GeminiParsedResponse = (CompletionResponse, Vec<ToolCall>, HashMap<String, String>) obscures API intent. The third field's purpose is non-obvious at call sites. Per CLAUDE.md preference for strong types, use a named struct instead (e.g., struct GeminiParsedResponse { completion, tool_calls, thought_sigs }).

https://github.com/anthropics/claude-code/blob/684a9d30/src/llm/gemini_oauth.rs#L932-L933

  1. [MEDIUM:82] #[allow(clippy::too_many_arguments)] suppresses valid lint instead of refactoring. to_gemini_request() now has 8 parameters. Per CLAUDE.md "keep functions focused," consider consolidating parameters into a request builder struct to improve API clarity and maintainability.

https://github.com/anthropics/claude-code/blob/684a9d30/src/llm/gemini_oauth.rs#L1614

  1. [MEDIUM:80] HashSet allocated on every complete_with_tools() call (lines 2142-2149) just for membership checking. For workloads with many tool calls, consider optimizing by computing live IDs incrementally or using a generation counter.

https://github.com/anthropics/claude-code/blob/684a9d30/src/llm/gemini_oauth.rs#L2142-L2150

  1. [MEDIUM:70] HashMap capacity never shrinks after retain(). If map grows to N entries then shrinks, reserved capacity stays at N forever. Consider calling shrink_to_fit() after pruning for long-running processes with variable load.

https://github.com/anthropics/claude-code/blob/684a9d30/src/llm/gemini_oauth.rs#L2150

…rom LLM (#1748)

* fix(builder): accept inline-table and object-map dependency formats from LLM

* review: return Option from flatten_dep to skip invalid TOML values

---------

Co-authored-by: Illia Polosukhin <ilblackdragon@gmail.com>
G7CNF and others added 15 commits April 1, 2026 15:33
* feat(config): unify all settings to DB > env > default priority

Previously only LLM settings used DB-first priority while all other
subsystems (agent, channels, tunnel, heartbeat, embeddings, sandbox,
wasm, safety, builder, transcription, routines, skills, hygiene,
search) used env-first. This made web UI settings changes unreliable
for non-LLM config — env vars would silently override DB values.

Now all subsystems follow the same priority: DB > env > TOML > default.

- Add db_first_or_default, db_first_bool, db_first_optional_string,
  db_first_option helpers to config/helpers.rs with shadow warnings
- Flip 10 Group 1 resolvers (agent, channels, tunnel, heartbeat,
  embeddings, sandbox, wasm, safety, builder, transcription) from
  parse_optional_env/parse_bool_env to db_first_* equivalents
- Add Settings structs for 4 Group 2 resolvers (routines, skills,
  hygiene, search) that previously had no DB persistence
- Update Config::build() call sites and cli/doctor.rs caller
- Security-sensitive fields stay env-only: allow_local_tools,
  allow_full_access, cost/rate limits, auth tokens, API keys
- Bootstrap configs (database, secrets) stay env-only

Closes #1119 (partial — config unification phases 1-2)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address PR review feedback

- Stop logging raw values in shadow warnings to prevent leaking
  sensitive data (tunnel tokens, API keys) to logs
- Propagate optional_env errors for OLLAMA_BASE_URL instead of
  silently swallowing them with .ok().flatten()
- Make tunnel auth tokens (cf_token, ngrok_token) env-only like
  gateway_auth_token — sensitive credentials should not come from DB
- Fix transcription enabled tri-state: explicit DB false now correctly
  overrides TRANSCRIPTION_ENABLED env var (was collapsing to "unset")
- Switch SearchSettings fts_weight/vector_weight to Option<f32> so
  0.5 can be explicitly configured without being treated as "unset"

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address @ilblackdragon review feedback

- Document default-equality heuristic limitation in db_first_or_default
  (a DB value equal to the default is treated as "unset")
- Remove dead _db_value parameter from warn_if_db_shadows_env
- Replace misleading db_first_or_default for embedding dimension with
  direct parse_optional_env (dimension depends on model, not DB)
- Use db_first_option for search weights to emit shadow warnings
  consistently with other resolvers
- Add migration warnings for auth tokens (gateway_auth_token,
  cf_token, ngrok_token) that are now env-only — warns at startup
  if these fields are set in DB/TOML but being ignored
- Improve signal error message to mention signal_enabled setting
- Clarify module docs and TOML header about default-equality caveat

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* chore: minor cleanups from self-review

- Simplify warn_if_db_shadows_env: use is_ok_and() instead of
  binding + drop
- Add comment explaining u32→usize cast for max_parallel_jobs

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* feat(jobs): per-job MCP server filtering and max_iterations cap

Add mcp_servers and max_iterations optional params to create_job.
mcp_servers filters which MCP servers are mounted into worker
containers (gated behind MCP_PER_JOB_ENABLED, default false).
max_iterations caps the worker agent loop (default 50, max 500).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address review feedback on per-job MCP filtering

- Fix max_iterations dead code: add env = "IRONCLAW_MAX_ITERATIONS"
  to clap arg so worker CLI reads the env var injected by orchestrator
- Fix max_iterations: 0 allowed: use .clamp(1, 500) instead of .min(500)
- Replace hardcoded /tmp/ironclaw-mcp-configs with std::env::temp_dir()
- Make MCP server name matching case-insensitive
- Add test for case-insensitive matching
- Add test verifying max_iterations env var name matches clap definition

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address Copilot review feedback on per-job MCP filtering

- Guard IRONCLAW_MAX_ITERATIONS injection to Worker mode only (ClaudeCode uses max_turns)
- Extract WORKER_MCP_CONFIG_PATH as constant (no more hardcoded path)
- Fix TOCTOU race in cleanup_job: use remove_file directly, match on NotFound
- Fix schema_version default: 0 → 1 to match McpServersFile default
- Propagate serialization errors instead of silently writing empty config
- Add type validation warnings for mcp_servers and max_iterations params

* test: add regression tests and security hardening for per-job MCP filtering

Add 5 regression tests covering CI-required scenarios:
- Filtered config contains only the requested server (no leaks)
- Feature flag disabled skips MCP filtering entirely
- Temp file cleanup removes per-job config
- cleanup_job is idempotent (no panic on missing file/handle)
- Temp directory has restrictive 0o700 permissions (unix)

Security: set 0o700 permissions on /tmp/ironclaw-mcp-configs/ to prevent
other users on the host from reading filtered MCP server configs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address code review — server-side clamp, JobCreationParams, name validation

Critical:
1. Server-side max_iterations clamp in create_job_inner — defense no longer
   relies solely on tool parameter parsing. Uses MAX_WORKER_ITERATIONS constant
   (matching worker/job.rs) so the cap is enforced even for direct API calls.

2. Introduce JobCreationParams struct to bundle credential_grants, mcp_servers,
   and max_iterations. Removes #[allow(clippy::too_many_arguments)] from both
   create_job and execute_sandbox (7→5 and 9→7 positional args).

Important:
3. Validate MCP server names: reject path separators (/\), null bytes, and
   names longer than 128 chars to prevent future misuse.

5. Add test verifying max_iterations is NOT injected for ClaudeCode mode.
   Add test verifying server-side clamp uses MAX_WORKER_ITERATIONS constant.
   Add test verifying name validation rejects path separators and null bytes.

* fix: async I/O in generate_worker_mcp_config, shared MAX_WORKER_ITERATIONS

1. Convert generate_worker_mcp_config from sync std::fs to async tokio::fs.
   The function is called from async create_job_inner — sync I/O was blocking
   the tokio runtime thread. All test callers converted to #[tokio::test].

2. Move MAX_WORKER_ITERATIONS (500) to ironclaw_common as single source of
   truth. Both src/orchestrator/job_manager.rs and src/worker/job.rs now
   import from the shared crate, preventing drift.

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Improve command execution parameter validation

Enhance workdir and timeout parameter handling for command execution.

* fix(worker): refine timeout parameter validation logic

Refactor timeout parameter handling to ensure it is a positive integer.

* fix(shell): add timeout and workdir validation with regression tests

Parse timeout strictly: reject non-integer (float/string), zero, and null values; normalize blank/whitespace-only workdir to None.
Add six regression tests covering each edge case flagged in review.

* fix(shell): improve parameter validation consistency

Add "minimum": 1 to timeout schema so LLMs get constraint upfront
Reject non-string workdir types (was silently ignored before)
Clarify error message: "positive integer" instead of "integer"
Add test for non-string workdir rejection
…#1848)

For channel mentions, the relay channel used event.channel_id (e.g.
"C088K6C3SQZ") as the thread_id fallback. Slack requires thread_ts to
be a message timestamp, so it silently ignored this and posted a
top-level message instead of threading.

Now uses event.id (the Slack message ts, e.g. "1609459200.000100") as
the fallback, so responses are always threaded under the user's message.
Also fixes metadata["thread_id"] to use the same value.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* test(routines): add issue 1781 coverage

* fix: address PR review feedback
* test(e2e): cover chat approval parity across channels

* fix: harden chat approval prompt rendering
* Expand GitHub WASM tool surface

* Tighten GitHub tool input validation
* test(e2e): add agent loop recovery coverage

* test(e2e): harden mock message text parsing
…5626

chore: promote staging to staging-promote/2f2ad260-23866616993 (2026-04-01 22:10 UTC)
…6993

chore: promote staging to staging-promote/510fe19a-23863770631 (2026-04-01 19:23 UTC)
…0631

chore: promote staging to staging-promote/eb3fa0e6-23858863254 (2026-04-01 18:14 UTC)
…3254

chore: promote staging to staging-promote/58b01f15-23856539614 (2026-04-01 16:18 UTC)
…9614

chore: promote staging to staging-promote/27a2fab1-23853914907 (2026-04-01 15:27 UTC)
…4907

chore: promote staging to staging-promote/73759253-23837266309 (2026-04-01 14:31 UTC)
…6309

chore: promote staging to staging-promote/684a9d30-23835295614 (2026-04-01 07:30 UTC)
@henrypark133
henrypark133 merged commit dbc5b2e into staging-promote/f441d788-23825523544 Apr 1, 2026
12 of 13 checks passed
@henrypark133
henrypark133 deleted the staging-promote/684a9d30-23835295614 branch April 1, 2026 22:34
@github-actions github-actions Bot added scope: agent Agent core (agent loop, router, scheduler) scope: channel Channel infrastructure scope: channel/cli TUI / CLI channel scope: channel/web Web gateway channel scope: channel/wasm WASM channel runtime scope: tool Tool infrastructure scope: tool/builtin Built-in tools scope: tool/wasm WASM tool sandbox scope: tool/builder Dynamic tool builder scope: orchestrator Container orchestrator scope: worker Container worker scope: config Configuration scope: setup Onboarding / setup scope: ci CI/CD workflows scope: docs Documentation size: XL 500+ changed lines risk: high Safety, secrets, auth, or critical infrastructure and removed size: L 200-499 changed lines risk: low Changes to docs, tests, or low-risk modules labels Apr 1, 2026
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
…3835295614

chore: promote staging to staging-promote/bdb41161-23825523544 (2026-04-01 06:31 UTC)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: high Safety, secrets, auth, or critical infrastructure scope: agent Agent core (agent loop, router, scheduler) scope: channel/cli TUI / CLI channel scope: channel/wasm WASM channel runtime scope: channel/web Web gateway channel scope: channel Channel infrastructure scope: ci CI/CD workflows scope: config Configuration scope: docs Documentation scope: llm LLM integration scope: orchestrator Container orchestrator scope: setup Onboarding / setup scope: tool/builder Dynamic tool builder scope: tool/builtin Built-in tools scope: tool/wasm WASM tool sandbox scope: tool Tool infrastructure scope: worker Container worker size: XL 500+ changed lines staging-promotion

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants