Conversation
|
You have reached your Codex usage limits for security reviews. Please try again later. |
![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: 6bc66dea3f
ℹ️ 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".
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
Comment on lines
+110
to
+124
| # the terminal's foreground-process-group signal reaches the active command | ||
| # without killing this driver, and the evaluating function gives the trap a | ||
| # return boundary that aborts the rest of the operation while preserving state | ||
| # in this Bash process. | ||
| trap 'return 130' INT | ||
| # Testing the call rather than running it bare keeps `errexit` enabled by the | ||
| # operation from exiting this shell before a result is reported, and the | ||
| # status is parked in a positional parameter rather than a local: Bash's | ||
| # dynamic scoping would expose any local to the sourced code, where an | ||
| # ordinary `status=...` would be silently discarded and a `readonly status` | ||
| # would make the bookkeeping assignment fail and kill the driver. | ||
| if awsh_run_operation; then | ||
| set -- "$1" 0 | ||
| else | ||
| set -- "$1" "$?" |
There was a problem hiding this comment.
💡 Edge Case: SIGINT trap only aborts the innermost workload function
The trap 'return 130' INT relies on return unwinding awsh_run_operation, but Bash's return in a signal handler returns only from the innermost executing function. If the interrupt lands while the workload is running inside a function it defined (e.g. w(){ sleep 3; ...; }; w), the trap returns from w, leaving the sourced tail and awsh_run_operation to continue; the operation is not aborted and completed reports status 0 instead of 130. I verified this scoping behavior directly (inner function unwinds, caller continues, status=0). The documented guarantee that the trap "aborts the rest of the operation" therefore holds only for interrupts delivered at the source's top level. For this cooperative, trusted-input prototype the impact is limited, but it's an untested gap worth a note or a regression test.
Was this helpful? React with 👍 / 👎
Owner
Author
|
Formal restack approval for exact head |
This was referenced Aug 23, 2026
| # without killing this driver, and the evaluating function gives the trap a | ||
| # return boundary that aborts the rest of the operation while preserving state | ||
| # in this Bash process. | ||
| trap 'return 130' INT |
There was a problem hiding this comment.
💡 Edge Case: SIGINT after operation, before re-arm, skips completed event
In awsh_execute_operation the trap 'return 130' INT armed at line 114 stays active through the if awsh_run_operation block until it is replaced by trap '' INT at line 131. If a stray Ctrl-C is delivered in the narrow window after awsh_run_operation returns (line 125) but before line 131, the trap fires return 130, so awsh_execute_operation returns without reaching awsh_emit completed at line 132 — the same hung-client failure this PR set out to prevent, just at a different point. The probability is low (no foreground child is running there), but to close it, re-arm trap '' INT immediately after the operation returns (before parking the status) or emit the completed result even on the interrupt path.
Was this helpful? React with 👍 / 👎
Owner
Author
|
PR-cycle state — maintained automatically. Do not edit by hand. {
"approval_evidence": null,
"approval_fingerprint": null,
"approved": false,
"approved_revision_fingerprint": null,
"candidate_revision_fingerprint": null,
"checks": [],
"cycle_closed": false,
"deferrals": [
{
"claim": "Guarantee whole-operation Ctrl-C unwinding through arbitrary nested Bash functions.",
"evidence": "The feasibility prototype will narrow its claim; Bash return from a signal trap unwinds only the innermost active function.",
"id": "review-comment-3819345605",
"owner": "B2 Awsh boundary alignment",
"source": {
"head_sha": "a1009c3b8335a76dfbf0ddeda79231beec7b368d",
"id": 3819345605,
"thread_id": "PRRT_kwDOTOFvvM6at9e1",
"type": "review_comment"
},
"status": "open",
"trigger": "Production Awsh cancellation and control-plane implementation"
},
{
"claim": "Guarantee structured completion across a SIGINT arriving after source return and before idle re-arm.",
"evidence": "The feasibility prototype will disclose the narrow signal window rather than claim production-grade cancellation.",
"id": "review-comment-3838864477",
"owner": "B2 Awsh boundary alignment",
"source": {
"head_sha": "a1009c3b8335a76dfbf0ddeda79231beec7b368d",
"id": 3838864477,
"thread_id": "PRRT_kwDOTOFvvM6bgMiY",
"type": "review_comment"
},
"status": "open",
"trigger": "Production Awsh cancellation and control-plane implementation"
}
],
"findings": [
{
"classification": "additive-delivery",
"disposition_history": [
{
"disposition": "current-slice-defect",
"evidence": "unresolved review thread",
"head_sha": "a1009c3b8335a76dfbf0ddeda79231beec7b368d"
},
{
"disposition": "deferred-to-delivery",
"evidence": "prototype claim narrowed; B2 owns production cancellation",
"head_sha": "be2294ffbc923b872ee9ac6237d21693c9c28cad"
}
],
"id": "review-comment-3819345605",
"invariant": "Ctrl-C must not claim to abort an entire operation when Bash can unwind only the innermost workload function.",
"location": {
"line": 124,
"path": "docs/future/prototype/awsh/awsh-driver.bash"
},
"source": {
"head_sha": "a1009c3b8335a76dfbf0ddeda79231beec7b368d",
"id": 3819345605,
"review_id": 4979961126,
"type": "review_comment"
},
"status": "deferred-to-delivery",
"thread_id": "PRRT_kwDOTOFvvM6at9e1",
"thread_status": "open"
},
{
"classification": "additive-delivery",
"disposition_history": [
{
"disposition": "current-slice-defect",
"evidence": "unresolved review thread",
"head_sha": "a1009c3b8335a76dfbf0ddeda79231beec7b368d"
},
{
"disposition": "deferred-to-delivery",
"evidence": "prototype claim narrowed; B2 owns production cancellation",
"head_sha": "be2294ffbc923b872ee9ac6237d21693c9c28cad"
}
],
"id": "review-comment-3838864477",
"invariant": "A SIGINT arriving after source evaluation must not bypass the structured completion event.",
"location": {
"line": 114,
"path": "docs/future/prototype/awsh/awsh-driver.bash"
},
"source": {
"head_sha": "a1009c3b8335a76dfbf0ddeda79231beec7b368d",
"id": 3838864477,
"review_id": 5002715917,
"type": "review_comment"
},
"status": "deferred-to-delivery",
"thread_id": "PRRT_kwDOTOFvvM6bgMiY",
"thread_status": "open"
},
{
"classification": "current-slice-defect",
"disposition_history": [
{
"disposition": "current-slice-defect",
"evidence": "current-head Codex review",
"head_sha": "a1009c3b8335a76dfbf0ddeda79231beec7b368d"
},
{
"disposition": "addressed-pending-review",
"evidence": "regression coverage passed and review thread resolved",
"head_sha": "be2294ffbc923b872ee9ac6237d21693c9c28cad"
},
{
"disposition": "verified-addressed",
"evidence": "the next current-head Codex review did not repeat the resolved finding",
"head_sha": "be2294ffbc923b872ee9ac6237d21693c9c28cad"
}
],
"id": "review-comment-3846383594",
"invariant": "Workload declarations must not make storage of the next pending source terminate the driver before started.",
"location": {
"line": 106,
"path": "docs/future/prototype/awsh/awsh-driver.bash"
},
"resolution": "The driver now passes request source as a positional argument and reads it byte-exactly through reserved file descriptor 22, so workload declarations cannot make pending-source storage readonly.",
"resulting_head": "be2294ffbc923b872ee9ac6237d21693c9c28cad",
"source": {
"head_sha": "a1009c3b8335a76dfbf0ddeda79231beec7b368d",
"id": 3846383594,
"review_id": 5011315279,
"type": "review_comment"
},
"status": "verified-addressed",
"thread_id": "PRRT_kwDOTOFvvM6bz1gR",
"thread_status": "resolved"
},
{
"classification": "current-slice-defect",
"disposition_history": [
{
"disposition": "current-slice-defect",
"evidence": "current-head Codex review",
"head_sha": "a1009c3b8335a76dfbf0ddeda79231beec7b368d"
},
{
"disposition": "addressed-pending-review",
"evidence": "regression coverage passed and review thread resolved",
"head_sha": "be2294ffbc923b872ee9ac6237d21693c9c28cad"
},
{
"disposition": "verified-addressed",
"evidence": "the next current-head Codex review did not repeat the resolved finding",
"head_sha": "be2294ffbc923b872ee9ac6237d21693c9c28cad"
}
],
"id": "review-comment-3846383600",
"invariant": "Completion framing must not depend on an emitter-local name that workload source can make readonly.",
"location": {
"line": 59,
"path": "docs/future/prototype/awsh/awsh-driver.bash"
},
"resolution": "Completion frames are now emitted by one direct printf with no dynamically scoped emitter-local variable that workload source can make readonly.",
"resulting_head": "be2294ffbc923b872ee9ac6237d21693c9c28cad",
"source": {
"head_sha": "a1009c3b8335a76dfbf0ddeda79231beec7b368d",
"id": 3846383600,
"review_id": 5011315279,
"type": "review_comment"
},
"status": "verified-addressed",
"thread_id": "PRRT_kwDOTOFvvM6bz1gW",
"thread_status": "resolved"
},
{
"classification": "current-slice-defect",
"disposition_history": [
{
"disposition": "current-slice-defect",
"evidence": "current-head Codex review",
"head_sha": "be2294ffbc923b872ee9ac6237d21693c9c28cad"
},
{
"disposition": "addressed-pending-review",
"evidence": "focused regression and repository validation passed; obsolete thread resolved",
"head_sha": "e8e2d7e34116af953aa35a61abf8662829f58519"
},
{
"disposition": "verified-addressed",
"evidence": "the next current-head Codex review did not repeat the resolved parser-frame finding",
"head_sha": "e8e2d7e34116af953aa35a61abf8662829f58519"
}
],
"id": "review-comment-3846711156",
"invariant": "Request-parser bookkeeping must survive workload declarations of parser variable names as readonly.",
"location": {
"line": 152,
"path": "docs/future/prototype/awsh/awsh-driver.bash"
},
"resolution": "Request parsing now runs in a short-lived function frame, so readonly parser names disappear before the next request; a two-request regression covers all four names.",
"resulting_head": "e8e2d7e34116af953aa35a61abf8662829f58519",
"source": {
"head_sha": "be2294ffbc923b872ee9ac6237d21693c9c28cad",
"id": 3846711156,
"review_id": 5011683684,
"type": "review_comment"
},
"status": "verified-addressed",
"thread_id": "PRRT_kwDOTOFvvM6b0rbp",
"thread_status": "resolved"
},
{
"classification": "current-slice-defect",
"disposition_history": [
{
"disposition": "current-slice-defect",
"evidence": "current-head Codex review",
"head_sha": "e8e2d7e34116af953aa35a61abf8662829f58519"
},
{
"disposition": "addressed-pending-review",
"evidence": "focused regression and mutation proof passed; review thread resolved",
"head_sha": "9ee3c3b6899d82d5451d94471b5a13b8385e1e3e"
}
],
"id": "review-comment-3847323540",
"invariant": "Field-reader bookkeeping must survive workload declarations of helper-local names as readonly.",
"location": {
"line": 72,
"path": "docs/future/prototype/awsh/awsh-driver.bash"
},
"resolution": "The field reader now expands the caller positional target directly and introduces no helper-local target name that workload code can make readonly.",
"resulting_head": "9ee3c3b6899d82d5451d94471b5a13b8385e1e3e",
"source": {
"head_sha": "e8e2d7e34116af953aa35a61abf8662829f58519",
"id": 3847323540,
"review_id": 5012391365,
"type": "review_comment"
},
"status": "addressed-pending-review",
"thread_id": "PRRT_kwDOTOFvvM6b2P_E",
"thread_status": "resolved"
},
{
"classification": "current-slice-defect",
"disposition_history": [
{
"disposition": "current-slice-defect",
"evidence": "current-head Codex review",
"head_sha": "e8e2d7e34116af953aa35a61abf8662829f58519"
},
{
"disposition": "addressed-pending-review",
"evidence": "focused regression and mutation proof passed; review thread resolved",
"head_sha": "9ee3c3b6899d82d5451d94471b5a13b8385e1e3e"
}
],
"id": "review-comment-3847323549",
"invariant": "Completion must report the actual shell cwd even when workload code unsets PWD under nounset.",
"location": {
"line": 134,
"path": "docs/future/prototype/awsh/awsh-driver.bash"
},
"resolution": "Completion obtains the cwd from the Bash pwd builtin instead of expanding workload-mutable PWD, including after set -u and unset PWD.",
"resulting_head": "9ee3c3b6899d82d5451d94471b5a13b8385e1e3e",
"source": {
"head_sha": "e8e2d7e34116af953aa35a61abf8662829f58519",
"id": 3847323549,
"review_id": 5012391365,
"type": "review_comment"
},
"status": "addressed-pending-review",
"thread_id": "PRRT_kwDOTOFvvM6b2P_M",
"thread_status": "resolved"
},
{
"classification": "current-slice-defect",
"disposition_history": [
{
"disposition": "current-slice-defect",
"evidence": "current-head Codex review",
"head_sha": "9ee3c3b6899d82d5451d94471b5a13b8385e1e3e"
},
{
"disposition": "addressed-pending-review",
"evidence": "focused regressions, mutation proof, repository validation, and resolved review thread",
"head_sha": "a4218a81b485bd6b153239ef23f4e40cf9ebefec"
}
],
"id": "review-comment-3847490055",
"invariant": "EOF and shutdown closure events must report cwd without depending on workload-mutable PWD.",
"location": {
"line": 154,
"path": "docs/future/prototype/awsh/awsh-driver.bash"
},
"resolution": "EOF and shutdown closure paths now derive cwd through the Bash pwd builtin instead of expanding workload-mutable PWD.",
"resulting_head": "a4218a81b485bd6b153239ef23f4e40cf9ebefec",
"source": {
"head_sha": "9ee3c3b6899d82d5451d94471b5a13b8385e1e3e",
"id": 3847490055,
"review_id": 5012575173,
"type": "review_comment"
},
"status": "addressed-pending-review",
"thread_id": "PRRT_kwDOTOFvvM6b2rNp",
"thread_status": "resolved"
},
{
"classification": "current-slice-defect",
"disposition_history": [
{
"disposition": "current-slice-defect",
"evidence": "current-head Codex review",
"head_sha": "9ee3c3b6899d82d5451d94471b5a13b8385e1e3e"
},
{
"disposition": "addressed-pending-review",
"evidence": "focused regressions, mutation proof, repository validation, and resolved review thread",
"head_sha": "a4218a81b485bd6b153239ef23f4e40cf9ebefec"
}
],
"id": "review-comment-3847490064",
"invariant": "Protocol-error bookkeeping must remain correct after workload code makes matching global names readonly.",
"location": {
"line": 78,
"path": "docs/future/prototype/awsh/awsh-driver.bash"
},
"resolution": "Protocol-error fields now remain in positional parameters, so persisted readonly workload globals cannot forge or block the emitted error.",
"resulting_head": "a4218a81b485bd6b153239ef23f4e40cf9ebefec",
"source": {
"head_sha": "9ee3c3b6899d82d5451d94471b5a13b8385e1e3e",
"id": 3847490064,
"review_id": 5012575173,
"type": "review_comment"
},
"status": "addressed-pending-review",
"thread_id": "PRRT_kwDOTOFvvM6b2rNw",
"thread_status": "resolved"
},
{
"classification": "current-slice-defect",
"disposition_history": [
{
"disposition": "current-slice-defect",
"evidence": "current-head Codex review",
"head_sha": "a4218a81b485bd6b153239ef23f4e40cf9ebefec"
},
{
"disposition": "addressed-pending-review",
"evidence": "focused regressions, mutation proof, repository validation, and resolved review thread",
"head_sha": "b8e2e3c8e4bc29bf5268e505f9ccb22ac18bd612"
}
],
"id": "review-comment-3847597439",
"invariant": "Top-level declarations made by an operation must remain visible to later operations in the persistent Bash session.",
"location": {
"line": 99,
"path": "docs/future/prototype/awsh/awsh-driver.bash"
},
"resolution": "The evaluator now captures declarations still local after source return and promotes them globally, preserving operation-top-level scalar and array declarations while nested-function locals remain scoped.",
"resulting_head": "b8e2e3c8e4bc29bf5268e505f9ccb22ac18bd612",
"severity": "P1",
"source": {
"head_sha": "a4218a81b485bd6b153239ef23f4e40cf9ebefec",
"id": 3847597439,
"review_id": 5012692890,
"type": "review_comment"
},
"status": "addressed-pending-review",
"thread_id": "PRRT_kwDOTOFvvM6b28wj",
"thread_status": "resolved"
},
{
"classification": "current-slice-defect",
"disposition_history": [
{
"disposition": "current-slice-defect",
"evidence": "current-head Codex review",
"head_sha": "a4218a81b485bd6b153239ef23f4e40cf9ebefec"
},
{
"disposition": "addressed-pending-review",
"evidence": "focused regressions, mutation proof, repository validation, and resolved review thread",
"head_sha": "b8e2e3c8e4bc29bf5268e505f9ccb22ac18bd612"
}
],
"id": "review-comment-3847597446",
"invariant": "Request and result descriptors must have one canonical decimal interpretation before reserved-descriptor checks.",
"location": {
"line": 56,
"path": "docs/future/prototype/awsh/awsh-driver.bash"
},
"resolution": "Descriptor arguments now require canonical nonzero decimal spelling, rejecting leading-zero forms before arithmetic and descriptor operations can interpret them differently.",
"resulting_head": "b8e2e3c8e4bc29bf5268e505f9ccb22ac18bd612",
"severity": "P2",
"source": {
"head_sha": "a4218a81b485bd6b153239ef23f4e40cf9ebefec",
"id": 3847597446,
"review_id": 5012692890,
"type": "review_comment"
},
"status": "addressed-pending-review",
"thread_id": "PRRT_kwDOTOFvvM6b28wo",
"thread_status": "resolved"
},
{
"classification": "current-slice-defect",
"disposition_history": [
{
"disposition": "current-slice-defect",
"evidence": "current-head Codex review",
"head_sha": "b8e2e3c8e4bc29bf5268e505f9ccb22ac18bd612"
},
{
"disposition": "addressed-pending-review",
"evidence": "focused regression, mutation proof, repository validation, and resolved review thread",
"head_sha": "e0dde8d683d0aa1fc2ef365a736824630dc11bb3"
}
],
"id": "review-comment-3847678443",
"invariant": "Persisted workload functions must not intercept builtins used for protocol emission or driver bookkeeping.",
"location": {
"line": 69,
"path": "docs/future/prototype/awsh/awsh-driver.bash"
},
"resolution": "Protocol emission and operation-source materialization now invoke Bash's builtin printf explicitly; the #8 broker descendant carries the same protection through every parent-side control write and assignment.",
"resulting_head": "e0dde8d683d0aa1fc2ef365a736824630dc11bb3",
"severity": "P2",
"source": {
"head_sha": "b8e2e3c8e4bc29bf5268e505f9ccb22ac18bd612",
"id": 3847678443,
"review_id": 5012783827,
"type": "review_comment"
},
"status": "addressed-pending-review",
"thread_id": "PRRT_kwDOTOFvvM6b3KE7",
"thread_status": "resolved"
},
{
"classification": "current-slice-defect",
"disposition_history": [
{
"disposition": "current-slice-defect",
"evidence": "current-head Codex review",
"head_sha": "e0dde8d683d0aa1fc2ef365a736824630dc11bb3"
},
{
"disposition": "addressed-pending-review",
"evidence": "focused regressions, mutation proof, repository validation, and resolved review thread",
"head_sha": "b66584c58b05ef01674ec77682348208890c84fa"
}
],
"id": "review-comment-3847776698",
"invariant": "Declaration persistence must never promote workload declarations into the driver-owned awsh_/AWSH_ namespace.",
"location": {
"line": 113,
"path": "docs/future/prototype/awsh/awsh-driver.bash"
},
"resolution": "Declaration promotion now filters the driver-owned awsh_/AWSH_ namespace before global evaluation, so readonly workload declarations cannot poison parser state.",
"resulting_head": "b66584c58b05ef01674ec77682348208890c84fa",
"severity": "P2",
"source": {
"head_sha": "e0dde8d683d0aa1fc2ef365a736824630dc11bb3",
"id": 3847776698,
"review_id": 5012907189,
"type": "review_comment"
},
"status": "addressed-pending-review",
"thread_id": "PRRT_kwDOTOFvvM6b3aK5",
"thread_status": "resolved"
},
{
"classification": "current-slice-defect",
"disposition_history": [
{
"disposition": "current-slice-defect",
"evidence": "current-head Codex review",
"head_sha": "e0dde8d683d0aa1fc2ef365a736824630dc11bb3"
},
{
"disposition": "addressed-pending-review",
"evidence": "focused regressions, mutation proof, repository validation, and resolved review thread",
"head_sha": "b66584c58b05ef01674ec77682348208890c84fa"
}
],
"id": "review-comment-3847776707",
"invariant": "Persisted workload functions named source must not intercept evaluation of later operations.",
"location": {
"line": 102,
"path": "docs/future/prototype/awsh/awsh-driver.bash"
},
"resolution": "Operation evaluation now uses Bash's dot builtin spelling, which cannot be intercepted by a persisted function named source and preserves the tested errexit behavior.",
"resulting_head": "b66584c58b05ef01674ec77682348208890c84fa",
"severity": "P2",
"source": {
"head_sha": "e0dde8d683d0aa1fc2ef365a736824630dc11bb3",
"id": 3847776707,
"review_id": 5012907189,
"type": "review_comment"
},
"status": "addressed-pending-review",
"thread_id": "PRRT_kwDOTOFvvM6b3aK_",
"thread_status": "resolved"
},
{
"classification": "current-slice-defect",
"disposition_history": [
{
"disposition": "current-slice-defect",
"evidence": "current-head Codex review",
"head_sha": "b66584c58b05ef01674ec77682348208890c84fa"
},
{
"disposition": "addressed-pending-review",
"evidence": "focused regressions, mutation proof, repository validation, and resolved review thread",
"head_sha": "03e449afdfdea790a1834177356908682d3f5e6c"
}
],
"id": "review-comment-3847859764",
"invariant": "Workload code must not replace the builtin dispatcher used by driver protocol and bookkeeping paths.",
"location": {
"line": 69,
"path": "docs/future/prototype/awsh/awsh-driver.bash"
},
"resolution": "The driver now preserves the operation status and removes any workload function named builtin before driver bookkeeping resumes, reserving the real Bash dispatcher without adding a scope-changing wrapper.",
"resulting_head": "03e449afdfdea790a1834177356908682d3f5e6c",
"severity": "P2",
"source": {
"head_sha": "b66584c58b05ef01674ec77682348208890c84fa",
"id": 3847859764,
"review_id": 5013005415,
"type": "review_comment"
},
"status": "addressed-pending-review",
"thread_id": "PRRT_kwDOTOFvvM6b3noI",
"thread_status": "resolved"
},
{
"classification": "current-slice-defect",
"disposition_history": [
{
"disposition": "current-slice-defect",
"evidence": "current-head Codex review",
"head_sha": "b66584c58b05ef01674ec77682348208890c84fa"
},
{
"disposition": "addressed-pending-review",
"evidence": "focused regressions, mutation proof, repository validation, and resolved review thread",
"head_sha": "03e449afdfdea790a1834177356908682d3f5e6c"
}
],
"id": "review-comment-3847859769",
"invariant": "Explicit globals in the driver-owned parser namespace must be rejected before another request is consumed.",
"location": {
"line": 113,
"path": "docs/future/prototype/awsh/awsh-driver.bash"
},
"resolution": "After each completed operation, the driver detects explicit global parser names and emits a reserved_namespace protocol error before consuming another request; readonly poison is therefore handled fail-closed rather than misparsed.",
"resulting_head": "03e449afdfdea790a1834177356908682d3f5e6c",
"severity": "P2",
"source": {
"head_sha": "b66584c58b05ef01674ec77682348208890c84fa",
"id": 3847859769,
"review_id": 5013005415,
"type": "review_comment"
},
"status": "addressed-pending-review",
"thread_id": "PRRT_kwDOTOFvvM6b3noK",
"thread_status": "resolved"
},
{
"classification": "current-slice-defect",
"disposition_history": [
{
"disposition": "current-slice-defect",
"evidence": "current-head Codex review",
"head_sha": "03e449afdfdea790a1834177356908682d3f5e6c"
}
],
"id": "review-comment-3847940485",
"invariant": "Persisted workload functions named dot must not replace the evaluator used for later operation source.",
"location": {
"line": 105,
"path": "docs/future/prototype/awsh/awsh-driver.bash"
},
"severity": "P2",
"source": {
"head_sha": "03e449afdfdea790a1834177356908682d3f5e6c",
"id": 3847940485,
"review_id": 5013114139,
"type": "review_comment"
},
"status": "current-slice-defect",
"thread_id": "PRRT_kwDOTOFvvM6b30yK",
"thread_status": "open"
},
{
"classification": "current-slice-defect",
"disposition_history": [
{
"disposition": "current-slice-defect",
"evidence": "current-head Codex review",
"head_sha": "03e449afdfdea790a1834177356908682d3f5e6c"
}
],
"id": "review-comment-3847940492",
"invariant": "Persisted workload functions named exec must not intercept source-descriptor setup or close.",
"location": {
"line": 100,
"path": "docs/future/prototype/awsh/awsh-driver.bash"
},
"severity": "P2",
"source": {
"head_sha": "03e449afdfdea790a1834177356908682d3f5e6c",
"id": 3847940492,
"review_id": 5013114139,
"type": "review_comment"
},
"status": "current-slice-defect",
"thread_id": "PRRT_kwDOTOFvvM6b30yO",
"thread_status": "open"
},
{
"classification": "current-slice-defect",
"disposition_history": [
{
"disposition": "current-slice-defect",
"evidence": "current-head Codex review",
"head_sha": "03e449afdfdea790a1834177356908682d3f5e6c"
}
],
"id": "review-comment-3847940495",
"invariant": "Cleanup of reserved driver primitives must not itself depend on a workload-shadowable unset command.",
"location": {
"line": 114,
"path": "docs/future/prototype/awsh/awsh-driver.bash"
},
"severity": "P2",
"source": {
"head_sha": "03e449afdfdea790a1834177356908682d3f5e6c",
"id": 3847940495,
"review_id": 5013114139,
"type": "review_comment"
},
"status": "current-slice-defect",
"thread_id": "PRRT_kwDOTOFvvM6b30yS",
"thread_status": "open"
},
{
"classification": "current-slice-defect",
"disposition_history": [
{
"disposition": "current-slice-defect",
"evidence": "current-head Codex review",
"head_sha": "03e449afdfdea790a1834177356908682d3f5e6c"
}
],
"id": "review-comment-3847940500",
"invariant": "Persisted workload functions named return must not change the operation status reported by the evaluator.",
"location": {
"line": 124,
"path": "docs/future/prototype/awsh/awsh-driver.bash"
},
"severity": "P2",
"source": {
"head_sha": "03e449afdfdea790a1834177356908682d3f5e6c",
"id": 3847940500,
"review_id": 5013114139,
"type": "review_comment"
},
"status": "current-slice-defect",
"thread_id": "PRRT_kwDOTOFvvM6b30yV",
"thread_status": "open"
}
],
"pr": {
"base_ref": "pr23",
"base_sha": "6d1e566faa80d87d93d7f40131ff8703b42db270",
"body_digest": "sha256:5f31659c1a5acf5fb3b356b5d02e72706de7e24a33ebba36eafc1ea8f49ba03b",
"diff_digest": "sha256:58a82141d17d6ad120e832d591eea76a14990bd0b9393f9a7c07a47aac8a91ba",
"head_ref": "pr17",
"head_sha": "03e449afdfdea790a1834177356908682d3f5e6c",
"pr": 18,
"repository": "omry/omegaflow",
"scope_authority": [
{
"digest": "sha256:bfc323c0c0827033b278185c84d36a50137210a257f5d131d68cfaa2b069bca2",
"path": "docs/.review/BACKLOG.md"
},
{
"digest": "sha256:0f38919d9d8b909b2f5fbd12a3b5371957b1df3c2aaba948e06d958250800a7a",
"path": "docs/.review/runtime-dependencies.md"
},
{
"digest": "sha256:7c8806d142b11af6603dfc4fd1067005090106c97dcd14d773821315bbb2b03b",
"path": "docs/BACKLOG.md"
},
{
"digest": "sha256:a3fd7a68f5fafd31a30c97d7196c02e654cc2bc04736c84321ac210d1a0e0b46",
"path": "docs/design/envoy-protocol-v1.md"
},
{
"digest": "sha256:eb579960ef7ea2b2ba0fe8427fdc47fec93e66c38294d010dfeb99de1565fa0b",
"path": "docs/design/omegaflow-envoy-design.md"
},
{
"digest": "sha256:4ebbefefe348f30161aec8ea4f6a135acb22c170f0a23e658f38875785e3919a",
"path": "docs/design/reploy-environments-design.md"
},
{
"digest": "sha256:dad245428e52256b349c805154d089e3220cb28d87288e881f23c161ba1398f2",
"path": "docs/design/reploy-integration-implementation-plan.md"
},
{
"digest": "sha256:ea989e6cff43e71b8346f226d1eb89db4d246006ce57288ce393928d86f2a349",
"path": "docs/runtime-dependencies.md"
}
],
"title_digest": "sha256:e3807715e16439ad3e1760972ab4d4e812c807d26db60ff94cdd6b20b7790533"
},
"proposed_fix": {
"footprint": [
"choose a reserved workload-function contract or isolate driver control from the workload function namespace"
],
"head_sha": "03e449afdfdea790a1834177356908682d3f5e6c",
"status": "needs-design-direction"
},
"record_version": 27,
"review_request": {
"created_at": "2026-08-24T22:17:19Z",
"head_sha": "03e449afdfdea790a1834177356908682d3f5e6c",
"id": 5402188141
},
"review_result": {
"body_digest": "sha256:eb5b9c33d4c216b480fc74b889db179953f23309c387fad9ce36c33065d59d3f",
"head_sha": "03e449afdfdea790a1834177356908682d3f5e6c",
"id": 5013114139,
"state": "COMMENTED",
"submitted_at": "2026-08-24T22:20:20Z"
},
"revision_fingerprint": "sha256:f309f946db3ec1b35d7de8377b31aa16ff3fb9c0c12e7b0db2053232e7a6e836",
"rounds": [
{
"applied_fix_footprint": [
"docs/future/prototype/awsh/README.md",
"docs/future/prototype/awsh/awsh",
"docs/future/prototype/awsh/awsh-driver.bash",
"docs/future/prototype/awsh/test_awsh.py"
],
"checks": [
"20 focused Awsh tests passed",
"mutation checks proved all four added regression tests exercise their fixes",
"repository CI passed: 998 tests plus schema-doc and Towncrier checks",
"three existing design attestations were unaffected and no stamp changed",
"native stack #26 synchronized with unchanged metadata and an idempotent no-op recheck",
"two fixed review threads resolved and two B2 delivery-deferral threads left open"
],
"effective_diff_digest": "sha256:c34f3b8b6f6c5f8cc9937ca0ace3787e1654eeaa56851ff7a4acbd74f3679458",
"finding_ids": [
"review-comment-3819345605",
"review-comment-3838864477",
"review-comment-3846383594",
"review-comment-3846383600"
],
"open_delivery_deferrals": 2,
"outcome": "findings",
"proposed_fix_footprint": [
"isolate source handoff from workload declarations",
"emit completion frames without dynamically scoped scratch state",
"reserve the source-handoff descriptor",
"narrow prototype cancellation claims and defer production semantics to B2"
],
"resulting_head": "be2294ffbc923b872ee9ac6237d21693c9c28cad",
"review_request_id": 5399638509,
"review_result_id": 5011315279,
"reviewed_head": "a1009c3b8335a76dfbf0ddeda79231beec7b368d",
"root_cause": "The prototype exposed dynamically scoped driver names to sourced workload code and described Ctrl-C behavior more strongly than Bash trap unwinding can guarantee.",
"round": 1,
"unresolved_current_slice_findings": 0,
"unresolved_design_blockers": 0
},
{
"applied_fix_footprint": [
"docs/future/prototype/awsh/awsh-driver.bash",
"docs/future/prototype/awsh/test_awsh.py"
],
"checks": [
"focused readonly-parser regression passed",
"mutation coverage proved the regression fails before the fix and passes after it",
"repository validation passed",
"current published head CI is green",
"existing design attestations were unaffected and no stamp changed",
"native stack synchronization published the exact rewritten head without metadata drift",
"the fixed parser thread was resolved and two B2 delivery-deferral threads remain open"
],
"effective_diff_digest": "sha256:51c1bc8f6a3daf4bad6bb0685cbbf509055eae8fab54cb8ecef58a200f07ed6d",
"finding_ids": [
"review-comment-3846711156"
],
"open_delivery_deferrals": 2,
"outcome": "findings",
"proposed_fix_footprint": [
"parse each request in a disposable function frame",
"add a two-request readonly parser-name regression"
],
"resulting_head": "e8e2d7e34116af953aa35a61abf8662829f58519",
"review_request_id": 5400117216,
"review_result_id": 5011683684,
"reviewed_head": "be2294ffbc923b872ee9ac6237d21693c9c28cad",
"root_cause": "Long-lived request-parser variables remained dynamically visible to sourced workload code and could be made readonly before the next request.",
"round": 2,
"unresolved_current_slice_findings": 0,
"unresolved_design_blockers": 0
},
{
"applied_fix_footprint": [
"docs/future/prototype/awsh/awsh-driver.bash",
"docs/future/prototype/awsh/test_awsh.py"
],
"checks": [
"23 focused Awsh tests passed at PR 18",
"33 Awsh tests passed at the rewritten stack tip",
"both new regressions failed against the pre-fix implementation and passed after restoration",
"repository CI passed: 998 tests plus schema-doc and Towncrier checks",
"existing design attestations were unaffected and no stamp changed",
"native stack 26 synchronized five predecessor heads with unchanged metadata and topology",
"the two addressed review threads were resolved and the two B2 delivery-deferral threads remain open"
],
"effective_diff_digest": "sha256:ff64fb847c0ef38d710454a814cc577af2c7b724233a76251408ed54060e0333",
"finding_ids": [
"review-comment-3847323540",
"review-comment-3847323549"
],
"open_delivery_deferrals": 2,
"outcome": "findings",
"proposed_fix_footprint": [
"remove the field reader helper-local target",
"derive completion cwd without PWD"
],
"resulting_head": "9ee3c3b6899d82d5451d94471b5a13b8385e1e3e",
"review_request_id": 5401103901,
"review_result_id": 5012391365,
"reviewed_head": "e8e2d7e34116af953aa35a61abf8662829f58519",
"root_cause": "Two remaining completion-path assumptions exposed driver bookkeeping to persisted workload namespace and shell-state mutations.",
"round": 3,
"unresolved_current_slice_findings": 0,
"unresolved_design_blockers": 0
},
{
"applied_fix_footprint": [
"docs/future/prototype/awsh/awsh-driver.bash",
"docs/future/prototype/awsh/test_awsh.py"
],
"checks": [
"26 focused Awsh tests passed at PR 18",
"35 Awsh tests passed at the rewritten stack tip",
"all three new regressions failed against the pre-fix implementations and passed after restoration",
"repository CI passed: 998 tests plus schema-doc and Towncrier checks",
"existing design attestations were unaffected and no stamp changed",
"all five rewritten PR heads were synchronized with unchanged stack topology, titles, bases, and readiness",
"the two addressed review threads were resolved and the two B2 delivery-deferral threads remain open"
],
"effective_diff_digest": "sha256:0b4133276c95edb56583b60986d018dddda2d0adcee5bd90d87ef29b2189d8a4",
"finding_ids": [
"review-comment-3847490055",
"review-comment-3847490064"
],
"open_delivery_deferrals": 2,
"outcome": "findings",
"proposed_fix_footprint": [
"derive EOF and shutdown cwd through the Bash pwd builtin",
"remove named local protocol-error fields from the workload namespace",
"add focused persistent-state regressions"
],
"resulting_head": "a4218a81b485bd6b153239ef23f4e40cf9ebefec",
"review_request_id": 5401379114,
"review_result_id": 5012575173,
"reviewed_head": "9ee3c3b6899d82d5451d94471b5a13b8385e1e3e",
"root_cause": "Two closure and error paths still trusted persistent workload-controlled shell names after completion had already been hardened.",
"round": 4,
"unresolved_current_slice_findings": 0,
"unresolved_design_blockers": 0
},
{
"applied_fix_footprint": [
"docs/future/prototype/awsh/awsh-driver.bash",
"docs/future/prototype/awsh/test_awsh.py"
],
"checks": [
"29 focused Awsh tests passed at PR 18",
"38 Awsh tests passed at the rewritten stack tip",
"the declaration regression and both leading-zero descriptor cases failed against the pre-fix implementations and passed after restoration",
"repository CI passed: 998 tests plus schema-doc and Towncrier checks",
"existing design attestations were unaffected and no stamp changed",
"all five rewritten PR heads were synchronized with unchanged stack topology, titles, bases, and readiness",
"the two addressed review threads were resolved and the two B2 delivery-deferral threads remain open"
],
"effective_diff_digest": "sha256:a30790a70dd6c19e3aa659690437c216eeaeb9575fa43e6842631065a1ebd623",
"finding_ids": [
"review-comment-3847597439",
"review-comment-3847597446"
],
"open_delivery_deferrals": 2,
"outcome": "findings",
"proposed_fix_footprint": [
"promote only operation-top-level declarations out of the evaluator frame",
"reject non-canonical decimal descriptor strings",
"add focused persistence and descriptor regressions"
],
"resulting_head": "b8e2e3c8e4bc29bf5268e505f9ccb22ac18bd612",
"review_request_id": 5401539565,
"review_result_id": 5012692890,
"reviewed_head": "a4218a81b485bd6b153239ef23f4e40cf9ebefec",
"root_cause": "The evaluator function contained loop control but unintentionally changed top-level declaration scope, while descriptor validation admitted strings with conflicting decimal and octal interpretations.",
"round": 5,
"unresolved_current_slice_findings": 0,
"unresolved_design_blockers": 0
},
{
"applied_fix_footprint": [
"docs/future/prototype/awsh/awsh-driver.bash",
"docs/future/prototype/awsh/test_awsh.py"
],
"checks": [
"30 focused Awsh tests passed at PR 18",
"39 Awsh tests passed at the rewritten stack tip",
"the persisted-printf regression timed out against the pre-fix implementation and passed after restoration",
"repository CI passed: 998 tests plus schema-doc and Towncrier checks",
"existing design attestations were unaffected and no stamp changed",
"all five rewritten PR heads were synchronized with unchanged stack topology, titles, bases, and readiness",
"the addressed review thread was resolved and the two B2 delivery-deferral threads remain open"
],
"effective_diff_digest": "sha256:f966b57f20a8c736e6d0663be041994243e964271b2d3b1c3f902b37920976ec",
"finding_ids": [
"review-comment-3847678443"
],
"open_delivery_deferrals": 2,
"outcome": "findings",
"proposed_fix_footprint": [
"invoke Bash's builtin printf for protocol emission and source handoff",
"add a persisted printf-function regression"
],
"resulting_head": "e0dde8d683d0aa1fc2ef365a736824630dc11bb3",
"review_request_id": 5401668306,
"review_result_id": 5012783827,
"reviewed_head": "b8e2e3c8e4bc29bf5268e505f9ccb22ac18bd612",
"root_cause": "Protocol emission and source handoff invoked printf by normal command lookup even though workload function definitions intentionally persist.",
"round": 6,
"unresolved_current_slice_findings": 0,
"unresolved_design_blockers": 0
},
{
"applied_fix_footprint": [
"docs/future/prototype/awsh/awsh-driver.bash",
"docs/future/prototype/awsh/test_awsh.py"
],
"checks": [
"32 focused Awsh tests passed at PR 18",
"41 Awsh tests passed at the rewritten stack tip",
"both new regressions failed against the pre-fix implementations and passed after restoration",
"repository CI passed: 998 tests plus schema-doc and Towncrier checks",
"existing design attestations were unaffected and no stamp changed",
"the rewritten stack was synchronized without metadata or topology changes",
"both addressed threads were resolved; two B2 delivery deferrals remain open"
],
"effective_diff_digest": "sha256:a5b0b265c0dd1279b7928ab5351b471da572844ac9e563915836a2749da01ca0",
"finding_ids": [
"review-comment-3847776698",
"review-comment-3847776707"
],
"open_delivery_deferrals": 2,
"outcome": "findings",
"proposed_fix_footprint": [
"filter awsh_/AWSH_ declarations before promotion",
"evaluate through the dot builtin",
"add namespace and source-shadowing regressions"
],
"resulting_head": "b66584c58b05ef01674ec77682348208890c84fa",
"review_request_id": 5401830334,
"review_result_id": 5012907189,
"reviewed_head": "e0dde8d683d0aa1fc2ef365a736824630dc11bb3",
"root_cause": "Declaration promotion did not exclude the driver's reserved namespace, and source evaluation still used an interceptable compatibility name.",
"round": 7,
"unresolved_current_slice_findings": 0,
"unresolved_design_blockers": 0
},
{
"applied_fix_footprint": [
"docs/future/prototype/awsh/awsh-driver.bash",
"docs/future/prototype/awsh/test_awsh.py"
],
"checks": [
"34 focused Awsh tests passed at PR 18",
"43 Awsh tests passed at stack tip",
"both new regressions failed without their guards and passed after restoration",
"repository CI passed: 998 tests plus schema-doc and Towncrier checks",
"existing design attestations were unaffected and no stamp changed",
"the rewritten stack was synchronized without metadata or topology changes",
"both addressed threads were resolved; two B2 delivery deferrals remain open"
],
"effective_diff_digest": "sha256:58a82141d17d6ad120e832d591eea76a14990bd0b9393f9a7c07a47aac8a91ba",
"finding_ids": [
"review-comment-3847859764",
"review-comment-3847859769"
],
"open_delivery_deferrals": 2,
"outcome": "findings",
"proposed_fix_footprint": [
"reserve builtin by removing a workload function before bookkeeping",
"fail closed after explicit global parser poisoning",
"add focused regressions"
],
"resulting_head": "03e449afdfdea790a1834177356908682d3f5e6c",
"review_request_id": 5401979901,
"review_result_id": 5013005415,
"reviewed_head": "b66584c58b05ef01674ec77682348208890c84fa",
"root_cause": "The dispatcher escape hatch remained function-shadowable, while explicit global declarations bypassed local promotion filtering and were not checked before the next request.",
"round": 8,
"unresolved_current_slice_findings": 0,
"unresolved_design_blockers": 0
}
],
"schema": "awd:swe:pr-cycle-state",
"version": 1
} |
Owner
Author
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1009c3b83
ℹ️ 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".
Owner
Author
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be2294ffbc
ℹ️ 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".
Owner
Author
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8e2d7e341
ℹ️ 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".
Owner
Author
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ee3c3b689
ℹ️ 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".
Owner
Author
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4218a81b4
ℹ️ 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".
Owner
Author
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8e2e3c8e4
ℹ️ 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".
Owner
Author
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0dde8d683
ℹ️ 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".
Owner
Author
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b66584c58b
ℹ️ 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".
Owner
Author
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03e449afdf
ℹ️ 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".
| # Use the dot builtin spelling so a persisted function named `source` cannot | ||
| # intercept evaluation. `builtin source` is not equivalent here: sourced | ||
| # `set -e` can trigger Bash's pop_var_context failure in this function frame. | ||
| . /dev/fd/22 |
There was a problem hiding this comment.
When an operation defines .() { :; }, Bash persists that function and the next request resolves this command to it instead of the dot builtin. The driver then emits a successful completed event without evaluating any of the supplied source, silently corrupting experiment results; invoke the evaluator in a way that cannot be intercepted by a workload function.
Useful? React with 👍 / 👎.
| # function's positional parameters. This keeps the source out of the | ||
| # workload's `$@` without parking it in a shell variable that the preceding | ||
| # operation could make readonly. | ||
| exec 22< <(builtin printf '%s' "$1") |
There was a problem hiding this comment.
When an operation defines exec() { :; }, it intercepts the descriptor close later in this invocation and the descriptor setup on the next request. Descriptor 22 therefore remains at EOF, so the next operation is reported as successfully completed without executing its source. Ensure this control-plane descriptor manipulation cannot resolve through persisted workload functions.
Useful? React with 👍 / 👎.
| set -- "$?" | ||
| # `builtin` is a reserved driver primitive. Remove a workload function with | ||
| # that name before any driver call needs the real Bash dispatcher. | ||
| unset -f builtin 2>/dev/null || true |
There was a problem hiding this comment.
When one operation defines both unset() { :; } and builtin() { :; }, the workload's unset intercepts this cleanup, leaving the fake builtin installed; the subsequent completion emit is then swallowed and the client waits indefinitely. Fresh evidence in this revision is that the new cleanup for the previously reported builtin collision is itself dispatched through a workload-shadowable command.
Useful? React with 👍 / 👎.
| set -- "$1" "${2//$'\n'declare /$'\n'declare -g }" | ||
| builtin eval "$2" | ||
| fi | ||
| return "$1" |
There was a problem hiding this comment.
When an operation defines return() { :; }, this command invokes that persisted function rather than Bash's return builtin. The evaluator consequently falls through and returns the fake function's zero status, so later failing operations such as false are reported as successful completions, corrupting the recorded status.
Useful? React with 👍 / 👎.
Introduce the Awsh Bash feasibility prototype as its own reviewable commit rather than embedding it in a design-document slice. The POSIX `sh` launcher replaces itself with one selected Bash; the Bash-resident driver executes sequential source in one persistent shell, keeps terminal I/O on the PTY, and reports structured status and cwd over a separate descriptor. Harden the prototype's cooperative boundary: - contain top-level `break` and `continue` inside the evaluator; - keep operation identity, status, field-reader targets, and protocol-error fields out of Bash's dynamically scoped workload namespace; - keep protocol emission and source handoff on Bash's builtin `printf` even when a workload persists a function with that name; - reserve the `builtin` dispatcher itself by removing a workload function with that name before driver bookkeeping resumes; - evaluate operation text through the dot builtin so a persisted function named `source` cannot intercept later operations; - preserve top-level `declare` and `typeset` state across operations while keeping declarations inside workload-defined functions local and excluding the driver-owned `awsh_`/`AWSH_` namespace; - fail closed after an operation explicitly creates global parser state in the reserved driver namespace, before another request is consumed; - clear workload positional arguments before evaluation; - carry byte-exact pending source on reserved descriptor 22 rather than in a workload-visible variable; - emit complete frames without a dynamically scoped loop variable that a workload can make readonly; - parse each request in a short-lived function frame so readonly workload declarations cannot poison the parser state used by the next request; - derive completion and closure cwd from Bash itself rather than workload- mutable `PWD`; and - reject non-canonical descriptor spellings before decimal and arithmetic interpretations can disagree. The README distinguishes this compact prototype grammar from the frozen private `awsh-v1` protocol. It also limits the direct Ctrl-C claim to the tested top-level source shape and records nested-function and post-source signal windows for delivery slice B2's production cancellation boundary. Validation: - 34 Awsh prototype tests at this commit; - mutation checks proving the namespace, descriptor, byte-preservation, cwd, and protocol-error regressions fail when their corresponding hardening is removed; and - full repository CI: 998 tests, schema documentation check, and Towncrier draft validation.
Code Review 👍 Approved with suggestions 0 resolved / 2 findingsAdds the Awsh Bash feasibility prototype with POSIX shell launching and structured PTY communication. Consider adjusting the SIGINT trap handling to ensure correct unwinding and event delivery between operations. 💡 Edge Case: SIGINT trap only aborts the innermost workload function📄 docs/future/prototype/awsh/awsh-driver.bash:110-124 The 💡 Edge Case: SIGINT after operation, before re-arm, skips completed event📄 docs/future/prototype/awsh/awsh-driver.bash:114 📄 docs/future/prototype/awsh/awsh-driver.bash:121-132 In awsh_execute_operation the 🤖 Prompt for agentsOptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
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
1 participant
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.
Introduce the Awsh Bash feasibility prototype as its own reviewable commit
rather than embedding it in a design-document slice. The POSIX
shlauncherreplaces itself with one selected Bash; the Bash-resident driver executes
sequential source in one persistent shell, keeps terminal I/O on the PTY, and
reports structured status and cwd over a separate descriptor.
Harden the prototype's cooperative boundary:
breakandcontinueinside the evaluator;fields out of Bash's dynamically scoped workload namespace;
printfevenwhen a workload persists a function with that name;
builtindispatcher itself by removing a workload function withthat name before driver bookkeeping resumes;
named
sourcecannot intercept later operations;declareandtypesetstate across operations whilekeeping declarations inside workload-defined functions local and excluding
the driver-owned
awsh_/AWSH_namespace;reserved driver namespace, before another request is consumed;
workload-visible variable;
workload can make readonly;
declarations cannot poison the parser state used by the next request;
mutable
PWD; andinterpretations can disagree.
The README distinguishes this compact prototype grammar from the frozen
private
awsh-v1protocol. It also limits the direct Ctrl-C claim to the testedtop-level source shape and records nested-function and post-source signal
windows for delivery slice B2's production cancellation boundary.
Validation:
and protocol-error regressions fail when their corresponding hardening is
removed; and
draft validation.