Repository navigation
roadmap recut review - #13649
Closed
gunbai-bot[bot] wants to merge 32 commits into
Closed
roadmap recut review#13649gunbai-bot[bot] wants to merge 32 commits into
gunbai-bot[bot] wants to merge 32 commits into
Conversation
…it WitnessBin.Run. The rust handler now drains stderr under the declared WitnessStderrCapturePolicy (concurrent tail or Complete-within-budget) instead of refusing the three accounting channels, which had blocked any floor whose closure reached extdeps.gunbc after #13437. Co-authored-by: Cursor <cursoragent@cursor.com>
… Complete overflow. A missing policy input is not a 16 KiB tail, and a Complete stream past GUNBC_MEMORY_BUDGET_BYTES is not a successful truncated answer (review 77024). Co-authored-by: Cursor <cursoragent@cursor.com>
review 77031 asked for a discriminating RED on BoundedTail and Complete, not a .contains() on the drain string. Co-authored-by: Cursor <cursoragent@cursor.com>
v2 self-compile treated {cause}, {stderr_total_u64} and {max_bytes} as dag variables, which refused the generated lane.
Co-authored-by: Cursor <cursoragent@cursor.com>
…tors. silent-lark-156's NO-LAND on ee1ab5: validate WitnessStderrCapturePolicy before the child runs, drain concurrently with stdin, refuse Complete overflow as a typed error, and roster the seed-growth items pending the exact-scope ruling. Co-authored-by: Cursor <cursoragent@cursor.com>
…v var. review 77056: the interpreter uses read_host_budget_bytes (gunbc.host_budget_source). The emitted path now calls the same function on v1_rt, and the seed wrapper delegates to it so the two cannot diverge on a raw GUNBC_MEMORY_BUDGET_BYTES read. Co-authored-by: Cursor <cursoragent@cursor.com>
DESIGN section 5/6 refuse an author-declared scaffold approval; keep capture-accounting membership in one predicate. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…d emit. Co-authored-by: Cursor <cursoragent@cursor.com>
Inner // annotations refuse at parse (floor on adedd46). Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
… sysctlbyname. Co-authored-by: Cursor <cursoragent@cursor.com>
The seed-growth row now lists the inline-module path and every new fixture_closure_union_tests item. Emit wall and rust renderer share operation_declares_required_stderr_capture_policy so a String, optional, or collection named stderr_capture refuses as TransportEmissionNotModeled. Co-authored-by: Cursor <cursoragent@cursor.com>
The generated job refused because the committed emit, emit_rust, runtime_rust and v1_rt mirrors were not one seed emission of the current .dag sources. Co-authored-by: Cursor <cursoragent@cursor.com>
One remote required-regen loop copied v1_rt.rs from the candidate; the second pass reported first_generation_equal=true. Co-authored-by: Cursor <cursoragent@cursor.com>
A fixture-local WitnessStderrCapturePolicy must refuse as transport not modeled; only std.shell_stream_capture.WitnessStderrCapturePolicy admits the rust shell capture channels. Co-authored-by: Cursor <cursoragent@cursor.com>
Governor REDs now exercise the same walk and v1 parse Complete uses; the memory_governor copies are wrappers, matching Darwin physical. Co-authored-by: Cursor <cursoragent@cursor.com>
Map HostBudgetCgroupV1 at the existing wrapper call sites so the consolidation adds no new hand-Rust item. Co-authored-by: Cursor <cursoragent@cursor.com>
…get observers. required-regen at c764 drifted v1_compiler_emit.rs and rust_runtime_source; v1_rt stays the live observers until the next generation emits from this source. Co-authored-by: Cursor <cursoragent@cursor.com>
Mechanical merge for #13472 queue drop. Stage0 mirrors that both sides changed were re-emitted; emit_host keeps both capture-gap and stderr-capture tests. Co-authored-by: Cursor <cursoragent@cursor.com>
The merge-head seed emits list concat via append; the pre-merge mirror still used extend. Co-authored-by: Cursor <cursoragent@cursor.com>
The capture-gap attribute landed on STDERR_CAPTURE_MEMBER and clippy refused the lint step. Co-authored-by: Cursor <cursoragent@cursor.com>
…ure. After the main merge the floor still required a capture-gap exclusion that this branch closed. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
… channels. The constructed ShellChannelNotRealizedByTarget fact is unchanged; rust_stderr_capture_channel_not_realized_facts is empty because shell_channel_realized_by_target is now true for those channels. Install the required-regen emit_rust mirror after merging main. Co-authored-by: Cursor <cursoragent@cursor.com>
…source. The seed-growth row already scoped the handler; review 77633 read the host-budget join as uncovered hand seed. Co-authored-by: Cursor <cursoragent@cursor.com>
The restoration trigger fired with #13472: empty the exclusion, delete the union strip, and mark the drop Retired so the standing ledger matches the capability. Co-authored-by: Cursor <cursoragent@cursor.com>
…so both modules rust-emit The floor's union emit refuses both with EffectfulSelfRecursionUnrealized (non-tail async recursion has no realization). entry_presence walks ancestors through entry_presence_above, carrying the cause read one level down; walk_cgroup walks a preorder pending frontier through walk_cgroup_pending. Results and refusal arms are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
v1_compiler_emit_rust.rs carries main's side provisionally; regenerated in a following commit. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… into integration/silent-lark-156
briansrls
marked this pull request as ready for review
October 9, 2026 18:22
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 181dd1b91b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
|
|
||
| data shell_capture_drain_start: String = "let mut stdout_pipe = output.stdout.take();\nlet mut stderr_pipe = output.stderr.take();\nlet stdout_thread = std::thread::spawn(move || -> std::io::Result<Vec<u8>> {\n let mut buf = Vec::new();\n if let Some(ref mut reader) = stdout_pipe {\n std::io::Read::read_to_end(reader, &mut buf)?;\n }\n Ok(buf)\n});\nlet stderr_thread = std::thread::spawn(move || -> std::io::Result<(Vec<u8>, u64, bool)> {\n let mut reader = match stderr_pipe {\n Some(r) => r,\n None => return Ok((Vec::new(), 0, false)),\n };\n let mut chunk = [0u8; 65536];\n let mut total: u64 = 0;\n if let Some(max_bytes) = __stderr_complete_limit {\n let mut retained = Vec::new();\n loop {\n let n = std::io::Read::read(&mut reader, &mut chunk)?;\n if n == 0 { break; }\n total += n as u64;\n if total <= max_bytes as u64 { retained.extend_from_slice(&chunk[..n]); }\n }\n let truncated = total > max_bytes as u64;\n return Ok((if truncated { Vec::new() } else { retained }, total, truncated));\n }\n let cap = __stderr_tail_bytes;\n let mut ring = Vec::with_capacity(cap);\n let mut start = 0usize;\n loop {\n let n = std::io::Read::read(&mut reader, &mut chunk)?;\n if n == 0 { break; }\n total += n as u64;\n for &b in &chunk[..n] {\n if cap == 0 { continue; }\n if ring.len() < cap {\n ring.push(b);\n } else {\n ring[start] = b;\n start = (start + 1) % cap;\n }\n }\n }\n let retained = if ring.len() < cap || cap == 0 {\n ring\n } else {\n let mut out = Vec::with_capacity(cap);\n for i in 0..cap { out.push(ring[(start + i) % cap]); }\n out\n };\n let retained_len = retained.len() as u64;\n Ok((retained, total, total > retained_len))\n});\n" |
There was a problem hiding this comment.
When a Rust shell operation requests any stderr accounting field, it enters this branch and drains stdout with read_to_end into an unbounded Vec. The interpreter instead applies default_shell_stdout_capture_policy() with an 8 MiB ceiling and refuses overflow in shell_result_from_capture, so a child producing large or endless stdout can grow the emitted process until OOM and behave differently from interpretation. Drain while counting against the same ceiling and refuse after the stream is drained rather than retaining arbitrary output.
Useful? React with 👍 / 👎.
|
|
||
| data shell_capture_drain_start: String = "let mut stdout_pipe = output.stdout.take();\nlet mut stderr_pipe = output.stderr.take();\nlet stdout_thread = std::thread::spawn(move || -> std::io::Result<Vec<u8>> {\n let mut buf = Vec::new();\n if let Some(ref mut reader) = stdout_pipe {\n std::io::Read::read_to_end(reader, &mut buf)?;\n }\n Ok(buf)\n});\nlet stderr_thread = std::thread::spawn(move || -> std::io::Result<(Vec<u8>, u64, bool)> {\n let mut reader = match stderr_pipe {\n Some(r) => r,\n None => return Ok((Vec::new(), 0, false)),\n };\n let mut chunk = [0u8; 65536];\n let mut total: u64 = 0;\n if let Some(max_bytes) = __stderr_complete_limit {\n let mut retained = Vec::new();\n loop {\n let n = std::io::Read::read(&mut reader, &mut chunk)?;\n if n == 0 { break; }\n total += n as u64;\n if total <= max_bytes as u64 { retained.extend_from_slice(&chunk[..n]); }\n }\n let truncated = total > max_bytes as u64;\n return Ok((if truncated { Vec::new() } else { retained }, total, truncated));\n }\n let cap = __stderr_tail_bytes;\n let mut ring = Vec::with_capacity(cap);\n let mut start = 0usize;\n loop {\n let n = std::io::Read::read(&mut reader, &mut chunk)?;\n if n == 0 { break; }\n total += n as u64;\n for &b in &chunk[..n] {\n if cap == 0 { continue; }\n if ring.len() < cap {\n ring.push(b);\n } else {\n ring[start] = b;\n start = (start + 1) % cap;\n }\n }\n }\n let retained = if ring.len() < cap || cap == 0 {\n ring\n } else {\n let mut out = Vec::with_capacity(cap);\n for i in 0..cap { out.push(ring[(start + i) % cap]); }\n out\n };\n let retained_len = retained.len() as u64;\n Ok((retained, total, total > retained_len))\n});\n" | ||
|
|
||
| data shell_capture_join_project: String = "let status = output.wait()?;\n__stdin_thread.join().map_err(|_| std::io::Error::new(std::io::ErrorKind::Other, \"stdin write thread panicked\"))??;\nlet stdout_bytes = stdout_thread.join().map_err(|_| std::io::Error::new(std::io::ErrorKind::Other, \"stdout drain thread panicked\"))??;\nlet (stderr_bytes, stderr_total_u64, stderr_truncated) = stderr_thread.join().map_err(|_| std::io::Error::new(std::io::ErrorKind::Other, \"stderr drain thread panicked\"))??;\nif let Some(max_bytes) = __stderr_complete_limit {\n if stderr_total_u64 > max_bytes as u64 {\n return Err(format!(\"WitnessStderrCaptureCompleteBudgetExceeded: stderr total {} exceeds host budget {} ({})\", stderr_total_u64, max_bytes, __stderr_complete_source).into());\n }\n}\nlet stderr_total_bytes = stderr_total_u64 as i64;\nlet stderr_retained_bytes = stderr_bytes.len() as i64;\nlet stdout = String::from_utf8_lossy(&stdout_bytes).to_string();\nlet stderr = String::from_utf8_lossy(&stderr_bytes).trim_end().to_string();\nlet output = std::process::Output { status, stdout: stdout_bytes, stderr: stderr_bytes };\n" |
There was a problem hiding this comment.
When a capture-accounting operation supplies stdin and the child exits or closes stdin before consuming the payload, the writer returns BrokenPipe and the second ? turns the invocation into a host error even if the child's declared exit outcome should be authoritative. The interpreter explicitly discards this write result for ordinary POSIX early-close behavior (v1_interpreter.rs lines 18233-18240); this emitted path should likewise report thread panics but ignore the writer's I/O result.
Useful? React with 👍 / 👎.
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
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.
Auto-opened by session-dashboard for session
silent-lark-156.Pushing to
integration/silent-lark-156advances this PR.Worker attestation
Before flipping this PR to ready for review, confirm each item:
npm test,cargo test) and the result.Closes #Ndirective.Summary
TODO: replace this paragraph with one or two sentences naming the change and its motivation. Reviewers read this first.
Test plan