Skip to content

feat(iii-worker): champion-tier agent DX for sandbox::*#1699

Merged
andersonleal merged 9 commits into
mainfrom
feat/sandbox-agent-dx
May 28, 2026
Merged

feat(iii-worker): champion-tier agent DX for sandbox::*#1699
andersonleal merged 9 commits into
mainfrom
feat/sandbox-agent-dx

Conversation

@andersonleal

@andersonleal andersonleal commented May 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Ships the full sandbox::* AI-agent DX work in one commit. Reduces workflow TTHW (time-to-hello-world) from 4 tool calls to 1, returns structured/resubmittable errors from every sandbox function, and wires handler-boundary tracing across all 17 sandbox handlers.

Highlights

  • sandbox::run — meta-function composing create + fs::write + exec + stop into a single call. Auto-stops the VM on success/failure unless keep_sandbox: true.
  • sandbox::catalog::list — new function returning the daemon's image catalog (bundled presets + operator-registered custom_images). Closes the "what images exist here" discovery loop.
  • Flexible input shapes
    • exec.cmd accepts cmd + args, argv array, or shell-line cmd (shlex-split).
    • exec.env / create.env accept both Vec<"K=V"> and { K: V } map shapes.
    • is_valid_env_name pins names to [A-Za-z_][A-Za-z0-9_]* (rejects digit-leading / /---= with S001).
  • fs::read inline body — returns UTF-8 string for text under 1 MiB; large/binary still streams via StreamChannelRef.
  • Structured errors — every sandbox::* returns { code, type, message, docs_url, retryable, fix, fix_note }. docs_url anchored at in-repo README (#S001, #S211, ...). fix is a non-null JSON payload the agent can resubmit verbatim when recovery is unambiguous (e.g. FsParentNotFound carries { "parents": true }).
  • RunStepFailed — when a sandbox::run sub-step fails, inner_code surfaces transparently and fix.context names the failing step plus fix.sandbox_id when keep_sandbox: true.
  • SandboxErrorWire wrapper makes Display emit the JSON payload through the iii_sdk async handler boundary so envelopes round-trip without per-handler serialization.
  • Default exec timeout 30s -> 300s (5m) — sized for cold npm install / pip install / cargo build. The old 30s default fired as an opaque engine-gate denial before the daemon could return a structured timed_out:true response.
  • Handler-boundary tracinglog_handler_result emits tracing::info! on both success and error with a stable field set (function_id, sandbox_id, success, error_code, error_type, retryable, duration_ms). Wired into all 17 sandbox handlers.
  • All 10 sandbox::fs::* functions migrated from legacy RegisterFunctionMessage to RegisterFunction::new_async with full JSON Schema, field docs, and worked examples. FsEntry, FsMatch, FsSedFileResult, FsReadMeta, StreamChannelRef, ChannelDirection all gain JsonSchema derives.
  • Agent-facing docs — README gets per-S-code anchored subsections (<a id="Sxxx"></a>) plus an Agent Quickstart. SANDBOX_AGENT_GUIDE is a paste-into-system-prompt cheat sheet (doctest pins contents against wire format).

New tests

  • sandbox_docs_anchor_stability.rs — every SandboxErrorCode has a README anchor.
  • sandbox_fs_decode_error_wire.rs — decode-error wire shape.
  • sandbox_fs_read_buffer_boundary.rs — byte-loss boundary check for the inline-UTF-8 read fast path.

Backwards compatibility

Wire shape stays additive: missing argv/env fields default to empty, existing Vec env shape still deserializes, error code propagation still works via map_iii_err's three-path matching. Shell worker (workers/shell) regression-checked: 20/20 sandbox dispatch tests pass unchanged.

Test plan

  • iii-worker lib tests pass (770+, incl. 5 new: catalog::list, RunStepFailed wire shape, FsParentNotFound fix payload, handler-boundary trace shape)
  • iii-worker integration tests pass (1228 / 1228, 7 pre-existing ignored)
  • workers/shell sandbox dispatch tests pass (20/20)
  • Doctest pins SANDBOX_AGENT_GUIDE contents against the wire format
  • Manual smoke: sandbox::run end-to-end with a real workload
  • Manual smoke: sandbox::catalog::list against a daemon with custom_images registered

Summary by CodeRabbit

  • New Features

    • Added sandbox::run meta-function and sandbox::catalog::list.
    • Exec/create accept multiple command shapes (cmd+args, argv, shlex'd cmd) and env as vector or map; argv wins when present.
    • Many sandbox fs endpoints now publish typed request/response schemas with examples, per-call timing/tracing, and better error payloads.
    • Console trigger config is now optional and trigger summaries include full config payload.
  • Breaking Changes

    • sandbox::fs::read may include an inline UTF‑8 body for small files (additive).
    • Default sandbox::exec timeout increased from 30s to 300s.

Review Change Stack

Ships the full sandbox::* AI-agent DX work in one squash: TTHW=1
meta-function, structured S-code errors with resubmittable fix
payloads, multi-shape exec input, expanded catalog discovery, and
end-to-end handler-boundary tracing.

- sandbox::run: meta-function that composes create + fs::write +
  exec + stop into one call. Workflow TTHW (time-to-hello-world)
  drops from 4 tool calls to 1. Auto-stops the VM on both success
  and failure unless keep_sandbox: true.
- sandbox::catalog::list: NEW function returning the daemon's
  image catalog (bundled presets + operator-registered
  custom_images). Closes the "what custom images exist here"
  loop without requiring operator hand-off.

- sandbox::exec.cmd now accepts THREE shapes:
  * cmd + args (classic POSIX)
  * argv array
  * shell-line cmd (shlex-split when args/argv empty)
- sandbox::exec.env and sandbox::create.env accept BOTH
  Vec<"K=V"> and { K: V } map shapes (untagged enum).
- is_valid_env_name pins env-var names to [A-Za-z_][A-Za-z0-9_]*
  (lowercase fine; digits-leading or `/`/`-`/`=` rejected S001).
- sandbox::fs::read returns body inline as a UTF-8 string for
  text under 1 MiB; large/binary still streams via StreamChannelRef.
  Removes the "always stream" friction for typical text reads.

- Every sandbox::* function returns a structured envelope:
    { code, type, message, docs_url, retryable, fix, fix_note }
- docs_url is anchored at the in-repo README (e.g. #S001, #S211).
- fix is a non-null JSON payload the agent can resubmit verbatim
  when the recovery is unambiguous. fix_note describes how to use
  the fix or, when fix is null, explains why no auto-fix exists.
- New FsParentNotFound variant: parent-missing S211s carry
  fix: { "parents": true } that the agent merges into the
  original request. Plain target-missing FsNotFound still returns
  null (caller-intent-dependent recovery).
- New RunStepFailed variant: when sandbox::run sub-step fails,
  inner_code surfaces transparently and fix.context names the
  failing step (e.g. "during sandbox::run step `fs::write (code)`")
  plus fix.sandbox_id when keep_sandbox: true.
- SandboxErrorWire wrapper makes Display emit the JSON payload
  through iii_sdk's async handler boundary, so the structured
  envelope round-trips without custom serialization in each
  handler.

- DEFAULT_EXEC_TIMEOUT_MS: 30_000 -> 300_000 (5 minutes). Sized
  for cold `npm install` / `pip install` / `cargo build`. The
  previous 30s default fired as an opaque engine-gate denial
  before the daemon could return a structured timed_out:true
  response; the 5m default lets typical install commands complete
  without manual timeout_ms override.

- log_handler_result emits tracing::info! on BOTH success and
  error with a stable field set: function_id, sandbox_id, success,
  error_code, error_type, retryable, duration_ms.
- Wired into every sandbox handler: create, exec, stop, list,
  catalog::list, run, and all 10 sandbox::fs::* functions.
- Operators can dashboard sandbox::* usage without grepping logs.

- All 10 sandbox::fs::* functions migrated from the legacy
  RegisterFunctionMessage shape to RegisterFunction::new_async.
  Full JSON Schema with field docs and worked examples on every
  request/response struct.
- FsEntry, FsMatch, FsSedFileResult, FsReadMeta gain JsonSchema
  derives so schemars round-trips cleanly through the SDK.
- iii-sdk channels (StreamChannelRef, ChannelDirection) gain
  JsonSchema derives for the same reason.

- README per-S-code anchored subsections (<a id="Sxxx"></a>) plus
  an Agent Quickstart that maps real questions to the right
  function in <60 seconds.
- SANDBOX_AGENT_GUIDE pub const: paste-into-system-prompt cheat
  sheet covering the workflow, cmd shapes, env shapes, error
  parsing, and S-code recovery table. Doctest pins the contents
  against the wire format.
- Three new tests pin doc/wire invariants:
  * sandbox_docs_anchor_stability.rs — every SandboxErrorCode has
    a README anchor
  * sandbox_fs_decode_error_wire.rs — decode-error wire shape
  * sandbox_fs_read_buffer_boundary.rs — byte-loss boundary check
    for the inline-UTF-8 read fast path

- Shell worker (workers/shell) regression-checked: 20/20 sandbox
  dispatch tests pass unchanged. Wire shape stays additive:
  missing argv/env fields default to empty, existing Vec env
  shape still deserializes, error code propagation still works
  via map_iii_err's three-path matching.

- iii-worker: 770+ lib tests pass (incl. 5 new for catalog::list,
  RunStepFailed wire shape, FsParentNotFound fix payload, and
  handler-boundary trace shape).
- iii-worker integration: 1228 / 1228 tests pass (7 ignored,
  all pre-existing).
@vercel

vercel Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
iii-website Skipped Skipped May 27, 2026 6:04pm

Request Review

@github-actions

github-actions Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

skill-check — docs

2 verified, 321 skipped.

Layer Result
structure
vale
ai

Three for three. Nicely done.

@github-actions

Copy link
Copy Markdown
Contributor

Terraform plan — infra/terraform/website

Click to expand
data.aws_route53_zone.iii_dev: Reading...
data.aws_caller_identity.current: Reading...
aws_cloudfront_function.redirects: Refreshing state... [id=iii-website-prod-redirects]
aws_iam_openid_connect_provider.github: Refreshing state... [id=arn:aws:iam::600627348446:oidc-provider/token.actions.githubusercontent.com]
aws_cloudfront_origin_access_control.site: Refreshing state... [id=E3GRB5YVVA4332]
aws_sns_topic.alarms: Refreshing state... [id=arn:aws:sns:us-east-1:600627348446:iii-website-prod-alarms]
aws_s3_bucket.site: Refreshing state... [id=iii-website-prod-us-east-1]
aws_cloudfront_response_headers_policy.site: Refreshing state... [id=033b1d66-8d53-48a1-a817-8280d8a60114]
aws_acm_certificate.site: Refreshing state... [id=arn:aws:acm:us-east-1:600627348446:certificate/b8e26c06-08b6-4b90-bd59-1f83bc231db2]
data.aws_caller_identity.current: Read complete after 0s [id=600627348446]
data.aws_iam_policy_document.github_trust: Reading...
data.aws_iam_policy_document.github_tf_plan_trust: Reading...
data.aws_iam_policy_document.github_trust: Read complete after 0s [id=932063200]
data.aws_iam_policy_document.github_tf_plan_trust: Read complete after 0s [id=2702237784]
aws_iam_role.github_deploy_website: Refreshing state... [id=iii-website-prod-github-deploy]
aws_iam_role.github_tf_plan: Refreshing state... [id=iii-infra-github-tf-plan]
aws_sns_topic_subscription.email: Refreshing state... [id=arn:aws:sns:us-east-1:600627348446:iii-website-prod-alarms:0ef0815c-13c7-4db5-b2d6-f544a17e3cdb]
aws_cloudwatch_metric_alarm.acm_days_to_expiry: Refreshing state... [id=iii-website-prod-acm-days-to-expiry]
data.aws_route53_zone.iii_dev: Read complete after 0s [id=Z05516132AI1ZGB3NLC6D]
aws_route53_record.cert_validation["www.iii.dev"]: Refreshing state... [id=Z05516132AI1ZGB3NLC6D__2bb3298c0bcca2494fce5e36c3d1427d.www.iii.dev._CNAME]
aws_route53_record.cert_validation["iii-preview.iii.dev"]: Refreshing state... [id=Z05516132AI1ZGB3NLC6D__0e43dc21d86b26a03742fb15aa5b4558.iii-preview.iii.dev._CNAME]
aws_route53_record.cert_validation["iii.dev"]: Refreshing state... [id=Z05516132AI1ZGB3NLC6D__7853369f3aea55a5d242d7c68506980e.iii.dev._CNAME]
aws_iam_role_policy_attachment.github_tf_plan_readonly: Refreshing state... [id=iii-infra-github-tf-plan-20260414113923643200000001]
aws_acm_certificate_validation.site: Refreshing state... [id=2026-04-14 00:00:29.305 +0000 UTC]
aws_s3_bucket_server_side_encryption_configuration.site: Refreshing state... [id=iii-website-prod-us-east-1]
aws_s3_bucket_versioning.site: Refreshing state... [id=iii-website-prod-us-east-1]
aws_s3_bucket_public_access_block.site: Refreshing state... [id=iii-website-prod-us-east-1]
aws_cloudfront_distribution.site: Refreshing state... [id=E3N35RPQE4YVFQ]
aws_cloudwatch_metric_alarm.cf_5xx_rate: Refreshing state... [id=iii-website-prod-cf-5xx-rate]
data.aws_iam_policy_document.github_deploy_website: Reading...
data.aws_iam_policy_document.site_bucket: Reading...
aws_route53_record.apex_aaaa[0]: Refreshing state... [id=Z05516132AI1ZGB3NLC6D_iii.dev_AAAA]
aws_route53_record.preview_a: Refreshing state... [id=Z05516132AI1ZGB3NLC6D_iii-preview.iii.dev_A]
aws_route53_record.preview_aaaa: Refreshing state... [id=Z05516132AI1ZGB3NLC6D_iii-preview.iii.dev_AAAA]
aws_route53_record.www_aaaa[0]: Refreshing state... [id=Z05516132AI1ZGB3NLC6D_www.iii.dev_AAAA]
aws_route53_record.www_a[0]: Refreshing state... [id=Z05516132AI1ZGB3NLC6D_www.iii.dev_A]
data.aws_iam_policy_document.site_bucket: Read complete after 0s [id=2990320740]
aws_route53_record.apex_a[0]: Refreshing state... [id=Z05516132AI1ZGB3NLC6D_iii.dev_A]
data.aws_iam_policy_document.github_deploy_website: Read complete after 0s [id=1235650417]
aws_s3_bucket_policy.site: Refreshing state... [id=iii-website-prod-us-east-1]
aws_iam_policy.github_deploy_website: Refreshing state... [id=arn:aws:iam::600627348446:policy/iii-website-prod-github-deploy]
aws_iam_role_policy_attachment.github_deploy_website: Refreshing state... [id=iii-website-prod-github-deploy-20260414000337121000000003]

Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
  + create
  ~ update in-place
  - destroy

Terraform will perform the following actions:

  # aws_cloudfront_function.redirects will be updated in-place
  ~ resource "aws_cloudfront_function" "redirects" {
      ~ code                         = <<-EOT
            // Viewer-request handler for the default (S3) behavior. Tested in redirects.test.js.
            
            function redirect(location) {
              return {
                statusCode: 301,
                statusDescription: 'Moved Permanently',
                headers: {
                  location: { value: location },
                  'cache-control': { value: 'public, max-age=3600' },
                },
              }
            }
            
          + // Return a real 404 for unknown extensionless paths instead of the previous
          + // SPA fallback to /index.html. The old behavior served homepage HTML with
          + // status 200 for any unknown path (e.g. /workers/iii-queue from broken docs
          + // links), which Google indexed as soft-duplicate-of-homepage. Returning 404
          + // here lets Search Console's "Not found (404)" bucket reflect reality and
          + // stops generating "Duplicate without user-selected canonical" entries.
          + function notFound() {
          +   return {
          +     statusCode: 404,
          +     statusDescription: 'Not Found',
          +     headers: {
          +       'content-type': { value: 'text/html; charset=utf-8' },
          +       'cache-control': { value: 'public, max-age=300' },
          +     },
          +     body: '<!doctype html><meta charset="utf-8"><title>404 — iii</title><meta name="robots" content="noindex"><style>body{font-family:system-ui,sans-serif;max-width:40rem;margin:4rem auto;padding:0 1rem;color:#1a1a1a}a{color:#1a5fbf}</style><h1>404 — page not found</h1><p>That URL doesn\'t exist on iii.dev.</p><p><a href="/">home</a> · <a href="/docs">docs</a> · <a href="/blog/">blog</a> · <a href="/manifesto">manifesto</a></p>',
          +   }
          + }
          + 
            // CloudFront Functions deliver request.querystring as
            //   { key: { value: string, multiValue?: [{ value: string }, ...] } }
            // where repeated params spill into multiValue. We re-encode and rejoin so the
            // host-redirect path below preserves the original query (otherwise `?a=1&a=2`
            // would silently drop on the 301).
            function serializeQuerystring(qs) {
              if (!qs) return ''
              var parts = []
              for (var key in qs) {
                if (!Object.prototype.hasOwnProperty.call(qs, key)) continue
                var entry = qs[key]
                if (!entry) continue
                var encodedKey = encodeURIComponent(key)
                var primary = entry.value == null ? '' : entry.value
                parts.push(encodedKey + '=' + encodeURIComponent(primary))
                if (entry.multiValue && entry.multiValue.length) {
                  for (var i = 0; i < entry.multiValue.length; i++) {
                    var extra = entry.multiValue[i]
                    var extraValue = extra && extra.value != null ? extra.value : ''
                    parts.push(encodedKey + '=' + encodeURIComponent(extraValue))
                  }
                }
              }
              return parts.length ? '?' + parts.join('&') : ''
            }
            
            // biome-ignore lint/correctness/noUnusedVariables: CloudFront Function entry point
            // biome-ignore lint/complexity/useOptionalChain: cloudfront-js-2.0 does NOT support optional chaining
            function handler(event) {
              var request = event.request
              var uri = request.uri
              var host = request.headers && request.headers.host ? request.headers.host.value : undefined
            
              if (host === 'www.iii.dev') {
                return redirect(`https://iii.dev${uri}${serializeQuerystring(request.querystring)}`)
              }
            
              if (uri.indexOf('/.well-known/') === 0) return request
            
              // Pretty URLs → matching *.html objects in S3 (Option A). Add a key when you
              // ship a new top-level page as `pagename.html`.
              var htmlPretty = {
                '/manifesto': '/manifesto.html',
              }
              var htmlTarget = htmlPretty[uri]
              if (htmlTarget !== undefined) {
                request.uri = htmlTarget
                return request
              }
            
              // /blog/* — Astro emits build.format: 'directory' with trailingSlash:
              // 'always', so canonical URLs are /blog/<slug>/. CloudFront's
              // default_root_object only applies to the apex, so we rewrite directory
              // URLs to .../index.html and 301 extensionless paths to the canonical
              // trailing-slash form. Must run before the SPA fallback so /blog/<slug>
              // doesn't get hijacked into /index.html.
              var redirectHost = host || 'iii.dev'
              if (uri === '/blog') {
                return redirect('https://' + redirectHost + '/blog/' + serializeQuerystring(request.querystring))
              }
              if (uri.indexOf('/blog/') === 0) {
                if (uri.charAt(uri.length - 1) === '/') {
                  request.uri = uri + 'index.html'
                  return request
                }
                var lastSlashB = uri.lastIndexOf('/')
                var lastSegmentB = uri.substring(lastSlashB + 1)
                if (lastSegmentB.indexOf('.') === -1) {
                  return redirect('https://' + redirectHost + uri + '/' + serializeQuerystring(request.querystring))
                }
                return request
              }
            
          -   // SPA fallback: extensionless path not ending in /
          +   // Real 404 for extensionless paths not in the pretty-URL map. Previously
          +   // this rewrote to /index.html (soft-404 cloning the homepage) — see
          +   // notFound() comment above for the SEO impact.
              if (uri !== '/' && uri.charAt(uri.length - 1) !== '/') {
                const lastSlash = uri.lastIndexOf('/')
                const lastSegment = uri.substring(lastSlash + 1)
                if (lastSegment.indexOf('.') === -1) {
          -       request.uri = '/index.html'
          -       return request
          +       return notFound()
                }
              }
            
              return request
            }
        EOT
        id                           = "iii-website-prod-redirects"
        name                         = "iii-website-prod-redirects"
        # (8 unchanged attributes hidden)
    }

  # aws_route53_record.apex_a[0] will be destroyed
  # (because index [0] is out of range for count)
  - resource "aws_route53_record" "apex_a" {
      - fqdn                             = "iii.dev" -> null
      - id                               = "Z05516132AI1ZGB3NLC6D_iii.dev_A" -> null
      - multivalue_answer_routing_policy = false -> null
      - name                             = "iii.dev" -> null
      - records                          = [] -> null
      - ttl                              = 0 -> null
      - type                             = "A" -> null
      - zone_id                          = "Z05516132AI1ZGB3NLC6D" -> null
        # (2 unchanged attributes hidden)

      - alias {
          - evaluate_target_health = false -> null
          - name                   = "d3de6i4fbqh35s.cloudfront.net" -> null
          - zone_id                = "Z2FDTNDATAQYW2" -> null
        }
    }

  # aws_route53_record.apex_aaaa[0] will be destroyed
  # (because index [0] is out of range for count)
  - resource "aws_route53_record" "apex_aaaa" {
      - fqdn                             = "iii.dev" -> null
      - id                               = "Z05516132AI1ZGB3NLC6D_iii.dev_AAAA" -> null
      - multivalue_answer_routing_policy = false -> null
      - name                             = "iii.dev" -> null
      - records                          = [] -> null
      - ttl                              = 0 -> null
      - type                             = "AAAA" -> null
      - zone_id                          = "Z05516132AI1ZGB3NLC6D" -> null
        # (2 unchanged attributes hidden)

      - alias {
          - evaluate_target_health = false -> null
          - name                   = "d3de6i4fbqh35s.cloudfront.net" -> null
          - zone_id                = "Z2FDTNDATAQYW2" -> null
        }
    }

  # aws_route53_record.www_a[0] will be destroyed
  # (because index [0] is out of range for count)
  - resource "aws_route53_record" "www_a" {
      - fqdn                             = "www.iii.dev" -> null
      - id                               = "Z05516132AI1ZGB3NLC6D_www.iii.dev_A" -> null
      - multivalue_answer_routing_policy = false -> null
      - name                             = "www.iii.dev" -> null
      - records                          = [] -> null
      - ttl                              = 0 -> null
      - type                             = "A" -> null
      - zone_id                          = "Z05516132AI1ZGB3NLC6D" -> null
        # (2 unchanged attributes hidden)

      - alias {
          - evaluate_target_health = false -> null
          - name                   = "d3de6i4fbqh35s.cloudfront.net" -> null
          - zone_id                = "Z2FDTNDATAQYW2" -> null
        }
    }

  # aws_route53_record.www_aaaa[0] will be destroyed
  # (because index [0] is out of range for count)
  - resource "aws_route53_record" "www_aaaa" {
      - fqdn                             = "www.iii.dev" -> null
      - id                               = "Z05516132AI1ZGB3NLC6D_www.iii.dev_AAAA" -> null
      - multivalue_answer_routing_policy = false -> null
      - name                             = "www.iii.dev" -> null
      - records                          = [] -> null
      - ttl                              = 0 -> null
      - type                             = "AAAA" -> null
      - zone_id                          = "Z05516132AI1ZGB3NLC6D" -> null
        # (2 unchanged attributes hidden)

      - alias {
          - evaluate_target_health = false -> null
          - name                   = "d3de6i4fbqh35s.cloudfront.net" -> null
          - zone_id                = "Z2FDTNDATAQYW2" -> null
        }
    }

  # aws_sns_topic_subscription.email will be created
  + resource "aws_sns_topic_subscription" "email" {
      + arn                             = (known after apply)
      + confirmation_timeout_in_minutes = 1
      + confirmation_was_authenticated  = (known after apply)
      + endpoint                        = "devops@motia.dev"
      + endpoint_auto_confirms          = false
      + filter_policy_scope             = (known after apply)
      + id                              = (known after apply)
      + owner_id                        = (known after apply)
      + pending_confirmation            = (known after apply)
      + protocol                        = "email"
      + raw_message_delivery            = false
      + topic_arn                       = "arn:aws:sns:us-east-1:600627348446:iii-website-prod-alarms"
    }

Plan: 1 to add, 1 to change, 4 to destroy.

@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Adds JsonSchema derives and schemars dependency; implements EnvShape and argv handling with shlex-based cmd splitting; raises exec timeout to 300s; introduces sandbox::run and sandbox::catalog::list; refactors sandbox::fs handlers to typed RegisterFunction::new_async with schema/examples; adds hybrid fs::read inline UTF‑8 fast‑path; expands errors with structured fix payloads, docs, and tests.

Changes

Sandbox Protocol Expansion and API Surface

Layer / File(s) Summary
Schema and Dependency Foundation
crates/iii-shell-proto/Cargo.toml, crates/iii-shell-proto/src/lib.rs, sdk/packages/rust/iii/src/channels.rs
Add schemars dependency and apply JsonSchema derives to public wire types (FsEntry, FsMatch, FsSedFileResult, FsReadMeta, ChannelDirection, StreamChannelRef) to enable cross-agent schema generation.
Error Variants, Environment Shape, and Structured Fix Payloads
crates/iii-worker/src/sandbox_daemon/errors.rs, crates/iii-worker/src/sandbox_daemon/exec.rs
Introduce FsParentNotFound and RunStepFailed error variants with step attribution and inner codes; add EnvShape enum supporting vector/map forms with validation; emit structured fix/fix_note payloads for auto-recovery hints.
Command Parsing, Environment Normalization, and Timeout Updates
crates/iii-worker/Cargo.toml, crates/iii-worker/src/cli/sandbox.rs, crates/iii-worker/src/cli/sandbox_daemon.rs, crates/iii-worker/src/sandbox_daemon/adapters.rs, crates/iii-worker/src/sandbox_daemon/create.rs, crates/iii-worker/src/sandbox_daemon/exec.rs
Add shlex dependency; implement resolve_cmd_shape() for whitespace-command splitting with argv precedence; normalize EnvShape (vector or map) to canonical Vec<String> with env-var name validation; bump exec timeout from 30s to 300s CLI/daemon-wide; wire normalized env through CreateRequest and handle_create.
Catalog List API and One-Call Sandbox::run Orchestrator
crates/iii-worker/src/sandbox_daemon/catalog.rs, crates/iii-worker/src/sandbox_daemon/run.rs
Implement CatalogListRequest/CatalogEntry/CatalogListResponse with preset+custom enumeration and deterministic ordering; define RunRequest/RunResponse/RunFile with interpreter selection, optional files, conditional sandbox_id; implement handle_run orchestrating create-write-exec-stop lifecycle with step-attributed error wrapping.
File Reading Refactor: Hybrid Inline UTF-8 and Streaming
crates/iii-worker/src/sandbox_daemon/fs/read.rs, crates/iii-worker/src/sandbox_daemon/fs/adapter.rs
Add INLINE_BUFFER_CAP (1 MiB) fast-path to return an inline body string for small UTF‑8 files while always returning content: StreamChannelRef; implement buffering/chain fallback at boundaries; distinguish FsParentNotFound in adapter mapping.
Filesystem Trigger Handlers: Typed Registration and Schema
crates/iii-worker/src/sandbox_daemon/fs/*.rs (chmod, grep, ls, mkdir, mv, rm, sed, stat, write, read)
Refactor all sandbox::fs::* triggers to derive JsonSchema with example generators, use RegisterFunction::new_async with typed request handlers, add per-request timing and unified logging via log_handler_result, convert errors through SandboxErrorWire.
Daemon Serve Entrypoint, Handler Wiring, and Logging Infrastructure
crates/iii-worker/src/sandbox_daemon/mod.rs
Rename run to serve; create shared fs_runner; register sandbox::catalog::list and sandbox::run; instrument create/exec/stop/list with duration and log_handler_result; add SANDBOX_AGENT_GUIDE and tests for logging helper.
Command and Environment Tests
crates/iii-worker/src/sandbox_daemon/exec.rs, crates/iii-worker/src/sandbox_daemon/create.rs
Update exec/create tests to use argv and EnvShape::default(); add tests for shlex-splitting, ambiguous cmd/args, argv precedence, unbalanced quotes, env map normalization and invalid-name rejection.
Integration Test Updates
crates/iii-worker/tests/*.rs (fake_sandbox_relay, sandbox_lifecycle_integration, sandbox_workflow_integration, sandbox_fakes)
Update test suites and fakes to use EnvShape::default() for env and include argv: vec![] in ExecRequest.
Testing Infrastructure
crates/iii-worker/tests/sandbox_docs_anchor_stability.rs, crates/iii-worker/tests/sandbox_fs_decode_error_wire.rs, crates/iii-worker/tests/sandbox_fs_read_buffer_boundary.rs
Add CI tests for documentation anchor stability, decode-error wire invariants for LsRequest deserialization, and fs::read buffer boundary conditions.
Documentation: README Quickstart, Error References, and Changelog
crates/iii-worker/src/sandbox_daemon/README.md, docs/changelog/index.mdx
Add Agent Quickstart, document multi-shape exec/env inputs, inline body behavior for fs::read, per-S-code anchors and fix semantics, and changelog noting timeout/API changes.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • iii-hq/iii#1578: Related SDK register_function refactor and handler-wiring changes.
  • iii-hq/iii#1554: Related sandbox daemon fs handler and S-code wiring changes.
  • iii-hq/iii#1675: Overlaps on RegisteredTriggerSummary and engine discovery DTOs.

Suggested reviewers

  • sergiofilhowz
  • guibeira
  • ytallo

"🐰 I hopped into the code with a schematic song,
Schemas added, commands parsed strong,
One-call run and catalog in view,
Small files inline, big streams still true,
Timeouts stretched — I celebrate along!"

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: delivering champion-tier developer experience improvements for the sandbox::* API family.
Description check ✅ Passed The PR description comprehensively covers the changeset with clear sections (Summary, Highlights, New tests, Backwards compatibility, Test plan) and explains the motivation, key features, and testing status.
Docstring Coverage ✅ Passed Docstring coverage is 96.05% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/sandbox-agent-dx

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🧹 Nitpick comments (4)
crates/iii-worker/src/sandbox_daemon/README.md (2)

444-460: 💤 Low value

Consider adding descriptions to filesystem error codes for consistency.

Error codes S211-S216 have minimal documentation (just titles), while other error codes like S001, S100, and S200 include detailed descriptions and guidance. For a more consistent agent experience, consider adding brief descriptions to each filesystem error:

  • S211: Clarify when this occurs (read/write/stat on missing path)
  • S212: Explain the operation attempted and the conflicting file type found
  • S213: Note which operations trigger this (mkdir, write with exclusive flag)
  • S214: Clarify this applies to rm operations
  • S215: Mention common causes (chmod restrictions, ownership)
  • S216: Distinguish from other error types

Even 1-2 sentences per code would help agents understand recovery paths.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/iii-worker/src/sandbox_daemon/README.md` around lines 444 - 460, Add
1–2 sentence descriptions for the filesystem error codes S211, S212, S213, S214,
S215, and S216 in the README where the error list is defined so they match the
level of detail of S001/S100/S200; for each code include when it occurs and
typical causes or recovery hints (e.g., S211: occurs on read/write/stat of a
missing path; S212: operation attempted vs. conflicting file type; S213:
triggered by mkdir or write with exclusive flag; S214: applies to rm failures;
S215: caused by permission/ownership/chmod issues; S216: explain how it differs
from other errors). Update the entries named S211–S216 in the README so each has
a short descriptive sentence and a brief recovery suggestion.

12-86: ⚡ Quick win

Consider moving example code to docs/ and linking from README.

The Agent Quickstart section includes extensive JSON code examples demonstrating sandbox::run, the surgical workflow, cmd shapes, env shapes, and error handling. As per coding guidelines, READMEs should link examples from docs/ rather than duplicate them inline.

Consider:

  1. Moving these examples to docs/api-reference/sandbox.mdx or a dedicated quickstart guide
  2. Replacing the inline examples with brief descriptions and links to the full examples in docs/

This reduces duplication and ensures examples stay synchronized across documentation surfaces.

As per coding guidelines: "**/README.md: Ensure that READMEs do not contain example code that is already in the docs/. READMEs should link these examples in the docs/."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/iii-worker/src/sandbox_daemon/README.md` around lines 12 - 86, The
README's "Agent Quickstart" contains large inline JSON/code examples (including
sandbox::run usage, the surgical workflow, cmd shapes, env shapes, and error
handling) that should be moved into a docs page and replaced with links; create
a new docs file (e.g., docs/api-reference/sandbox.mdx or
docs/quickstart/sandbox.mdx) containing the full examples and explanations, then
edit the sandbox_daemon README.md's Agent Quickstart section to remove the
duplicated examples and add concise descriptions plus links to the new docs page
and to the specific examples for sandbox::run, surgical workflow, cmd/env
shapes, and error handling so the README points readers to the canonical docs.
crates/iii-worker/src/sandbox_daemon/fs/adapter.rs (1)

27-42: ⚡ Quick win

Normalize the S211 message before parent-path detection.

This branch is currently case-sensitive, so small message-format shifts can downgrade FsParentNotFound to generic not-found and drop the intended recovery hint.

Proposed change
             "S211" => {
+                let msg_lc = message.to_ascii_lowercase();
                 // The in-VM shell uses S211 for both "the target path
                 // doesn't exist" and "an intermediate parent directory
                 // doesn't exist". Split them here so the daemon-side
@@
-                if message.contains("parent not found")
-                    || message.contains("parent directory not found")
+                if msg_lc.contains("parent not found")
+                    || msg_lc.contains("parent directory not found")
                 {
                     SandboxError::FsParentNotFound { path: message }
                 } else {
                     SandboxError::fs_not_found(message)
                 }
             }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/iii-worker/src/sandbox_daemon/fs/adapter.rs` around lines 27 - 42, The
S211 branch's parent-path detection is case-sensitive and can miss variants of
the error text; normalize the message (e.g., compute a lowercase/trimmed
version) before checking for "parent not found" or "parent directory not found"
so the condition reliably detects parent-missing errors, while still returning
the original message text to SandboxError::FsParentNotFound { path: ... } or
passing the original message into SandboxError::fs_not_found(...) to preserve
the original payload.
crates/iii-worker/src/sandbox_daemon/exec.rs (1)

231-233: 💤 Low value

Unreachable error path: argv is already confirmed non-empty.

The outer if !argv.is_empty() on line 223 ensures at least one element exists, so it.next() will always return Some. The ok_or_else closure is dead code. While harmless, consider removing it for clarity or adding a comment explaining it's defensive.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/iii-worker/src/sandbox_daemon/exec.rs` around lines 231 - 233, The
ok_or_else branch is unreachable because argv is already checked non-empty
earlier; replace the needless ok_or_else on the iterator (`let head =
it.next().ok_or_else(...)?`) with a direct unwrap/expect (e.g.,
`it.next().unwrap()` or `it.next().expect("argv non-empty")`) to clarify intent,
or if you prefer to keep defensive code, add a short comment referencing the
earlier `if !argv.is_empty()` check; make the change around the `argv`,
`it.next()` and `SandboxError::InvalidRequest` usage in exec.rs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/iii-worker/src/sandbox_daemon/create.rs`:
- Around line 172-177: BootingVm is being emitted before validating env, so if
req.env.into_kv_vec() fails clients get a spurious BootingVm; call
req.env.clone().into_kv_vec() and assign to env_vec before calling
on_event(SandboxCreateEvent::BootingVm), propagating the error as currently
done, then proceed to use env_vec for booting; refer to
SandboxCreateEvent::BootingVm, req.env, into_kv_vec(), env_vec, and on_event to
locate and reorder these operations.

In `@crates/iii-worker/src/sandbox_daemon/errors.rs`:
- Around line 288-291: Update the doc comment in errors.rs to match the actual
env-var validation used by is_valid_env_name in exec.rs: change the described
POSIX portable name pattern from `[A-Z_][A-Z0-9_]*` to the implemented
`[A-Za-z_][A-Za-z0-9_]*` (or equivalent wording), so the comment accurately
reflects the accepted uppercase and lowercase letters and digits; reference the
is_valid_env_name function to ensure consistency.
- Around line 18-19: The DOCS_BASE constant and the env-var validation mismatch
need addressing: either update the validation in exec.rs (function
is_valid_env_name) to enforce the documented regex by using is_ascii_uppercase
for the first char and is_ascii_uppercase/is_ascii_digit/_ for subsequent chars,
or update the error/doc text (and any README/regex mentions) to reflect the
current implementation that allows lowercase letters; also verify DOCS_BASE
(symbol DOCS_BASE) points to the correct README anchors used by
sandbox_docs_anchor_stability.rs and adjust the URL or comment if the repo path
assumption is inaccurate so SandboxErrorCode anchors remain valid.

In `@crates/iii-worker/src/sandbox_daemon/exec.rs`:
- Around line 52-55: The error text in the SandboxError::InvalidRequest call
incorrectly documents the expected env var pattern; update the message string to
reflect the actual validation used by is_valid_env_name by changing the regex in
the message to `[A-Za-z_][A-Za-z0-9_]*` so the error shown to users matches the
validator implementation.
- Around line 36-41: Update the doc comment on EnvShape::into_kv_vec to reflect
the actual validated env-name pattern: change the stated regex from
`[A-Z_][A-Z0-9_]*` to `[A-Za-z_][A-Za-z0-9_]*`, since the implementation in
is_valid_env_name permits lowercase for npm/pip/hg compatibility; keep the rest
of the doc (purpose, return type, SandboxError behavior) unchanged and ensure
the regex in the comment matches the behavior of into_kv_vec and
is_valid_env_name.

In `@crates/iii-worker/src/sandbox_daemon/README.md`:
- Around line 484-486: The README contains a forward reference to
docs/api-reference/sandbox.mdx#s-codes (and an inline TODO) that points to a
non-existent docs page; either create the referenced docs page
(docs/api-reference/sandbox.mdx with anchor s-codes) in this PR, or remove the
link and the TODO from the README and instead link to the relevant section of
this README; update the README entry that currently references
"docs/api-reference/sandbox.mdx#s-codes" so all references resolve to existing
documentation.

In `@crates/iii-worker/src/sandbox_daemon/run.rs`:
- Around line 123-136: The comment in interpreter_for is misleading and the
mapping incorrectly treats "bash" as /bin/sh; update the comment and mapping in
interpreter_for to reflect actual behavior: either remove the "-lc" mention from
the shell comment (since we pass a file path, not -lc) or change the
implementation so "bash" maps to "/bin/bash" if sandbox images provide bash;
alternatively remove "bash" from the explicit match so it falls to the
catch-all. Ensure you update the tuple returned for the "shell" branch and the
comment above it to be consistent with whichever choice you make, referencing
the interpreter_for function and the match arms for "shell" | "sh" | "bash".

In `@crates/iii-worker/tests/sandbox_docs_anchor_stability.rs`:
- Around line 44-48: The test currently accepts a lowercase-heading fallback
which contradicts the file-level contract that lowercase-renamed anchors should
fail; update the check in the test so it only accepts the explicit HTML id
anchor (use html_anchor and readme, remove the lowercase_heading check and its
creation) and adjust the accompanying comment to state that only the HTML id
anchor is allowed, ensuring that readme.contains is only tested against
html_anchor (refer to variables html_anchor, lowercase_heading, code_str, and
readme to locate and remove the lowercase fallback).

In `@crates/iii-worker/tests/sandbox_fs_read_buffer_boundary.rs`:
- Around line 113-152: The test
invalid_utf8_under_threshold_falls_through_to_stream doesn’t call handle_read so
it doesn’t verify buffer-boundary or stream-fallback behavior; either (A)
convert this unit test into an integration test that constructs a real
iii_sdk::III (or a test harness that can create_channel) and invokes handle_read
end-to-end using SandboxRegistry, FakeRunnerStream and ReadRequest to assert the
produced ReadContent variant, or (B) refactor the buffer/decision logic out of
handle_read into a pure helper function (e.g., decide_read_content_from_reader
or similar) that accepts the reader/metadata and returns a ReadContent, then
write unit tests that call that helper directly with deterministic
FakeRunnerStream/fixtures (including invalid_utf8_999kib) to assert Stream vs
Utf8 outcomes; pick one approach and update both tests (lines ~113-152 and
154-169) to exercise the chosen path.

In `@docs/changelog/index.mdx`:
- Line 63: Replace incorrect repository path in the docs URLs: update the JSON
"docs_url" value(s) and any textual breaking-change links that point to
"MotiaDev/motia" to the correct "iii-hq/iii" repository and proper file path;
specifically change URLs like
"https://github.com/MotiaDev/motia/.../README.md#Sxxx" to
"https://github.com/iii-hq/iii/blob/main/crates/iii-worker/src/sandbox_daemon/README.md#Sxxx"
wherever the "docs_url" key and the breaking change explanation link reference
the old repo.

---

Nitpick comments:
In `@crates/iii-worker/src/sandbox_daemon/exec.rs`:
- Around line 231-233: The ok_or_else branch is unreachable because argv is
already checked non-empty earlier; replace the needless ok_or_else on the
iterator (`let head = it.next().ok_or_else(...)?`) with a direct unwrap/expect
(e.g., `it.next().unwrap()` or `it.next().expect("argv non-empty")`) to clarify
intent, or if you prefer to keep defensive code, add a short comment referencing
the earlier `if !argv.is_empty()` check; make the change around the `argv`,
`it.next()` and `SandboxError::InvalidRequest` usage in exec.rs.

In `@crates/iii-worker/src/sandbox_daemon/fs/adapter.rs`:
- Around line 27-42: The S211 branch's parent-path detection is case-sensitive
and can miss variants of the error text; normalize the message (e.g., compute a
lowercase/trimmed version) before checking for "parent not found" or "parent
directory not found" so the condition reliably detects parent-missing errors,
while still returning the original message text to
SandboxError::FsParentNotFound { path: ... } or passing the original message
into SandboxError::fs_not_found(...) to preserve the original payload.

In `@crates/iii-worker/src/sandbox_daemon/README.md`:
- Around line 444-460: Add 1–2 sentence descriptions for the filesystem error
codes S211, S212, S213, S214, S215, and S216 in the README where the error list
is defined so they match the level of detail of S001/S100/S200; for each code
include when it occurs and typical causes or recovery hints (e.g., S211: occurs
on read/write/stat of a missing path; S212: operation attempted vs. conflicting
file type; S213: triggered by mkdir or write with exclusive flag; S214: applies
to rm failures; S215: caused by permission/ownership/chmod issues; S216: explain
how it differs from other errors). Update the entries named S211–S216 in the
README so each has a short descriptive sentence and a brief recovery suggestion.
- Around line 12-86: The README's "Agent Quickstart" contains large inline
JSON/code examples (including sandbox::run usage, the surgical workflow, cmd
shapes, env shapes, and error handling) that should be moved into a docs page
and replaced with links; create a new docs file (e.g.,
docs/api-reference/sandbox.mdx or docs/quickstart/sandbox.mdx) containing the
full examples and explanations, then edit the sandbox_daemon README.md's Agent
Quickstart section to remove the duplicated examples and add concise
descriptions plus links to the new docs page and to the specific examples for
sandbox::run, surgical workflow, cmd/env shapes, and error handling so the
README points readers to the canonical docs.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 49697d57-ba39-46e8-82eb-e1d819dc5c2b

📥 Commits

Reviewing files that changed from the base of the PR and between 576f738 and 9416694.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (33)
  • crates/iii-shell-proto/Cargo.toml
  • crates/iii-shell-proto/src/lib.rs
  • crates/iii-worker/Cargo.toml
  • crates/iii-worker/src/cli/sandbox.rs
  • crates/iii-worker/src/cli/sandbox_daemon.rs
  • crates/iii-worker/src/sandbox_daemon/README.md
  • crates/iii-worker/src/sandbox_daemon/adapters.rs
  • crates/iii-worker/src/sandbox_daemon/catalog.rs
  • crates/iii-worker/src/sandbox_daemon/create.rs
  • crates/iii-worker/src/sandbox_daemon/errors.rs
  • crates/iii-worker/src/sandbox_daemon/exec.rs
  • crates/iii-worker/src/sandbox_daemon/fs/adapter.rs
  • crates/iii-worker/src/sandbox_daemon/fs/chmod.rs
  • crates/iii-worker/src/sandbox_daemon/fs/grep.rs
  • crates/iii-worker/src/sandbox_daemon/fs/ls.rs
  • crates/iii-worker/src/sandbox_daemon/fs/mkdir.rs
  • crates/iii-worker/src/sandbox_daemon/fs/mv.rs
  • crates/iii-worker/src/sandbox_daemon/fs/read.rs
  • crates/iii-worker/src/sandbox_daemon/fs/rm.rs
  • crates/iii-worker/src/sandbox_daemon/fs/sed.rs
  • crates/iii-worker/src/sandbox_daemon/fs/stat.rs
  • crates/iii-worker/src/sandbox_daemon/fs/write.rs
  • crates/iii-worker/src/sandbox_daemon/mod.rs
  • crates/iii-worker/src/sandbox_daemon/run.rs
  • crates/iii-worker/tests/common/sandbox_fakes.rs
  • crates/iii-worker/tests/fake_sandbox_relay.rs
  • crates/iii-worker/tests/sandbox_docs_anchor_stability.rs
  • crates/iii-worker/tests/sandbox_fs_decode_error_wire.rs
  • crates/iii-worker/tests/sandbox_fs_read_buffer_boundary.rs
  • crates/iii-worker/tests/sandbox_lifecycle_integration.rs
  • crates/iii-worker/tests/sandbox_workflow_integration.rs
  • docs/changelog/index.mdx
  • sdk/packages/rust/iii/src/channels.rs

Comment thread crates/iii-worker/src/sandbox_daemon/create.rs Outdated
Comment thread crates/iii-worker/src/sandbox_daemon/errors.rs Outdated
Comment thread crates/iii-worker/src/sandbox_daemon/errors.rs
Comment thread crates/iii-worker/src/sandbox_daemon/exec.rs
Comment thread crates/iii-worker/src/sandbox_daemon/exec.rs
Comment thread crates/iii-worker/src/sandbox_daemon/README.md Outdated
Comment thread crates/iii-worker/src/sandbox_daemon/run.rs
Comment thread crates/iii-worker/tests/sandbox_docs_anchor_stability.rs Outdated
Comment thread crates/iii-worker/tests/sandbox_fs_read_buffer_boundary.rs
Comment thread docs/changelog/index.mdx
guibeira
guibeira previously approved these changes May 27, 2026
Comment thread crates/iii-shell-proto/Cargo.toml Outdated
Drop the untagged `ReadContent { Utf8 | Stream }` enum that
serializes `content` as a bare string for small text — that change
broke peers (notably `workers/shell`) that statically type
`content` as `StreamChannelRef`.

Restore `ReadResponse.content: StreamChannelRef` (always set,
always serves bytes through the channel) and surface the inline
UTF-8 fast path through an additive `body: Option<String>` field.
Both fields point at the same bytes for small text so legacy
subscribers keep working; new callers see `body.is_some()` and
short-circuit the channel subscription.

- `handle_read` routes the UTF-8 success branch through
  `stream_via_channel` with `body: Some(s)` and a Cursor over the
  buffered bytes. Other branches pass `body: None`.
- `stream_via_channel` takes an optional `body` parameter.
- `INLINE_BUFFER_CAP` doc + handler description updated.
- README `sandbox::fs::read` section rewritten for the new shape.
- Changelog drops the "Breaking" classification on this entry and
  describes the change as additive.
- Test `sandbox_fs_read_buffer_boundary.rs` drops `ReadContent`
  imports; structural assertions retargeted at the `body` field.
- 1228/1228 integration tests pass.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (2)
docs/changelog/index.mdx (1)

65-65: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix repository name in documentation URL.

The docs_url uses MotiaDev/motia but this PR is for the iii-hq/iii repository. The URL should be:

https://github.com/iii-hq/iii/blob/main/crates/iii-worker/src/sandbox_daemon/README.md#Sxxx
📝 Proposed fix
-  - `docs_url` anchors directly at the in-repo `S`-code subsection. **Breaking:** the base URL flipped from `https://iii.dev/docs/errors/sandbox/Sxxx` to `https://github.com/MotiaDev/motia/blob/main/crates/iii-worker/src/sandbox_daemon/README.md#Sxxx` while the canonical `iii.dev` error pages are still pending. Bookmarks and scrapers built on the old URL need to follow the new anchors.
+  - `docs_url` anchors directly at the in-repo `S`-code subsection. **Breaking:** the base URL flipped from `https://iii.dev/docs/errors/sandbox/Sxxx` to `https://github.com/iii-hq/iii/blob/main/crates/iii-worker/src/sandbox_daemon/README.md#Sxxx` while the canonical `iii.dev` error pages are still pending. Bookmarks and scrapers built on the old URL need to follow the new anchors.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/changelog/index.mdx` at line 65, Update the docs_url value to reference
the correct repository name: replace any occurrences of "MotiaDev/motia" with
"iii-hq/iii" so the link becomes
https://github.com/iii-hq/iii/blob/main/crates/iii-worker/src/sandbox_daemon/README.md#Sxxx;
specifically search for the docs_url constant/string in the changelog or docs
generator code and update the base GitHub path while keeping the README.md#Sxxx
anchor intact to preserve existing bookmarks.
crates/iii-worker/src/sandbox_daemon/README.md (1)

99-99: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Remove or update forward references to non-existent documentation.

Multiple lines reference docs/api-reference/sandbox.mdx which doesn't exist:

  • Line 99: "See S300 in docs/api-reference/sandbox.mdx"
  • Lines 487-489: "see docs/api-reference/sandbox.mdx#s-codes" with inline TODO
  • Line 494: Link to docs/api-reference/sandbox.mdx

As per coding guidelines, "Ensure all references to primitives reflect those defined in docs/" — references should point to actual documentation.

Since this README already defines all S-code anchors (lines 396-486), update these references to point to the relevant sections within this README instead:

  • Line 99: "See S300 below for the full diagnostic flow."
  • Lines 487-489: Remove the forward reference and TODO; the anchors are already documented in this file
  • Line 494: Either remove this link or change it to reference the Errors section of this README

As per coding guidelines: "**/*.md*: Ensure all references to primitives reflect those defined in docs/"

Also applies to: 487-489, 494-494

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/iii-worker/src/sandbox_daemon/README.md` at line 99, Replace external
references to docs/api-reference/sandbox.mdx with internal README anchors:
change the text that currently says "See `S300` in
`docs/api-reference/sandbox.mdx`" to "See [S300](`#S300`) below" (referencing the
S300 anchor already defined in this README); remove the forward-reference TODO
and any phrase "see `docs/api-reference/sandbox.mdx#s-codes`" (the S-code
anchors are already documented in this file), and update or remove the link that
points to `docs/api-reference/sandbox.mdx` so it instead points to this README's
Errors/S-codes section (e.g., "Errors" or the appropriate `#S300`/`#errors`
anchor) to ensure all S-code references are internal and consistent with the
anchors defined in this README.
🧹 Nitpick comments (2)
crates/iii-worker/src/sandbox_daemon/fs/read.rs (1)

144-147: std::io::Cursor is compatible with tokio::io::AsyncRead here
Tokio 1.x implements tokio::io::AsyncRead for std::io::Cursor<T>, so boxing std::io::Cursor<Vec<u8>> as Box<dyn tokio::io::AsyncRead + Unpin + Send> is valid. The .chain(reader) call aligns with Tokio’s AsyncReadExt::chain (given the boxed AsyncRead types), not std::io::Read::chain. [Optional] If those code paths are currently skipped, add/enable coverage to exercise them.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/iii-worker/src/sandbox_daemon/fs/read.rs` around lines 144 - 147, The
code is valid because std::io::Cursor<Vec<u8>> implements tokio::io::AsyncRead,
so keep creating bytes_for_channel, wrap it in std::io::Cursor::new(...) and box
it as Box<dyn tokio::io::AsyncRead + Unpin + Send> (the chained variable) and
call stream_via_channel(iii, chained, meta, Some(s), path). Ensure
tokio::io::AsyncReadExt is in scope if you later use .chain(...) on AsyncRead
objects (so the AsyncReadExt::chain method is used, not std::io::Read::chain),
and add/enable test coverage to exercise this code path if it’s currently
skipped.
docs/changelog/index.mdx (1)

58-58: ⚡ Quick win

Use the full path in the example docs_url for consistency.

Line 58 uses ... as a placeholder in the example URL:

"docs_url": "https://github.com/iii-hq/iii/.../README.md#S211"

For consistency with line 65 (which shows the full path) and to provide an accurate example, show the complete path:

"docs_url": "https://github.com/iii-hq/iii/blob/main/crates/iii-worker/src/sandbox_daemon/README.md#S211"

This matches what the actual error payload contains and helps users understand the exact URL format.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/changelog/index.mdx` at line 58, Replace the placeholder docs_url
example value so it shows the full repository path instead of "..." — update the
JSON example for the "docs_url" key (the example string on the line containing
"docs_url": "https://github.com/iii-hq/iii/.../README.md#S211") to the complete
path used in real payloads, e.g.
"https://github.com/iii-hq/iii/blob/main/crates/iii-worker/src/sandbox_daemon/README.md#S211",
ensuring the example matches the full path shown elsewhere in the changelog.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@crates/iii-worker/src/sandbox_daemon/README.md`:
- Line 99: Replace external references to docs/api-reference/sandbox.mdx with
internal README anchors: change the text that currently says "See `S300` in
`docs/api-reference/sandbox.mdx`" to "See [S300](`#S300`) below" (referencing the
S300 anchor already defined in this README); remove the forward-reference TODO
and any phrase "see `docs/api-reference/sandbox.mdx#s-codes`" (the S-code
anchors are already documented in this file), and update or remove the link that
points to `docs/api-reference/sandbox.mdx` so it instead points to this README's
Errors/S-codes section (e.g., "Errors" or the appropriate `#S300`/`#errors`
anchor) to ensure all S-code references are internal and consistent with the
anchors defined in this README.

