Skip to content

Improve coding-full parity defaults#8

Open
rohitg00 wants to merge 1 commit into
mainfrom
feature/coding-full-profile
Open

Improve coding-full parity defaults#8
rohitg00 wants to merge 1 commit into
mainfrom
feature/coding-full-profile

Conversation

@rohitg00
Copy link
Copy Markdown
Collaborator

@rohitg00 rohitg00 commented May 13, 2026

Summary

  • make run/resume/chat streaming defaults consistent at 30 minutes
  • verify live MCP and database functions in doctor --coding-full
  • relax the example shell policy for normal coding commands like npm run, node -e, python -c, find, sed, and awk
  • update first-turn guidance and parity docs toward Kimchi/Pi-style coding-agent behavior

Verification

  • cargo fmt -- --check
  • cargo test
  • cargo clippy --all-targets -- -D warnings
  • cargo install --path .
  • live iii-code doctor --coding-full
  • live shell::exec smoke checks for node -e and npm run --help
  • live iii-code chat --new "reply with hi" --stream-timeout-ms 120000

Summary by CodeRabbit

  • Documentation

    • Expanded setup and configuration documentation with additional shell command examples and verification details.
  • Configuration

    • Updated shell worker command allowlist with additional common utilities and tools.
  • Improvements

    • Enhanced health checks for coding-full setup verification.
    • Increased default operation timeout for better stability.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

📝 Walkthrough

Walkthrough

This PR enhances iii-code toward a more complete terminal coding agent by adding runtime function verification to doctor --coding-full, expanding shell command authorization policy, increasing operation timeouts, and updating agent behavior instructions with corresponding documentation of the coding-full parity goals and configuration.

Changes

Coding-full setup, runtime verification, and agent behavior enhancements

Layer / File(s) Summary
Coding-full runtime function requirements and verification
src/app.rs
CODING_FULL_RUNTIME_FUNCTIONS constant defines required coding-full functions. doctor --coding-full probe now queries engine::functions::list and fails when any required runtime functions are missing. A new missing_function_ids(...) helper refactors common function-id detection logic. New test validates the missing-function failure path.
Shell command allowlist and denylist policy
config.example.yaml, README.md
Shell worker allowlist expanded to include stream editors, package/language tools (sed, awk, perl, npm, pip, cargo, pytest, etc.), and utilities (curl, wget, mkdir, touch, cp, mv). denylist_patterns narrowed to only curl file-exfil and git-operation restrictions. README examples and lead-in text updated to reflect the expanded and focused policy.
Extended stream timeout defaults for run and resume
src/cli.rs
RunArgs and ResumeArgs default stream_timeout_ms increased from 600,000 ms (10 min) to 1,800,000 ms (30 min). Test assertions updated to validate the new defaults for both run and resume command parsing paths.
Full-coding-agent behavior context and instructions
src/payload.rs
III_CODE_CLIENT_CONTEXT prompt expanded with instructions: act as a complete coding agent, invoke matching worker functions in the same turn, correctly handle streamed/file-read outputs before concluding, pivot after repeated identical tool-shape failures. Test updated to verify "full coding agent" and "same turn" phrases are included in composed worker-aware messages.
Coding-full parity goals and configuration documentation
README.md, docs/feature-parity-gaps.md
Documentation refined to position iii-code as a complete terminal coding agent (with setup expectations adjusted accordingly); expanded setup --coding-full and doctor --coding-full verification descriptions to include MCP/database function registration checks; clarified shell command allowlist/denylist guidance; restructured parity gaps as "Highest-priority Kimchi/Pi parity gaps" with permission preset mappings, improved timeout recovery, and future contract/UX dependencies.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • rohitg00/iii-code#7: Modifies doctor --coding-full health-probe logic in src/app.rs and aligns related CLI stream-timeout and payload/context messaging in src/cli.rs and src/payload.rs.
  • rohitg00/iii-code#5: Extends doctor --coding-full/setup --coding-full health/probe behavior in src/app.rs to validate coding-full requirements with scaffolding for coding-full profile routing and verification.
  • rohitg00/iii-code#4: Modifies src/app.rs doctor/health probe logic to validate required "core/runtime function" sets via engine::functions::list when harness probing is unavailable.

