Skip to content

feat(#276): collect OpenShell logs before sandbox deletion - #555

Merged
rh-hemartin merged 1 commit into
mainfrom
agent/276-collect-openshell-logs
May 4, 2026
Merged

feat(#276): collect OpenShell logs before sandbox deletion#555
rh-hemartin merged 1 commit into
mainfrom
agent/276-collect-openshell-logs

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Add log collection step to the sandbox cleanup flow so that OpenShell gateway and sandbox logs are preserved in the run output directory for post-mortem debugging and CI artifact collection.

Changes:

  • sandbox.CollectLogs(): new function that runs
    openshell logs <name> --source <source> -n 0 with a 30s
    timeout to retrieve all available log lines
  • collectOpenshellLogs(): orchestrator that collects both
    sandbox and gateway log sources, writing them to
    /logs/openshell-sandbox.log and
    /logs/openshell-gateway.log
  • Log collection runs in the cleanup defer before
    sandbox.Delete(), ensuring logs are captured on both
    success and failure paths
  • Failures are warned but never block the run (best-effort)

Note: pre-commit could not run due to network restrictions in the sandbox environment (exit code 3, 403 Forbidden on hook installation). The post-script runs an authoritative pre-commit check on the runner.


Closes #276

Post-script verification

  • Branch is not main/master (agent/276-collect-openshell-logs)
  • Secret scan passed (gitleaks — 94f67c43a41ec9974542ec6d6db660282c50120c..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

Add log collection step to the sandbox cleanup flow so that
OpenShell gateway and sandbox logs are preserved in the run
output directory for post-mortem debugging and CI artifact
collection.

Changes:
- sandbox.CollectLogs(): new function that runs
  `openshell logs <name> --source <source> -n 0` with a 30s
  timeout to retrieve all available log lines
- collectOpenshellLogs(): orchestrator that collects both
  sandbox and gateway log sources, writing them to
  <run-dir>/logs/openshell-sandbox.log and
  <run-dir>/logs/openshell-gateway.log
- Log collection runs in the cleanup defer before
  sandbox.Delete(), ensuring logs are captured on both
  success and failure paths
- Failures are warned but never block the run (best-effort)

Note: pre-commit could not run due to network restrictions
in the sandbox environment (exit code 3, 403 Forbidden on
hook installation). The post-script runs an authoritative
pre-commit check on the runner.

Closes #276
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Apr 30, 2026
@github-actions

Copy link
Copy Markdown

Site preview

Preview: https://79d56dfd-site.fullsend-ai.workers.dev

Commit: e46cdea1a43c333950575b590d37e8d5ce99c449

@fullsend-ai-review

Copy link
Copy Markdown

Review: #555

Head SHA: e46cdea
Timestamp: 2026-04-30T00:00:00Z
Outcome: approve

Summary

Clean, well-scoped change that adds best-effort OpenShell log collection to the sandbox cleanup flow, matching the intent and scope of issue #276. The implementation follows existing patterns in the codebase (exec.CommandContext with separate args, timeout handling, warn-but-don't-block error handling). No security, correctness, or injection concerns found.

Findings

Critical

None.

High

None.

Medium

None.

Low

  • [correctness] internal/sandbox/sandbox.go:349CombinedOutput() mixes stdout and stderr, so diagnostic messages from openshell will appear in the collected log files. If clean log separation matters later, consider using separate Stdout/Stderr pipes. Acceptable for a best-effort debugging feature.

Info

  • [style] internal/sandbox/sandbox_test.go:109-121 — Both TestCollectLogs_OpenshellNotInPath and TestCollectLogs_InvalidSource test the same condition (openshell not in PATH). The second test name suggests it tests source validation, but it actually tests the same binary-not-found path. Consider renaming or adding a test that exercises a real invalid-source scenario when openshell is available.

Footer

Outcome: approve
This review applies to SHA e46cdea1a43c333950575b590d37e8d5ce99c449. Any push to the PR head clears this review and requires a new evaluation.

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment above for full details.

@rh-hemartin
rh-hemartin added this pull request to the merge queue May 4, 2026
Merged via the queue into main with commit 4d2a65d May 4, 2026
6 checks passed
@rh-hemartin
rh-hemartin deleted the agent/276-collect-openshell-logs branch May 4, 2026 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-review Triggers review agent dispatch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Collect OpenShell sandbox logs into the run output directory

1 participant