Skip to content

feat(awk): implement phase 4 investigation features#245

Open
matt-dz wants to merge 44 commits into
codex/awk-phase-3from
codex/awk-phase-4
Open

feat(awk): implement phase 4 investigation features#245
matt-dz wants to merge 44 commits into
codex/awk-phase-3from
codex/awk-phase-4

Conversation

@matt-dz
Copy link
Copy Markdown
Collaborator

@matt-dz matt-dz commented May 8, 2026

Summary

Completes the practical awk phase 4 delta on top of phase 3 by adding the last investigation-focused pieces instead of broad GNU awk parity. This phase adds user-defined functions, substitution and matching helpers, composite-key polish, getline input streams, and rshell-interpreted command pipes while tightening parse-time validation and help text around intentionally unsupported awk surfaces.

  • Adds sub, gsub, match, RSTART/RLENGTH, sprintf, ternary expressions, and exit [code] END-rule handling.
  • Adds user-defined functions with scalar and array parameters, recursion, return, and reserved-name/function-call validation.
  • Adds controlled command pipes plus getline, getline var, getline var < file, and "cmd" | getline var through rshell sandbox policy.
  • Expands enabled upstream awk coverage and stabilizes rewritten harness cases for byte regexes, pipe semantics, and getline behavior.
  • Clarifies awk --help for supported command pipes, rshell sandbox execution, and unsupported awk file-output redirection.

Validation

  • make fmt
  • go test ./analysis ./builtins/awk ./builtins/tests/awk ./tests
  • make test_against_gawk
  • make test_awk_rewritten
  • go test -race -v -timeout 10m ./...
  • Codex review loop clear on latest head 632d9c79

@matt-dz matt-dz changed the base branch from codex/awk-phase-3 to main May 8, 2026 17:33
Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 8, 2026

@codex please review this PR. Focus on actionable correctness, security, regression-risk, and missing-test issues for the phase 4 awk implementation. If everything looks good, please say so explicitly.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ec88f3c4ac

ℹ️ 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".

Comment thread builtins/awk/eval.go Outdated
Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 8, 2026

@codex please review the latest commit ba15f4d1 after the gsub anchor fix. If everything looks good now, please say so explicitly.

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

ℹ️ 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".

Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 8, 2026

@codex review please run another pass on the latest commit ba15f4d1. Please focus especially on phase 4 awk edge cases: regex substitution/match semantics, zero-width matches, command output pipes, user-defined functions and local scope, array/composite-key behavior, exit/END status handling, and missing regression coverage. If everything still looks good, please say so explicitly.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ba15f4d104

ℹ️ 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".

Comment thread builtins/awk/lexer.go
Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 8, 2026

@codex review please re-check the latest commit 2615aee2 after the awk octal string escape fix. Please focus on any remaining actionable correctness, security, regression-risk, or missing-test issues in the phase 4 awk implementation. If everything looks good now, please say so explicitly.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2615aee24c

ℹ️ 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".

Comment thread builtins/awk/runtime.go
Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 8, 2026

@codex review please re-check the latest commit 9d527e48 after the awk regex octal escape fix. Please focus on any remaining actionable correctness, security, regression-risk, or missing-test issues in the phase 4 awk implementation. If everything looks good now, please say so explicitly.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9d527e4843

ℹ️ 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".

Comment thread builtins/awk/runtime.go
Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 8, 2026

@codex review please re-check the latest commit d1019232 after the awk high-bit regex octal byte fix. Please focus on any remaining actionable correctness, security, regression-risk, or missing-test issues in the phase 4 awk implementation. If everything looks good now, please say so explicitly.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d1019232d9

ℹ️ 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".

Comment thread builtins/awk/runtime.go
Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 8, 2026

@codex review please re-check the latest commit 86d1688c after the dynamic awk regex high-bit byte fix. Please focus on any remaining actionable correctness, security, regression-risk, or missing-test issues in the phase 4 awk implementation. If everything looks good now, please say so explicitly.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 86d1688cfb

