Skip to content

chore: promote staging to staging-promote/b6b3ffa1-23819569437 (2026-04-01 00:16 UTC) - #1847

Merged
henrypark133 merged 23 commits into
staging-promote/b6b3ffa1-23819569437from
staging-promote/f441d788-23825523544
Apr 1, 2026
Merged

henrypark133 merged 23 commits into
staging-promote/b6b3ffa1-23819569437from
staging-promote/f441d788-23825523544

Conversation

@ironclaw-ci

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

Copy link
Copy Markdown
Contributor

Auto-promotion from staging CI

Batch range: b6b3ffa1a42fa4d27b51edbe3e43ac1a78be430d..f441d788b3838dd17e650b288cc6c11a9c4c907d
Promotion branch: staging-promote/f441d788-23825523544
Base: staging-promote/b6b3ffa1-23819569437
Triggered by: Staging CI batch at 2026-04-01 00:16 UTC

Commits in this batch (2):

Current commits in this promotion (0)

Current base: staging-promote/b6b3ffa1-23819569437
Current head: staging-promote/f441d788-23825523544
Current range: origin/staging-promote/b6b3ffa1-23819569437..origin/staging-promote/f441d788-23825523544

  • (no non-merge commits in range)

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

hanakannzashi and others added 2 commits March 31, 2026 16:49
…DMs (#1845)

* fix(relay): route async Slack messages to correct channel instead of DMs

Fixes three bugs causing async/cross-channel Slack messages to land in
DMs or fail silently:

1. routing_target_from_metadata now extracts channel_id for Slack relay
   messages, so proactive broadcasts target the originating channel
   instead of falling back to sender_id (user's DM)

2. Lightweight routine JobContext carries notify metadata (owner_id,
   notify_channel, notify_user) so the message tool can resolve the
   correct delivery target — previously ..Default::default() left
   metadata as null

3. Routine creation auto-captures source channel/target from
   ctx.metadata when the LLM omits delivery params, so routines
   created from a Slack channel know where to send results

Also:
- Clarified message tool channel/target parameter descriptions to
  prevent LLM confusion between transport names and Slack channel IDs
- IronClaw proxy_provider now checks Slack ok=false and surfaces
  errors instead of silently succeeding

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

* style: apply cargo fmt

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

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added scope: agent Agent core (agent loop, router, scheduler) scope: channel Channel infrastructure scope: tool/builtin Built-in tools size: M 50-199 changed lines risk: medium Business logic, config, or moderate-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 5 issues:

  1. [MEDIUM:70] Metadata field naming inconsistency between routine creation and execution

The PR correctly carries notify_channel from context in both routine_engine.rs (where it's set) and routine.rs (where it's read). However, this creates a subtle temporal ordering dependency: routines created via the tool read from the incoming context, while routines executed by the engine write the metadata. This works but obscures the data flow. Consider documenting the metadata propagation path more explicitly to clarify when/where notify_channel is available.

https://github.com/anthropics/ironclaw/blob/7113fba4fcd3356d90e499b8853a1ee52d0739b9/src/tools/builtin/routine.rs#L1184-L1189

  1. [MEDIUM:50] Redundant .clone() in Option fallback chain

The code calls .clone() before .or_else() on an Option that will be moved into the struct. Since or_else consumes self, the clone is unnecessary. This works but wastes a string allocation.

https://github.com/anthropics/ironclaw/blob/7113fba4fcd3356d90e499b8853a1ee52d0739b9/src/tools/builtin/routine.rs#L1184

Consider:

channel: normalized.delivery.channel.or_else(|| {
    ctx.metadata
        .get("notify_channel")
        .and_then(|v| v.as_str())
        .map(ToOwned::to_owned)
}),
  1. [LOW:75] Parameter descriptions in message.rs could be clearer about precedence

The schema descriptions mention "defaults to current channel" but don't clarify that this only applies when metadata is absent. The note: "Defaults to current conversation channel if no execution-local routing metadata exists." would be more accurate.

https://github.com/anthropics/ironclaw/blob/7113fba4fcd3356d90e499b8853a1ee52d0739b9/src/tools/builtin/message.rs#L207-L212

  1. [LOW:65] Routine engine regression test could verify full data flow

The test test_build_lightweight_prompt_preserves_notify_config validates that notify metadata appears in the prompt string, but doesn't exercise the full routing path (whether the message tool actually uses it). Consider adding an integration-style test that mocks tool execution.

https://github.com/anthropics/ironclaw/blob/7113fba4fcd3356d90e499b8853a1ee52d0739b9/src/agent/routine_engine.rs#L2588-L2610

  1. [LOW:60] RelayClient Slack error handling could be generalized

The ok=false error check is correctly added to proxy_provider(), but other Slack relay endpoints like initiate_oauth() and create_approval() may also return HTTP 200 with error semantics. Consider generalizing this pattern to all Slack API calls.

https://github.com/anthropics/ironclaw/blob/7113fba4fcd3356d90e499b8853a1ee52d0739b9/src/channels/relay/client.rs#L280-L301

Review summary: No security issues found. All changes follow CLAUDE.md patterns (no unwrap/expect in production, proper error handling, type-driven design). The PR correctly fixes message routing bugs with good test coverage. Issues flagged are minor code clarity and optimization opportunities.

h19overflow and others added 20 commits March 31, 2026 23:01
…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>
…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>
* 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)
…5614

chore: promote staging to staging-promote/f441d788-23825523544 (2026-04-01 06:31 UTC)
@henrypark133
henrypark133 merged commit e85e071 into staging-promote/b6b3ffa1-23819569437 Apr 1, 2026
13 checks passed
@github-actions github-actions Bot added size: XL 500+ changed lines and removed size: M 50-199 changed lines labels Apr 1, 2026
@henrypark133
henrypark133 deleted the staging-promote/f441d788-23825523544 branch April 1, 2026 22:34
@github-actions github-actions Bot added size: M 50-199 changed lines scope: channel/cli TUI / CLI channel scope: channel/web Web gateway channel scope: channel/wasm WASM channel runtime scope: tool Tool infrastructure scope: tool/wasm WASM tool sandbox scope: tool/builder Dynamic tool builder scope: llm LLM integration scope: orchestrator Container orchestrator scope: worker Container worker scope: config Configuration scope: setup Onboarding / setup scope: ci CI/CD workflows scope: docs Documentation risk: high Safety, secrets, auth, or critical infrastructure and removed size: XL 500+ changed lines risk: medium Business logic, config, or moderate-risk modules labels Apr 1, 2026
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
…3825523544

chore: promote staging to staging-promote/3411530a-23819569437 (2026-04-01 00:16 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: M 50-199 changed lines staging-promotion

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants