Skip to content

feat: multi-file desugar と router catch-all を追加 - #118

Merged
redpeacock78 merged 22 commits into
masterfrom
wiki-stage1a-serving
Jul 15, 2026
Merged

redpeacock78 merged 22 commits into
masterfrom
wiki-stage1a-serving

Conversation

@redpeacock78

@redpeacock78 redpeacock78 commented Jul 9, 2026 •

Copy link
Copy Markdown
Owner

Summary

wiki サイト構築プロジェクト段1(spec 機能1・機能2)。段2(templateブランチ)が @include でDSL v2ページファイルを読み込む方式に依存するため、その基盤を先に整備する。

  • router catch-all: 末尾セグメント *name を (.+) に変換し req["params:name"] に残りパス全体を渡す。非末尾の *name は従来どおりリテラル扱い。
  • hawk-libs desugar の multi-file化: entry から @include を再帰的に辿り dist/(HAWK_DIST で上書き可)にソースツリーをミラー出力するよう変更。desugar対象になった @include パスは dist/ 接頭の実パスに書き換える(gawk が / を含む @include パスを AWKPATH 探索しないための対応、実測確認済み)。循環include・存在しないinclude・desugar失敗(exit1+ファイル名)を網羅。
  • desugar出力の永続化: dist/ が終了後も残るようになったため、hawk-serve/hawk-check/hawk-emit の一時ファイル削除処理(trap rm)を除去。.gitignore に dist/ を追加。

Test plan

  • make test-unit: 581 passed, 0 failed
  • make test-desugar: 13 passed, 0 failed
  • make test-cli: 8 passed, 0 failed
  • make test-e2e: 12 passed, 0 failed
  • make ci: 全体 PASS
  • Subagent-driven development(タスク毎レビュー clean + 最終whole-branchレビュー Approve)で実装

Known 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。
  • hawk-libs desugar 実装の Minor 指摘3件(|| true のスコープ、変数名 in、$out.tmp 固定名)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • ルーティングで末尾のキャッチオール(*name)に対応し、残りパスを params[name] として取得可能に。
  • Bug Fixes
    • ルート末尾以外での *name はリテラルとして扱うよう整理。
    • 停止時の終了処理を見直し、一時出力の後始末挙動を変更。
    • desugar の multi-file @include をより堅牢に処理(循環や不正な書き出しを拒否)。
  • Documentation
    • Catch-all parameters(*name)の説明と例を追記。
  • Tests
    • ルータ/desugar のテストケースとフィクスチャを拡充。
  • Chores
    • test-desugar を追加し、dist/ を gitignore で無視。

redpeacock78 and others added 3 commits July 9, 2026 18:42
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>
@coderabbitai

coderabbitai Bot commented Jul 9, 2026 •

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

hawk-libs の desugar を dist/ 配下へ再帰出力する方式に変更し、include、循環、失敗、出力先検証を追加した。関連する実行後処理とテスト基盤も更新した。あわせて router に末尾 catch-all パラメータを追加した。

Changes

Multi-file desugar 出力方式の変更

Layer / File(s) Summary
desugar コア実装の再帰化
libexec/hawk-libs
入力と @include 依存を dist/ 配下へ再帰的に出力し、パス逸脱、alias、循環、アトミック配置を処理する。
生成物のライフサイクル変更
libexec/hawk-check, libexec/hawk-emit, libexec/hawk-serve
生成 AWK の終了時削除処理を削除し、shutdown 時の PID ファイル処理を更新する。
desugar ケース別フィクスチャ
tests/unit/desugar/fixtures/...
循環、失敗、missing include、多階層、alias、escape、単一ファイル、拡張子衝突用のフィクスチャを追加する。
desugar 統合テストと実行 wiring
tests/unit/desugar/run.sh, Makefile, .gitignore
再帰 include、エラー、出力先、scratch、不在入力を検証し、test-desugar と dist/ 無視を追加する。

Router catch-all パラメータ対応

