feat: insert_after/append_to_section/append + defensive replace check - #151
Open
dvystrcil wants to merge 1 commit into
Open
feat: insert_after/append_to_section/append + defensive replace check#151dvystrcil wants to merge 1 commit into
dvystrcil wants to merge 1 commit into
Conversation
Companion operations to replace_file_content for ADDING content rather than replacing it: - POST /files/insert_after -- insert after a matching anchor line - POST /files/append_to_section -- append at the end of a markdown section, immediately before the next heading at equal or shallower depth (or EOF if none follows) - POST /files/append -- append at end of file Also hardens replace_file_content itself: refuses with a specific, actionable error when target is identical to the first non-empty line of replacement -- the signature of a model trying to ADD a new section but expressing it as a find-and-replace against an anchor it just invented. Without this, the call falls through to a generic 'Target string not found' and the model often concludes the file was truncated and goes into an unproductive re-read loop. Verified end-to-end against the real running server: defensive check correctly refuses and leaves the file untouched; insert_after, append_to_section (including the sub-heading-skip case), and append all produce the expected output; a legitimate replace where target != replacement's first line is unaffected.
dvystrcil
added a commit
to dvystrcil/open-terminal-docker
that referenced
this pull request
Aug 1, 2026
…opy (#67) This repo's Dockerfile has always pulled a pre-built upstream image (FROM .../ghcr-proxy/open-webui/open-terminal:latest) and only added wrapper tooling on top. It never built or installed this repo's own vendored open_terminal/ package -- every fix this CHANGELOG has described as applied to open_terminal/main.py was real, tested, merged code that the running container never actually ran. See homelab#822 for the full incident writeup. Root cause found while investigating why homelab#720's security fix (process-log retention) wasn't live despite being merged days ago. Auditing the vendored package's full history surfaced three more real, undeployed fixes beyond that one, all now submitted upstream: - open-webui/open-terminal#148 -- configurable uvicorn keep-alive timeout (intermittent ConnectionResetError) - open-webui/open-terminal#149 -- process-log retention security fix - open-webui/open-terminal#150 -- two-tier process-result expiry (a slow caller could lose a finished command's result forever) - open-webui/open-terminal#151 -- insert_after/append_to_section/ append endpoints + a defensive replace_file_content check Plus one homelab-specific fix NOT appropriate for upstream (ties into our own GH App token-file convention, not something upstream has any hook for): refresh_github_token_env(), re-reads the current token from disk before every subprocess spawn, closing a gap BASH_ENV-based shell-profile sourcing doesn't cover (plain-shell and PTY spawn paths never source /etc/profile.d). ## What changed - Dockerfile: stage 1 now builds open_terminal from dvystrcil/open-terminal-app-fork (a real fork carrying all 5 fixes above) via git clone + pip install ., mirroring upstream's own Dockerfile exactly, instead of pulling the pre-built upstream image. TEMPORARY -- revert to a plain upstream FROM once all four PRs merge and a release picks them up. - docker.yml: resolves the fork's current commit SHA via `git ls-remote` and passes it as a build-arg on every run. Without this, Docker's build cache (keyed on RUN command text, not on what `git clone --branch main` actually fetches) would silently keep shipping whatever fork commit was cloned the FIRST time this layer built, even after new fixes land on the fork -- caught this empirically: an initial local build without the explicit build-arg produced a stale image missing later fixes despite a fresh fork push. - Removed the vendored open_terminal/ package and its tests -- dead weight now that the real fixes live in a fork with a real upstream relationship. Kept tests/test_actor_env.py (tests helpers/bible_bridge.py, which *is* deployed) and tests/__init__.py. Removed pyproject.toml, dev.sh, .python-version (all specific to developing the now-removed vendored package). - README: documents the new build shape and the incident. ## Testing Built the actual image locally (multi-stage, full apt/pip install, ~2 min), ran it, and verified all 5 fixes are present in the running container (direct imports of the sweep function, keep-alive/expiry env values, refresh_github_token_env, and the three new endpoint handlers) plus the health endpoint and all wrapper tools (kubectl, gh, yq, argocd, act) and the entrypoint's token-refresh profile. Confirmed the FORK_SHA cache-bust actually works: an explicit --build-arg forces a genuine re-clone (visible in build output, not a CACHED layer) rather than silently reusing a stale one. tests/test_actor_env.py still passes standalone (7/7) against the real helpers/bible_bridge.py, unaffected by the removed package.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
I noticed a recurring failure pattern when an LLM agent wants to ADD a new section to a file: it expresses the intent as a
replace_file_contentcall with a made-uptarget(usually the heading it wants to add) and areplacementthat starts with that same text. Since the "target" never existed in the file, this just fails with a generic"Target string not found", and the model often concludes the file was truncated and goes into an unproductive re-read loop rather than realizing it needed a different operation.What this adds
Three companion operations to
replace_file_content, for ADDING rather than replacing:POST /files/insert_after-- insert content on a new line immediately after the first line matching an anchor substring.allow_multipleto insert after every match instead of erroring on ambiguity.POST /files/append_to_section-- find a markdown heading by substring, append content at the end of that section (immediately before the next heading at equal-or-shallower depth, or EOF). Correctly skips past deeper sub-headings within the section.POST /files/append-- append at end of file, handling missing trailing newlines on both sides.Also hardens
replace_file_contentitself to catch the pattern directly: whentargetis identical to the first non-empty line ofreplacement, it refuses with a specific error naming the three ops above, instead of the generic "not found" message.I know adding new endpoints is a bigger ask than a small bug fix -- happy to adjust naming/shape, split this into smaller pieces, or drop the defensive-check part if you'd rather keep
replace_file_contentuntouched. Also totally understand if this is outside what you want in scope; no worries either way.Testing
No test harness exists in this repo currently, so verified end-to-end against the real running server:
insert_afterinserts correctly after the matched anchor.append_to_sectionappends before the next heading, correctly skipping past a sub-heading's body.appendappends with correct newline handling.target != replacement's first line is unaffected by the new check.