Granular "feeling safe" control - #5045
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a granular 'feeling safe' mechanism to allow enabling unsafe behaviors selectively. To improve the architecture, avoid using global state for tracking allowed functionalities. Make the DangerousFunctionality class and formatting functions context-aware by passing the Common object (or self) to retrieve options directly. Remove unused global variables, imports, and helper functions like allow_functionalities. Additionally, fix a typo in the TODO path for the local provision plugin and import Common directly under TYPE_CHECKING to avoid re-exports.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
ed3d98f to
30c7fcc
Compare
35b7784 to
79d239e
Compare
79d239e to
482ae37
Compare
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ 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 |
a5f5af4 to
125539c
Compare
LecrisUT
left a comment
There was a problem hiding this comment.
Other than having a more explicit none keyword, all LGTM.
125539c to
f402698
Compare
LecrisUT
left a comment
There was a problem hiding this comment.
Lgtm other than a couple of non-blocking implementation details
eaedae0 to
ee61f50
Compare
thrix
left a comment
There was a problem hiding this comment.
Three findings could not be anchored to diff lines because the PR does not touch those files. All three confirmed present at ee61f50c:
| File | Problem |
|---|---|
docs/overview.rst:390 |
"Set this variable to 1" — TMT_FEELING_SAFE=1 now exits 2 |
agents/AGENTS.md:72 |
tmt --feeling-safe -vv run ... — the option swallows -vv and errors |
.claude/CLAUDE.md:72 |
Same command, same break |
Generated-by: Claude Code
8566fd4 to
ce90270
Compare
tmt 1.78.0 dropped the step-level '--feeling-safe' (teemtee/tmt#5045, "Granular feeling safe control"): step and plugin commands now get only PLUGIN_SECURITY_OPTIONS, and the flag lives on the 'tmt' command alone. Our invocation put it after 'provision --how=local', so every run with a pip-installed tmt died before a single test ran: Error: No such option '--feeling-safe'. [Debug] tmt exit code: 2 which then cascaded into no results.yaml, no log.txt and a final "Test run did not complete: tmt exit '2', 0 results recorded." Move ${tmt_options} right after 'tmt'. The top-level '--feeling-safe' has existed since 1.38, so this keeps working with the 1.77 EPEL still ships for the 8/9 jobs -- which would have broken the same way as soon as EPEL picks up 1.78.
Instead of one big all-or-nothing switch, finer control over various dangerous action is needed. Users can still allow everything, with
--feeling-safe[=all], or allow individual dangerous functionalities.Pull Request Checklist