Skip to content

chore: promote staging to staging-promote/315c4cf8-24151502580 (2026-04-08 19:29 UTC) - #2163

Merged
henrypark133 merged 4 commits into
staging-promote/315c4cf8-24151502580from
staging-promote/bb2c3e1d-24154330911
Apr 9, 2026
Merged

chore: promote staging to staging-promote/315c4cf8-24151502580 (2026-04-08 19:29 UTC)#2163
henrypark133 merged 4 commits into
staging-promote/315c4cf8-24151502580from
staging-promote/bb2c3e1d-24154330911

Conversation

@ironclaw-ci

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

Copy link
Copy Markdown
Contributor

Auto-promotion from staging CI

Batch range: a55aff980a4e235590c3af57ded2542512e2f9f6..bb2c3e1dd17c9fe40c0f5490fc0c28e1680c02cb
Promotion branch: staging-promote/bb2c3e1d-24154330911
Base: staging-promote/315c4cf8-24151502580
Triggered by: Staging CI batch at 2026-04-08 19:29 UTC

Commits in this batch (62):

Current commits in this promotion (0)

Current base: staging-promote/315c4cf8-24151502580
Current head: staging-promote/bb2c3e1d-24154330911
Current range: origin/staging-promote/315c4cf8-24151502580..origin/staging-promote/bb2c3e1d-24154330911

  • (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

serrrfirat and others added 2 commits April 8, 2026 21:28
@serrrfirat @claude @ilblackdragon
feat(workspace): admin system prompt shared with all users (#2109)
d75e2b2 
* feat(workspace): admin system prompt shared with all users (#2088)

Introduce SYSTEM.md in a well-known __admin__ scope so admins can set a
system prompt that all tenants receive. Gated behind multi-tenant mode
(WorkspacePool sets admin_prompt_enabled on each workspace; owner
workspace in app.rs also gets the flag when has_any_users() is true).

New endpoints:
- GET  /api/admin/system-prompt — read admin system prompt
- PUT  /api/admin/system-prompt — set admin system prompt (64 KB limit)

Safety:
- SYSTEM.md added to injection scan list
- is_reserved_scope() guard on user creation (defense-in-depth)
- Multi-tenancy gate on both API and prompt assembly layers

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

* chore: remove review audit file from tracked files

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

* fix: add 64 KB size limit to admin system prompt PUT handler

Addresses PR review feedback:
- Enforce 64 KB limit on system prompt content to prevent token budget
  exhaustion (the content is injected into every user's system prompt)
- Add regression tests for the size limit (413 for oversized, not-413
  for at-limit)
- Document that is_multi_tenant is evaluated once at startup and the
  owner workspace requires a restart after the first user is created

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

* fix: address remaining review feedback on admin system prompt

- Restore rustdoc comments stripped from document.rs (DocumentMetadata,
  HygieneMetadata, DocumentVersion, VersionSummary, PatchResult, etc.)
  to keep the diff focused on feature additions only
- Replace silent error swallowing (if let Ok) with discriminated match
  in admin prompt read — only DocumentNotFound is silent, other errors
  logged at debug! level
- Cache admin system prompt on WorkspacePool to avoid an extra DB read
  on every turn; invalidated on PUT via invalidate_admin_prompt()
- Add cache invalidation integration test

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

* fix(workspace): tighten reserved-scope check and admin-prompt body limit

- is_reserved_scope: case-insensitive, whitespace-tolerant, and reserves
  the entire `__*__` namespace so future system scopes (alongside
  `__admin__`) cannot be impersonated by hand-crafted user IDs
- admin system-prompt route: layer-level DefaultBodyLimit of 128 KB
  rejects oversized payloads before JSON parse, complementing the
  in-handler 64 KB content cap
- system_prompt put_handler: clarify that the in-handler size check is
  a clearer-error fallback for the layer cap
- users_create_handler: drop the dead is_reserved_scope check on a
  freshly-minted UUID; the guard belongs at a code path that actually
  accepts user-supplied IDs
- expand is_reserved_scope tests for case, whitespace, and the wider
  `__*__` namespace

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

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Illia Polosukhin <ilblackdragon@gmail.com>
@serrrfirat @claude @ilblackdragon
fix (skills) installs for invalid catalog names (#2040)
bb2c3e1 
* Fix skill installs for invalid catalog names

* Fix clippy test module ordering

* fix: address PR review feedback

* fix: use PairingStore::new_noop() in SSRF test after merge with staging

The staging branch introduced a new test (test_http_request_rejects_private_ip_targets)
that calls PairingStore::new(), but this branch changed the signature to require
db and cache arguments. Use new_noop() since this is a test context.

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

* fix: address PR #2040 review — remove expect() and dead strip_prefix

- Restructure download_key flow in skills_install_handler to use the
  value directly instead of round-tripping through Option + expect(),
  satisfying the no-expect-in-production-code rule.
- Remove dead strip_prefix("---\n") in render_skill_md — serde_yml does
  not emit a leading document marker for structs.

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

* fix(skills): preserve unknown frontmatter and tighten install matching

- rewrite install-recovery to mutate the `name` field via raw YAML
  Value rather than re-serializing the typed SkillManifest, so unknown
  frontmatter keys (vendor extensions, future fields) survive the
  install rewrite
- catalog_entry_is_installed: case-insensitive comparison for the
  display-name and normalized-slug branches, matching the slug branch
- normalize_skill_identifier: document non-ASCII handling
- normalizing-invalid-name log: warn -> debug (REPL/TUI rule)
- add round-trip test asserting unknown top-level keys, nested
  mappings, and sequences survive install recovery

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

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Illia Polosukhin <ilblackdragon@gmail.com>
@github-actions github-actions Bot added size: XL 500+ changed lines risk: medium Business logic, config, or moderate-risk modules contributor: core 20+ merged PRs scope: channel/web Web gateway channel scope: tool/builtin Built-in tools scope: workspace Persistent memory / workspace labels Apr 8, 2026
@claude

claude Bot commented Apr 8, 2026

Copy link
Copy Markdown

Code review

Found 13 issues:

  1. [MEDIUM:85] Redundant normalization calls in catalog resolution hot path - In resolve_catalog_slug_for_name(), the second match closure calls normalize_catalog_identity() twice per entry (on both entry.name and slug_suffix). Results in O(2n) operations for catalogs with hundreds of entries.
    https://github.com/anthropics/ironclaw/blob/d848910b4287decfc647b1cbfae7bc939084f96c/crates/ironclaw_skills/src/catalog.rs#L128-L131

  2. [MEDIUM:85] Hardcoded error response in handlers - The system_prompt handlers return Result<Json, (StatusCode, String)> with manually constructed status codes and error strings. Per CLAUDE.md, errors should use thiserror for domain semantics.
    https://github.com/anthropics/ironclaw/blob/d848910b4287decfc647b1cbfae7bc939084f96c/src/channels/web/handlers/system_prompt.rs#L16-L48

  3. [MEDIUM:80] Unnecessary string allocations in catalog slug resolution - The collect_matches closure clones every matching entry's slug into a Vec, then sorts and deduplicates. This Vec is fully allocated on every call.
    https://github.com/anthropics/ironclaw/blob/d848910b4287decfc647b1cbfae7bc939084f96c/crates/ironclaw_skills/src/catalog.rs#L104-L114

  4. [MEDIUM:80] Startup-time admin prompt activation creates stale state risk - The admin prompt is enabled based on db.has_any_users() evaluated once at server startup. If users are added later, the flag stays false without a restart.
    https://github.com/anthropics/ironclaw/blob/d848910b4287decfc647b1cbfae7bc939084f96c/src/app.rs#L388-L396

  5. [MEDIUM:78] Workspace cache invalidation pattern relies on manual clearing - Invalidation of admin_prompt_cache relies on manual cache clearing after writes. This is prone to race conditions where a workspace could read stale cache.
    https://github.com/anthropics/ironclaw/blob/d848910b4287decfc647b1cbfae7bc939084f96c/src/workspace/mod.rs#L1615-L1650

  6. [MEDIUM:75] Double clone of admin prompt in cache population - In read_admin_prompt(), the content is cloned to an Option, then cloned again when inserting into Arc. Happens on every cache-miss during system_prompt_for_context().
    https://github.com/anthropics/ironclaw/blob/d848910b4287decfc647b1cbfae7bc939084f96c/src/workspace/mod.rs#L1644-L1647

  7. [MEDIUM:75] slug_suffix() uses unwrap_or() unnecessarily - rsplit('/').next() always returns Some for non-empty iterators, so the unwrap_or() will never trigger. This is safe but semantically misleading.
    https://github.com/anthropics/ironclaw/blob/d848910b4287decfc647b1cbfae7bc939084f96c/crates/ironclaw_skills/src/catalog.rs#L90-L92

  8. [MEDIUM:70] Quadratic YAML string rewriting - In rewrite_frontmatter_name(), deserialize YAML, mutate, re-serialize, then strip delimiters with multiple string operations. Multi-pass processing is inefficient for large manifests.

  9. [LOW:70] with_admin_prompt_cache() parameter lacks validation - The shared cache is passed without validation that it belongs to the same pool. Stale data could leak if a workspace gets a cache from a different pool.
    https://github.com/anthropics/ironclaw/blob/d848910b4287decfc647b1cbfae7bc939084f96c/src/workspace/mod.rs#L714-L718

  10. [LOW:65] Admin prompt caching state machine lacks explicit type - Uses Option where None='not loaded' and Some('')='loaded but empty'. Consider using explicit enum like CacheState::Uninitialized | Loaded(String).
    https://github.com/anthropics/ironclaw/blob/d848910b4287decfc647b1cbfae7bc939084f96c/src/workspace/mod.rs#L1615-L1625

  11. [LOW:65] Multiple normalization functions defined independently - PR adds normalize_catalog_identity() and reuses normalize_skill_identifier(). Consider centralizing to prevent divergence in normalization logic.
    https://github.com/anthropics/ironclaw/blob/d848910b4287decfc647b1cbfae7bc939084f96c/crates/ironclaw_skills/src/catalog.rs#L82-L88

  12. [LOW:65] Multiple to_ascii_lowercase calls per entry - Line 116 computes name.to_ascii_lowercase() once, but predicate on line 117 calls it once per entry in the catalog.
    https://github.com/anthropics/ironclaw/blob/d848910b4287decfc647b1cbfae7bc939084f96c/crates/ironclaw_skills/src/catalog.rs#L116-L120

  13. [LOW:60] Unnecessary sort+dedup on unique data - Catalog slugs should be unique by definition. Using HashSet during collection or BTreeMap would be more idiomatic than sort+dedup (O(n log n)).
    https://github.com/anthropics/ironclaw/blob/d848910b4287decfc647b1cbfae7bc939084f96c/crates/ironclaw_skills/src/catalog.rs#L111-L112

Summary: No critical or security issues found. The code is well-structured with proper auth gating and error handling. All findings are performance optimizations and type-safety improvements. Main concerns: cache consistency patterns (items 4-6) and catalog resolution hotspots (items 1-3).

henrypark133 and others added 2 commits April 8, 2026 14:34
@henrypark133 @claude
fix(test): use canonical extension name in setup submit test (#2158)
1eaea59 
The test used a hyphenated channel name ("test-failing-channel") but
canonicalize_extension_name() converts hyphens to underscores. This
caused configure() to look for "test_failing_channel.capabilities.json"
which didn't exist, returning an early Err before reaching the
activation code path the test was designed to exercise.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@henrypark133
Merge pull request #2170 from nearai/staging-promote/1eaea594-2416018…
e3789a3 
…1770

chore: promote staging to staging-promote/bb2c3e1d-24154330911 (2026-04-08 21:41 UTC)
@henrypark133
henrypark133 merged commit 9e89f77 into staging-promote/315c4cf8-24151502580 Apr 9, 2026
5 checks passed
@henrypark133
henrypark133 deleted the staging-promote/bb2c3e1d-24154330911 branch April 9, 2026 04:14
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
@henrypark133
Merge pull request nearai#2163 from nearai/staging-promote/0d3714fd-2…
4760c09 
…4154330911

chore: promote staging to staging-promote/a1b88640-24151502580 (2026-04-08 19:29 UTC)
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 scope: workspace Persistent memory / workspace size: XL 500+ changed lines staging-promotion

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@henrypark133 @serrrfirat