fix(skillkit): install OpenCode skills via native symlinks

[mikemikimike]

Problem

OpenCode users currently receive AgentField skills by appending a marker block to ~/.config/opencode/AGENTS.md. This injects the full skill guidance into every OpenCode session and does not use OpenCode's native skill discovery.

Fix

Fixes #813

• install newly selected skills as ~/.config/opencode/skills/<name> symlinked to the canonical ~/.agentfield/skills/<name>/current directory;
• report symlink installation and target version through af skill list status;
• remove the installed skill links during uninstall;
• add deterministic install/uninstall coverage and update existing target-path contracts.

This fixes the installation path for new installs and updates. It does not attempt to
migrate or remove marker blocks written by older versions from AGENTS.md; that is kept
out of this focused change to avoid deleting user-authored OpenCode instructions.

Validation

• go test ./internal/skillkit -run 'TestOpenCodeTargetInstallsSkillSymlink|TestTargetSpecificEdgeCases/marker_targets_report_empty_status_after_plain_file_uninstall' -count=1 — passed
• go vet ./internal/skillkit — passed
• git diff --check — passed

The focused tests verify the actual skillkit install/uninstall path and filesystem
symlink target, without requiring a real OpenCode process or credentials. A real
OpenCode CLI end-to-end check was not run. The full go test ./internal/skillkit run
is currently affected by pre-existing Windows test-environment assumptions around
HOME/USERPROFILE and embedded path separators; the changed OpenCode tests pass
independently.

Known limitations

The replacement currently removes an existing target entry before creating the new
symlink, so installation is not crash-atomic. The target is expected to be an
AgentField-managed skill path; protecting arbitrary user content at that exact path
and adding failure-injection coverage are follow-up hardening work.

AI disclosure

This PR was prepared with GPT-5 Codex. The implementation was reviewed against the existing Claude Code symlink lifecycle and the OpenCode target tests.

[santoshkumarradha]

Thanks for pushing this. I reproduced the local go test ./internal/skillkit -count=1 failure, and there are two concrete regressions to fix before this is ready: the OpenCode uninstall path now hides missing-home errors, and the status path reports current instead of the installed skill version.

[github-actions]

📊 Coverage gate

Thresholds from .coverage-gate.toml: per-surface ≥ 84%, aggregate ≥ 85%, max per-surface regression ≤ 1.0 pp, max aggregate regression ≤ 0.50 pp.

Surface	Current	Baseline	Δ
control-plane	87.30%	87.40%	↓ -0.10 pp	🟡
sdk-go	92.90%	92.00%	↑ +0.90 pp	🟢
sdk-python	94.21%	93.73%	↑ +0.48 pp	🟢
sdk-typescript	91.39%	90.42%	↑ +0.97 pp	🟢
web-ui	84.76%	84.79%	↓ -0.03 pp	🟡
aggregate	85.70%	85.75%	↓ -0.05 pp	🟡

✅ Gate passed

No surface regressed past the allowed threshold and the aggregate stayed above the floor.

[github-actions]

📐 Patch coverage gate

Threshold: 80% on lines this PR touches vs origin/main (from .coverage-gate.toml:thresholds.min_patch).

Surface	Touched lines	Patch coverage	Status
control-plane	0	—	➖ no changes
sdk-go	0	—	➖ no changes
sdk-python	0	—	➖ no changes
sdk-typescript	0	—	➖ no changes
web-ui	0	—	➖ no changes

✅ Patch gate passed

Every surface whose lines were touched by this PR has patch coverage at or above the threshold.

[santoshkumarradha]

Thanks for addressing the two regressions I called out earlier. I revalidated the OpenCode install/status paths locally and the updated symlink version handling looks right now.

[AbirAbbas]

Thanks — the symlink move is the right call and the target itself looks good. I've pushed four commits on top rather than block on a review round.

You wrote that the legacy AGENTS.md migration was left out "to avoid deleting user-authored OpenCode instructions" — that caution is exactly right, and it's why the Codex helper couldn't just be reused here. But leaving it out means the block can now never be removed, so I've added a migration built around that constraint instead.