ℹ️ 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".

Comment thread builtins/awk/runtime.go
Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 8, 2026

@codex review please re-check the latest commit b9f55063 after the mixed UTF-8 plus high-bit byte awk regex fix. Please focus on any remaining actionable correctness, security, regression-risk, or missing-test issues in the phase 4 awk implementation. If everything looks good now, please say so explicitly.

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

ℹ️ 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".

@matt-dz matt-dz changed the base branch from main to codex/awk-phase-3 May 8, 2026 20:49
Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 8, 2026

@codex review please run a fresh pass now that this PR is based on codex/awk-phase-3 and the diff is focused on phase 4 only. Please focus on actionable correctness, security, regression-risk, and missing-test issues in the phase 4 awk implementation: substitutions/match semantics, regex byte and UTF-8 behavior, command pipes, user-defined functions and scoping, composite array keys, exit/END status handling, and enabled gawk/BWK coverage. If everything looks good, please say so explicitly.

Reviewed head: b9f55063.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b9f55063c5

ℹ️ 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".

Comment thread builtins/awk/eval.go Outdated
Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 8, 2026

@codex review please re-check the latest commit 3b5a21d1 after the byte-mode match() offset fix. Please focus on any remaining actionable correctness, security, regression-risk, or missing-test issues in the phase 4 awk implementation. If everything looks good now, please say so explicitly.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3b5a21d195

ℹ️ 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".

Comment thread builtins/awk/runtime.go
Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 8, 2026

@codex review please re-check the latest commit 3b5a21d1 after the ASCII octal regex metacharacter thread was answered with pinned GNU awk 5.4.0 oracle evidence and resolved as invalid. Please focus on any remaining actionable correctness, security, regression-risk, or missing-test issues in the phase 4 awk implementation. If everything looks good now, please say so explicitly.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3b5a21d195

ℹ️ 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".

Comment thread builtins/awk/runtime.go
Comment thread builtins/awk/runtime.go Outdated
Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 8, 2026

@codex please review the latest phase 4 changes on head 2cb1911d. I addressed the valid function-parameter alias and implicit pipe-close comments with regressions, and the branch remains based on codex/awk-phase-3.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2cb1911d84

ℹ️ 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".

Comment thread builtins/awk/parser.go Outdated
Comment thread builtins/awk/runtime.go
Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 8, 2026

@codex review please re-check the latest commit a80598bd after the ternary else-arm assignment and shared parameter scalar/array typing fixes. Please focus on any remaining actionable correctness, security, regression-risk, or missing-test issues in the phase 4 awk implementation. If everything looks good now, please say so explicitly.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a80598bdc1

ℹ️ 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".

Comment thread builtins/awk/runtime.go
Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 8, 2026

@codex review please re-check the latest commit 04e1b97c after the function-parameter scalar-read typing fix. Please focus on any remaining actionable correctness, security, regression-risk, or missing-test issues in the phase 4 awk implementation. If everything looks good now, please say so explicitly.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 04e1b97cb2

ℹ️ 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".

Comment thread builtins/awk/runtime.go
b.WriteByte('}')
return
}
b.WriteByte(value)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Escape octal regex bytes that are regexp metacharacters

When an awk regex contains an octal escape for a regexp metacharacter, this writes the decoded ASCII byte directly into the Go regexp pattern. For inputs such as awk 'BEGIN { print "*" ~ /\52/ }' or /\134/, gawk treats the octal escape as the literal character, but this normalizes to * or a dangling \, causing a regexp compile error or different matching semantics. The byte escape path should quote regexp metacharacters when the octal escape is meant to denote a literal byte.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I validated this against the pinned GNU awk oracle and this claim does not appear to be valid for the cited cases. GNU awk 5.4.0 does not treat octal escapes for regexp metacharacters as quoted literals here:

  • gawk 'BEGIN { print "*" ~ /\52/ }' fails with ? * + or {interval} not preceded by valid subpattern: /\52/
  • gawk --posix 'BEGIN { print "*" ~ /\52/ }' fails the same way
  • gawk 'BEGIN { print "\\" ~ /\134/ }' fails with invalid trailing backslash: /\134/
  • gawk 'BEGIN { print "x" ~ /\56/ }' prints 1, which shows the decoded . is acting as a regexp metacharacter, not as a quoted literal dot

