Conversation
…#405 S1) Restructure the model-initiated bash tool_call tail into an ordered, named policy list (BASH_COMMAND_POLICIES) evaluated by evaluateBashPolicies: first non-undefined verdict wins, undefined defers to the next policy, overall undefined allows. The only entry is the runtime-guardrails adapter over the byte-identical confirmCommand, so every verdict, block reason, emitted event sequence, headless fail-safe, and config precedence is unchanged. Exposed via __testing with order/short-circuit/allow tests. Work unit S1 of Gentleman-Programming#405: no new policy modules, no config schema change, no second tool_call handler.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe bash tool-call path now evaluates an ordered policy list. The initial ChangesBash policy evaluation
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Refactor Sequence Diagram(s)sequenceDiagram
participant BashToolCallHandler
participant evaluateBashPolicies
participant runtime-guardrails
participant confirmCommand
BashToolCallHandler->>evaluateBashPolicies: evaluate command
evaluateBashPolicies->>runtime-guardrails: evaluate command
runtime-guardrails->>confirmCommand: confirm command
confirmCommand-->>runtime-guardrails: return verdict
runtime-guardrails-->>evaluateBashPolicies: return verdict
evaluateBashPolicies-->>BashToolCallHandler: return first non-undefined verdict
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The command-safety behavior remains preserved while gaining an ordered policy seam. No actionable merge risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
…interface
The seam harness mocked ExtensionAPI["events"] as
{ emit: () => {}, on: () => {} } with an `as` cast, which trips TS2352:
EventBus.on must return an unsubscribe function, and a void-returning
stub does not overlap. Give the mock the real emit/on signatures so it
matches EventBus structurally and the cast disappears entirely, keeping
the typecheck ratchet baseline at 205.
|
Adding review provenance: the branch went through a four-lens native review (risk, resilience, readability, reliability) on top of CI and CodeRabbit: approved with 8 informational advisories and no corrections required. The advisories worth a skim before merge, all non-blocking:
Verification state: 56/56 autonomous-guard tests pass, biome clean, CI green, CodeRabbit reports no actionable comments. No code changed since the last push, so the diff under review is unchanged. |
|
@Alan-TheGentleman consolidated status of my open gentle-shell PRs, since they all sit green and unlabeled and I want to make sure they are on your radar rather than lost. Merge-ready, blocked only on the
Needs triage rather than review: #605 closes #593, which has no labels yet. The PR itself merges clean, so an approval decision on the issue is the only open question. #915, #1009 and #605 were dry-run verified today as zero-conflict against current main. Happy to rebase, split, or close any of them if the direction changed on main. |
Part of #405 (work unit S1 of 4; the issue stays open until S2-S4 land).
Problem
#405 asks for structured, ordered evaluation of high-risk model-initiated Bash operations. Today the bash
tool_calltail evaluates protections as an ordered if/else chain inside the single handler, which leaves no deterministic place to compose future policy modules (npm/pnpm package acquisition, destructive direct SQL) without duplicating authority or making precedence hard to reason about.What this adds
Unit S1 from the decomposition posted on the issue: a behavior-preserving evaluation seam.
BASH_COMMAND_POLICIES) evaluated byevaluateBashPolicies: first non-undefined verdict wins,undefineddefers to the next policy, overallundefinedallows.confirmCommand, so every verdict, block reason, emitted event sequence, headless fail-safe, and config precedence is unchanged.__testingwith order, short-circuit, and allow tests (tests/autonomous-guard.test.ts).No new policy modules, no second authority, no sandbox semantics; S2 (npm/pnpm recognition), S3 (probes and evidence), and S4 (direct SQL DROP) build on this seam as separate chained units.
extensions/gentle-ai.tsevaluateBashPolicies+ guardrails adapter (+63)tests/autonomous-guard.test.tsVerification
evaluateBashPoliciesseam tests: 5/5.tests/autonomous-guard.test.ts: 56/56.tests/gentle-ai.test.ts(integration, guards the unchanged-behavior claim): 36/36.main(127 commits, clean, no conflicts) withgit range-diffreviewed and all suites re-run green.Label request:
type:feature(pull-only author, maintainer needs to apply it).Summary by CodeRabbit
rm -rf /, remain blocked or require confirmation, while safe commands such asecho hellocan proceed.