Layer / File(s) Summary
catch-all ルーティング実装
core/router.awk, docs/routing.md
末尾の *name が残りのパス全体を捕捉する処理と仕様説明を追加する。
catch-all テスト wiring
tests/unit/test_router.awk, tests/unit/run.awk
単独、named param 併用、非テール位置のリテラル扱いを検証するテストを追加する。

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 を返す
Loading

Possibly related issues

Possibly related PRs

Poem

ぴょん、と兎は道を駆け
dist/ にファイルをそろえ
include の輪もほどけゆき
*name は道の尾を受け
テストの葉っぱが舞い踊る

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed multi-file desugar と router catch-all という PR の主な変更点を簡潔に示しており、内容とも一致しています。
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between e29b58f and f4d4488.

📒 Files selected for processing (20)
  • .gitignore
  • Makefile
  • core/router.awk
  • docs/routing.md
  • libexec/hawk-check
  • libexec/hawk-emit
  • libexec/hawk-libs
  • libexec/hawk-serve
  • tests/unit/desugar/fixtures/cyc/a.awk
  • tests/unit/desugar/fixtures/cyc/b.awk
  • tests/unit/desugar/fixtures/fail/bad.awk
  • tests/unit/desugar/fixtures/fail/main.awk
  • tests/unit/desugar/fixtures/missing/main.awk
  • tests/unit/desugar/fixtures/proj/app/page.awk
  • tests/unit/desugar/fixtures/proj/app/sub/util.awk
  • tests/unit/desugar/fixtures/proj/main.awk
  • tests/unit/desugar/fixtures/solo.awk
  • tests/unit/desugar/run.sh
  • tests/unit/run.awk
  • tests/unit/test_router.awk
💤 Files with no reviewable changes (3)
  • libexec/hawk-check
  • libexec/hawk-emit
  • libexec/hawk-serve

Comment thread libexec/hawk-libs Outdated

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

Copy link
Copy Markdown

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: 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".

Comment thread libexec/hawk-libs
case "$_seen" in *" $rel "*) return 0 ;; esac
_seen="$_seen$rel "
in="$src_root/$rel"
out="$dist/$rel"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。.. を含む include パスは dist の外に出るため、desugar 失敗として exit 1 + エラーメッセージにしています(黙ってスキップすると実行時に壊れた @include が残るため、失敗させる方針にしました)。テストケース parent_include_escape_rejected / parent_include_no_file_written_outside_dist を追加しています。

Comment thread libexec/hawk-libs Outdated
# dsl/desugar.awk 自身が @include "dsl/util.awk" 等の相対パスを持ち、
# gawk は '/' を含む @include をカレントディレクトリ相対で解決するため、
# HAWK_LIB (リポジトリルート) に cd してから実行する。
if ! ( cd "$HAWK_LIB" && gawk -f dsl/desugar.awk "$in" ) > "$out"; then

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

調査しました。現状の「ファイルごとに個別 gawk プロセスで desugar」というアーキテクチャを覆す規模の変更が必要と判明したため、このPRのスコープからは切り出し、#119 に別issueとして起票しました。wiki 段2(templateブランチ)着手前に対応予定です。

- @include に '..' が含まれ dist の外へ書き込まれうる問題を修正。
  検出時は desugar 失敗として exit 1(P1, codex 指摘)
- @include パス書き換えの gawk 呼び出し失敗時にエラーメッセージなしで
  黙って続行していた箇所を明示的な exit 1 + ファイル名表示に変更
  (coderabbit 指摘)

複数ファイルにまたがる型宣言共有の課題(P2)は #119 に切り出し。

Co-Authored-By: Claude <noreply@anthropic.com>

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

Copy link
Copy Markdown

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: 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".

Comment thread libexec/hawk-libs Outdated
# dsl/desugar.awk 自身が @include "dsl/util.awk" 等の相対パスを持ち、
# gawk は '/' を含む @include をカレントディレクトリ相対で解決するため、
# HAWK_LIB (リポジトリルート) に cd してから実行する。
if ! ( cd "$HAWK_LIB" && gawk -f dsl/desugar.awk "$in" ) > "$out"; then

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。dist を絶対パスに正規化した上で、in/out が同一パスに解決する(HAWK_DIST が source ツリーにエイリアスする)ケースを検出し、desugar 失敗として exit 1 にしています。テストケース alias_dist_rejected / alias_dist_source_untouched(source が改変されていないことを確認)を追加しています。