Poem

🐰 A rabbit hops through timeout lands,
Commands are blessed, denied by hands,
The agent learns to code complete,
Runtime checks make healing sweet,
Full-featured now, the parity's neat!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Improve coding-full parity defaults' accurately reflects the main objectives of the pull request—improving defaults and parity for the coding-full profile across multiple components.
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 feature/coding-full-profile

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.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
config.example.yaml (1)

129-141: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add wget exfiltration guardrails (or remove it from allowlist).

Line 129 allows wget, but denylist protections at Line 140 are curl-specific. This leaves a gap for outbound data upload patterns via wget.

🛡️ Suggested fix
       denylist_patterns:
@@
       - \bcurl\b[^|;&]*(file://|-o\s|--output-dir\b|-F\s+@)
+      - \bwget\b[^|;&]*(--post-|--method\s+POST|--body-data|--body-file)
🤖 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 `@config.example.yaml` around lines 129 - 141, The config currently allows
"wget" (see the allowlist entry) but denylist_patterns only target curl/git;
update denylist_patterns to include wget-specific exfiltration flags (e.g.
matches for \bwget\b and common upload/exfil flags like -O|--output-document,
--post-data, --post-file, --method) so wget-based exfiltration is caught, and/or
remove "wget" from the allowlist; reference denylist_patterns and the existing
wget entry to implement the change.
🤖 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 `@config.example.yaml`:
- Around line 116-117: The allowlist currently contains the shell entries "sh"
and "bash" which permit use of sh -c / bash -lc and can bypass command-level
allowlisting; remove the "sh" and "bash" entries from the allowlist so arbitrary
shell invocation is not permitted, and instead explicitly enumerate permitted
executables/commands in the allowlist (or add stricter command-level patterns)
to prevent policy bypass; update any references to ALLOWLIST entries that
assumed shell semantics (e.g., callers that used "sh" to run compound commands)
to use the explicit permitted commands or a controlled wrapper.

---

Outside diff comments:
In `@config.example.yaml`:
- Around line 129-141: The config currently allows "wget" (see the allowlist
entry) but denylist_patterns only target curl/git; update denylist_patterns to
include wget-specific exfiltration flags (e.g. matches for \bwget\b and common
upload/exfil flags like -O|--output-document, --post-data, --post-file,
--method) so wget-based exfiltration is caught, and/or remove "wget" from the
allowlist; reference denylist_patterns and the existing wget entry to implement
the change.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8ad95786-8fca-43d7-bb75-2ad3839ef796

📥 Commits

Reviewing files that changed from the base of the PR and between 62b56c6 and e4e22e5.

📒 Files selected for processing (6)
  • README.md
  • config.example.yaml
  • docs/feature-parity-gaps.md
  • src/app.rs
  • src/cli.rs
  • src/payload.rs

Comment thread config.example.yaml
Comment on lines +116 to +117
- sh
- bash
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Remove sh/bash from the allowlist to prevent policy bypass.

Line 116 and Line 117 allow arbitrary command execution via sh -c/bash -lc, which defeats command-level allowlisting and weakens denylist protection.

🔒 Suggested fix
-      - sh
-      - bash
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- sh
- bash
🤖 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 `@config.example.yaml` around lines 116 - 117, The allowlist currently contains
the shell entries "sh" and "bash" which permit use of sh -c / bash -lc and can
bypass command-level allowlisting; remove the "sh" and "bash" entries from the
allowlist so arbitrary shell invocation is not permitted, and instead explicitly
enumerate permitted executables/commands in the allowlist (or add stricter
command-level patterns) to prevent policy bypass; update any references to
ALLOWLIST entries that assumed shell semantics (e.g., callers that used "sh" to
run compound commands) to use the explicit permitted commands or a controlled
wrapper.

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.

1 participant