chore(harness): prune transitive deps from iii.worker.yaml#134
Conversation
Drop deps that other declared deps already pull in: auth-credentials (providers), session/hook-fanout/provider-router (turn-orchestrator), llm-budget (provider-router). Reorder iii-* engine deps first.
skill-check — worker0 verified, 25 skipped (no docs/).
Three for three. Nicely done. |
📝 WalkthroughWalkthroughThe PR updates the worker harness configuration file by modifying the ChangesWorker Dependencies Configuration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
harness/iii.worker.yaml (1)
16-30:⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoffReconsider removing session, hook-fanout, provider-router, and llm-budget from the deployment manifest.
The PR claims these were removed because they're pulled in transitively by other dependencies. However,
harness/iii.worker.yamlis a runtime worker deployment manifest, not a Cargo dependency list. Whileauth-credentialsis genuinely transitive viaprovider-anthropicandprovider-openaiCargo packages, the other four removed workers (session,hook-fanout,provider-router, andllm-budget) do not appear as dependencies in any Cargo.toml files and are explicitly referenced in harness documentation (ARCHITECTURE.md, skill.md) and code (fanout.rs) as required peers.Removing them from the manifest means they won't be deployed alongside the harness unless deployed separately—they won't be automatically available at runtime. Tests pass because the harness binary itself doesn't compile-depend on them (the interface comes from
iii-sdk), but the harness is documented to expect these workers to be present during operation. Clarify whether this is intentional (workers will be deployed through other means) or if the removal is premature.🤖 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 `@harness/iii.worker.yaml` around lines 16 - 30, The deployment manifest removed four runtime workers (session, hook-fanout, provider-router, llm-budget) that are expected at runtime by the harness (see ARCHITECTURE.md, skill.md, and fanout.rs) even though they are not Cargo transitive dependencies; restore these worker entries in harness/iii.worker.yaml (add back session, hook-fanout, provider-router, and llm-budget) so they are deployed with the harness, or if the removal is intentional, update ARCHITECTURE.md, skill.md, and any references in fanout.rs/iii-sdk to document the new deployment plan and where those services will be provisioned instead; ensure auth-credentials remains handled transitively via provider-anthropic/provider-openai if that is correct.
🤖 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.
Outside diff comments:
In `@harness/iii.worker.yaml`:
- Around line 16-30: The deployment manifest removed four runtime workers
(session, hook-fanout, provider-router, llm-budget) that are expected at runtime
by the harness (see ARCHITECTURE.md, skill.md, and fanout.rs) even though they
are not Cargo transitive dependencies; restore these worker entries in
harness/iii.worker.yaml (add back session, hook-fanout, provider-router, and
llm-budget) so they are deployed with the harness, or if the removal is
intentional, update ARCHITECTURE.md, skill.md, and any references in
fanout.rs/iii-sdk to document the new deployment plan and where those services
will be provisioned instead; ensure auth-credentials remains handled
transitively via provider-anthropic/provider-openai if that is correct.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6338bca5-a661-4f17-b247-0328473cce36
📒 Files selected for processing (1)
harness/iii.worker.yaml
1 participant
Summary
harness/iii.worker.yaml:auth-credentials,session,hook-fanout,provider-router,llm-budget. Each is already pulled in by another declared dep (providers, turn-orchestrator, provider-router).iii-*engine deps first for readability.EXPECTED_WORKERS(generated byharness/build.rsfrom this manifest) shrinks accordingly, so the workers strip in the UI tracks only direct deps. The pruned workers still run on the bus via their parents.Test plan
cargo buildcleancargo test --lib— 41/41 pass (includingexpected_workers_includes_approval_gateandexpected_workers_includes_shell_consolidatedguards)Summary by CodeRabbit