feat: multi-file desugar と router catch-all を追加 - #118
Conversation
entry ファイルから @include を再帰的に辿り、dist/ にソースツリーを ミラー出力する multi-file 方式に置き換える。従来は entry 1 ファイルのみ desugar して mktemp に書き出し、@include 先は未desugarのまま gawk の ネイティブ include に委ねていたため、include 先で DSL 構文 (let 等) を 使うと実行時に構文エラーになっていた。 - 訪問済み管理は空白区切り文字列 _seen (bash 3 系互換) - 循環 include は 2 度目以降辿らない、存在しない include は素通し - @include 行は dist/ 接頭の実パスに書き換える (gawk は '/' を含む @include を AWKPATH 探索せずカレントディレクトリ 相対で解決するため) - dsl/desugar.awk 自身の内部 @include (dsl/util.awk 等) も同じ理由で cwd 依存のため、HAWK_LIB に cd してから起動する - hawk-serve/hawk-check/hawk-emit の呼び出し契約 (entry 1 引数、stdout に desugar 済みパス) は変更なし tests/unit/desugar/run.sh を追加し、make test / make ci に配線した。 Co-Authored-By: Claude <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughhawk-libs の desugar を ChangesMulti-file desugar 出力方式の変更
Router catch-all パラメータ対応
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Desugar as desugar
participant Source as source AWK
participant Dist as dist/
participant Gawk as gawk
Desugar->>Source: 入力と `@include` を読み込む
Desugar->>Gawk: 各ファイルを desugar する
Gawk-->>Desugar: 生成済み AWK と include 参照
Desugar->>Dist: closure 全体を配置する
Dist-->>Desugar: dist/$entry_rel を返す
Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@libexec/hawk-libs`:
- Around line 82-91: The `@include` rewrite step in hawk-libs is missing explicit
failure handling around the gawk pipeline, so a gawk error can fail silently or
leave $out partially unreplaced. Update the tmp-to-out rewrite path in hawk-libs
to detect gawk failures explicitly, emit a clear error message before exiting,
and avoid moving the temporary file when rewriting did not succeed. Use the
existing tmp/$out flow and the gawk invocation site to add the handling so the
dist tree never ends up with stale `@include` lines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 300e8fce-979d-4e59-beca-c1ddbf0e7ea9
📒 Files selected for processing (20)
.gitignoreMakefilecore/router.awkdocs/routing.mdlibexec/hawk-checklibexec/hawk-emitlibexec/hawk-libslibexec/hawk-servetests/unit/desugar/fixtures/cyc/a.awktests/unit/desugar/fixtures/cyc/b.awktests/unit/desugar/fixtures/fail/bad.awktests/unit/desugar/fixtures/fail/main.awktests/unit/desugar/fixtures/missing/main.awktests/unit/desugar/fixtures/proj/app/page.awktests/unit/desugar/fixtures/proj/app/sub/util.awktests/unit/desugar/fixtures/proj/main.awktests/unit/desugar/fixtures/solo.awktests/unit/desugar/run.shtests/unit/run.awktests/unit/test_router.awk
💤 Files with no reviewable changes (3)
- libexec/hawk-check
- libexec/hawk-emit
- libexec/hawk-serve
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4d4488d24
ℹ️ 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".
| case "$_seen" in *" $rel "*) return 0 ;; esac | ||
| _seen="$_seen$rel " | ||
| in="$src_root/$rel" | ||
| out="$dist/$rel" |
There was a problem hiding this comment.
Prevent parent includes from escaping dist
When an app in a subdirectory includes a real parent file such as @include "../common.awk", _desugar_one is called with rel=../common.awk and this composes out as dist/../common.awk. With the normal project-root cwd, the subsequent desugar redirection writes outside dist and can truncate/overwrite the original source file before gawk reads it; the rewritten include also points outside the mirrored tree. Normalize or reject .. segments before building the output path, or map them into a safe location under dist.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。.. を含む include パスは dist の外に出るため、desugar 失敗として exit 1 + エラーメッセージにしています(黙ってスキップすると実行時に壊れた @include が残るため、失敗させる方針にしました)。テストケース parent_include_escape_rejected / parent_include_no_file_written_outside_dist を追加しています。
| # dsl/desugar.awk 自身が @include "dsl/util.awk" 等の相対パスを持ち、 | ||
| # gawk は '/' を含む @include をカレントディレクトリ相対で解決するため、 | ||
| # HAWK_LIB (リポジトリルート) に cd してから実行する。 | ||
| if ! ( cd "$HAWK_LIB" && gawk -f dsl/desugar.awk "$in" ) > "$out"; then |
There was a problem hiding this comment.
Preserve shared DSL declarations across includes
In multi-file apps that put a DSL type alias/record declaration in one included file and use it from the entry or another included file, each file is desugared by a separate gawk -f dsl/desugar.awk "$in" invocation, so the checker never sees declarations from earlier @includes. For example, @include "types.awk" followed by let u: User = { ... } in the entry now fails desugar because User was collected only in types.awk; process the include closure with shared declarations or otherwise pass the included context into each desugar run.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
調査しました。現状の「ファイルごとに個別 gawk プロセスで desugar」というアーキテクチャを覆す規模の変更が必要と判明したため、このPRのスコープからは切り出し、#119 に別issueとして起票しました。wiki 段2(templateブランチ)着手前に対応予定です。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73e6a7ad12
ℹ️ 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".
| # dsl/desugar.awk 自身が @include "dsl/util.awk" 等の相対パスを持ち、 | ||
| # gawk は '/' を含む @include をカレントディレクトリ相対で解決するため、 | ||
| # HAWK_LIB (リポジトリルート) に cd してから実行する。 | ||
| if ! ( cd "$HAWK_LIB" && gawk -f dsl/desugar.awk "$in" ) > "$out"; then |
There was a problem hiding this comment.
Reject desugar outputs that alias sources
When HAWK_DIST is set to the source directory, such as HAWK_DIST=. for main.awk, or a file already under the default dist/ is passed back into desugar, out resolves to the same file as in; the shell opens $out for truncation before gawk reads $in, so this can silently erase the source and still exit 0 for an empty file. Please canonicalize/reject output paths that alias input files before redirecting.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。dist を絶対パスに正規化した上で、in/out が同一パスに解決する(HAWK_DIST が source ツリーにエイリアスする)ケースを検出し、desugar 失敗として exit 1 にしています。テストケース alias_dist_rejected / alias_dist_source_untouched(source が改変されていないことを確認)を追加しています。
| return 0 | ||
| } | ||
|
|
||
| entry_rel="$(basename "$src")" |
There was a problem hiding this comment.
Keep dist entry paths unique per source root
Because this drops the directory and always emits dist/<basename>, two running apps such as sites/a/app.awk and sites/b/app.awk share the same dist/app.awk; starting or checking the second app overwrites the artifact that the first server's restart loop/supervisor will later reload after a worker exit. The old mktemp path avoided this cross-app collision, so the dist path needs to include the source root or another unique component.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
これは dist 出力パスの命名規約自体を変える設計判断が要る規模と判断し、#119 に追記しました(型宣言共有の課題と根が近く、同じ issue に集約しています)。wiki 段2着手前に対応予定です。
HAWK_DIST が source ディレクトリと同じ場所を指す場合、 `> "$out"` が gawk 実行前に source ファイルを truncate し、 desugar が空ファイルを exit 0 で返して source を消してしまう 問題を修正(codex 指摘、新規P2)。 dist を絶対パスに正規化した上で in/out のエイリアスを検出し、 desugar 失敗として exit 1 する。 複数アプリでの dist 出力パス衝突(同codex指摘の別課題)は #119 に追記済み。 Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57899f8756
ℹ️ 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".
| _seen="$_seen$rel " | ||
| in="$src_root/$rel" | ||
| out="$dist/$rel" | ||
| if [[ "$in" == "$dist_abs/$rel" ]]; then |
There was a problem hiding this comment.
Canonicalize dist/source alias checks
Fresh evidence after the alias fix: this still compares logical path strings, so HAWK_DIST can point at the source directory through a symlink and bypass the guard. For example, with main.awk in /tmp/src and HAWK_DIST=/tmp/link where link -> /tmp/src, the redirection to $out opens the same file as $in through the symlink and truncates the source before gawk reads it; compare canonical paths or write to a temp file outside the target before replacing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。dist_abs / src_root の正規化を pwd -P(物理パス)に変更し、symlink 経由のエイリアスも検出するようにしています。テストケース alias_dist_symlink_rejected / alias_dist_symlink_source_untouched を追加しています。
| if [[ -n "$incs" ]]; then | ||
| # gawk は '/' を含む @include パスを AWKPATH 探索しないため、 | ||
| # dist 接頭の実パスに書き換える | ||
| tmp="$out.tmp" |
There was a problem hiding this comment.
Use a temp path that cannot collide with outputs
When an included source path itself ends in .tmp, this fixed rewrite filename can alias another desugared artifact. For example, if main.awk includes both a.awk.tmp and a.awk, and a.awk has any include to rewrite, rewriting a.awk uses dist/a.awk.tmp as scratch, overwrites/removes the already-emitted dist/a.awk.tmp, and the final entry still includes a missing file at runtime; use mktemp in the same directory or another collision-free scratch name.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。scratch を固定名 $out.tmp から mktemp(.hawk-desugar.XXXXXX、出力先と同じディレクトリ)に変更しています。a.awk.tmp と a.awk を両方 include するフィクスチャ(tmpcol/)で、書き換え後も dist/a.awk.tmp が生存することを検証するテスト tmp_suffix_include_survives_rewrite を追加しています。
| # dsl/desugar.awk 自身が @include "dsl/util.awk" 等の相対パスを持ち、 | ||
| # gawk は '/' を含む @include をカレントディレクトリ相対で解決するため、 | ||
| # HAWK_LIB (リポジトリルート) に cd してから実行する。 | ||
| if ! ( cd "$HAWK_LIB" && gawk -f dsl/desugar.awk "$in" ) > "$out"; then |
There was a problem hiding this comment.
Update dist artifacts atomically
Because persistent dist files are opened directly for redirection here, any concurrent hawk check/emit/serve for the same app truncates dist/main.awk and included outputs before the new content and rewritten includes are complete. An already-running hawk-serve restarts workers from the same APP_AWK, so a worker exit during this window can load an empty, partial, or not-yet-rewritten artifact and crash; write each output to a unique temp file and atomically rename it into place only after the rewrite succeeds.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。desugar 出力と @include 書き換えをすべて mktemp な scratch 上で行い、完成後に mv でアトミックに配置する構造に変更しています。$out を直接 truncate する窓はなくなり、稼働中の hawk-serve worker は常に完全な旧成果物か完全な新成果物のどちらかを読むようになります。scratch の取り残しがないことを検証する no_scratch_leftovers テストも追加しています。
codex レビュー追撃3件(PR #118)への対応: - source エイリアス検査を pwd -P による物理パス比較に変更。 HAWK_DIST が symlink 経由で source ディレクトリを指すケースの すり抜けを防ぐ - 書き換え scratch を固定名 ($out.tmp) から mktemp に変更。 include 対象自体が .tmp で終わる名前のとき既出力の成果物を 破壊する問題を解消 - desugar 出力を scratch に書き、書き換え完了後に mv で アトミックに配置。稼働中の hawk-serve worker が truncate 途中の 成果物を再ロードする窓を除去 Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e197ba41f8
ℹ️ 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".
| mv "$tmp2" "$tmp" | ||
| fi | ||
| chmod 644 "$tmp" | ||
| mv "$tmp" "$out" |
There was a problem hiding this comment.
Publish parent artifacts only after children pass
Fresh evidence after the temp-file fix: this mv publishes the rewritten parent before recursively desugaring its includes. If a child include later fails desugar (or is rejected for ..), the command exits nonzero but leaves dist/main.awk updated to reference a missing/invalid child; an already-running hawk-serve that restarts a worker from dist/main.awk can then crash even though the check/serve attempt failed. Stage the whole include closure or recurse/validate children before replacing any existing dist artifacts.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。子 include を先に再帰 desugar し、全て成功してから親を mv で publish する post-order 構造に変更しています。子の失敗時は親の scratch を EXIT trap で掃除し、既存の dist 成果物は旧状態のまま残ります。テスト parent_not_published_on_child_failure / no_scratch_leftovers_on_failure を追加しています。
| # src_root 相対で実在する @include を dist 接頭に書き換えて再帰する。 | ||
| _desugar_one() { | ||
| local rel="$1" in out tmp inc incs | ||
| case "$_seen" in *" $rel "*) return 0 ;; esac |
There was a problem hiding this comment.
Reject entry include cycles during desugar
When the entry participates in an include cycle, this early return only stops the recursion; it still leaves the rewritten cycle in dist (for example a.awk includes b.awk and b.awk includes a.awk). hawk-libs desugar and plain hawk check then exit 0, but gawk -f dist/a.awk fails with a fatal error because the program file is included again, so hawk serve can fail after a successful desugar/check. Track the active include stack and fail cycles that re-enter the entry, or remove/avoid rewriting the back-edge.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。実測で確認したところ、gawk は entry(-f で渡した program file)への再 include のみ fatal で、entry を経由しない循環は重複 include としてスキップされます。そのため entry への back-edge を desugar 時点で exit 1(include cycle back to entry メッセージ)で拒否し、非 entry 循環は従来どおり許容する実装にしています。テストは entry_cycle_rejected / entry_cycle_names_cause(entry 循環の拒否)と、新フィクスチャ cyc2/(main→x→y→x)で非 entry 循環の desugar 成功 + 実行成功(cycle_tree_runs)を検証しています。
| _seen="$_seen$rel " | ||
| in="$src_root/$rel" | ||
| out="$dist/$rel" | ||
| if [[ "$in" == "$dist_abs/$rel" ]]; then |
There was a problem hiding this comment.
Reject symlinked sources that resolve into dist
Fresh evidence after the dist canonicalization fix: this guard compares the textual $in path, so an app passed through a symlink can still alias the output. For example, if linkdir/main.awk -> src/main.awk and HAWK_DIST=src, $in is linkdir/main.awk while $dist_abs/$rel is src/main.awk, the check passes, and the final mv replaces the real source with desugared output. Compare the resolved input file (or device/inode) against the resolved output before writing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。エイリアス検査を文字列比較から bash の -ef(device/inode 比較)に変更し、HAWK_DIST 側・source 側どちらが symlink のケースも検出するようにしています。テスト symlinked_source_alias_rejected / symlinked_source_untouched(linkdir/main.awk -> srcdir/main.awk + HAWK_DIST=srcdir で拒否・source 無傷)を追加しています。
codex レビュー追撃3件(PR #118)への対応: - 子 include を先に desugar し、全て成功してから親を mv で publish する post-order に変更。子の失敗時に「存在しない子を 参照する新しい親」が dist に残る問題を解消。途中失敗時の scratch は EXIT trap で掃除 - entry に戻る循環 include を desugar 時点で exit 1 で拒否。 gawk は program file の再 include を実行時 fatal にするため、 check/desugar 成功 + serve 失敗という不整合を防ぐ (entry を経由しない循環は gawk が重複 include をスキップする ため従来どおり許容、cyc2 フィクスチャで検証) - source エイリアス検査を -ef (device/inode 比較) に変更。 source 側が symlink で dist に解決するケースも検出 Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ea4a00b12
ℹ️ 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".
| # "cannot include `X' and use it as a program file" で fatal になるため | ||
| # desugar 時点で拒否する (entry を経由しない循環は gawk が重複 include を | ||
| # スキップするので許容) | ||
| if [[ "$inc" == "$entry_rel" ]]; then |
There was a problem hiding this comment.
Normalize entry-cycle includes before comparing
Fresh evidence after the entry-cycle guard: this compares the raw include text, so an entry a.awk can still be re-included as ./a.awk from b.awk. That path is accepted and rewritten to dist/./a.awk; hawk-libs desugar exits 0, but gawk -f dist/a.awk fails at worker startup with gawk's program-file include fatal error. Normalize or resolve include paths before checking whether they point back to the entry.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。desugar 直後に @include パスの ./(先頭の ./ と中間の /./)を正規化するパスを追加しています。これにより entry 循環検査・_seen 重複排除・dist 書き換えの対応表がすべて "./a.awk" と "a.awk" を同一視します。テスト entry_cycle_dotslash_rejected(./ 経由の back-edge 拒否)と dotslash_include_normalized / dotslash_tree_runs(正常な ./x.awk include の正規化 + 実行)を追加しています。
| chmod 644 "$tmp" | ||
| mv "$tmp" "$out" | ||
| return 0 | ||
| } |
There was a problem hiding this comment.
Stage all outputs before publishing any child
Fresh evidence after the post-order change: this mv still publishes each successfully processed child before the caller has finished later siblings. With an existing valid dist, if main.awk includes a.awk then b.awk, a new valid a.awk is moved into place before a failing b.awk aborts the command, leaving running hawk-serve restarts to load an old parent with a new child after a failed check. Keep completed temp files for the whole include closure and publish them only after every file succeeds.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。publish を「closure 全体の desugar 成功後に一括 mv」の2フェーズに変更しています。各ファイルは scratch に完成させて publish リストに積み、entry の再帰が完走してから深い子→entry の順で配置します。途中失敗時は何も publish されず既存の dist が旧状態のまま残ります。テスト sibling_failure_publishes_nothing(good.awk 成功 + bad.awk 失敗の兄弟構成で、good も main も publish されないこと)を追加しています。
codex レビュー追撃2件(PR #118)への対応: - desugar 直後に @include パスの ./ を正規化するパスを追加。 "./a.awk" 形式の entry back-edge が生テキスト比較を すり抜けて実行時 fatal になる問題を解消。_seen 重複排除・ dist 書き換え対応表も同一視できるようになる - publish を「closure 全体の成功後に一括 mv」の2フェーズに変更。 兄弟 include の失敗時に新旧成果物が dist に混在する窓を除去 (publish 順は深い子から、entry が最後) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8dedff434
ℹ️ 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".
| while read -r t o; do | ||
| [[ -n "$t" ]] || continue | ||
| chmod 644 "$t" | ||
| mv "$t" "$o" |
There was a problem hiding this comment.
Reject directory destinations before publishing
When a stale output path already exists as a directory, such as dist/main.awk/ from an earlier layout and the current source is main.awk, this mv succeeds by moving the temp artifact inside that directory instead of replacing it; I checked mv --help, which documents SOURCE DEST as rename-or-move-to-directory unless -T is used. hawk-libs desugar then exits 0 and echoes dist/main.awk, but gawk -f dist/main.awk fails because it is still a directory, so non-strict check can pass and serve can start a crash loop; reject directory outputs or force file-target semantics before publishing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。staging 時(publish 前)に出力先がディレクトリの場合を検出して exit 1 にしています(publish ループ内でなく staging 時に検査することで、一括 publish の原子性も保っています)。テスト dir_output_rejected を追加しています。
| out="$dist/$rel" | ||
| # -ef は device/inode 比較。symlink 経由 (HAWK_DIST・source どちら側でも) | ||
| # の source エイリアスを文字列比較より確実に検出する | ||
| if [[ -e "$out" && "$in" -ef "$out" ]]; then |
There was a problem hiding this comment.
Reject dist outputs that collide with source files
Fresh evidence beyond the -ef alias guard: if HAWK_DIST points at a source subdirectory rather than the exact same file, this check allows publishing over a different source file. For example, with src/main.awk and an existing src/sub/main.awk, running HAWK_DIST=src/sub hawk-libs desugar src/main.awk exits 0 and replaces src/sub/main.awk with the entry output because src/main.awk and src/sub/main.awk are not -ef; reject output paths that resolve inside the source tree outside the dedicated dist area or otherwise refuse to overwrite pre-existing source files.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。dist ルートに .hawk-dist マーカーファイルを導入し、マーカーのないディレクトリ内の既存ファイルへの上書きを拒否するようにしています(マーカーは publish 成功後に作成)。HAWK_DIST=src/sub のように source 木の中を指した場合、初回の publish 時点で refusing to overwrite existing file in non-dist directory で exit 1 になります。テスト dist_over_source_rejected / dist_over_source_untouched と、マーカーのある正規 dist への再実行が成功する rerun_over_marker_ok を追加しています。
| { | ||
| if (match($0, /^([[:space:]]*@include[[:space:]]+")([^"]+)(".*)$/, m)) { | ||
| p = m[2] | ||
| sub(/^(\.\/)+/, "", p) |
There was a problem hiding this comment.
Keep unresolved ./ includes from falling into AWKPATH
When a source has @include "./helper.awk" but that file is absent under the app root, this normalization still rewrites it to @include "helper.awk" and leaves it as a missing include. I verified with gawk that the original ./helper.awk form fails locally, while the normalized no-slash form is resolved through AWKPATH, so an environment with an unrelated helper.awk on AWKPATH can turn a missing include into silently loaded code; only normalize/rewrite includes after confirming the target is one of the files being mirrored.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。./ の正規化を「正規化後のパスが src_root 相対で実在するファイルの場合」に限定しています。実在しない ./helper.awk は元の形のまま残るため、AWKPATH に落ちる挙動変化は起きません。テスト unresolved_dotslash_kept(実在しない ./nosuch.awk が書き換えられずそのまま残ること)を追加しています。
| # "cannot include `X' and use it as a program file" で fatal になるため | ||
| # desugar 時点で拒否する (entry を経由しない循環は gawk が重複 include を | ||
| # スキップするので許容) | ||
| if [[ "$inc" == "$entry_rel" ]]; then |
There was a problem hiding this comment.
Detect entry cycles through symlinked includes
Fresh evidence after the entry-cycle guard: this compares only the include text, so a.awk -> b.awk -> alias.awk is accepted when alias.awk is a symlink or hard link to a.awk. Desugar then copies the entry under dist/alias.awk and exits 0, but gawk -f dist/a.awk fails later with duplicate entry definitions; compare the included file's resolved identity with the entry file (for example with -ef) before accepting the edge.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。entry 循環検査をテキスト比較 + -ef(device/inode 比較)の併用にし、symlink / hardlink 経由で entry を指す include も cycle back to entry として拒否するようにしています。テスト entry_cycle_symlink_rejected(alias.awk -> a.awk の symlink 経由 back-edge)を追加しています。
codex レビュー追撃4件(PR #118)への対応: - 出力先が既存ディレクトリの場合は exit 1 (mv がディレクトリ内へ移動して exit 0 のまま壊れるのを防止) - dist ルートに .hawk-dist マーカーを導入。マーカーのない ディレクトリ内の既存ファイルは source とみなし上書き拒否 (HAWK_DIST が source 木の中の別ファイルを指すケースは -ef では検出できないため) - ./ 正規化を src_root 相対で実在するファイルに限定。 実在しない "./x.awk" を "x.awk" に書き換えると素通り後に AWKPATH で無関係のファイルを拾う挙動変化が起きるため - entry 循環検査に -ef を追加し、symlink/hardlink 経由の entry エイリアス include も検出 Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/unit/desugar/run.sh (1)
57-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
run_out=$(gawk ...)のset -e下での早期終了リスク
set -eが有効な状態でrun_out=$(gawk -f "$dist/main.awk" </dev/null)が失敗すると、スクリプトが即座に終了し、後続のng "dotslash_tree_runs"が報告されません。desugar が成功しても gawk 実行が失敗するエッジケースで、どのテストが失敗したかの情報が失われます。Line 79 のcycle_tree_runsも同様です。♻️ 提案する修正
if grep -qF "`@include` \"$dist/x.awk\"" "$dist/main.awk"; then ok "dotslash_include_normalized"; else ng "dotslash_include_normalized"; fi - run_out=$(gawk -f "$dist/main.awk" </dev/null) + set +e + run_out=$(gawk -f "$dist/main.awk" </dev/null) + set -e if [[ "$run_out" == "dotslash-x" ]]; then ok "dotslash_tree_runs"; else ng "dotslash_tree_runs" "$run_out"; fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/desugar/run.sh` around lines 57 - 74, Protect the gawk execution in the dotslash_tree_runs check from set -e so failures are reported through ng instead of terminating the script. Capture the command’s exit status while assigning run_out, then emit ok only for a successful command with the expected output, otherwise emit ng with the captured output. Apply the same handling to the cycle_tree_runs check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/unit/desugar/run.sh`:
- Around line 57-74: Protect the gawk execution in the dotslash_tree_runs check
from set -e so failures are reported through ng instead of terminating the
script. Capture the command’s exit status while assigning run_out, then emit ok
only for a successful command with the expected output, otherwise emit ng with
the captured output. Apply the same handling to the cycle_tree_runs check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7da49bd9-9709-4045-8053-cec25062652a
📒 Files selected for processing (16)
libexec/hawk-libstests/unit/desugar/fixtures/cyc2/main.awktests/unit/desugar/fixtures/cyc2/x.awktests/unit/desugar/fixtures/cyc2/y.awktests/unit/desugar/fixtures/cyc3/a.awktests/unit/desugar/fixtures/cyc3/b.awktests/unit/desugar/fixtures/dotmiss/main.awktests/unit/desugar/fixtures/dotslash/main.awktests/unit/desugar/fixtures/dotslash/x.awktests/unit/desugar/fixtures/sib/a_good.awktests/unit/desugar/fixtures/sib/main.awktests/unit/desugar/fixtures/sib/z_bad.awktests/unit/desugar/fixtures/symcyc/a.awktests/unit/desugar/fixtures/symcyc/b.awktests/unit/desugar/fixtures/symsrc/srcdir/main.awktests/unit/desugar/run.sh
✅ Files skipped from review due to trivial changes (10)
- tests/unit/desugar/fixtures/sib/a_good.awk
- tests/unit/desugar/fixtures/cyc2/main.awk
- tests/unit/desugar/fixtures/symcyc/b.awk
- tests/unit/desugar/fixtures/sib/main.awk
- tests/unit/desugar/fixtures/cyc2/x.awk
- tests/unit/desugar/fixtures/sib/z_bad.awk
- tests/unit/desugar/fixtures/symcyc/a.awk
- tests/unit/desugar/fixtures/dotslash/x.awk
- tests/unit/desugar/fixtures/symsrc/srcdir/main.awk
- tests/unit/desugar/fixtures/cyc2/y.awk
🚧 Files skipped from review as they are similar to previous changes (1)
- libexec/hawk-libs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe177c0cf2
ℹ️ 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".
| while read -r t o; do | ||
| [[ -n "$t" ]] || continue | ||
| chmod 644 "$t" | ||
| mv "$t" "$o" |
There was a problem hiding this comment.
Use a delimiter-safe publish queue
When HAWK_DIST is set to an absolute directory containing spaces, the scratch path stored in _publish also contains spaces. This loop parses each queued pair with read -r t o, so t becomes only the first path component (for example /tmp/hawk) and chmod/mv fail or target the wrong file after desugar has already succeeded; store the publish pairs in arrays or use a delimiter that cannot appear in paths.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。publish キューを空白区切り文字列から bash 配列(_pub_tmp / _pub_out の並行配列)に変更しています。テスト spaced_dist_publishes(空白入り HAWK_DIST での多段 include publish)を追加しています。
| # src_root 相対で実在する @include を dist 接頭に書き換えて再帰する。 | ||
| _desugar_one() { | ||
| local rel="$1" in out tmp inc incs | ||
| case "$_seen" in *" $rel "*) return 0 ;; esac |
There was a problem hiding this comment.
Coalesce symlinked includes before mirroring
When an included file is reachable through two names that are the same inode but are not the entry, such as main.awk including both x.awk and a symlink alias.awk -> x.awk, this text-keyed _seen check processes both names and publishes two separate dist files. Plain gawk suppresses the duplicate include by file identity, but after mirroring the files have different inodes, so BEGIN side effects and definitions run twice; key the closure by resolved identity or rewrite aliases to one artifact.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。coalesce(別名を正規パスに書き換えて1成果物に集約)ではなく、同一実体の別名 include を -ef で検出して exit 1 で拒否する方針にしています。app 内で同じファイルを symlink 別名で二重 include する構成は正当な用途が考えにくく、静かに二重定義になるより明示的に失敗させる方が安全と判断しました。テスト alias_include_rejected(x.awk + alias.awk -> x.awk の両 include で拒否)を追加しています。
| while read -r t o; do | ||
| [[ -n "$t" ]] || continue | ||
| chmod 644 "$t" | ||
| mv "$t" "$o" |
There was a problem hiding this comment.
Avoid exposing mixed dist generations
Fresh evidence after the closure-staging fix: this loop still publishes a successful closure one file at a time, with children queued before the entry. If hawk check or emit runs while an existing hawk-serve is supervising the same dist and a worker restarts after a child has been moved but before the parent is replaced, it can load the old parent together with a new child; incompatible parent/child edits then crash despite the new closure being valid. Publish through a generation directory or another single switch point so workers see only all-old or all-new files.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
これは generation ディレクトリ + 単一スイッチポイント(dist/gen-N/ + symlink 付け替え等)という dist レイアウト規約の再設計が必要な変更のため、#119 に課題3として追記しました。dist 出力パスの一意化(課題2)と同じ領域なので、まとめて設計します。本PRの一括 publish で窓は「desugar 全体」から「連続する数回の mv」まで縮んでおり、稼働中 serve と並行して非互換編集を desugar する運用は現時点で想定していないため、先送りとしています。
codex レビュー追撃2件(PR #118)への対応: - publish キューを空白区切り文字列から bash 配列に変更。 HAWK_DIST に空白入りパスを渡すと read の解析が壊れて publish が失敗/誤動作する問題を解消 - 同一実体 (symlink/hardlink) を別名で include するケースを exit 1 で拒否。mirror 後は別 inode になり gawk の重複 include 排除が効かず、定義・BEGIN が二重実行されるため publish の世代混在窓 (generation directory による単一スイッチ ポイント化) は dist レイアウト規約の再設計が必要なため #119 に追記。 Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3f3e31e66
ℹ️ 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".
| # src_root 相対で実在する @include を dist 接頭に書き換えて再帰する。 | ||
| _desugar_one() { | ||
| local rel="$1" in out tmp inc incs prior | ||
| case "$_seen" in *" $rel "*) return 0 ;; esac |
There was a problem hiding this comment.
Track seen includes without space delimiters
When an app includes a file whose relative path contains a space and later includes a path matching one of its space-separated tokens, this substring check treats the later file as already seen. For example, @include "x y.awk" followed by @include "x" rewrites both lines to dist/..., but _desugar_one "x" returns here without creating dist/x, so gawk -f dist/main.awk fails at runtime with a missing include. Store seen paths in an associative array or use a delimiter-safe representation for arbitrary path names.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。空白入り include パスは include path contains spaces (unsupported) で staging 時に exit 1 にしています(bash 3 互換で連想配列を使わない設計のため、サポートではなく明示的拒否を選びました。黙って欠けた dist を作るより安全です)。テスト spaced_include_rejected / spaced_include_publishes_nothing を追加しています。
| chmod 644 "${_pub_tmp[$i]}" | ||
| mv "${_pub_tmp[$i]}" "${_pub_out[$i]}" | ||
| done | ||
| : > "$dist/.hawk-dist" |
There was a problem hiding this comment.
Validate the dist marker before publishing
If $dist/.hawk-dist exists as a directory or otherwise cannot be overwritten, this redirection is the first marker validation and it runs after the loop has already moved every staged output into place. In that stale-marker state hawk-libs desugar exits 1 here but leaves files like dist/main.awk updated, so a failed hawk check/serve attempt can still change artifacts that an existing supervisor may reload; reject an invalid marker before the publish loop or create it during staging.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。desugar 開始時(staging 前)に .hawk-dist が regular file 以外の場合を検証して exit 1 にしています。publish 済み成果物が残ったまま失敗する不整合は起きなくなります。テスト invalid_marker_rejected / invalid_marker_publishes_nothing(marker がディレクトリの場合に何も publish されないこと)を追加しています。
codex レビュー追撃2件(PR #118)への対応: - 空白入り include パスを exit 1 で拒否。_seen の空白区切り 訪問済み判定がトークン単位の部分一致を起こし、dist に欠けた ファイルを黙って作るため。bash 3 互換 (連想配列不使用) を 保ったまま fail loud にする - .hawk-dist marker が regular file 以外の場合を staging 前に 検証して exit 1。publish 後の marker 作成失敗だと 「exit 1 なのに成果物は更新済み」の不整合が起きるため Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/desugar/run.sh`:
- Around line 183-193: alias_include_rejected テストに、desugar が exit 1 になった後に dist
ディレクトリへ成果物が生成されていないことを検証する publishes_nothing チェックを追加してください。既存の spaced_include
および invalid_marker テストと同じ判定方法で "$dist" の出力有無を確認し、エラーメッセージ確認と合わせて成功条件に含めてください。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c52f88fa-013a-412e-8475-c365650d5ccd
📒 Files selected for processing (6)
libexec/hawk-libstests/unit/desugar/fixtures/aliasinc/main.awktests/unit/desugar/fixtures/aliasinc/x.awktests/unit/desugar/fixtures/spacename/main.awktests/unit/desugar/fixtures/spacename/x y.awktests/unit/desugar/run.sh
✅ Files skipped from review due to trivial changes (3)
- tests/unit/desugar/fixtures/spacename/x y.awk
- tests/unit/desugar/fixtures/aliasinc/x.awk
- tests/unit/desugar/fixtures/spacename/main.awk
🚧 Files skipped from review as they are similar to previous changes (1)
- libexec/hawk-libs
他の失敗系テストと同様の publishes_nothing チェックが alias_include_rejected にだけ欠けていた (coderabbit 指摘)。 Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2206ba49ee
ℹ️ 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".
| for ((i = 0; i < ${#_pub_tmp[@]}; i++)); do | ||
| chmod 644 "${_pub_tmp[$i]}" | ||
| mv "${_pub_tmp[$i]}" "${_pub_out[$i]}" | ||
| done | ||
| : > "$dist/.hawk-dist" |
There was a problem hiding this comment.
Validate marker writability before publishing
When dist/.hawk-dist is a regular file but the current user cannot truncate it, for example after chmod a-w dist/.hawk-dist, the publish loop has already moved every staged artifact into dist before this redirection fails under set -e. That makes a failed hawk check/serve attempt still replace files that an existing supervisor may reload; preflight the marker by checking/writing it before the mv loop or stage the marker with the rest of the closure.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。marker が既存で書込不可の場合を staging 前(desugar 開始時)に検証して exit 1 にしています。テスト unwritable_marker_rejected / unwritable_marker_publishes_nothing(既存 dist の成果物が無変更で残ること)を追加しています。
| _scratches="$_scratches $tmp" | ||
| # shellcheck disable=SC2064 | ||
| trap "rm -f$_scratches" EXIT |
There was a problem hiding this comment.
Quote scratch paths in cleanup trap
When HAWK_DIST contains spaces, which the new tests explicitly support, this trap stores the scratch paths as unquoted shell words. On exit a path like /tmp/dist with spaces/.hawk-desugar.X is parsed as separate arguments, so a successful desugar can delete an unrelated prefix file such as /tmp/dist, and a failing desugar leaves the scratch behind; keep scratch paths in an array or install a cleanup function that quotes each path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。scratch の管理を空白区切り文字列 + 文字列 trap から、配列 _scratch_arr + quote 安全な cleanup 関数(trap _cleanup_scratches EXIT)に変更しています。テスト spaced_dist_no_scratch_leftovers(空白入り HAWK_DIST でも scratch が正しく処理されること)を追加しています。
| mkdir -p "$dist" | ||
| # marker が regular file 以外だと publish 後の作成で失敗し | ||
| # 「exit 1 なのに成果物は更新済み」になるため、staging 前に検証する | ||
| if [[ -e "$dist/.hawk-dist" && ! -f "$dist/.hawk-dist" ]]; then |
There was a problem hiding this comment.
A symlink to a regular file passes this marker validation because -f follows symlinks, and it also makes the later ! -e "$dist/.hawk-dist" overwrite guard treat the directory as a trusted dist. For example, with HAWK_DIST=src/sub and src/sub/.hawk-dist -> /tmp/marker, an existing src/sub/main.awk is overwritten by desugaring src/main.awk, and the external marker target is truncated; reject marker symlinks before staging.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。marker 検証の先頭に -L チェックを追加し、symlink の marker を staging 前に拒否しています(-f が symlink を辿る問題への対処)。テスト symlink_marker_rejected / symlink_marker_target_untouched(外部ターゲットが truncate されないこと)を追加しています。
codex レビュー追撃3件(PR #118)への対応: - marker が書込不可の場合を staging 前に検証。publish 完了後の marker 更新失敗だと「exit 1 なのに成果物は更新済み」になるため - symlink の marker を拒否。-f が symlink を辿るため、marker 偽装で 上書き保護を迂回でき、外部ターゲットの truncate も起きるため - scratch cleanup を空白区切り文字列 trap から配列 + cleanup 関数に 変更。空白入り HAWK_DIST で trap の word splitting が無関係の ファイルを削除しうるため Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a48f18afa8
ℹ️ 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".
| dist="$TMP/distmkw" | ||
| HAWK_DIST="$dist" "$LIBS" desugar "$FIX/solo.awk" >/dev/null | ||
| before_hash=$(cksum < "$dist/solo.awk") | ||
| chmod a-w "$dist/.hawk-dist" |
There was a problem hiding this comment.
Make marker writability test work as root
In the root container context I inspected (id -u is 0), chmod a-w does not make this marker fail Bash's -w check or prevent truncation, so running ./tests/unit/desugar/run.sh gets exit=0 here and fails unwritable_marker_rejected / unwritable_marker_publishes_nothing; because make test now includes this script, CI environments that run as root will fail. Use a fixture that is unwritable for root too, or skip this assertion when running as uid 0.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。root(uid 0)では chmod a-w が -w 判定にも truncate 阻止にも効かないため、unwritable_marker_rejected / unwritable_marker_publishes_nothing を uid 0 のとき SKIP(PASS/FAIL どちらにもカウントしない)にしています。
| # 過去の desugar が作った dist であることを .hawk-dist マーカーで確認する。 | ||
| # マーカーのないディレクトリ内の既存ファイルは source とみなし上書きしない | ||
| # (HAWK_DIST が source 木の中を指すと -ef では検出できない別ファイルを潰すため) | ||
| if [[ -e "$out" && ! -e "$dist/.hawk-dist" ]]; then |
There was a problem hiding this comment.
Do not let a prior marker bless source directories
Fresh evidence after the marker-based overwrite guard: if HAWK_DIST is accidentally pointed at a source subdirectory where the first entry basename does not already exist, the first successful desugar creates .hawk-dist there; after that, this condition treats the source directory as trusted and a later entry whose output name collides with an existing source file is overwritten instead of rejected. For example, HAWK_DIST=src/sub desugaring src/main.awk plants the marker, and a later desugar of src/other.awk replaces an existing src/sub/other.awk.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。.hawk-dist marker を空ファイルから「publish した rel パスの1行1件記録」に変更し、既存ファイルの上書きは marker に記録済みの rel に限定しています(grep -qxF で照合、marker 更新は mktemp + mv でアトミック)。ご指摘のシナリオ(先に marker が植えられた source サブディレクトリで、後続 desugar が既存 source と衝突)はテスト marker_does_not_bless_unrelated_files / marker_does_not_bless_unrelated_files_untouched で検証しています。
| # 同一実体 (symlink/hardlink) を別名で include すると、mirror 後は | ||
| # 別 inode になり gawk の重複 include 排除が効かず定義が二重実行される。 | ||
| # 別名での include は拒否する | ||
| for prior in $_seen; do |
There was a problem hiding this comment.
Disable globbing when walking seen includes
Fresh evidence after the space-path rejection: _seen is still expanded as unquoted shell words here, so include names containing glob characters are matched against the current directory. When an app has both a literal x*.awk and xfoo.awk and includes them in that order from the app root, the loop expands the earlier x*.awk to xfoo.awk and falsely rejects the second include as an alias of itself; disable globbing or store seen paths without shell expansion.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。alias 検出ループを set -f / set +f で囲み、_seen の未クォート展開時のパス名展開を止めています。テスト glob_include_names_not_confused(リテラル x*.awk と xfoo.awk の両 include が誤 alias 判定されず両方 publish されること)を追加しています。
codex レビュー追撃3件(PR #118)への対応: - .hawk-dist marker を空ファイルから「publish した rel パスの 1行1件記録」に変更。空 marker だと最初の成功後にディレクトリ 全体を信頼してしまい、HAWK_DIST が source 木の中を指す場合に 無関係な source ファイルまで上書き対象になるため。上書き許可は marker に記録済みの rel に限定 - alias 検出ループを set -f で囲み glob 展開を停止。include 名の glob 文字 (x*.awk 等) がカレントディレクトリに展開されて 無関係ファイルを誤 alias 判定するため - root では chmod a-w が -w 判定に効かないため、unwritable_marker テストを uid 0 のとき SKIP に変更 (root コンテナ CI 対策) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 620aa1270c
ℹ️ 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".
| [[ -f "$dist/.hawk-dist" ]] && cat "$dist/.hawk-dist" | ||
| printf '%s\n' "${_pub_rel[@]}" | ||
| } | sort -u > "$_marker_tmp" | ||
| mv "$_marker_tmp" "$dist/.hawk-dist" |
There was a problem hiding this comment.
When an app includes a real source file named .hawk-dist, it is staged and published to dist/.hawk-dist like any other include, but this marker update then uses the same path for bookkeeping; the command exits 0 while the included artifact is replaced/appended with marker lines, so gawk -f dist/main.awk later fails on the rewritten @include ".../.hawk-dist". Reject .hawk-dist as a source rel or move the marker outside the mirrored output namespace.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。.hawk-dist を予約名とし、この名前の include を staging 時に exit 1 で拒否しています。テスト marker_name_include_rejected を追加しています。
| echo "[hawk-libs] desugar failed: include cycle back to entry: $rel includes $inc" >&2 | ||
| exit 1 | ||
| fi | ||
| [[ -f "$src_root/$inc" ]] && incs="$incs$inc"$'\n' |
There was a problem hiding this comment.
Resolve absolute includes before mirroring
Fresh evidence beyond the symlink-alias check: when an included file is also referenced by absolute path, such as main.awk including x.awk and x.awk including /tmp/app/x.awk, this source-root-prefixed existence check does not recognize the absolute include as the same source file. Plain gawk suppresses that duplicate by file identity, but after mirroring dist/x.awk includes the original source path and BEGIN side effects/definitions run twice; resolve absolute include paths as themselves before alias checks and recursion, or reject them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。絶対パス include のうち、物理パス解決(pwd -P)で src_root 配下を指すものを exit 1 で拒否しています(プロジェクト内は相対パスに限定)。プロジェクト外を指す絶対パスは共有ライブラリ用途を想定して従来どおり素通りです。テスト abs_include_inside_root_rejected / abs_include_external_passthrough を追加しています。
codex レビュー追撃2件(PR #118)への対応: - ".hawk-dist" という include 名を予約名として exit 1。 dist ルートの marker と同じパスに publish されると、publish 後の marker 更新が成果物を書き潰して実行時に壊れるため - プロジェクト内 (src_root 配下) を指す絶対パス include を exit 1。 mirror 後も原本 (未 desugar) を直接ロードして定義が二重実行される ため相対パスに限定する。外部の絶対パス (共有ライブラリ等) は 従来どおり素通り Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb968d3272
ℹ️ 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".
| { | ||
| [[ -f "$dist/.hawk-dist" ]] && cat "$dist/.hawk-dist" | ||
| printf '%s\n' "${_pub_rel[@]}" | ||
| } | sort -u > "$_marker_tmp" |
There was a problem hiding this comment.
Validate marker readability before rewriting
If an existing .hawk-dist marker is writable but not readable for the current user, this cat fails inside the pipeline, but the pipeline can still exit 0 via sort; the command has already published the staged artifacts and then replaces the marker with an empty or incomplete list. In that state, later desugars of previously published files are rejected as attempts to overwrite non-dist files, so preflight -r/read the marker before publishing or make the marker update fail before any mv.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。marker が既存の場合の staging 前検証に -r(読込可能)を追加し、読めない marker は publish 前に exit 1 にしています。テスト unreadable_marker_rejected(root では chmod が効かないため uid 0 時は SKIP)を追加しています。
| # 潰すため。marker を空ファイルにするだけだと最初の成功後はディレクトリ全体を | ||
| # 信頼してしまい、無関係な source ファイルまで上書き対象になる) | ||
| if [[ -e "$out" ]]; then | ||
| if [[ ! -f "$dist/.hawk-dist" ]] || ! grep -qxF "$rel" "$dist/.hawk-dist"; then |
There was a problem hiding this comment.
Stop grep from parsing rel names as options
When a generated rel path starts with -, for example an entry named -main.awk or an included -x.awk, the first desugar records that rel in .hawk-dist but a rerun reaches this check and grep treats the rel as an option rather than the fixed-string pattern. I checked grep --help: its usage is grep [OPTION]... PATTERNS [FILE] and -m is an option, which matches the observed invalid max count failure for -main.awk; use grep -qxF -- "$rel" ... so valid dash-prefixed filenames remain rerunnable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。grep -qxF -- "$rel" に変更しています。テスト dash_prefixed_entry_rerunnable(-main.awk という entry の2回目の desugar が成功すること)を追加しています。
| # プロジェクト内は相対パスに限定し、外部の絶対パス (共有ライブラリ等) は | ||
| # 従来どおり素通りで残す | ||
| if [[ "$inc" == /* ]]; then | ||
| if [[ -f "$inc" && "$(cd "$(dirname "$inc")" && pwd -P)/" == "$src_root/"* ]]; then |
There was a problem hiding this comment.
Resolve absolute include symlinks before passing them through
Fresh evidence after the absolute-include fix: this check only canonicalizes the containing directory, so an absolute path outside the project that is a symlink to a project file is treated as an external library and left in the mirrored output. If main.awk includes x.awk and x.awk includes /tmp/outside/alias.awk -> $src_root/x.awk, desugar exits 0 but gawk -f dist/main.awk later loads both dist/x.awk and the original source through the symlink, producing duplicate definitions; resolve the include file identity itself (for example with -ef) before deciding it is external.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。絶対パス include の外部判定で、ディレクトリの正規化に加えてファイル自体の symlink チェーンを readlink ループ(最大40回、超過は symlink loop として exit 1)で解決してから src_root 配下かを判定するようにしています。テスト abs_symlink_include_to_project_rejected(/tmp/outside/alias.awk -> project/x.awk 型のすり抜けを拒否)を追加しています。
| # 上書きしない (HAWK_DIST が source 木の中を指すと -ef では検出できない別ファイルを | ||
| # 潰すため。marker を空ファイルにするだけだと最初の成功後はディレクトリ全体を | ||
| # 信頼してしまい、無関係な source ファイルまで上書き対象になる) | ||
| if [[ -e "$out" ]]; then |
There was a problem hiding this comment.
Treat dangling output symlinks as existing collisions
Fresh evidence after the marker-based overwrite guard: Bash -e is false for dangling symlinks, so a stale source symlink at the output path bypasses this protection and is replaced by the desugared artifact. For example, with HAWK_DIST=src/sub and src/sub/main.awk -> /missing, desugaring src/main.awk exits 0 and overwrites that source-tree symlink; check -L as well as -e before deciding the destination is safe to publish.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。上書き保護の存在判定を [[ -e "$out" || -L "$out" ]] に変更し、dangling symlink も既存衝突として扱うようにしています。テスト dangling_symlink_output_rejected(出力先の dangling symlink が置き換えられず exit 1 になること)を追加しています。
codex レビュー追撃4件(PR #118)への対応: - marker が書込可だが読込不可の場合を staging 前に検証。 publish 後の marker 更新パイプラインで cat の失敗が sort の exit 0 に隠れ、marker が空/不完全になって以降の再実行が 全部拒否されるため - marker 照合の grep に -- を追加。"-main.awk" のような ダッシュ始まりの正当なファイル名がオプションと誤解釈され 再実行不能になるため - 絶対パス include の外部判定で、ディレクトリだけでなく ファイル自体の symlink チェーンも解決 (最大40回、超過は symlink loop として exit 1)。外部パスを装った symlink が プロジェクト内の原本を指すと定義が二重実行されるため - 上書き保護の存在判定に -L を追加。dangling symlink は -e が false になり、出力先に残った古い source symlink が保護を すり抜けて上書きされるため Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dec66f3716
ℹ️ 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".
| if [[ "$(cd "$(dirname "$_abs_inc_resolved")" && pwd -P)/" == "$src_root/"* ]]; then | ||
| rm -f "$tmp" | ||
| echo "[hawk-libs] desugar failed: absolute include points inside project root (use a relative path): $inc" >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Reject absolute hard links back into the project
Fresh evidence beyond the symlink absolute-include guard: this only checks the resolved pathname's directory, so an absolute hard link outside src_root to a project file is treated as an external library and left in the mirrored artifact. If main.awk includes x.awk and x.awk includes /tmp/outside/alias.awk hard-linked to $src_root/x.awk, desugar exits 0, but gawk -f dist/main.awk loads both dist/x.awk and the original hard link, causing duplicate definitions/BEGIN side effects. Compare the include's device/inode against already-seen project files before continuing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。symlink チェーン解決 + パス比較に加えて、訪問済みプロジェクトファイルとの -ef(device/inode 比較)を追加し、hard link 経由の同一実体も検出して exit 1 にしています。テスト abs_hardlink_include_to_project_rejected(x.awk が自身への hard link を絶対パスで include するケース)を追加しています。
| # dsl/desugar.awk 自身が @include "dsl/util.awk" 等の相対パスを持ち、 | ||
| # gawk は '/' を含む @include をカレントディレクトリ相対で解決するため、 | ||
| # HAWK_LIB (リポジトリルート) に cd してから実行する。 | ||
| if ! ( cd "$HAWK_LIB" && gawk -f dsl/desugar.awk "$in" ) > "$tmp"; then |
There was a problem hiding this comment.
Share signatures before desugaring child files
When an included DSL file calls a function declared in a sibling or entry file, this per-file invocation checks that child in isolation and dsl/check.awk reports unknown function even though gawk allows functions to be split across included files. For example, main.awk including a.awk and b.awk fails if a.awk contains function a() -> Int { return b() } and b() is declared in b.awk, so valid multi-file typed apps cannot be checked or served; collect signatures across the include closure before checking each file, or otherwise allow these cross-file references.
Useful? React with 👍 / 👎.
codex レビュー追撃(PR #118)への対応。 hard link は readlink で辿れずパス名も外部のままなので、symlink チェーン解決 + パス prefix 比較では検出できない。訪問済み プロジェクトファイルとの device/inode 比較 (-ef) を追加し、 mirror 後に原本 (未 desugar) が二重ロードされるのを防ぐ。 関数シグネチャのファイル跨ぎ参照 (checker が unknown function に なる問題) は型宣言共有と同根の設計制約のため #119 に追記。 Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d44bdcdd7c
ℹ️ 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".
| for prior in $_seen; do | ||
| if [[ "$inc" -ef "$src_root/$prior" ]]; then |
There was a problem hiding this comment.
Reject absolute hard links to not-yet-seen project files
Fresh evidence after the hard-link guard: this only compares an absolute include to _seen, which at this point contains ancestors and any children processed earlier, not later siblings in the same closure. If main.awk includes /tmp/alias.awk hard-linked to x.awk before @include "x.awk" (or a child points at a later sibling), desugar exits 0 and leaves both the external absolute include and dist/x.awk; gawk then loads the same definitions twice. Check absolute includes against the full closure before publishing, or reject hard links by inode even when the relative file has not been visited yet.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。絶対パス include の hard link 照合を「staging 中に _seen と照合」から「closure 確定後(publish 前)に _abs_ext キューを全 _seen と -ef 照合」に移しています。後続 sibling への hard link も検出され、失敗時は何も publish されません。テスト abs_hardlink_to_later_sibling_rejected(alias → x.awk の順で include するケース)を追加し、既存の abs_hardlink_include_to_project_rejected も引き続き通っています。
| return 0 | ||
| } | ||
|
|
||
| entry_rel="$(basename "$src")" |
There was a problem hiding this comment.
Use source-specific dist paths for entry artifacts
When two app entries in different directories share the same basename and use the same HAWK_DIST (for example apps/a/main.awk and apps/b/main.awk), both publish to dist/main.awk and the marker records only main.awk, so the second successful check/emit silently replaces the first app's artifact. An already-running hawk-serve for the first app restarts workers from that same APP_AWK path and can load the second app instead; include enough of the source path or source identity in the dist key, or reject cross-source reuse of an existing marked rel.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。dist パス自体への source 識別子の埋め込みは dist レイアウト規約の再設計(#119 課題2)に含めるとして、このPRでは「reject cross-source reuse」を採っています。marker の行形式を rel から src_root<TAB>rel のペアに変更し、既存ファイルの上書きは「同じ src_root からの同じ rel」が marker に記録されている場合に限定しました。別 source の同名 entry は refusing to overwrite で exit 1 になります。テスト cross_source_same_basename_rejected(appsA/main.awk 成功後の appsB/main.awk が拒否され、dist の中身が A のまま)を追加し、同一 entry の再実行 rerun_over_marker_ok は引き続き成功します。
| exit 1 | ||
| fi | ||
| fi | ||
| mkdir -p "$(dirname "$out")" |
There was a problem hiding this comment.
Reject symlinked output directories
When an output parent inside dist is a symlink, this creates the scratch file through the link and the later mv publishes outside the dist tree. For example, after sub/x.awk has been recorded in .hawk-dist, replacing dist/sub with a symlink to another directory lets a rerun overwrite that directory's x.awk while desugar exits 0; validate each output directory component is a real dist directory before staging or publishing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。mkdir -p の直後に出力先の親ディレクトリを pwd -P で物理パス解決し、dist の物理パス配下でなければ output directory escapes dist via symlink で exit 1 にしています。テスト symlinked_output_dir_rejected(marker 記録済みの dist/app を外部ディレクトリへの symlink に差し替えた再実行が拒否され、外部にファイルが書かれないこと)を追加しています。
codex レビュー追撃3件(PR #118)への対応: - 絶対パス include の hard link 照合を closure 確定後 (publish 前) に移動。staging 中の _seen は処理順依存で、後続 sibling への hard link を見逃すため - marker の行形式を rel から「src_root TAB rel」のペアに変更。 rel だけの照合だと、別ディレクトリの同名 entry (apps/a/main.awk と apps/b/main.awk) が同じ dist を使ったとき後勝ちが前の成果物を 無警告で置換するため。dist パス自体の一意化は #119 の設計課題 - 出力先の親ディレクトリの物理パスが dist の物理パス配下であることを staging 時に検証。dist 配下のディレクトリが外部への symlink に 差し替わると publish が dist の外に書いてしまうため Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cfd53fcceb
ℹ️ 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".
| echo "[hawk-libs] desugar failed: include path contains spaces (unsupported): $rel" >&2 | ||
| exit 1 | ||
| ;; | ||
| .hawk-dist) |
There was a problem hiding this comment.
Reserve the marker namespace, not just the file
When an app includes a file under .hawk-dist/, such as .hawk-dist/x.awk, this exact-name check lets it publish into dist/.hawk-dist/ as a directory. The later marker mv then moves the marker temp file inside that directory and still exits 0, leaving the desugared tree runnable once but with no valid marker and making the next desugar fail because .hawk-dist is not a regular file; reject rel paths whose first component is .hawk-dist.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。予約名チェックを .hawk-dist|.hawk-dist/* に拡張し、第一成分が .hawk-dist の rel をすべて staging 時に拒否しています。テスト marker_namespace_include_rejected(.hawk-dist/x.awk の include が exit 1 になり、dist/.hawk-dist がディレクトリとして作られないこと)を追加しています。
codex レビュー追撃(PR #118)への対応。 完全一致のみの検査だと ".hawk-dist/x.awk" のような rel が素通りし、 publish で dist/.hawk-dist/ がディレクトリとして作られる。その後の marker mv がディレクトリ内への移動になって exit 0 のまま marker が 壊れ、次回 desugar が必ず失敗する。第一成分が .hawk-dist の rel を すべて拒否する。 実装は agmsg 経由で Codex が担当、レビューは cc。 Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c5de88eca
ℹ️ 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".
| # closure 全体が成功してから一括 publish (深い子から、entry が最後) | ||
| for ((i = 0; i < ${#_pub_tmp[@]}; i++)); do | ||
| chmod 644 "${_pub_tmp[$i]}" | ||
| mv "${_pub_tmp[$i]}" "${_pub_out[$i]}" |
There was a problem hiding this comment.
Prevent strict-failed checks from publishing artifacts
When hawk check --strict or hawk emit --strict runs, desugar returns only after this mv has replaced the persistent dist files; the strict gawk --sandbox ... -f "$APP_AWK" validation happens later in the caller. A raw AWK syntax error such as an unmatched { therefore makes the command exit 1 while leaving dist/main.awk updated, so an existing hawk-serve using the same dist can restart into an artifact from a failed check. Keep the staged files unpublished until the caller's strict validation succeeds, or validate strict mode against an isolated dist.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。check / emit は検証・出力が目的で永続 dist を必要としないため、呼び出し元の HAWK_DIST も使わず mktemp の隔離 dist に出力し終了時に削除するようにしています。strict 検証の失敗が稼働中 serve の dist に影響する経路自体がなくなります。テスト check_leaves_no_cwd_dist / emit_leaves_no_cwd_dist を追加しています。
codex レビュー指摘(PR #118)への対応。 check --strict / emit --strict は desugar が dist を publish した後に gawk --sandbox の strict 検証を実行する。検証失敗でも永続 dist は 更新済みになり、同じ dist を使う稼働中の hawk-serve が「失敗した check の成果物」を worker 再起動時に読んでしまう。 check は検証、emit は出力が目的で永続 dist を必要としないため、 呼び出し元の HAWK_DIST も使わず mktemp の隔離 dist に出力し、 終了時に削除する。永続 dist を温めたい場合は hawk-libs desugar を 直接使う。 Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e5211298e
ℹ️ 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".
| # 出力を読んでしまう。呼び出し元の HAWK_DIST も使わず隔離した一時 dist に出す | ||
| HAWK_DIST="$(mktemp -d /tmp/hawk-emit.XXXXXX)" | ||
| export HAWK_DIST | ||
| trap 'rm -rf "$HAWK_DIST"' EXIT |
There was a problem hiding this comment.
Keep emitted include dependencies available
When hawk emit is used on a multi-file app, desugar rewrites local includes to paths under this temporary HAWK_DIST, but the EXIT trap deletes that directory immediately after only the entry file is printed. For example, hawk emit main.awk > out.awk where main.awk includes x.awk produces an out.awk containing @include "/tmp/hawk-emit.../x.awk", and running it later fails because the helper was removed; emit should either output a self-contained closure or keep/rewrite dependencies to stable paths.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。emit は隔離 dist 内のローカル @include を再帰的にインライン展開(重複 include は gawk の dedup と同じく1回だけ)し、外部 include はそのまま残す self-contained な出力にしています。一時 dist の削除後も出力単独で実行できます。テスト emit_multifile_self_contained(2ファイル構成の emit 出力に一時パスの @include が残らず、gawk で実行できること)を追加しています。
| exit 1 | ||
| fi | ||
| fi | ||
| mkdir -p "$(dirname "$out")" |
There was a problem hiding this comment.
Validate symlinked parents before mkdir
When a stale output parent is a symlink and an include targets a missing deeper path such as app/sub/x.awk, this mkdir -p follows the symlink and creates sub/ in the external target before the later physical-path guard rejects the run. Fresh evidence after the symlink-output fix is that rejected desugars still mutate outside HAWK_DIST; validate existing parent components before creating new directories through them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。mkdir -p の前に dist からの相対で親パスの既存成分を1段ずつ検査し、symlink を見つけた時点で(mkdir せずに)exit 1 するようにしています。拒否される desugar が dist 外にディレクトリを作る副作用はなくなります。テスト symlinked_parent_no_external_mkdir(dist/app が外部 symlink の状態で app/sub/x.awk を含む構成が拒否され、外部に sub/ が作られないこと)を追加しています。
一時 dist の include を展開して emit 結果を単独実行可能にし、dist 内の symlink 親を mkdir 前に拒否して外部への副作用を防ぐ。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f40968a03
ℹ️ 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".
| # desugar はローカル include を隔離 dist 内の絶対パスへ書き換えるが、 | ||
| # emit の stdout は一時 dist を trap で削除した後も単独で実行できる必要がある。 | ||
| # dist 内の include だけを再帰展開し、外部 include は gawk に任せてそのまま残す。 | ||
| declare -A EMIT_SEEN=() |
There was a problem hiding this comment.
Avoid associative arrays in hawk emit
When hawk emit runs under the macOS default Bash 3.2 (or any non-supervisor environment still using Bash 3), this declare -A exits with an invalid-option error before emitting anything. docs/setup.md scopes the Bash 4.3+ requirement to the multi-worker supervisor, and libexec/hawk-supervise is the script with an explicit Bash-version guard, so making the single-file emit path require associative arrays breaks that documented lower-Bash use case; use indexed arrays/string tracking here or add a clear version guard and update the requirements.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
対応しました。EMIT_SEEN を declare -A から libexec/hawk-libs の _seen と同じ空白区切り文字列 + case 照合に戻し、bash 4 要件を emit パスに波及させないようにしています。macOS 標準の /bin/bash 3.2.57 で実機検証済みです。
単発 emit に supervisor と同じ Bash 4 要件を波及させないため、include の訪問済み集合を連想配列から既存の文字列照合へ戻す。
Summary
wiki サイト構築プロジェクト段1(spec 機能1・機能2)。段2(templateブランチ)が
@includeでDSL v2ページファイルを読み込む方式に依存するため、その基盤を先に整備する。*nameを(.+)に変換しreq["params:name"]に残りパス全体を渡す。非末尾の*nameは従来どおりリテラル扱い。@includeを再帰的に辿りdist/(HAWK_DISTで上書き可)にソースツリーをミラー出力するよう変更。desugar対象になった@includeパスはdist/接頭の実パスに書き換える(gawk が/を含む@includeパスを AWKPATH 探索しないための対応、実測確認済み)。循環include・存在しないinclude・desugar失敗(exit1+ファイル名)を網羅。dist/が終了後も残るようになったため、hawk-serve/hawk-check/hawk-emit の一時ファイル削除処理(trap rm)を除去。.gitignoreにdist/を追加。Test plan
make test-unit: 581 passed, 0 failedmake test-desugar: 13 passed, 0 failedmake test-cli: 8 passed, 0 failedmake test-e2e: 12 passed, 0 failedmake ci: 全体 PASSKnown follow-ups(別PRで対応予定)
--debugフラグが本PRの trap 削除で no-op 化。docs/cli.md/docs/ja/cli.ja.md/docs/dsl.md/docs/ja/dsl.ja.mdの一時ファイル記述が stale。|| trueのスコープ、変数名in、$out.tmp固定名)🤖 Generated with Claude Code
Summary by CodeRabbit
*name)に対応し、残りパスをparams[name]として取得可能に。*nameはリテラルとして扱うよう整理。desugarの multi-file@includeをより堅牢に処理(循環や不正な書き出しを拒否)。*name)の説明と例を追記。desugarのテストケースとフィクスチャを拡充。test-desugarを追加し、dist/をgitignoreで無視。