Remove post-hook feature and move post-hook to mkosi recipe

[canercidam]

The mkosi recipe can now take in the builder registration responsibility off the playground code.

This helps us remove the complex and unncessary post-hook feature and get rid of an orchestration problem about host dependencies. It is feasible for future to think of and plan ways to orchestrate containers and host services (binaries, custom services with lifecycle hooks) together, with cross-dependencies at start, but the post-hook feature gets in the way of further improvements (observed from a recent personal experiment).

[claude]

Claude finished @canercidam's task in 2m 39s —— View job

---

PR Review: Remove post-hook feature and move post-hook to mkosi recipe

• Gather context and read changed files
• Review code changes
• Post review feedback

---

The Go-side cleanup is clean — the post-hook abstraction, registration logic, and related types are removed consistently. A few issues in the new shell script and some leftover dead code in Go:

Issues flagged inline:

1. Potential bug — wrong port field (register-builder.sh:48-51): The script reads .port from the manifest JSON, which is the container port. The old Go code used .HostPort (the host-mapped port). If these differ, the script will fail to connect.

2. Dead code — $? check after set -e (register-builder.sh:26-30): The yq failure will exit the script before $? is ever checked.

3. Hardcoded IP vs variable (register-builder.sh:97): LIBP2P_ADDR hardcodes 10.0.2.2 instead of using the $HOST_IP variable defined at the top.

4. Misleading comment (register-builder.sh:10): Says "default to 127.0.0.1" but actual default is 10.0.2.2.

5. Fragile sed on JSON (register-builder.sh:127-128): Template replacement via sed on JSON can corrupt output if values contain & or other metacharacters. jq would be safer.

Not inline (lines outside diff):

6. Dead code — BuilderHostIPAddress (recipe_buildernet.go:9): This constant is no longer referenced after removing registerBuilder. Should be deleted.

7. Dead code — BuilderIP/BuilderConfig fields (components.go:936-937): BuilderHub.BuilderIP and BuilderHub.BuilderConfig are now unused since the registration logic moved to the shell script. The corresponding CLI flags (--builder-ip, --builder-config in recipe_buildernet.go:33-34) are also dead.

8. Missing dependency check: The script requires yq and jq at runtime but doesn't verify their presence. A quick check at the top would give a clearer error than a cryptic "command not found".

[claude]

Claude finished @canercidam's task in 1m 24s —— View job

---

PR Review: Remove post-hook feature and move post-hook to mkosi recipe

• Gather context and read changed files
• Review code changes (re-review after 3 follow-up commits)
• Post review feedback

---

Good progress — the follow-up commits addressed three of the original issues (misleading comment, dead $? check, hardcoded IP). The author explicitly declined the .port vs .HostPort suggestion and the sed fragility concern, so those are settled.

Remaining dead code that should be cleaned up:

1. BuilderHostIPAddress constant (playground/recipe_buildernet.go:9): No longer referenced anywhere after removing registerBuilder. Should be deleted.

2. BuilderHub.BuilderIP / BuilderHub.BuilderConfig fields (playground/components.go:936-937): These struct fields are set in recipe_buildernet.go:49-50 but never read inside BuilderHub.Apply() — the registration logic that consumed them is now in the shell script. The corresponding CLI flags --builder-ip and --builder-config (recipe_buildernet.go:33-34) and their backing fields (recipe_buildernet.go:19-20) are also dead. The auto-generated docs (docs/recipes/buildernet.md:8-9) reference these flags too.

   Fix this →

These are minor but worth cleaning up to avoid confusion — users will see --builder-ip and --builder-config flags that do nothing.

---

[claude]

Claude finished @canercidam's task in 1m 20s —— View job

---

PR Review: Remove post-hook feature and move post-hook to mkosi recipe

• Gather context and read changed files
• Review code changes
• Post review feedback

---

The latest push addresses most of the shell script issues from the previous review (comment fix, $? dead code, HOST_IP variable usage). The Go-side removal of the post-hook feature is clean.