Comment thread libexec/hawk-libs
return 0
}

entry_rel="$(basename "$src")"

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

これは 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>

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

Copy link
Copy Markdown

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: 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".

Comment thread libexec/hawk-libs Outdated
_seen="$_seen$rel "
in="$src_root/$rel"
out="$dist/$rel"
if [[ "$in" == "$dist_abs/$rel" ]]; then

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。dist_abs / src_root の正規化を pwd -P(物理パス)に変更し、symlink 経由のエイリアスも検出するようにしています。テストケース alias_dist_symlink_rejected / alias_dist_symlink_source_untouched を追加しています。

Comment thread libexec/hawk-libs Outdated
if [[ -n "$incs" ]]; then
# gawk は '/' を含む @include パスを AWKPATH 探索しないため、
# dist 接頭の実パスに書き換える
tmp="$out.tmp"

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。scratch を固定名 $out.tmp から mktemp(.hawk-desugar.XXXXXX、出力先と同じディレクトリ)に変更しています。a.awk.tmp と a.awk を両方 include するフィクスチャ(tmpcol/)で、書き換え後も dist/a.awk.tmp が生存することを検証するテスト tmp_suffix_include_survives_rewrite を追加しています。

Comment thread libexec/hawk-libs Outdated
# dsl/desugar.awk 自身が @include "dsl/util.awk" 等の相対パスを持ち、
# gawk は '/' を含む @include をカレントディレクトリ相対で解決するため、
# HAWK_LIB (リポジトリルート) に cd してから実行する。
if ! ( cd "$HAWK_LIB" && gawk -f dsl/desugar.awk "$in" ) > "$out"; then

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。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>

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

Copy link
Copy Markdown

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: 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".

Comment thread libexec/hawk-libs Outdated
mv "$tmp2" "$tmp"
fi
chmod 644 "$tmp"
mv "$tmp" "$out"

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。子 include を先に再帰 desugar し、全て成功してから親を mv で publish する post-order 構造に変更しています。子の失敗時は親の scratch を EXIT trap で掃除し、既存の dist 成果物は旧状態のまま残ります。テスト parent_not_published_on_child_failure / no_scratch_leftovers_on_failure を追加しています。

Comment thread libexec/hawk-libs
# src_root 相対で実在する @include を dist 接頭に書き換えて再帰する。
_desugar_one() {
local rel="$1" in out tmp inc incs
case "$_seen" in *" $rel "*) return 0 ;; esac

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。実測で確認したところ、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)を検証しています。

Comment thread libexec/hawk-libs Outdated
_seen="$_seen$rel "
in="$src_root/$rel"
out="$dist/$rel"
if [[ "$in" == "$dist_abs/$rel" ]]; then

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。エイリアス検査を文字列比較から 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>

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

Copy link
Copy Markdown

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: 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".

Comment thread libexec/hawk-libs Outdated
# "cannot include `X' and use it as a program file" で fatal になるため
# desugar 時点で拒否する (entry を経由しない循環は gawk が重複 include を
# スキップするので許容)
if [[ "$inc" == "$entry_rel" ]]; then

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。desugar 直後に @include パスの ./(先頭の ./ と中間の /./)を正規化するパスを追加しています。これにより entry 循環検査・_seen 重複排除・dist 書き換えの対応表がすべて "./a.awk" と "a.awk" を同一視します。テスト entry_cycle_dotslash_rejected(./ 経由の back-edge 拒否)と dotslash_include_normalized / dotslash_tree_runs(正常な ./x.awk include の正規化 + 実行)を追加しています。

Comment thread libexec/hawk-libs
chmod 644 "$tmp"
mv "$tmp" "$out"
return 0
}

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。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>

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