rshell currently matches that behavior for the cited \52 case by surfacing a regexp compile error, so I did not change code for this thread.

Comment thread builtins/awk/eval.go Outdated
if len(e.args) == 0 {
return numberValue(float64(len([]rune(rt.field(0).String())))), nil
}
if arg, ok := e.args[0].(*varExpr); ok && rt.isArray(arg.name) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Materialize ENVIRON before length(array) detection

This only treats a bare argument as an array when rt.isArray is already true, but built-in arrays like ENVIRON are lazy and are not present in rt.arrays until ensureEnviron runs. As a result, awk 'BEGIN { print length(ENVIRON) }' falls through to scalar evaluation and errors with cannot use array ENVIRON as scalar, even though the implementation supports both length(array) and ENVIRON; ensure built-in arrays are considered here before falling back to scalar length.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Validated against the pinned GNU awk oracle; this was real. length(ENVIRON) should materialize the lazy built-in array before deciding whether the bare argument is an array, while rshell was falling through to scalar evaluation and erroring.

Fixed in 16f6496a by materializing built-in arrays before the length(var) array check. Added a gawk-oracle regression in tests/scenarios/cmd/awk/basic/environ_numeric_string.yaml that checks length(ENVIRON) without depending on the exact environment size.

Validation run:

  • make fmt
  • direct repro via go run ./cmd/rshell --allow-all-commands -c ...
  • go test ./builtins/awk ./builtins/tests/awk
  • focused TestShellScenariosAgainstGawk/cmd/awk/basic/environ_numeric_string
  • make test_against_gawk
  • make test_awk_rewritten
  • go test ./... outside the sandbox after the sandboxed run hit unrelated macOS ps/ss permission failures

Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 14, 2026

@codex please re-check PR #245 from latest head commit 3182fdb586690240dbeff137c923958b361be158 after the empty-stdout output-pipe flush fix.

The latest update keeps awk output command pipes open across printf "" while still flushing before stdout writes that emit bytes, with regression coverage for the wc -l case and the prior observable ordering case. Please focus on actionable correctness, security/sandboxing, regressions, or missing tests in the phase 4 delta against codex/awk-phase-3. If everything looks good, please say so explicitly.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3182fdb586

ℹ️ 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".

Comment thread builtins/awk/runtime.go Outdated
Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 14, 2026

@codex please re-check PR #245 from latest head commit e8540cc7619d100b1545f40e2645eefa00743d6a after the analyzer allowlist fix.

The latest update removes the disallowed context.Background() fallback from the awk output-pipe stdout flush path, keeping it on the active awk execution context. Please focus on actionable correctness, security/sandboxing, regressions, CI failures, or missing tests in the phase 4 delta against codex/awk-phase-3. If everything looks good, please say so explicitly.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e8540cc761

ℹ️ 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".

Comment thread builtins/awk/runtime.go Outdated
Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 14, 2026

@codex please re-check PR #245 from latest head commit 00b2733666657d3394dbfd61657f3ac88de3e29d after the output-pipe reuse fix.

The latest update keeps reused awk output command pipes open across intervening stdout writes while still flushing one-shot pipes before observable stdout output. Please focus on actionable correctness, security/sandboxing, regressions, CI failures, or missing tests in the phase 4 delta against codex/awk-phase-3. If everything looks good, please say so explicitly.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 00b2733666

ℹ️ 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".

Comment thread builtins/awk/runtime.go Outdated
Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 14, 2026

@codex please re-check PR #245 from latest head commit 2e88b735acafedc2ab7069c8620dadd9ff035e30 after the loop-reused output-pipe fix.