In `@docs/changelog/index.mdx`:
- Line 65: Update the docs_url value to reference the correct repository name:
replace any occurrences of "MotiaDev/motia" with "iii-hq/iii" so the link
becomes
https://github.com/iii-hq/iii/blob/main/crates/iii-worker/src/sandbox_daemon/README.md#Sxxx;
specifically search for the docs_url constant/string in the changelog or docs
generator code and update the base GitHub path while keeping the README.md#Sxxx
anchor intact to preserve existing bookmarks.

---

Nitpick comments:
In `@crates/iii-worker/src/sandbox_daemon/fs/read.rs`:
- Around line 144-147: The code is valid because std::io::Cursor<Vec<u8>>
implements tokio::io::AsyncRead, so keep creating bytes_for_channel, wrap it in
std::io::Cursor::new(...) and box it as Box<dyn tokio::io::AsyncRead + Unpin +
Send> (the chained variable) and call stream_via_channel(iii, chained, meta,
Some(s), path). Ensure tokio::io::AsyncReadExt is in scope if you later use
.chain(...) on AsyncRead objects (so the AsyncReadExt::chain method is used, not
std::io::Read::chain), and add/enable test coverage to exercise this code path
if it’s currently skipped.

In `@docs/changelog/index.mdx`:
- Line 58: Replace the placeholder docs_url example value so it shows the full
repository path instead of "..." — update the JSON example for the "docs_url"
key (the example string on the line containing "docs_url":
"https://github.com/iii-hq/iii/.../README.md#S211") to the complete path used in
real payloads, e.g.
"https://github.com/iii-hq/iii/blob/main/crates/iii-worker/src/sandbox_daemon/README.md#S211",
ensuring the example matches the full path shown elsewhere in the changelog.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 8f7aa9be-4dd1-44e4-8fd0-520f23c0650c

