non-interactive tty should not touch the allowlists commands#168
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a non-interactive gate helper and uses it to block scripted runs of ChangesNon-interactive gate and related updates
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@README.md`:
- Around line 64-66: Insert a blank line before the "### Allowlist.toml" heading
and correct the typo "excplicitly" to "explicitly" in the README.md section that
describes the allowlist (refer to the "Allowlist.toml" heading and the path
"~/.config/dela/allowlist.toml"), and ensure the commands `dela allow <task>`
and `dela deny <task>` remain unchanged.
In `@src/commands/deny.rs`:
- Around line 7-22: The non-interactive gating logic (the should_block function
and its error/exit behavior) is duplicated between deny::execute and
allow::execute; extract this into a single shared helper (e.g.,
ensure_interactive or gate_non_interactive) in a common module and have both
deny::execute and allow::execute call that helper instead of their local
should_block; the helper should accept the same inputs (is_terminal, is_test,
force_interactive or compute them internally), perform the same eprintln +
std::process::exit(1) behavior, and remove the local should_block from
src/commands/deny.rs so both modules remain aligned as the policy evolves.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dc770f1c-6b6f-49d3-8475-b80ea78c7b94
📒 Files selected for processing (9)
README.mdcargo_crap_baseline.jsondev_docs/project_plan.mdsrc/allowlist.rssrc/commands/allow.rssrc/commands/deny.rssrc/commands/mod.rssrc/main.rstests/docker_noinit/test_noinit.sh
There was a problem hiding this comment.
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 `@tests/docker_noinit/test_noinit.sh`:
- Line 847: The assertions use "script -qc ..." which doesn't propagate the
wrapped child's exit code; update every "script -qc" invocation in the test file
to include the --return/-e flag (i.e. change "script -qc" to "script -eqc") so
the exit status of the enclosed command (e.g. the "dela allow print-args"
invocations) is returned and the conditional branches behave deterministically;
search for all occurrences of "script -qc" in tests/docker_noinit/test_noinit.sh
and replace them with "script -eqc".
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: bbfad0af-088a-4d60-94c6-2e6731dcac48
📒 Files selected for processing (8)
README.mdcargo_crap_baseline.jsonsrc/commands/allow.rssrc/commands/deny.rssrc/commands/mod.rssrc/main.rstests/docker_noinit/Dockerfiletests/docker_noinit/test_noinit.sh
Check for interaction sessions for
dela allowanddela denyand stop on them.Summary by CodeRabbit