af606e78 — strip the legacy ~/.config/opencode/AGENTS.md block

Switching Method() to symlink makes uninstallMarkerBlock unreachable for this target, so nothing can ever remove the block older af binaries wrote. I checked on disk: after both Install and Uninstall on this branch, the block survives byte-identical. An upgrading user ends up with the native skill and the stale instructions still injected into every session — which is the AGENTS.md bloat #813 is actually complaining about, so the issue isn't closed for existing users without this. Codex hit the same thing in #910 and shipped removeLegacyMarkerBlock.

Install (right after the symlink lands) and Uninstall (per catalog skill) now clean the block up.

The part worth reviewing closely is that I deliberately did not copy the Codex helper, for the reason you gave. ~/.codex/AGENTS.override.md was created by af for itself; ~/.config/opencode/AGENTS.md is written by the user and read by OpenCode. uninstallMarkerBlock rewrites any file it can read — measured, a user AGENTS.md with no agentfield block goes 23 → 21 bytes — and the Codex wrapper deletes the file when only whitespace survives, which would silently remove a deliberately-empty AGENTS.md on every install. So the OpenCode version is stricter:

• no block of ours in the file → it is not opened for writing at all (bytes and mtime untouched)
• the file is removed only when stripping our block is what emptied it
• other tools' blocks and user prose on both sides of ours are preserved
• a malformed block (open marker, no close) is left alone rather than truncated to EOF
• missing file is a no-op

b547f480 — test hygiene

TestOpenCodeTargetUninstallRemovesCatalogEntries was the one OpenCode test not calling withTempHome, so it worked in the package-wide home instead of its own. And realHomeSnapshot still only fingerprinted the old ~/.config/opencode/AGENTS.md, so the real-home pollution guard didn't cover the new ~/.config/opencode/skills root — added.

3108d3cf — route the cleanup through the package's filesystem seams

My first pass called os.Remove / os.WriteFile / os.Rename directly, which none of the rest of this package does: every write path goes through the reconcile* seams in reconcile.go, which exist so a test can force a failure. The effect was that the helper's remove / write / rename into branches were unreachable from a test — six lines that never executed once, so a wrongly-wrapped error in any of them would have shipped silently. They now use the seams and each failure is covered through Uninstall, modelled on the reconciler's own rewrite-failure subtests.

Same commit drops legacyRulesPath's error return. It could only fail when TargetPath() fails, and both call sites had already proven TargetPath() succeeds before reaching it — dead code that also advertised a failure mode that doesn't exist. It now takes the resolved skills root, which lets Uninstall use the TargetPath() result it was already computing and throwing away instead of re-resolving it per skill.

a2762e14 — don't fail a live install over a rules file we can't clean

This is a behaviour change to my own first commit, and the one I'd most like a second opinion on. Install created the symlink and then returned an error if the cleanup failed — say ~/.config/opencode/AGENTS.md is mode 000 from a botched dotfiles restore, or a directory is sitting at that path. install.go records nothing for a failed target, so the outcome was the worst of both: the integration live on disk, af skill list reporting OpenCode as not installed, and every later af skill install exiting non-zero — all over a stale block that has no bearing on whether OpenCode can load the skill.

The cleanup is now advisory on the install path (one line to stderr) and still fatal on Uninstall, where stripping the block is the entire point of the call. The ordering is unchanged and deliberate: the native skill goes in before the old block comes out, so an interrupted migration leaves a working integration rather than neither.

Validation contract → test