📥 Commits

Reviewing files that changed from the base of the PR and between 9416694 and bd3c429.

📒 Files selected for processing (4)
  • crates/iii-worker/src/sandbox_daemon/README.md
  • crates/iii-worker/src/sandbox_daemon/fs/read.rs
  • crates/iii-worker/tests/sandbox_fs_read_buffer_boundary.rs
  • docs/changelog/index.mdx

- Validate env in sandbox::create BEFORE emitting BootingVm so a
  bad env never triggers a phantom boot-start event (CodeRabbit).
- Fix DOCS_BASE and changelog URLs from MotiaDev/motia to iii-hq/iii
  (canonical repo name; GitHub redirect could break).
- Update env-name docs and the user-facing InvalidRequest message in
  exec.rs to the actually-validated pattern `[A-Za-z_][A-Za-z0-9_]*`.
- Drop the lowercase-heading fallback in
  sandbox_docs_anchor_stability so a lowercase rename breaks CI
  (the test header already claimed this contract).
- Replace the misleading "/bin/sh -lc" comment in run.rs::interpreter_for
  and document why bash falls back to /bin/sh.
- Remove the dead forward reference to
  docs/api-reference/sandbox.mdx#s-codes from the README; the "See
  also" block below still links the page.
- Drop the schemars doc comment in iii-shell-proto/Cargo.toml per
  reviewer request.