The latest update carries future-statement context through nested blocks and loop bodies so output command pipes reused on later loop iterations stay open until close/END, while prior one-shot pipe ordering remains covered. Please focus on actionable correctness, security/sandboxing, regressions, CI failures, or missing tests in the phase 4 delta against codex/awk-phase-3. If everything looks good, please say so explicitly.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2e88b735ac

ℹ️ 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".

Comment thread builtins/awk/eval.go Outdated
Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 14, 2026

@codex please re-check PR #245 from latest head commit e77a79b7e023d51faa9abb3cc9ba23e47fee20d7 after the function pipe-context fix.

The latest update carries future-statement context into user function bodies and teaches the output-pipe look-ahead to inspect function calls/bodies so command pipes reused across calls remain open until close/END. Please focus on actionable correctness, security/sandboxing, regressions, CI failures, or missing tests in the phase 4 delta against codex/awk-phase-3. If everything looks good, please say so explicitly.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e77a79b7e0

ℹ️ 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".

Comment thread builtins/awk/runtime.go Outdated
Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 14, 2026

@codex please re-check PR #245 from latest head commit 47847afde6ed3318366e24ff4185849d684d1b4e after the input cleanup fix.

The latest update defers awk input cleanup so main input, getline file sources, and command-input sources are closed on runtime error paths as well as success paths, with a close-tracking regression test. Please focus on actionable correctness, security/sandboxing, regressions, CI failures, or missing tests in the phase 4 delta against codex/awk-phase-3. If everything looks good, please say so explicitly.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 47847afde6

ℹ️ 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".

Comment thread builtins/awk/runtime.go
Comment thread builtins/awk/eval.go Outdated
Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 14, 2026

@codex please re-check PR #245 from latest head commit 01543d9c after the IGNORECASE fixes.

The latest update makes IGNORECASE use AWK truthiness, adds case-folded asorti index ordering when IGNORECASE is enabled, and enables the corresponding upstream regression scenarios. Please focus on actionable correctness, security/sandboxing, regressions, CI failures, or missing tests in the phase 4 delta against codex/awk-phase-3. If everything looks good, please say so explicitly.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 01543d9c17

ℹ️ 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".

Comment thread builtins/awk/runtime.go Outdated
Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 14, 2026

@codex please re-check PR #245 from latest head commit 9c151978 after the command-pipe stdout ordering fix.

The latest update delays stdout while an open awk output command pipe has a future write or explicit close, so reused pipes match GNU awk ordering without regressing explicit close() or the prior function/sort cases. Please focus on actionable correctness, security/sandboxing, regressions, CI failures, or missing tests in the phase 4 delta against codex/awk-phase-3. If everything looks good, please say so explicitly.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9c151978eb

ℹ️ 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".

Comment thread builtins/awk/runtime.go
Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 14, 2026

@codex please re-check PR #245 from latest head commit 798982c6 after the cross-record command-pipe ordering fix.

The latest update carries future-statement context across normal rule iterations and END actions so output command pipes stay open across input records and explicit close() preserves GNU awk ordering. Please focus on actionable correctness, security/sandboxing, regressions, CI failures, or missing tests in the phase 4 delta against codex/awk-phase-3. If everything looks good, please say so explicitly.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 798982c6d2

ℹ️ 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".

Comment thread builtins/awk/runtime.go Outdated
Copy link
Copy Markdown
Collaborator Author

matt-dz commented May 14, 2026

@codex please re-check PR #245 from latest head commit 77f2b4e8 after the dynamic command-pipe close lookahead fix.

The latest update treats non-literal close(...) targets conservatively as possible closes for open awk output command pipes, while keeping literal close targets precise. Please focus on actionable correctness, security/sandboxing, regressions, CI failures, or missing tests in the phase 4 delta against codex/awk-phase-3. If everything looks good, please say so explicitly.

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

ℹ️ 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".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant