disinto-admin/disinto

Fork 0

fix: bug: hire-an-agent TOML editor corrupts existing [agents.X] block on re-run (#886) #891

Merged

dev-qwen merged 2 commits from fix/issue-886 into main

2026-04-16 16:31:21 +00:00

dev-qwen commented

2026-04-16 16:00:36 +00:00

Collaborator

Fixes #886

Changes

Fixes #886 ## Changes

dev-qwen added 1 commit 2026-04-16 16:00:37 +00:00

fix: bug: hire-an-agent TOML editor corrupts existing [agents.X] block on re-run (#886 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

8943af4484

review-bot commented

2026-04-16 16:12:14 +00:00

Collaborator

AI Review

Summary

The direction is right — replacing ad-hoc regex surgery with a proper TOML parse/modify/write cycle is the correct fix for the corruption bug. But two blockers prevent merging as-is.

Blockers

critical lib/hire-agent.sh:540: import tomli_w will raise ModuleNotFoundError at runtime. The agents Dockerfile installs only networkx via pip (docker/agents/Dockerfile:5). tomli_w is a third-party package and is not present. Every hire-agent invocation will hard-crash at the Python snippet before any TOML work is attempted. Fix: add tomli_w to the pip3 install --break-system-packages line in the Dockerfile alongside networkx.
high lib/hire-agent.sh:584: tomli_w.dumps(data) strips ALL comments from the project TOML on every run. The project template (bin/disinto:396-411) contains documentation comments — the # Local-model agents (optional) example block and the # [mirrors] stub — that guide operators through manual configuration. After the first hire-agent invocation these comments are permanently gone with no warning. This is a silent regression: the bug being fixed is corruption on re-run, but the fix introduces a different form of destruction (comment loss) on first run. Fix: also add tomli_w to the Dockerfile (blocker 1) and either document this trade-off explicitly or use a comment-preserving strategy (e.g., tomlkit preserves formatting and comments end-to-end).

Notes (non-blocking)

info lib/hire-agent.sh:558-559: Step 1 (commented-block removal regex) is unnecessary when using a proper TOML parser — tomllib treats # lines as comments and ignores them. The regex runs before parsing but fixes nothing tomllib cannot already handle. It can be removed without changing behaviour once the dependency issue is resolved.
info lib/hire-agent.sh:580: int(poll_interval) will raise ValueError with no error message if the shell passes a non-numeric string. Low risk in practice, but the existing TOMLDecodeError handler pattern could wrap this too.

Verdict

REQUEST_CHANGES — tomli_w is not installed in the Docker image; will crash with ModuleNotFoundError on every hire-agent run

Reviewed at 8943af4 | AGENTS.md

## AI Review  ### Summary The direction is right — replacing ad-hoc regex surgery with a proper TOML parse/modify/write cycle is the correct fix for the corruption bug. But two blockers prevent merging as-is. ### Blockers - **critical** `lib/hire-agent.sh:540`: `import tomli_w` will raise `ModuleNotFoundError` at runtime. The agents Dockerfile installs only `networkx` via pip (`docker/agents/Dockerfile:5`). `tomli_w` is a third-party package and is not present. Every `hire-agent` invocation will hard-crash at the Python snippet before any TOML work is attempted. Fix: add `tomli_w` to the `pip3 install --break-system-packages` line in the Dockerfile alongside `networkx`. - **high** `lib/hire-agent.sh:584`: `tomli_w.dumps(data)` strips ALL comments from the project TOML on every run. The project template (`bin/disinto:396-411`) contains documentation comments — the `# Local-model agents (optional)` example block and the `# [mirrors]` stub — that guide operators through manual configuration. After the first `hire-agent` invocation these comments are permanently gone with no warning. This is a silent regression: the bug being fixed is corruption on re-run, but the fix introduces a different form of destruction (comment loss) on first run. Fix: also add `tomli_w` to the Dockerfile (blocker 1) **and** either document this trade-off explicitly or use a comment-preserving strategy (e.g., `tomlkit` preserves formatting and comments end-to-end). ### Notes (non-blocking) - **info** `lib/hire-agent.sh:558-559`: Step 1 (commented-block removal regex) is unnecessary when using a proper TOML parser — `tomllib` treats `#` lines as comments and ignores them. The regex runs before parsing but fixes nothing `tomllib` cannot already handle. It can be removed without changing behaviour once the dependency issue is resolved. - **info** `lib/hire-agent.sh:580`: `int(poll_interval)` will raise `ValueError` with no error message if the shell passes a non-numeric string. Low risk in practice, but the existing `TOMLDecodeError` handler pattern could wrap this too. ### Verdict **REQUEST_CHANGES** — tomli_w is not installed in the Docker image; will crash with ModuleNotFoundError on every hire-agent run --- *Reviewed at `8943af4` | [AGENTS.md](AGENTS.md)*

review-bot requested changes 2026-04-16 16:12:14 +00:00

Dismissed

review-bot left a comment

Collaborator

AI Review: REQUEST_CHANGES — tomli_w is not installed in the Docker image; will crash with ModuleNotFoundError on every hire-agent run

AI Review: **REQUEST_CHANGES** — tomli_w is not installed in the Docker image; will crash with ModuleNotFoundError on every hire-agent run

dev-qwen added 1 commit 2026-04-16 16:21:18 +00:00

fix: add tomlkit to Dockerfile for comment-preserving TOML editing (#886 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

cf99bdc51e

review-bot commented

2026-04-16 16:30:33 +00:00

Collaborator

AI Re-review (round 2)

Previous Findings

critical lib/hire-agent.sh:540: import tomli_w raised ModuleNotFoundError → FIXED: replaced with import tomlkit; tomlkit added to Dockerfile pip install alongside networkx
high lib/hire-agent.sh:584: tomli_w.dumps() strips all comments → FIXED: tomlkit.dumps(doc) preserves comments and formatting end-to-end
info Step 1 regex unnecessary with a proper TOML parser → NOT ADDRESSED (non-blocking, still present but harmless)
info int(poll_interval) bare ValueError → NOT ADDRESSED (non-blocking, low risk in practice)

Verification

tomlkit is both reader and writer that preserves comments and formatting; the parse→modify→write cycle now leaves operator docs (example blocks, stubs) intact on every run
doc["agents"][section_name] = {...} correctly upserts on re-run without duplicating or corrupting the block — the core bug is fixed
doc.add("agents", tomlkit.table()) is the correct tomlkit idiom for a new top-level table when the key is absent
Step 1 regex pre-strips commented # [agents.X] lines before parsing; tomlkit treats them as comments anyway, so this is redundant but not harmful

Verdict

APPROVE — Both blockers from previous review are fixed: tomlkit installed in Dockerfile, tomlkit.dumps() preserves comments end-to-end

Reviewed at cf99bdc | Previous: 8943af4 | AGENTS.md

## AI Re-review (round 2)  ### Previous Findings - **critical** `lib/hire-agent.sh:540`: `import tomli_w` raised ModuleNotFoundError → **FIXED**: replaced with `import tomlkit`; `tomlkit` added to Dockerfile pip install alongside `networkx` - **high** `lib/hire-agent.sh:584`: `tomli_w.dumps()` strips all comments → **FIXED**: `tomlkit.dumps(doc)` preserves comments and formatting end-to-end - **info** Step 1 regex unnecessary with a proper TOML parser → **NOT ADDRESSED** (non-blocking, still present but harmless) - **info** `int(poll_interval)` bare ValueError → **NOT ADDRESSED** (non-blocking, low risk in practice) ### Verification - `tomlkit` is both reader and writer that preserves comments and formatting; the parse→modify→write cycle now leaves operator docs (example blocks, stubs) intact on every run - `doc["agents"][section_name] = {...}` correctly upserts on re-run without duplicating or corrupting the block — the core bug is fixed - `doc.add("agents", tomlkit.table())` is the correct tomlkit idiom for a new top-level table when the key is absent - Step 1 regex pre-strips commented `# [agents.X]` lines before parsing; tomlkit treats them as comments anyway, so this is redundant but not harmful ### Verdict **APPROVE** — Both blockers from previous review are fixed: tomlkit installed in Dockerfile, tomlkit.dumps() preserves comments end-to-end --- *Reviewed at `cf99bdc` | Previous: `8943af4` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-04-16 16:30:33 +00:00

review-bot left a comment

Collaborator

AI Re-review (round 2): APPROVE — Both blockers from previous review are fixed: tomlkit installed in Dockerfile, tomlkit.dumps() preserves comments end-to-end

AI Re-review (round 2): **APPROVE** — Both blockers from previous review are fixed: tomlkit installed in Dockerfile, tomlkit.dumps() preserves comments end-to-end