1228/1228 integration tests pass; cargo fmt clean.
Ignore .serena/ (Serena tooling cache: memories, project config) and
CLAUDE.md (project-level agent guidance with host-specific gbrain
setup notes). Per-contributor state, not shared.
…Summary

Adds `config: Value` to `RegisteredTriggerSummary` (the API response
shape) alongside the existing `config_summary` display string.
`config_summary` is kept for backward compatibility but is a
truncated/lossy representation; consumers that need the structured
payload (api_path, http_method, topic, expression, …) now have a
canonical field to read from.

Frontend (console-frontend) consumes it defensively with `?? {}`
fallbacks so older / leaner summaries that omit the field don't
crash list rendering or invocation code.

- engine_fn/mod.rs: RegisteredTriggerSummary gains `config: Value`,
  populated in both list paths. Unit test asserts the field is
  serialized to JSON.
- console-frontend `TriggerInfo.config` is now optional; triggers
  list view, flows normalisation, and the invocation forms all
  narrow with `?? {}` before reading nested fields.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@console/packages/console-frontend/src/routes/triggers.tsx`:
- Around line 443-447: The invokeHttp function should validate required trigger
config fields before making network calls: in invokeHttp, check that
config.api_path is present and non-empty (after trimming slashes) and that the
resolved method is valid, and if missing/invalid return or throw a clear
client-side error instead of proceeding to call root "/"; likewise, for the
event invoke path (the code around lines handling event invokes e.g., the
function that uses topic in lines ~555-558), validate that the topic is
non-empty before sending and surface an immediate error; locate these checks by
the invokeHttp function name and the event-invoke block and add guard clauses
that fail fast with descriptive errors when required fields are absent.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0a4a4b39-7aca-491a-82ae-088dbd93f06e

📥 Commits

Reviewing files that changed from the base of the PR and between 67a22a3 and f965bc4.

📒 Files selected for processing (5)
  • .gitignore
  • console/packages/console-frontend/src/api/events/functions.ts
  • console/packages/console-frontend/src/api/flows/flows.ts
  • console/packages/console-frontend/src/routes/triggers.tsx
  • engine/src/workers/engine_fn/mod.rs
✅ Files skipped from review due to trivial changes (1)
  • .gitignore

Comment thread console/packages/console-frontend/src/routes/triggers.tsx
@andersonleal andersonleal merged commit f7ab0ec into main May 28, 2026
38 checks passed
@andersonleal andersonleal deleted the feat/sandbox-agent-dx branch May 28, 2026 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants