Skip to content

chore: promote staging to staging-promote/336bdb1e-24684983862 (2026-04-21 02:12 UTC)#2768

Merged
henrypark133 merged 4 commits intomainfrom
staging-promote/6d4935a6-24700448305
Apr 21, 2026
Merged

chore: promote staging to staging-promote/336bdb1e-24684983862 (2026-04-21 02:12 UTC)#2768
henrypark133 merged 4 commits intomainfrom
staging-promote/6d4935a6-24700448305

Conversation

@ironclaw-ci
Copy link
Copy Markdown
Contributor

@ironclaw-ci ironclaw-ci Bot commented Apr 21, 2026 •
edited by github-actions Bot
Loading

Auto-promotion from staging CI

Batch range: 7fb41555a9e55677d1aaea29ca567a5b369c2b05..6d4935a617d89c6ee6d0a032d1e36f48c704fb9a
Promotion branch: staging-promote/6d4935a6-24700448305
Base: staging-promote/336bdb1e-24684983862
Triggered by: Staging CI batch at 2026-04-21 02:12 UTC

Commits in this batch (56):

Current commits in this promotion (4)

Current base: staging-promote/336bdb1e-24684983862
Current head: staging-promote/6d4935a6-24700448305
Current range: origin/staging-promote/336bdb1e-24684983862..origin/staging-promote/6d4935a6-24700448305

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

@henrypark133
[codex] Fix gateway slash autocomplete and attachment rendering (#2763)
a4966c6 
* fix gateway slash autocomplete and attachment rendering

* fix(web): restore attachment uploads and binary fallback

* fix(web): preserve in-progress attachment turns on reload
@henrypark133
[codex] Refresh skill tool ZIP extraction test fixtures (#2764)
8b12764 
* test(skill_tools): refresh ZIP extraction fixtures

* fix(review): tighten skill ZIP test coverage
@henrypark133
[codex] fix(gateway): make multi-tenant mode config-driven (#2762)
714cc41 
* fix(gateway): make multi-tenant mode config-driven

* fix(web): address henrypark133 review - add startup multi-tenant coverage (#2762)

* fix(web): address review - restore workspace isolation (#2762)
@henrypark133
[codex] Stabilize web settings LLM hot reload (#2765)
6d4935a 
* Stabilize web settings LLM hot reload

* Keep web settings hot reload DB-scoped

* fix(config): preserve TOML overlay in llm re-resolve

* test(config): allow env lock in async toml re-resolve test

* fix(web): fail closed on hot reload db read errors
@github-actions github-actions Bot added size: XL 500+ changed lines scope: channel/web Web gateway channel scope: tool/builtin Built-in tools risk: medium Business logic, config, or moderate-risk modules contributor: core 20+ merged PRs and removed size: XL 500+ changed lines labels Apr 21, 2026
@claude
Copy link
Copy Markdown

claude Bot commented Apr 21, 2026

Code Review

Found 5 issues across the JS and Rust changes:

HIGH severity

  1. [HIGH:85] Pending message deduplication logic bug—potential duplicate renders

    • File: crates/ironclaw_gateway/static/js/core/history.js
    • Lines 40–164 (full loadHistory function)
    • Issue: After rendering pending messages during the turn loop (lines 68–89) with .shift() calls to consume queued items, the final "render remaining pending" block (lines 150–164) reconstructs the array by flattening pendingByContent.values(). The shifted (empty or partially-consumed) queues still exist in the map, causing the reconstruct to collect items that were already rendered during the turn iteration. If a user sends the same message multiple times with attachments, some instances will render twice.
    • Example flow:
      1. User sends "hello" 3 times, last two are pending (in queue)
      2. First "hello" is in DB (database turn), gets rendered at line 77
      3. Second "hello" (pending with images) matches at line 71, renders at line 89, queue gets shift()
      4. Final Array.from(pendingByContent.values()).flat() at line 151 still includes the queue (now with remaining pending), renders duplicate
    • Fix: Track consumed pending IDs or use a consumed set to skip already-rendered messages

  2. [HIGH:75] File input reset removed, breaking UX for duplicate file selection

    • File: crates/ironclaw_gateway/static/js/surfaces/chat.js
    • Lines ~226–229
    • Issue: Removed e.target.value = '' reset after calling handleImageFiles(). This prevents users from selecting the same file twice (browser caches the last selected file path and won't trigger change event on repeat). While the Array.from() refactor is good, the value reset is essential for UX.
    • Fix: Restore e.target.value = ''; after the handleImageFiles call

MEDIUM severity

  1. [MEDIUM:75] Missing optional chaining in parseUserMessageContent() call

    • File: crates/ironclaw_gateway/static/js/core/history.js
    • Line 76
    • Issue: parseUserMessageContent(turn.user_input).attachments assumes the function always returns an object. If it returns null/undefined, this throws. The typeof check on line 75 doesn't guarantee a truthy return.
    • Fix: Use optional chaining: parseUserMessageContent(turn.user_input)?.attachments ?? []

  2. [MEDIUM:68] ZIP extraction test refactoring reflects security policy change without explanation

    • File: src/tools/builtin/skill_tools.rs
    • Lines affected: Test test_zip_extract_nested_single_skill_supported()
    • Issue: Test comment changed from "must NOT match nested paths" to "should still extract nested SKILL.md". This inverts a security assumption. The refactor from manual ZIP construction to build_zip_archive() helper is good (DRY), but the test behavior change (now accepting nested SKILL.md instead of rejecting) is a security policy change that deserves a dedicated commit or comment explaining why nested paths are now safe.
    • Fix: Add a docstring to test explaining the security rationale, or move to separate commit with security review

  3. [MEDIUM:75] Inefficient pending message deduplication rebuilt on every history load

    • File: crates/ironclaw_gateway/static/js/core/history.js
    • Lines 40–56
    • Issue: The pendingByContent map is built with nested .forEach() calls on every loadHistory() invocation. For chats with many duplicate pending messages, this is O(n) overhead that repeats across page loads, pagination, and in-progress refreshes.
    • Performance: One iteration filters, another builds map (two passes). Could use .reduce() in single pass.
    • Fix: Optimize map construction to single-pass reduce, or lazy-build only if visuals detected

LOW severity

  1. [LOW:50] N+1 DOM reflows when rendering pending messages
    • File: crates/ironclaw_gateway/static/js/core/history.js
    • Lines 46–53, 115–122, 137–143 (pattern repeated 3×)
    • Issue: Code calls addMessage() then conditionally calls appendImagesToMessage() separately. Each call triggers DOM reflow. Should batch by passing images to addMessage() once.
    • Example: const div = addMessage(...); if (images) appendImagesToMessage(div, images); → creates element, then modifies it (2 reflows)
    • Fix: Refactor addMessage() to accept optional images/attachments parameter

Summary: 2 bugs blocking production (HIGH), 4 issues requiring attention (MEDIUM–LOW). The pending message logic requires careful review to prevent duplicate renders in cases where multiple identical messages are sent with attachments.

Base automatically changed from staging-promote/336bdb1e-24684983862 to main April 21, 2026 03:18
@henrypark133 henrypark133 merged commit 6d4935a into main Apr 21, 2026
65 of 91 checks passed
@henrypark133 henrypark133 deleted the staging-promote/6d4935a6-24700448305 branch April 21, 2026 03:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

contributor: core 20+ merged PRs risk: medium Business logic, config, or moderate-risk modules scope: channel/web Web gateway channel scope: tool/builtin Built-in tools staging-promotion

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@henrypark133