Behaviour	Test
Install strips our block; user prose before and after it survives	TestOpenCodeInstallStripsLegacyMarkerBlockAndKeepsForeignContent
Another tool's block in the same file survives install and uninstall	same, plus TestOpenCodeLeavesARulesFileWithoutOurBlockUntouched/foreign_block_only
A file that was only our block (± whitespace) is removed	TestOpenCodeInstallDeletesLegacyRulesFileItOwnedAlone
A file with no agentfield block is byte-identical and its mtime unchanged	TestOpenCodeLeavesARulesFileWithoutOurBlockUntouched (prose / foreign-block / whitespace-only / empty)
Uninstall clears every catalog skill's block and keeps user content	TestOpenCodeUninstallRemovesLinksAndLegacyBlocks
Missing file is a no-op, and is not created	TestOpenCodeCleanupIsANoOpWithoutALegacyRulesFile
Uninstall reports a rules file it cannot read	TestOpenCodeUninstallReportsAnUnreadableLegacyRulesFile
Uninstall reports a failed write / rename / remove of the rules file, naming the file and the cause	TestOpenCodeUninstallReportsLegacyRewriteFailures (3 subtests)
An uncleanable rules file leaves the install reported as installed, the symlink on disk, and the target recorded in state	TestOpenCodeInstallSurvivesALegacyRulesFileItCannotClean

Both directions verified. With target_opencode.go reverted to this branch's version the four original behavioural tests fail; with the Codex helper dropped in verbatim, all four ...WithoutOurBlockUntouched cases fail (two on mtime, two because the file was deleted). And for the two new commits, reverting just those two changes fails exactly the new tests and nothing else:

--- FAIL: TestOpenCodeInstallSurvivesALegacyRulesFileItCannotClean
    an uncleanable legacy rules file failed the install: [{TargetName:opencode Err:read legacy OpenCode rules file .../AGENTS.md: is a directory}]
--- FAIL: TestOpenCodeUninstallReportsLegacyRewriteFailures/write
--- FAIL: TestOpenCodeUninstallReportsLegacyRewriteFailures/rename
--- FAIL: TestOpenCodeUninstallReportsLegacyRewriteFailures/remove

Gates (from control-plane, on top of cf40b601)

GOFLAGS=-buildvcs=false go build ./...                              exit 0
go vet ./...                                                        exit 0
gofmt -l ./internal/skillkit                                        (no output)
golangci-lint run ./internal/skillkit/...                           exit 0
go test ./internal/skillkit/... ./internal/cli/... -count=1 -race    ok skillkit 24.281s / ok cli 107.481s
                                                                    / ok cli/commands 1.040s / ok cli/framework 1.034s
go test -tags sqlite_fts5 -coverprofile=... ./...                   ok skillkit 4.102s, coverage 91.4% of statements
./scripts/patch-coverage-gate.sh                                    | control-plane | 99 | 87.00% | OK |  → gate passed

Patch coverage is the one worth calling out, since coverage-summary is a required check and min_patch = 80.0. My first two commits took control-plane patch coverage to 77%, which would have turned that check red on a branch that was green at cf40b601 — every missing line an untested error branch, four of them the unreachable ones above. With 3108d3cf and a2762e14 it's 99 touched lines / 12 missing / 87.00%, measured with the same gocover-cobertura + diff-cover pipeline the workflow uses. The 12 remaining are all pre-existing os-level error branches in Install, Uninstall and Status that predate my commits.

Two suite-level failures on my machine, both pre-existing and both reproduced in a clean checkout of cf40b601 itself: TestDevServiceRunDev in internal/core/services (port discovery against localhost in the 8001-8999 range — passes on current main, where dev_service.go has since changed, so a rebase clears it) and the TestDispatchAgentRequest* family in internal/handlers (the known WSL loopback artefact — all four fail at cf40b601 on a clean tree). Neither package's failure is reachable from this diff. golangci-lint run ./... reports the same 35 issues at cf40b601 and at HEAD, none in skillkit; gofmt -l . lists the same 84 files on both, none in skillkit.

Only the Go control plane is touched, so the SDK gates don't apply.

Refs #813.

[AbirAbbas]

Re-approving after my four commits on top of cf40b60 (Santosh's approval was auto-dismissed by the push). Contributor's commits are untouched; the additions are the legacy AGENTS.md cleanup + its tests and the seam refactor described in my comment above. Gates and patch coverage (87%) in the comment.