feat(#462): container shell page — Pico CSS + htmx terminal for agent containers#505
feat(#462): container shell page — Pico CSS + htmx terminal for agent containers#505hognek wants to merge 2 commits into
Conversation
… agent containers
- GET /api/container-shell/{agent_id}: HTML terminal page with dark theme
- POST /api/container-shell/{agent_id}/exec: execute commands via incus exec
- Uses Pico CSS for styling and htmx for AJAX command execution
- ARIA-annotated for screen readers, output log with auto-scroll
- 23 tests covering auth, terminal, escaping, incus flow
- Strips ANSI escape sequences for safe display
Recreated from closed PR jaylfc#501 (fork had unrelated git history)
|
Warning Review limit reached
More reviews will be available in 7 minutes and 40 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✨ 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 |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| pass | ||
| return 124, f"(command timed out after {_EXEC_TIMEOUT}s)" | ||
| except Exception as exc: | ||
| return 1, f"(error: {exc})" |
There was a problem hiding this comment.
WARNING: Returning internal exception details to users
Exposing internal error information could leak sensitive details about the system or application internals. Consider returning a generic error message instead.
| try: | ||
| proc.kill() | ||
| await proc.wait() | ||
| except Exception: |
There was a problem hiding this comment.
WARNING: Bare except clause
A bare except: catches all exceptions including system-exiting ones like KeyboardInterrupt and SystemExit. This can make the program harder to debug and interrupt. Consider catching only specific exceptions or at least logging the caught exception.
| </div> | ||
|
|
||
| <!-- htmx from CDN --> | ||
| <script src="$$HTMX_JS$$" integrity="sha384-L6YHG9qPUPXKpqZ1jJGp3Z3vGL5pJE1vJBlbQ3eJR7okILhYZcsW0E6+5jFxVxFE" crossorigin="anonymous"></script> |
There was a problem hiding this comment.
WARNING: Potentially truncated integrity hash
The Subresource Integrity hash appears to be truncated (ends with "+5jFxVxFE"). A proper SHA384 integrity hash should be longer and properly base64-encoded. This could weaken the integrity protection against tampered resources.
| stdout=asyncio.subprocess.PIPE, | ||
| stderr=asyncio.subprocess.STDOUT, | ||
| ) | ||
| stdout_bytes, _ = await asyncio.wait_for( |
There was a problem hiding this comment.
WARNING: Unlimited command output size could lead to DoS
Reading unlimited output from commands into memory could lead to denial of service through memory exhaustion. Consider implementing output size limits or streaming the output to prevent excessive memory consumption.
|
Test comment |
Code Review SummaryStatus: 4 Issues Found | Recommendation: Address before merge |
Code Review SummaryStatus: 4 Issues Found | Recommendation: Address before merge Overview
|
Code Review SummaryStatus: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
|
Code Review SummaryStatus: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Other Observations (not in diff)No issues found in unchanged code that cannot receive inline comments. |
Code Review SummaryStatus: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Other Observations (not in diff)No issues found in unchanged code that cannot receive inline comments. Files Reviewed (3 files)
Reviewed by gemma-4-26b-a4b-it-20260403 · 4,251 tokens |
1 participant
What
Container shell page at
/api/container-shell/{agent_id}— a browser-based terminal for agent containers usingincus exec.Why
This is a clean recreation of the closed PR #501. The original PR's fork had unrelated git history, making it unmergeable. This branch is built directly from upstream master and contains only the container shell feature.
Testing
No JavaScript bundling required — uses Pico CSS (local) and htmx (CDN).