Remaining dead code in Go (carried over from previous review, not yet addressed):

1. BuilderHostIPAddress constant (recipe_buildernet.go:9): No longer referenced anywhere after removing registerBuilder. Should be deleted.

2. BuilderHub.BuilderIP and BuilderHub.BuilderConfig fields (components.go:936-937): These fields are set in recipe_buildernet.go:49-50 but never read inside BuilderHub.Apply() — the registration logic that used them was removed. The corresponding CLI flags --builder-ip and --builder-config (recipe_buildernet.go:33-34) are also dead. All four can be removed.

Fix this →

---

[claude]

Claude finished @canercidam's task in 1m 30s —— View job

---

PR Review: Remove post-hook feature and move post-hook to mkosi recipe

• Gather context and read changed files
• Review code changes
• Post review feedback

---

Good cleanup overall — the post-hook abstraction is removed cleanly from the Go side and the registration logic is faithfully ported to the shell script. I see the latest commits addressed several issues from the prior review (comment fix, dead $? check, hardcoded IP).

Remaining issues:

1. Dead code — BuilderHostIPAddress constant (recipe_buildernet.go:9): No longer referenced anywhere after removing registerBuilder. Should be deleted.

2. Dead code — BuilderHub.BuilderIP / BuilderHub.BuilderConfig fields (components.go:936-937): These fields are set in recipe_buildernet.go:49-50 but never read inside BuilderHub.Apply() — the registration logic that consumed them is gone. The corresponding CLI flags --builder-ip and --builder-config (recipe_buildernet.go:33-34) and the struct fields on BuilderNetRecipe (recipe_buildernet.go:18-19) are also dead. This should all be cleaned up.

3. Hardcoded port 9001 in libp2p address (register-builder.sh:96): The TCP port is hardcoded to 9001. If the beacon's p2p port ever changes in the component definition, this will silently break. Consider extracting it from the manifest or the identity response.

Fix dead code →

[claude]

Claude finished @canercidam's task in 2m 0s —— View job

---

PR Review: Remove post-hook feature and move post-hook to mkosi recipe

• Gather context and read changed files
• Review code changes (after latest remove dead code commit)
• Post review feedback

---

The latest commit (15be5c1 remove dead code) addressed all the dead code issues from the previous reviews — BuilderHostIPAddress, BuilderHub.BuilderIP/BuilderConfig, and the corresponding flags are all removed. The Go-side cleanup is now clean and complete.

No new issues found. The post-hook abstraction, registration logic, and all related types/fields/flags are removed consistently. The shell script correctly ports the registration flow. The goccy/go-yaml import removal from components.go and recipe_buildernet.go is correct (still used in recipe_yaml.go, custom_recipes.go, and tests).

Pre-existing issue (not introduced by this PR): The mount validation loop in manifest.go:351-377 is duplicated — the same "validate that the mounts are correct" block appears twice consecutively. Not a blocker for this PR.

---

[claude]

Claude finished @canercidam's task in 1m 28s —— View job

---

PR Review: Remove post-hook feature and move post-hook to mkosi recipe

• Gather context and read changed files
• Review code changes (after latest commits including dead code cleanup)
• Post review feedback

---

The Go-side cleanup is now complete — all dead code (BuilderHostIPAddress, BuilderHub.BuilderIP/BuilderConfig, unused flags, postHook type, registerBuilderHook) has been removed. The post-hook abstraction is cleanly excised from manifest.go, components.go, recipe_buildernet.go, and main.go. No dangling references remain.

The shell script faithfully ports the registration flow and the previous review issues (misleading comment, dead $? check, hardcoded IP) were addressed in follow-up commits.

No new issues introduced by this PR.

Pre-existing issue (not introduced by this PR): The mount validation loop in manifest.go:351-377 is duplicated — the identical "validate that the mounts are correct" block appears twice consecutively. Worth a separate cleanup.

---