Copy link
Copy Markdown

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: 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".

Comment thread libexec/hawk-libs Outdated
while read -r t o; do
[[ -n "$t" ]] || continue
chmod 644 "$t"
mv "$t" "$o"

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。staging 時(publish 前)に出力先がディレクトリの場合を検出して exit 1 にしています(publish ループ内でなく staging 時に検査することで、一括 publish の原子性も保っています)。テスト dir_output_rejected を追加しています。

Comment thread libexec/hawk-libs
out="$dist/$rel"
# -ef は device/inode 比較。symlink 経由 (HAWK_DIST・source どちら側でも)
# の source エイリアスを文字列比較より確実に検出する
if [[ -e "$out" && "$in" -ef "$out" ]]; then

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。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 を追加しています。

Comment thread libexec/hawk-libs
{
if (match($0, /^([[:space:]]*@include[[:space:]]+")([^"]+)(".*)$/, m)) {
p = m[2]
sub(/^(\.\/)+/, "", p)

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。./ の正規化を「正規化後のパスが src_root 相対で実在するファイルの場合」に限定しています。実在しない ./helper.awk は元の形のまま残るため、AWKPATH に落ちる挙動変化は起きません。テスト unresolved_dotslash_kept(実在しない ./nosuch.awk が書き換えられずそのまま残ること)を追加しています。

Comment thread libexec/hawk-libs Outdated
# "cannot include `X' and use it as a program file" で fatal になるため
# desugar 時点で拒否する (entry を経由しない循環は gawk が重複 include を
# スキップするので許容)
if [[ "$inc" == "$entry_rel" ]]; then

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。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>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 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

📥 Commits

Reviewing files that changed from the base of the PR and between e197ba4 and fe177c0.

📒 Files selected for processing (16)
  • libexec/hawk-libs
  • tests/unit/desugar/fixtures/cyc2/main.awk
  • tests/unit/desugar/fixtures/cyc2/x.awk
  • tests/unit/desugar/fixtures/cyc2/y.awk
  • tests/unit/desugar/fixtures/cyc3/a.awk
  • tests/unit/desugar/fixtures/cyc3/b.awk
  • tests/unit/desugar/fixtures/dotmiss/main.awk
  • tests/unit/desugar/fixtures/dotslash/main.awk
  • tests/unit/desugar/fixtures/dotslash/x.awk
  • tests/unit/desugar/fixtures/sib/a_good.awk
  • tests/unit/desugar/fixtures/sib/main.awk
  • tests/unit/desugar/fixtures/sib/z_bad.awk
  • tests/unit/desugar/fixtures/symcyc/a.awk
  • tests/unit/desugar/fixtures/symcyc/b.awk
  • tests/unit/desugar/fixtures/symsrc/srcdir/main.awk
  • tests/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

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

Copy link
Copy Markdown

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: 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".

Comment thread libexec/hawk-libs Outdated
Comment on lines +193 to +196
while read -r t o; do
[[ -n "$t" ]] || continue
chmod 644 "$t"
mv "$t" "$o"

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。publish キューを空白区切り文字列から bash 配列(_pub_tmp / _pub_out の並行配列)に変更しています。テスト spaced_dist_publishes(空白入り HAWK_DIST での多段 include publish)を追加しています。

Comment thread libexec/hawk-libs
# src_root 相対で実在する @include を dist 接頭に書き換えて再帰する。
_desugar_one() {
local rel="$1" in out tmp inc incs
case "$_seen" in *" $rel "*) return 0 ;; esac

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。coalesce(別名を正規パスに書き換えて1成果物に集約)ではなく、同一実体の別名 include を -ef で検出して exit 1 で拒否する方針にしています。app 内で同じファイルを symlink 別名で二重 include する構成は正当な用途が考えにくく、静かに二重定義になるより明示的に失敗させる方が安全と判断しました。テスト alias_include_rejected(x.awk + alias.awk -> x.awk の両 include で拒否)を追加しています。

Comment thread libexec/hawk-libs Outdated
while read -r t o; do
[[ -n "$t" ]] || continue
chmod 644 "$t"
mv "$t" "$o"

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

これは 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>

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

Copy link
Copy Markdown

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: 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".

Comment thread libexec/hawk-libs
# src_root 相対で実在する @include を dist 接頭に書き換えて再帰する。
_desugar_one() {
local rel="$1" in out tmp inc incs prior
case "$_seen" in *" $rel "*) return 0 ;; esac

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。空白入り include パスは include path contains spaces (unsupported) で staging 時に exit 1 にしています(bash 3 互換で連想配列を使わない設計のため、サポートではなく明示的拒否を選びました。黙って欠けた dist を作るより安全です)。テスト spaced_include_rejected / spaced_include_publishes_nothing を追加しています。

Comment thread libexec/hawk-libs Outdated
chmod 644 "${_pub_tmp[$i]}"
mv "${_pub_tmp[$i]}" "${_pub_out[$i]}"
done
: > "$dist/.hawk-dist"

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。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>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between fe177c0 and a722a0b.

📒 Files selected for processing (6)
  • libexec/hawk-libs
  • tests/unit/desugar/fixtures/aliasinc/main.awk
  • tests/unit/desugar/fixtures/aliasinc/x.awk
  • tests/unit/desugar/fixtures/spacename/main.awk
  • tests/unit/desugar/fixtures/spacename/x y.awk
  • tests/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

Comment thread tests/unit/desugar/run.sh
他の失敗系テストと同様の publishes_nothing チェックが
alias_include_rejected にだけ欠けていた (coderabbit 指摘)。

Co-Authored-By: Claude <noreply@anthropic.com>

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

Copy link
Copy Markdown

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: 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".

Comment thread libexec/hawk-libs Outdated
Comment on lines +217 to +221
for ((i = 0; i < ${#_pub_tmp[@]}; i++)); do
chmod 644 "${_pub_tmp[$i]}"
mv "${_pub_tmp[$i]}" "${_pub_out[$i]}"
done
: > "$dist/.hawk-dist"

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。marker が既存で書込不可の場合を staging 前(desugar 開始時)に検証して exit 1 にしています。テスト unwritable_marker_rejected / unwritable_marker_publishes_nothing(既存 dist の成果物が無変更で残ること)を追加しています。

Comment thread libexec/hawk-libs Outdated
Comment on lines +124 to +126
_scratches="$_scratches $tmp"
# shellcheck disable=SC2064
trap "rm -f$_scratches" EXIT

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。scratch の管理を空白区切り文字列 + 文字列 trap から、配列 _scratch_arr + quote 安全な cleanup 関数(trap _cleanup_scratches EXIT)に変更しています。テスト spaced_dist_no_scratch_leftovers(空白入り HAWK_DIST でも scratch が正しく処理されること)を追加しています。

Comment thread libexec/hawk-libs Outdated
mkdir -p "$dist"
# marker が regular file 以外だと publish 後の作成で失敗し
# 「exit 1 なのに成果物は更新済み」になるため、staging 前に検証する
if [[ -e "$dist/.hawk-dist" && ! -f "$dist/.hawk-dist" ]]; then

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 Reject symlinked dist markers

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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。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>

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

Copy link
Copy Markdown

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: 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".

Comment thread tests/unit/desugar/run.sh Outdated
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"

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。root(uid 0)では chmod a-w が -w 判定にも truncate 阻止にも効かないため、unwritable_marker_rejected / unwritable_marker_publishes_nothing を uid 0 のとき SKIP(PASS/FAIL どちらにもカウントしない)にしています。

Comment thread libexec/hawk-libs Outdated
# 過去の desugar が作った dist であることを .hawk-dist マーカーで確認する。
# マーカーのないディレクトリ内の既存ファイルは source とみなし上書きしない
# (HAWK_DIST が source 木の中を指すと -ef では検出できない別ファイルを潰すため)
if [[ -e "$out" && ! -e "$dist/.hawk-dist" ]]; then

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。.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 で検証しています。

Comment thread libexec/hawk-libs
# 同一実体 (symlink/hardlink) を別名で include すると、mirror 後は
# 別 inode になり gawk の重複 include 排除が効かず定義が二重実行される。
# 別名での include は拒否する
for prior in $_seen; do

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。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>

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

Copy link
Copy Markdown

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: 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".

Comment thread libexec/hawk-libs
[[ -f "$dist/.hawk-dist" ]] && cat "$dist/.hawk-dist"
printf '%s\n' "${_pub_rel[@]}"
} | sort -u > "$_marker_tmp"
mv "$_marker_tmp" "$dist/.hawk-dist"

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 Reserve the dist marker name

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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。.hawk-dist を予約名とし、この名前の include を staging 時に exit 1 で拒否しています。テスト marker_name_include_rejected を追加しています。

Comment thread libexec/hawk-libs
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'

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。絶対パス 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>

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

Copy link
Copy Markdown

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: 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".

Comment thread libexec/hawk-libs
Comment on lines +270 to +273
{
[[ -f "$dist/.hawk-dist" ]] && cat "$dist/.hawk-dist"
printf '%s\n' "${_pub_rel[@]}"
} | sort -u > "$_marker_tmp"

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。marker が既存の場合の staging 前検証に -r(読込可能)を追加し、読めない marker は publish 前に exit 1 にしています。テスト unreadable_marker_rejected(root では chmod が効かないため uid 0 時は SKIP)を追加しています。

Comment thread libexec/hawk-libs Outdated
# 潰すため。marker を空ファイルにするだけだと最初の成功後はディレクトリ全体を
# 信頼してしまい、無関係な source ファイルまで上書き対象になる)
if [[ -e "$out" ]]; then
if [[ ! -f "$dist/.hawk-dist" ]] || ! grep -qxF "$rel" "$dist/.hawk-dist"; then

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。grep -qxF -- "$rel" に変更しています。テスト dash_prefixed_entry_rerunnable(-main.awk という entry の2回目の desugar が成功すること)を追加しています。

Comment thread libexec/hawk-libs Outdated
# プロジェクト内は相対パスに限定し、外部の絶対パス (共有ライブラリ等) は
# 従来どおり素通りで残す
if [[ "$inc" == /* ]]; then
if [[ -f "$inc" && "$(cd "$(dirname "$inc")" && pwd -P)/" == "$src_root/"* ]]; then

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。絶対パス include の外部判定で、ディレクトリの正規化に加えてファイル自体の symlink チェーンを readlink ループ(最大40回、超過は symlink loop として exit 1)で解決してから src_root 配下かを判定するようにしています。テスト abs_symlink_include_to_project_rejected(/tmp/outside/alias.awk -> project/x.awk 型のすり抜けを拒否)を追加しています。

Comment thread libexec/hawk-libs Outdated
# 上書きしない (HAWK_DIST が source 木の中を指すと -ef では検出できない別ファイルを
# 潰すため。marker を空ファイルにするだけだと最初の成功後はディレクトリ全体を
# 信頼してしまい、無関係な source ファイルまで上書き対象になる)
if [[ -e "$out" ]]; then

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。上書き保護の存在判定を [[ -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>

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

Copy link
Copy Markdown

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: 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".

Comment thread libexec/hawk-libs
Comment on lines +235 to +239
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

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。symlink チェーン解決 + パス比較に加えて、訪問済みプロジェクトファイルとの -ef(device/inode 比較)を追加し、hard link 経由の同一実体も検出して exit 1 にしています。テスト abs_hardlink_include_to_project_rejected(x.awk が自身への hard link を絶対パスで include するケース)を追加しています。

Comment thread libexec/hawk-libs
# dsl/desugar.awk 自身が @include "dsl/util.awk" 等の相対パスを持ち、
# gawk は '/' を含む @include をカレントディレクトリ相対で解決するため、
# HAWK_LIB (リポジトリルート) に cd してから実行する。
if ! ( cd "$HAWK_LIB" && gawk -f dsl/desugar.awk "$in" ) > "$tmp"; then

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

これは #119 の課題1(型宣言のファイル跨ぎ共有)と同根の「ファイルごとに個別 gawk プロセスで desugar / check する」設計制約のため、#119 に補足として追記しました。課題1の解決方式(include closure の一括 compile または宣言の事前抽出 + 注入)で関数シグネチャの収集も同時に扱います。wiki 段2着手前に対応予定です。

codex レビュー追撃(PR #118)への対応。

hard link は readlink で辿れずパス名も外部のままなので、symlink
チェーン解決 + パス prefix 比較では検出できない。訪問済み
プロジェクトファイルとの device/inode 比較 (-ef) を追加し、
mirror 後に原本 (未 desugar) が二重ロードされるのを防ぐ。

関数シグネチャのファイル跨ぎ参照 (checker が unknown function に
なる問題) は型宣言共有と同根の設計制約のため #119 に追記。

Co-Authored-By: Claude <noreply@anthropic.com>

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

Copy link
Copy Markdown

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: 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".

Comment thread libexec/hawk-libs Outdated
Comment on lines +244 to +245
for prior in $_seen; do
if [[ "$inc" -ef "$src_root/$prior" ]]; then

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。絶対パス 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 も引き続き通っています。

Comment thread libexec/hawk-libs
return 0
}

entry_rel="$(basename "$src")"

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。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 は引き続き成功します。

Comment thread libexec/hawk-libs Outdated
exit 1
fi
fi
mkdir -p "$(dirname "$out")"

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。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>

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

Copy link
Copy Markdown

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: 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".

Comment thread libexec/hawk-libs Outdated
echo "[hawk-libs] desugar failed: include path contains spaces (unsupported): $rel" >&2
exit 1
;;
.hawk-dist)

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。予約名チェックを .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>

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

Copy link
Copy Markdown

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: 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".

Comment thread libexec/hawk-libs
# closure 全体が成功してから一括 publish (深い子から、entry が最後)
for ((i = 0; i < ${#_pub_tmp[@]}; i++)); do
chmod 644 "${_pub_tmp[$i]}"
mv "${_pub_tmp[$i]}" "${_pub_out[$i]}"

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。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>

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

Copy link
Copy Markdown

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: 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".

Comment thread libexec/hawk-emit
# 出力を読んでしまう。呼び出し元の HAWK_DIST も使わず隔離した一時 dist に出す
HAWK_DIST="$(mktemp -d /tmp/hawk-emit.XXXXXX)"
export HAWK_DIST
trap 'rm -rf "$HAWK_DIST"' EXIT

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。emit は隔離 dist 内のローカル @include を再帰的にインライン展開(重複 include は gawk の dedup と同じく1回だけ)し、外部 include はそのまま残す self-contained な出力にしています。一時 dist の削除後も出力単独で実行できます。テスト emit_multifile_self_contained(2ファイル構成の emit 出力に一時パスの @include が残らず、gawk で実行できること)を追加しています。

Comment thread libexec/hawk-libs Outdated
exit 1
fi
fi
mkdir -p "$(dirname "$out")"

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。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 前に拒否して外部への副作用を防ぐ。

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

Copy link
Copy Markdown

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: 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".

Comment thread libexec/hawk-emit Outdated
# desugar はローカル include を隔離 dist 内の絶対パスへ書き換えるが、
# emit の stdout は一時 dist を trap で削除した後も単独で実行できる必要がある。
# dist 内の include だけを再帰展開し、外部 include は gawk に任せてそのまま残す。
declare -A EMIT_SEEN=()

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

対応しました。EMIT_SEEN を declare -A から libexec/hawk-libs の _seen と同じ空白区切り文字列 + case 照合に戻し、bash 4 要件を emit パスに波及させないようにしています。macOS 標準の /bin/bash 3.2.57 で実機検証済みです。

単発 emit に supervisor と同じ Bash 4 要件を波及させないため、include の訪問済み集合を連想配列から既存の文字列照合へ戻す。
@redpeacock78
redpeacock78 merged commit a619766 into master Jul 15, 2026
5 checks passed
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