fix(skillkit,harness): restore the OpenCode legacy AGENTS.md cleanup dropped by #947's squash; mark nested harness sessions - #1003
Merged
Conversation
Moving the OpenCode target to a native ~/.config/opencode/skills/<name> symlink leaves the marker block older af binaries appended to ~/.config/opencode/AGENTS.md behind forever: uninstallMarkerBlock is no longer reachable for this target, so nothing can remove it. Upgrading users end up with the native skill *and* the stale instructions — the AGENTS.md bloat #813 was actually about. Codex made the same migration in #910 and shipped removeLegacyMarkerBlock for exactly this reason. Install (once the symlink is in place) and Uninstall (per catalog skill) now strip that block. The rules are deliberately stricter than the Codex helper, because the two files are not alike: Codex's AGENTS.override.md was created by af for itself, while ~/.config/opencode/AGENTS.md is written by the user and read by OpenCode. So a file holding no block of ours is never opened for writing — bytes and mtime stay exactly as the user left them — and the file is deleted only when removing our block is what emptied it. Reusing uninstallMarkerBlock verbatim would instead rewrite any AGENTS.md it can read (measured: a user file with no agentfield block goes 23 -> 21 bytes) and delete a deliberately empty one on every install. Other tools' marker blocks and user prose on both sides of ours survive; a missing file is a no-op; read/write failures propagate, matching the target's existing Uninstall error contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…new root TestOpenCodeTargetUninstallRemovesCatalogEntries was the one OpenCode test that did not call withTempHome, so it built and tore down catalog entries in the home shared by the whole package instead of its own. realHomeSnapshot also still only fingerprinted the old ~/.config/opencode/AGENTS.md. Now that OpenCode installs a directory of symlinks, add ~/.config/opencode/skills so the real-home pollution guard covers the path this target actually writes to. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…kage seams removeLegacyMarkerBlock called os.Remove/os.WriteFile/os.Rename directly while every other write path in the package goes through the reconcile* seams (reconcile.go), which exist precisely so a test can force a failure. The consequence was that its "remove", "write" and "rename into" branches could not be exercised at all: six lines that never ran once, and error strings that could ship wrongly wrapped without anything noticing. Switch the four filesystem calls to reconcileReadFile/reconcileRemove/ reconcileWriteFile/reconcileRename and cover each failure through Uninstall, modelled on the reconciler's own rewrite-failure subtests. Also drop legacyRulesPath's error return. It could only fail when TargetPath() fails, and both call sites have already proven TargetPath() succeeds before reaching it — so the branch was unreachable and told a reader about a failure mode that does not exist. It now takes the resolved skills root, which lets Uninstall use the TargetPath() result it was already computing and discarding instead of re-resolving it per skill. No behaviour change: same files read, same files written, same errors returned. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Documents AGENTFIELD_HARNESS_DEPTH and removes unsupported OpenCode tool and permission claims.
Contributor
Performance
⚠ Regression detected:
|
Contributor
📊 Coverage gateThresholds from
✅ Gate passedNo surface regressed past the allowed threshold and the aggregate stayed above the floor. |
Contributor
📐 Patch coverage gateThreshold: 80% on lines this PR touches vs
✅ Patch gate passedEvery surface whose lines were touched by this PR has patch coverage at or above the threshold. |
santoshkumarradha
approved these changes
Aug 28, 2026
santoshkumarradha
left a comment
santoshkumarradha
left a comment
Member
There was a problem hiding this comment.
Tested this locally with go test ./internal/skillkit -count=1 in control-plane plus uv run --project sdk/python --extra dev pytest sdk/python/tests/test_run_cli_env.py -q. The legacy OpenCode cleanup behavior and the nested harness-depth propagation both look consistent across the touched surfaces.
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
2 participants
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.
Summary
Restores code that was reviewed on #947 but never reached
main. The squash commit for #947 (9a14e21e) was built from the 08-25 head, not the enqueued heada2762e14: the four commitsaf606e78,b547f480,3108d3cf,a2762e14—opencodeTarget.legacyRulesPath(),removeLegacyMarkerBlock(), the 346-line cleanup contract test, and thetestmain_test.gohome snapshot — are absent (257 of the PR's 379 added lines). Every machine that ranaf skill installbefore 0.1.136 therefore still carries three<!-- agentfield-skill:… -->blocks in~/.config/opencode/AGENTS.md, which OpenCode loads into every session as always-on rules — including harness sessions AgentField itself spawns. That is the recursion reported on #960, and apermission.skilldeny cannot touch it. The skillkit files on this branch are byte-identical toa2762e14.Deliberately not the codex helper:
~/.config/opencode/AGENTS.mdis user-authored, so the restored cleanup never opens it for writing unless one of our blocks is present (mtime preserved), deletes it only when removing our block emptied it, and is advisory on install / fatal on uninstall.Nested-harness marker. Every CLI harness provider in Python, Go and TypeScript (including the Pi/OMP providers from #913) now sets
AGENTFIELD_HARNESS_DEPTHin the child environment —1, or parent+1 — with a caller-supplied value winning. Nothing else in the environment is removed or altered (whether harness children should be stripped ofAGENTFIELD_SERVER/API key by default is a separate product decision). Both shipped skills gain a note: inside an AgentField harness session, do not dispatch to AgentField agents unless explicitly asked.Docs.
docs/harness-providers.mdcredited OpenCode with native tool/permission flags; no OpenCode provider readstoolsorpermission_modeat all. The table and prose now say what OpenCode actually receives (model, directory, prompt).Validation contract → tests
TestOpenCodeInstallStripsLegacyMarkerBlockAndKeepsForeignContent,TestOpenCodeInstallDeletesLegacyRulesFileItOwnedAlone,TestOpenCodeLeavesARulesFileWithoutOurBlockUntouched,TestOpenCodeUninstallRemovesLinksAndLegacyBlocks,TestOpenCodeCleanupIsANoOpWithoutALegacyRulesFile,TestOpenCodeUninstallReportsAnUnreadableLegacyRulesFile,TestOpenCodeInstallSurvivesALegacyRulesFileItCannotClean,TestOpenCodeUninstallReportsLegacyRewriteFailurestest_run_cli_sets_harness_depth,test_run_cli_caller_harness_depth_wins(py);TestRunCLI/sets_and_increments_harness_depth_while_caller_env_wins(go);sets harness depth from parent %s and caller %s(ts)TestAgentfieldUseEmbeddedFallbackContractMatchesSource,TestSkillCatalogAndEmbeddedMirrorsStayAligned,TestEmbeddedSkillSyncCheckTest plan
cd control-plane && go build ./... && go test ./internal/skillkit/... -count=1cd sdk/python && ruff check . && ./scripts/run_pytest.shcd sdk/go && go mod tidy && go build ./... && go test ./... -count=1cd sdk/typescript && npm ci && npm run build && npm testgofmt -lon touched Go files: cleanRefs #960 #947 #813
🤖 Generated with Claude Code