Conversation
…mon tests Snapshot commit so nothing is lost. Authored by the main agent in violation of the delegate-code-writing rule; handed to an implementer for triage of the red gate and to an independent reviewer for acceptance.
…ation once Count max_lines as stderr lines while preserving meaningful restart boundaries.\n\nAdvertise supervisor.stderr_tail in server.describe and its golden mirrors.\n\nKeep truncated text as a prefix, cover real ring-to-wire conversion, and format the workspace.
Enforce max_line_bytes <= max_bytes at construction time, clamping oversized line caps so stderr diagnostics degrade instead of blocking supervisor startup.
…inventing newlines Drain each stderr pump before a replacement boundary, preserving valid partial tails as incomplete captures, forwarding unterminated fragments byte-for-byte, and polling stderr assertions on the conditions they verify.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 15 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/subc-core/src/stderr_tail.rs">
<violation number="1" location="crates/subc-core/src/stderr_tail.rs:370">
P2: When `source.read` fails after a partial line, `pump_stderr_into` drops the already-read fragment before returning. Flush `pending` through `emit_line` before marking the capture incomplete.</violation>
</file>
<file name="crates/subc-core/src/supervise.rs">
<violation number="1" location="crates/subc-core/src/supervise.rs:2722">
P1: When the daemon's stderr is a slow or blocked pipe, this Tokio task performs blocking `write_all`, so a noisy child can block a runtime worker and eventually fill its stderr pipe. Multiple such modules can stall health checks, command handling, and restarts; forward through nonblocking/async I/O or isolate the blocking write with bounded backpressure.</violation>
</file>
You're on the cubic free plan with 12 free PR reviews remaining this month. Upgrade for unlimited reviews.
Re-trigger cubic
| ring.lock() | ||
| .unwrap_or_else(|poisoned| poisoned.into_inner()) | ||
| .push_process_start(); | ||
| Some(tokio::spawn(pump_stderr(stderr, Arc::clone(ring)))) |
There was a problem hiding this comment.
P1: When the daemon's stderr is a slow or blocked pipe, this Tokio task performs blocking write_all, so a noisy child can block a runtime worker and eventually fill its stderr pipe. Multiple such modules can stall health checks, command handling, and restarts; forward through nonblocking/async I/O or isolate the blocking write with bounded backpressure.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/subc-core/src/supervise.rs, line 2722:
<comment>When the daemon's stderr is a slow or blocked pipe, this Tokio task performs blocking `write_all`, so a noisy child can block a runtime worker and eventually fill its stderr pipe. Multiple such modules can stall health checks, command handling, and restarts; forward through nonblocking/async I/O or isolate the blocking write with bounded backpressure.</comment>
<file context>
@@ -2580,35 +2672,74 @@ fn spawn_child(
+ ring.lock()
+ .unwrap_or_else(|poisoned| poisoned.into_inner())
+ .push_process_start();
+ Some(tokio::spawn(pump_stderr(stderr, Arc::clone(ring))))
+ }
+ None => {
</file context>
| S: LineSink, | ||
| { | ||
| { | ||
| let mut guard = lock_ring(&ring); |
There was a problem hiding this comment.
P2: When source.read fails after a partial line, pump_stderr_into drops the already-read fragment before returning. Flush pending through emit_line before marking the capture incomplete.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/subc-core/src/stderr_tail.rs, line 370:
<comment>When `source.read` fails after a partial line, `pump_stderr_into` drops the already-read fragment before returning. Flush `pending` through `emit_line` before marking the capture incomplete.</comment>
<file context>
@@ -0,0 +1,923 @@
+ S: LineSink,
+{
+ {
+ let mut guard = lock_ring(&ring);
+ guard.mark_captured();
+ }
</file context>
Contributor
Author
…erage
The internal CI twin (origin/ci/pr10-stderr-tail) of this PR ran the full
platform matrix: ubuntu green, windows red. Five stderr-tail tests spawned
PathBuf::from("/bin/sh") directly, which resolves to nothing on Windows.
Cargo's fail-fast stopped after the first visible failure, hiding the other
four.
Extends the existing fake-aft-stub rather than #[cfg(unix)]-gating the
tests, so the stderr tail stays verified on the platform whose CRT stdio
behaviour differs most from Linux.
New stub knobs (same const/env-var convention as the ~30 existing ones):
- FAKE_AFT_EXIT_CODE: presence-triggered short-circuit that writes the
configured stderr line and exits with this code BEFORE ever touching
the connection file or dialling subc -- verified the normal connect/
HELLO/register path never runs for these five tests, so a connect
failure can't pollute the very stderr ring under test.
- FAKE_AFT_STDERR_LINE: text to write first, with a {pid} token
substituted with this process's real pid (for the restart-boundary
test, which needs successive generations to be distinguishable).
- FAKE_AFT_ORPHAN_WRITER_DELAY_MS / FAKE_AFT_ORPHAN_WRITER_LINE: makes
the stub re-exec itself (env::current_exe) with a marker env var and
Stdio::inherit() on stderr, then exit without waiting on the child.
Dropping a std::process::Child does not kill it, so the orphan keeps
this process's stderr write-end duplicate open past the parent's own
exit -- reproducing the wedged-pump property the fifth test asserts
on (an old generation's stderr must not cross the restart boundary),
portably, with no shell, no 1933377, no & backgrounding, no sleep binary.
The control.rs unit test (unlike tests/*.rs integration tests) doesn't get
CARGO_BIN_EXE_* from cargo at all; added fake_aft_stub_path() to locate the
sibling binary via current_exe() instead.
Golden fixtures for supervisor.stderr_tail are untouched (verified via
git diff against 9ab5a82). Full subc-core + subc-control suites, clippy,
and fmt all pass; the four stderr-tail supervision tests pass 10/10 in a
tight loop, including the timing-sensitive wedged-pump case.
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Internal twin of #10 (iceteaSA's stderr-tail feature) to run the full CI matrix — fork PRs only get Socket/cubic here, so the Rust legs never ran. Closes after the verdict; the merge happens on #10 itself.
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds a bounded per-module stderr tail retained in
subc-coreand exposes it via a new control op andckverb, so crash causes remain readable even if the daemon log rotated. Previously stderr only went to the daemon’s inherited fd; now each module’s stderr is piped into a ring buffer and still forwarded verbatim to the daemon log.module_idand optionalmax_lines/max_bytes; response returnscapturestate (captured/incomplete/not_captured),entries(lines withtruncatedflag and in-bandprocess_startmarkers), anddropped_lines.ck module stderr <id> [-n <count>]prints the retained tail (with dropped count header, truncation markers, and not-captured/incomplete messages).--jsonreturns raw wire.subc-core/src/stderr_tail.rs; spawn/monitor changes insubc-core/src/supervise.rs; RPC handler insubc-core/src/control.rs; wire types and goldens insubc-control. No migrations required;server.describenow advertises supervisor.stderr_tail.Written for commit 9886d1e. Summary will update on new commits.