From 43b69e0c3f9c21be3410e5cf1db1c0d7f9a1ae4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Philippe=20D=C3=A9=C3=AFs=20Nuel?= Date: Mon, 13 Jul 2026 22:44:51 +0200 Subject: [PATCH 1/6] =?UTF-8?q?chore:=20P0=20hardening=20=E2=80=94=20CI,?= =?UTF-8?q?=20vulns,=20docs,=20fuzz?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Robustness and correctness pass before a stable release: - CI: run tests with -race -shuffle=on; add govulncheck step. - CI: replace unpinned golangci-lint master install with golangci-lint-action@v8 (golangci-lint v2.12.2); pin checkout and setup-go actions by commit SHA. - Add .golangci.yml v2 config (standard linters + std-error-handling preset) so the enabled set does not drift with tool upgrades. - Bump toolchain to Go 1.25.12 and go-git v5.19.1, x/crypto v0.52.0, x/net v0.54.0 — clears all 38 code-reachable vulnerabilities to 0. (go-git v5.19.1 requires Go >= 1.25.) - Docs: correct the OpenCode prune description. The behavior is asymmetric — the package apply flow prunes owned nested entries via ManagedPruner, while `shenron push` stays upsert-only because it does not record per-leaf ownership. - Tests: add TestEndToEnd_PushAllTargetsIdempotent asserting every adapter reports "No changes" after a full push. - Tests: add FuzzMergeFile covering no-panic, valid-JSON output, foreign-key preservation, and idempotence of the OpenCode merge. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/ci.yml | 20 +++--- .golangci.yml | 13 ++++ Makefile | 8 ++- README.md | 9 ++- docs/ARCHITECTURE.md | 15 +++- go.mod | 21 +++--- go.sum | 67 +++++++++++++----- internal/adapter/opencode/fuzz_test.go | 94 ++++++++++++++++++++++++++ internal/integration_test.go | 24 +++++++ 9 files changed, 231 insertions(+), 40 deletions(-) create mode 100644 .golangci.yml create mode 100644 internal/adapter/opencode/fuzz_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4a06ca6..3ee6de8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,14 +9,18 @@ jobs: verify: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - uses: actions/setup-go@v5 + - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 + - uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0 with: - go-version: "1.24.2" - - name: Install golangci-lint - run: | - curl -sSf https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.62.2 + go-version: "1.25.12" - run: go vet ./... - run: gofmt -l . | tee /dev/stderr | (! read) # fail if any file needs formatting - - run: golangci-lint run - - run: go test ./... + - name: Lint + uses: golangci/golangci-lint-action@4afd733a84b1f43292c63897423277bb7f4313a9 # v8 + with: + version: v2.12.2 + - run: go test -race -shuffle=on ./... + - name: Vulnerability scan + env: + GOTOOLCHAIN: go1.25.12 + run: go run golang.org/x/vuln/cmd/govulncheck@v1.1.4 ./... diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..5902caf --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,13 @@ +# golangci-lint v2 configuration. +# Pinned explicitly so the enabled linter set does not drift with tool upgrades. +version: "2" + +linters: + # Standard set: errcheck, govet, ineffassign, staticcheck, unused. + default: standard + exclusions: + generated: lax + presets: + # Allow unchecked writes to os.Stdout/os.Stderr and similar + # fmt.Fprint* calls, matching the v1 default behavior for CLI output. + - std-error-handling diff --git a/Makefile b/Makefile index 4f6df1f..8d01694 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: build test lint vet fmt-check clean +.PHONY: build test test-race lint vet fmt-check vuln clean build: go build -o shenron ./cmd/shenron/ @@ -6,6 +6,12 @@ build: test: go test ./... +test-race: + go test -race -shuffle=on ./... + +vuln: + GOTOOLCHAIN=go1.25.12 go run golang.org/x/vuln/cmd/govulncheck@v1.1.4 ./... + lint: golangci-lint run diff --git a/README.md b/README.md index ad2838e..ef3af5e 100644 --- a/README.md +++ b/README.md @@ -334,9 +334,12 @@ Shenron upserts pivot agents and commands into the nested `agent` and preserved, and existing key order is retained where possible. The JSON document is parsed and serialized again, so byte-for-byte formatting is not guaranteed. -The merge is deliberately upsert-only. Removing an agent or command from the -pivot does not delete its nested OpenCode entry; remove stale JSON entries by -hand. Standalone managed files that are no longer generated can be reported as +For `shenron push`, the merge is upsert-only on nested entries: removing an +agent or command from the pivot does not delete its nested OpenCode entry, so +remove stale JSON entries by hand. (The package apply flow goes further — it +tracks the entries it owns in `.shenron-state.json` and prunes owned entries +that leave the pivot, while preserving native-only entries you added by hand.) +Standalone managed files that are no longer generated can be reported as orphaned, but Shenron still leaves deletion to you. ### Manual-edit protection diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 050173b..91bc3dd 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -251,8 +251,19 @@ merge. Its `orderedObject` implementation preserves existing key order and raw values while upserting managed `agent` and `command` entries. Unrelated top-level keys and native-only nested entries survive the merge. -OpenCode synchronization is deliberately upsert-only. Removing an item from the -pivot does not remove the corresponding nested entry from `opencode.json`. +Nested-entry pruning depends on ownership tracking, so the two write flows +differ: + +- The **package apply flow** records the `agent`/`command` leaves shenron writes + (in `.shenron-state.json` under `managed`). A later apply then uses the + `ManagedPruner` capability (`PruneManaged`) to remove owned leaves that left + the pivot, while preserving native-only entries and unrelated top-level keys. +- The **standalone `shenron push` flow** does not record per-leaf ownership, so + it is upsert-only for nested `opencode.json` entries: removing an item from + the pivot leaves its nested entry in place. + +In both flows, standalone managed files (prompt/command bodies) that are no +longer generated are reported as orphaned but never deleted. ### Codex diff --git a/go.mod b/go.mod index 1ef458f..cbae6ee 100644 --- a/go.mod +++ b/go.mod @@ -1,8 +1,11 @@ module github.com/S1933/Shenron -go 1.24.2 +go 1.25.0 + +toolchain go1.25.12 require ( + github.com/go-git/go-git/v5 v5.19.1 github.com/pelletier/go-toml/v2 v2.4.3 github.com/spf13/cobra v1.10.2 gopkg.in/yaml.v3 v3.0.1 @@ -12,23 +15,23 @@ require ( dario.cat/mergo v1.0.0 // indirect github.com/Microsoft/go-winio v0.6.2 // indirect github.com/ProtonMail/go-crypto v1.1.6 // indirect - github.com/cloudflare/circl v1.6.1 // indirect - github.com/cyphar/filepath-securejoin v0.4.1 // indirect + github.com/cloudflare/circl v1.6.3 // indirect + github.com/cyphar/filepath-securejoin v0.6.1 // indirect github.com/emirpasic/gods v1.18.1 // indirect github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect - github.com/go-git/go-billy/v5 v5.6.2 // indirect - github.com/go-git/go-git/v5 v5.16.3 // indirect + github.com/go-git/go-billy/v5 v5.9.0 // indirect github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect github.com/kevinburke/ssh_config v1.2.0 // indirect - github.com/pjbgf/sha1cd v0.3.2 // indirect + github.com/klauspost/cpuid/v2 v2.3.0 // indirect + github.com/pjbgf/sha1cd v0.6.0 // indirect github.com/sergi/go-diff v1.3.2-0.20230802210424-5b0b94c5c0d3 // indirect github.com/skeema/knownhosts v1.3.1 // indirect github.com/spf13/pflag v1.0.9 // indirect github.com/xanzy/ssh-agent v0.3.3 // indirect - golang.org/x/crypto v0.37.0 // indirect - golang.org/x/net v0.39.0 // indirect - golang.org/x/sys v0.32.0 // indirect + golang.org/x/crypto v0.52.0 // indirect + golang.org/x/net v0.54.0 // indirect + golang.org/x/sys v0.45.0 // indirect gopkg.in/warnings.v0 v0.1.2 // indirect ) diff --git a/go.sum b/go.sum index bc7311d..3cecb56 100644 --- a/go.sum +++ b/go.sum @@ -5,38 +5,63 @@ github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERo github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU= github.com/ProtonMail/go-crypto v1.1.6 h1:ZcV+Ropw6Qn0AX9brlQLAUXfqLBc7Bl+f/DmNxpLfdw= github.com/ProtonMail/go-crypto v1.1.6/go.mod h1:rA3QumHc/FZ8pAHreoekgiAbzpNsfQAosU5td4SnOrE= -github.com/cloudflare/circl v1.6.1 h1:zqIqSPIndyBh1bjLVVDHMPpVKqp8Su/V+6MeDzzQBQ0= -github.com/cloudflare/circl v1.6.1/go.mod h1:uddAzsPgqdMAYatqJ0lsjX1oECcQLIlRpzZh3pJrofs= +github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be h1:9AeTilPcZAjCFIImctFaOjnTIavg87rW78vTPkQqLI8= +github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be/go.mod h1:ySMOLuWl6zY27l47sB3qLNK6tF2fkHG55UZxx8oIVo4= +github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio= +github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= +github.com/cloudflare/circl v1.6.3 h1:9GPOhQGF9MCYUeXyMYlqTR6a5gTrgR/fBLXvUgtVcg8= +github.com/cloudflare/circl v1.6.3/go.mod h1:2eXP6Qfat4O/Yhh8BznvKnJ+uzEoTQ6jVKJRn81BiS4= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= -github.com/cyphar/filepath-securejoin v0.4.1 h1:JyxxyPEaktOD+GAnqIqTf9A8tHyAG22rowi7HkoSU1s= -github.com/cyphar/filepath-securejoin v0.4.1/go.mod h1:Sdj7gXlvMcPZsbhwhQ33GguGLDGQL7h7bg04C/+u9jI= +github.com/cyphar/filepath-securejoin v0.6.1 h1:5CeZ1jPXEiYt3+Z6zqprSAgSWiggmpVyciv8syjIpVE= +github.com/cyphar/filepath-securejoin v0.6.1/go.mod h1:A8hd4EnAeyujCJRrICiOWqjS1AX0a9kM5XL+NwKoYSc= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/elazarl/goproxy v1.7.2 h1:Y2o6urb7Eule09PjlhQRGNsqRfPmYI3KKQLFpCAV3+o= +github.com/elazarl/goproxy v1.7.2/go.mod h1:82vkLNir0ALaW14Rc399OTTjyNREgmdL2cVoIbS6XaE= github.com/emirpasic/gods v1.18.1 h1:FXtiHYKDGKCW2KzwZKx0iC0PQmdlorYgdFG9jPXJ1Bc= github.com/emirpasic/gods v1.18.1/go.mod h1:8tpGGwCnJ5H4r6BWwaV6OrWmMoPhUl5jm/FMNAnJvWQ= +github.com/gliderlabs/ssh v0.3.8 h1:a4YXD1V7xMF9g5nTkdfnja3Sxy1PVDCj1Zg4Wb8vY6c= +github.com/gliderlabs/ssh v0.3.8/go.mod h1:xYoytBv1sV0aL3CavoDuJIQNURXkkfPA/wxQ1pL1fAU= github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 h1:+zs/tPmkDkHx3U66DAb0lQFJrpS6731Oaa12ikc+DiI= github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376/go.mod h1:an3vInlBmSxCcxctByoQdvwPiA7DTK7jaaFDBTtu0ic= -github.com/go-git/go-billy/v5 v5.6.2 h1:6Q86EsPXMa7c3YZ3aLAQsMA0VlWmy43r6FHqa/UNbRM= -github.com/go-git/go-billy/v5 v5.6.2/go.mod h1:rcFC2rAsp/erv7CMz9GczHcuD0D32fWzH+MJAU+jaUU= -github.com/go-git/go-git/v5 v5.16.3 h1:Z8BtvxZ09bYm/yYNgPKCzgWtaRqDTgIKRgIRHBfU6Z8= -github.com/go-git/go-git/v5 v5.16.3/go.mod h1:4Ge4alE/5gPs30F2H1esi2gPd69R0C39lolkucHBOp8= +github.com/go-git/go-billy/v5 v5.9.0 h1:jItGXszUDRtR/AlferWPTMN4j38BQ88XnXKbilmmBPA= +github.com/go-git/go-billy/v5 v5.9.0/go.mod h1:jCnQMLj9eUgGU7+ludSTYoZL/GGmii14RxKFj7ROgHw= +github.com/go-git/go-git-fixtures/v4 v4.3.2-0.20231010084843-55a94097c399 h1:eMje31YglSBqCdIqdhKBW8lokaMrL3uTkpGYlE2OOT4= +github.com/go-git/go-git-fixtures/v4 v4.3.2-0.20231010084843-55a94097c399/go.mod h1:1OCfN199q1Jm3HZlxleg+Dw/mwps2Wbk9frAWm+4FII= +github.com/go-git/go-git/v5 v5.19.1 h1:nX27AnaU43/K5bKktKwgBmR9lawoYVe1Ckg0rgzzN00= +github.com/go-git/go-git/v5 v5.19.1/go.mod h1:Pb1v0c7/g8aGQJwx9Us09W85yGoyvSwuhEGMH7zjDKQ= github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8 h1:f+oWsMOmNPc8JmEHVZIycC7hBoQxHH9pNKQORJNozsQ= github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8/go.mod h1:wcDNUvekVysuuOpQKo3191zZyTpiI6se1N1ULghS0sw= +github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= +github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 h1:BQSFePA1RWJOlocH6Fxy8MmwDt+yVQYULKfN0RoTN8A= github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99/go.mod h1:1lJo3i6rXxKeerYnT8Nvf0QmHCRC1n8sfWVwXF2Frvo= github.com/kevinburke/ssh_config v1.2.0 h1:x584FjTGwHzMwvHx18PXxbBVzfnxogHaAReU4gf13a4= github.com/kevinburke/ssh_config v1.2.0/go.mod h1:CT57kijsi8u/K/BOFA39wgDQJ9CxiF4nAY/ojJ6r6mM= +github.com/klauspost/cpuid/v2 v2.3.0 h1:S4CRMLnYUhGeDFDqkGriYKdfoFlDnMtqTiI/sFzhA9Y= +github.com/klauspost/cpuid/v2 v2.3.0/go.mod h1:hqwkgyIinND0mEev00jJYCxPNVRVXFQeu1XKlok6oO0= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= +github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= +github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= +github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= +github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/onsi/gomega v1.34.1 h1:EUMJIKUjM8sKjYbtxQI9A4z2o+rruxnzNvpknOXie6k= +github.com/onsi/gomega v1.34.1/go.mod h1:kU1QgUvBDLXBJq618Xvm2LUX6rSAfRaFRTcdOeDLwwY= github.com/pelletier/go-toml/v2 v2.4.3 h1:GTRvJQutkOSftxIFD5xw9aepkYNuPWmVJpffdDPYVpY= github.com/pelletier/go-toml/v2 v2.4.3/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= -github.com/pjbgf/sha1cd v0.3.2 h1:a9wb0bp1oC2TGwStyn0Umc/IGKQnEgF0vVaZ8QF8eo4= -github.com/pjbgf/sha1cd v0.3.2/go.mod h1:zQWigSxVmsHEZow5qaLtPYxpcKMMQpa09ixqBxuCS6A= +github.com/pjbgf/sha1cd v0.6.0 h1:3WJ8Wz8gvDz29quX1OcEmkAlUg9diU4GxJHqs0/XiwU= +github.com/pjbgf/sha1cd v0.6.0/go.mod h1:lhpGlyHLpQZoxMv8HcgXvZEhcGs0PG/vsZnEJ7H0iCM= +github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ= +github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/sergi/go-diff v1.3.2-0.20230802210424-5b0b94c5c0d3 h1:n661drycOFuPLCN3Uc8sB6B/s6Z4t2xvBgU1htSHuq8= github.com/sergi/go-diff v1.3.2-0.20230802210424-5b0b94c5c0d3/go.mod h1:A0bzQcvG0E7Rwjx0REVgAGH58e96+X0MeOfepqsbeW4= @@ -50,30 +75,38 @@ github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/xanzy/ssh-agent v0.3.3 h1:+/15pJfg/RsTxqYcX6fHqOXZwwMP+2VyYWJeWM2qQFM= github.com/xanzy/ssh-agent v0.3.3/go.mod h1:6dzNDKs0J9rVPHPhaGCukekBHKqfl+L3KghI1Bc68Uw= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/crypto v0.0.0-20220622213112-05595931fe9d/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= -golang.org/x/crypto v0.37.0 h1:kJNSjF/Xp7kU0iB2Z+9viTPMW4EqqsrywMXLJOOsXSE= -golang.org/x/crypto v0.37.0/go.mod h1:vg+k43peMZ0pUMhYmVAWysMK35e6ioLh3wB8ZCAfbVc= +golang.org/x/crypto v0.52.0 h1:RMs7fP2rXdep0CftQlK8Uf+kibLm7qkCcradZWYz988= +golang.org/x/crypto v0.52.0/go.mod h1:1QgfPxDqh0T2M/elOJtp9RvuR95kVjir0e6/BvEmGbc= +golang.org/x/exp v0.0.0-20260410095643-746e56fc9e2f h1:W3F4c+6OLc6H2lb//N1q4WpJkhzJCK5J6kUi1NTVXfM= +golang.org/x/exp v0.0.0-20260410095643-746e56fc9e2f/go.mod h1:J1xhfL/vlindoeF/aINzNzt2Bket5bjo9sdOYzOsU80= golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= -golang.org/x/net v0.39.0 h1:ZCu7HMWDxpXpaiKdhzIfaltL9Lp31x/3fCP11bc6/fY= -golang.org/x/net v0.39.0/go.mod h1:X7NRbYVEA+ewNkCNyJ513WmMdQ3BineSwVtN2zD/d+E= +golang.org/x/net v0.54.0 h1:2zJIZAxAHV/OHCDTCOHAYehQzLfSXuf/5SoL/Dv6w/w= +golang.org/x/net v0.54.0/go.mod h1:Sj4oj8jK6XmHpBZU/zWHw3BV3abl4Kvi+Ut7cQcY+cQ= golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.32.0 h1:s77OFDvIQeibCmezSnk/q6iAfkdiQaJi4VzroCFrN20= -golang.org/x/sys v0.32.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= +golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= +golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= +golang.org/x/term v0.43.0 h1:S4RLU2sB31O/NCl+zFN9Aru9A/Cq2aqKpTZJ6B+DwT4= +golang.org/x/term v0.43.0/go.mod h1:lrhlHNdQJHO+1qVYiHfFKVuVioJIheAc3fBSMFYEIsk= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= +golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= -gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/warnings.v0 v0.1.2 h1:wFXVbFY8DY5/xOe1ECiWdKCzZlxgshcYVNkBHstARME= gopkg.in/warnings.v0 v0.1.2/go.mod h1:jksf8JmL6Qr/oQM2OXTHunEvvTAsrWBLb6OOjuVWRNI= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= diff --git a/internal/adapter/opencode/fuzz_test.go b/internal/adapter/opencode/fuzz_test.go new file mode 100644 index 0000000..b4451e5 --- /dev/null +++ b/internal/adapter/opencode/fuzz_test.go @@ -0,0 +1,94 @@ +package opencode + +import ( + "encoding/json" + "testing" + "unicode/utf8" +) + +// FuzzMergeFile exercises the OpenCode config merge on arbitrary existing +// documents and fragment ids. It asserts the core invariants the sync runtime +// relies on: +// +// - MergeFile never panics on any input. +// - When it succeeds, the output is always valid JSON. +// - Every top-level key present in a well-formed existing object survives the +// merge (the merge only upserts, so native/foreign keys are never dropped). +// - The merge is idempotent: merging its own output with the same fragments +// reproduces it byte-for-byte. +func FuzzMergeFile(f *testing.F) { + seeds := []string{ + "", + "{}", + `{"theme":"dark"}`, + `{"agent":{"native":{"mode":"subagent"}},"provider":{"default":"anthropic"}}`, + `{"command":{"ship":{"template":"go"}},"$schema":"https://opencode"}`, + `{"agent":{"build":{"mode":"primary"}},"command":{}}`, + `{"dup":1,"dup":2,"agent":{}}`, + `{"nested":{"a":{"b":[1,2,{"c":true}]}}}`, + `{"unicode":"café é 😀","agent":{"x":{}}}`, + `{} trailing garbage`, + `not json`, + `[1,2,3]`, + } + for _, s := range seeds { + f.Add([]byte(s), "build", "ship", "a subagent") + } + + adapter := NewAdapterWithBaseDir("", "") + const path = "opencode.json" + + f.Fuzz(func(t *testing.T, existing []byte, agentID, cmdID, body string) { + // Pivot ids always come from YAML, so they are valid UTF-8. Non-UTF-8 + // leaf ids are out of contract: json.Marshal rewrites their bytes to + // U+FFFD, which the raw in-memory key can no longer match on a second + // pass, breaking idempotence. Skip them rather than assert an invariant + // the real pipeline never has to uphold. + if !utf8.ValidString(agentID) || !utf8.ValidString(cmdID) { + return + } + + fragments := map[string]any{ + "agent." + agentID: map[string]any{"mode": "subagent", "description": body}, + "command." + cmdID: map[string]any{"template": body}, + } + + out, err := adapter.MergeFile(path, existing, fragments) + if err != nil { + // Malformed existing input legitimately errors; nothing else to check. + return + } + if out == nil { + t.Fatalf("MergeFile returned nil output without error for path %q", path) + } + + if !json.Valid(out) { + t.Fatalf("MergeFile produced invalid JSON:\n%s", out) + } + + // Foreign-key preservation: only checkable when the existing document is a + // standard JSON object (parseOrderedObject is more lenient about trailing + // data than encoding/json, so guard on a strict re-parse). + var existingObj map[string]json.RawMessage + if json.Unmarshal(existing, &existingObj) == nil { + var outObj map[string]json.RawMessage + if err := json.Unmarshal(out, &outObj); err != nil { + t.Fatalf("output not an object: %v\n%s", err, out) + } + for key := range existingObj { + if _, ok := outObj[key]; !ok { + t.Errorf("top-level key %q dropped by merge\nexisting: %s\nout: %s", key, existing, out) + } + } + } + + // Idempotence: re-merging the output with the same fragments is a no-op. + out2, err := adapter.MergeFile(path, out, fragments) + if err != nil { + t.Fatalf("second merge failed on valid output: %v\n%s", err, out) + } + if string(out2) != string(out) { + t.Errorf("merge not idempotent:\nfirst:\n%s\nsecond:\n%s", out, out2) + } + }) +} diff --git a/internal/integration_test.go b/internal/integration_test.go index 40e8015..d7ef880 100644 --- a/internal/integration_test.go +++ b/internal/integration_test.go @@ -347,6 +347,30 @@ func TestEndToEnd_PushBothTargets(t *testing.T) { } } +func TestEndToEnd_PushAllTargetsIdempotent(t *testing.T) { + env := newIntegrationEnv(t) + + if err := cli.RunPush(env.pushOpts("")); err != nil { + t.Fatalf("push all targets: %v", err) + } + + out, errOut, err := cli.CaptureOutput(func() error { + return cli.RunDiff(env.diffOpts("")) + }) + if err != nil { + t.Fatalf("diff after push: %v", err) + } + for _, name := range []string{"opencode", "claude-code", "codex"} { + want := "[" + name + "] No changes" + if !strings.Contains(out, want) { + t.Errorf("expected %q after push, got stdout:\n%s", want, out) + } + } + if strings.Contains(errOut, "warning: orphaned") { + t.Errorf("unexpected orphan warning after full push:\n%s", errOut) + } +} + func TestEndToEnd_PermissionMapping(t *testing.T) { env := newIntegrationEnv(t) From 77b347f2f8bccd94b762e4b8be3187821857e881 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Philippe=20D=C3=A9=C3=AFs=20Nuel?= Date: Mon, 13 Jul 2026 22:59:20 +0200 Subject: [PATCH 2/6] refactor(adapter): immutable generation + capability interfaces (P1.1-1.3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Harden the adapter boundary per the audit's P1 items: - P1.1: introduce adapter.GeneratedFile{Path,Content,Mode,Adapter, ResourceID} and GenerationResult{Files,Fragments}. The file mode now flows through to fsutil.WriteFileAtomic instead of a hardcoded 0o644. - P1.2: remove mutable adapter state. Both OpenCode (fragments) and Codex (nativeNames — a case the audit missed) built cross-resource state on the receiver and needed a reset between passes. Generation is now a single Generate(*pivot.PivotFile) pass that builds everything in local variables, so adapters are reentrant and safe to reuse. Drops ResetFragments/Fragments. Adds a reentrancy regression test. - P1.3: formalize optional behaviors as capability interfaces in capabilities.go — MergingAdapter, ManagedPruner, PivotDirectoryAware. MergeFile leaves the base Adapter interface; the nil-returning stubs on Claude and Codex are gone, as are the duplicate private interfaces that cli/sync.go declared. Generate (cli) now returns map[string][]adapter.GeneratedFile; the diff engine still consumes a path->content projection, while the push writer uses each file's mode. Co-Authored-By: Claude Opus 4.8 --- internal/adapter/adapter.go | 45 +++++--- internal/adapter/capabilities.go | 28 +++++ internal/adapter/claude/adapter.go | 52 ++++++--- internal/adapter/claude/adapter_test.go | 11 -- internal/adapter/codex/adapter.go | 108 +++++++++++-------- internal/adapter/codex/adapter_test.go | 34 +++--- internal/adapter/opencode/adapter.go | 96 ++++++++--------- internal/adapter/opencode/adapter_test.go | 33 +++++- internal/cli/package_apply.go | 22 ++-- internal/cli/sync.go | 125 ++++++++++------------ internal/cli/sync_runtime.go | 47 +++++--- internal/cli/sync_test.go | 4 +- 12 files changed, 367 insertions(+), 238 deletions(-) create mode 100644 internal/adapter/capabilities.go diff --git a/internal/adapter/adapter.go b/internal/adapter/adapter.go index 3968741..4dbb516 100644 --- a/internal/adapter/adapter.go +++ b/internal/adapter/adapter.go @@ -1,20 +1,41 @@ package adapter -import "github.com/S1933/Shenron/internal/pivot" +import ( + "io/fs" + "github.com/S1933/Shenron/internal/pivot" +) + +// GeneratedFile is one file an adapter wants written, carrying the metadata the +// sync runtime needs to route, write, and report it. +type GeneratedFile struct { + Path string // absolute destination path + Content []byte // file body + Mode fs.FileMode // permission bits for the atomic write + Adapter string // owning adapter name (e.g. "opencode") + ResourceID string // pivot resource id this file came from, when applicable +} + +// GenerationResult is the immutable output of one Generate call. Files are the +// standalone outputs; Fragments carries nested-config contributions (e.g. +// OpenCode's agent/command entries) that a MergingAdapter later folds into a +// shared file. Standalone-file adapters leave Fragments nil. +type GenerationResult struct { + Files []GeneratedFile + Fragments map[string]any +} + +// Adapter translates a pivot file into native configuration for one tool. +// +// Generate is a single, side-effect-free pass over the whole pivot: adapters +// build every file (and any config fragments) from local state and return them, +// so the same adapter instance can be reused and is safe under concurrency. +// Optional behaviors — merging fragments into a shared file, pruning managed +// entries, learning the pivot directory — are expressed as capability +// interfaces in capabilities.go rather than mandatory methods. type Adapter interface { Name() string ValidateAgent(pivot.AgentDefinition) error - GenerateAgent(pivot.AgentDefinition) (map[string]string, error) - GenerateCommand(pivot.CommandDefinition) (map[string]string, error) + Generate(*pivot.PivotFile) (GenerationResult, error) TargetPaths() []string - MergeFile(path string, existing []byte, fragments map[string]any) ([]byte, error) -} - -// ManagedPruner is an optional capability for adapters that merge into shared -// files. It removes leaves that shenron previously managed (recorded in -// state.Managed) but that the current pivot no longer generates. Standalone- -// file adapters do not implement it. -type ManagedPruner interface { - PruneManaged(path string, existing []byte, managed map[string][]string, fragments map[string]any) ([]byte, error) } diff --git a/internal/adapter/capabilities.go b/internal/adapter/capabilities.go new file mode 100644 index 0000000..b6e6868 --- /dev/null +++ b/internal/adapter/capabilities.go @@ -0,0 +1,28 @@ +package adapter + +// Optional adapter capabilities. The sync runtime probes for these with type +// assertions, so an adapter opts in simply by implementing the methods. + +// MergingAdapter merges accumulated fragments into an existing shared config +// file (e.g. opencode.json). Standalone-file adapters (Claude, Codex) do not +// implement it. +type MergingAdapter interface { + // MergeFile upserts fragments into existing and returns the new file bytes, + // or nil when path is not the shared config file the adapter owns. + MergeFile(path string, existing []byte, fragments map[string]any) ([]byte, error) + // ConfigPath returns the absolute path of the shared config file. + ConfigPath() string +} + +// ManagedPruner removes leaves that shenron previously managed (recorded in +// state.Managed) but that the current pivot no longer generates, before +// upserting the current fragments. It preserves entries shenron never owned. +type ManagedPruner interface { + PruneManaged(path string, existing []byte, managed map[string][]string, fragments map[string]any) ([]byte, error) +} + +// PivotDirectoryAware receives the directory of the pivot file, used to resolve +// relative promptFile references during generation. +type PivotDirectoryAware interface { + SetPivotDir(string) +} diff --git a/internal/adapter/claude/adapter.go b/internal/adapter/claude/adapter.go index ecb7fc1..171334e 100644 --- a/internal/adapter/claude/adapter.go +++ b/internal/adapter/claude/adapter.go @@ -4,10 +4,13 @@ import ( "fmt" "path/filepath" + "github.com/S1933/Shenron/internal/adapter" "github.com/S1933/Shenron/internal/fsutil" "github.com/S1933/Shenron/internal/pivot" ) +const fileMode = 0o644 + // Adapter implements the Claude Code target adapter. type Adapter struct { baseDir string @@ -42,17 +45,45 @@ func (a *Adapter) ValidateAgent(agent pivot.AgentDefinition) error { return nil } -// GenerateAgent produces a Claude Code agent Markdown file. -func (a *Adapter) GenerateAgent(agent pivot.AgentDefinition) (map[string]string, error) { - if err := a.ValidateAgent(agent); err != nil { - return nil, err +// Generate produces one Markdown file per agent and command. +func (a *Adapter) Generate(pf *pivot.PivotFile) (adapter.GenerationResult, error) { + var files []adapter.GeneratedFile + + for _, ag := range pf.Agents { + if err := a.ValidateAgent(ag); err != nil { + return adapter.GenerationResult{}, err + } + generated, err := generateAgentFile(ag, a.pivotDir, a.baseDir) + if err != nil { + return adapter.GenerationResult{}, fmt.Errorf("generate agent %q: %w", ag.ID, err) + } + files = append(files, a.filesFrom(generated, ag.ID)...) + } + + for _, cmd := range pf.Commands { + generated, err := generateCommandFile(cmd, a.baseDir) + if err != nil { + return adapter.GenerationResult{}, fmt.Errorf("generate command %q: %w", cmd.ID, err) + } + files = append(files, a.filesFrom(generated, cmd.ID)...) } - return generateAgentFile(agent, a.pivotDir, a.baseDir) + + return adapter.GenerationResult{Files: files}, nil } -// GenerateCommand produces a Claude Code command Markdown file. -func (a *Adapter) GenerateCommand(cmd pivot.CommandDefinition) (map[string]string, error) { - return generateCommandFile(cmd, a.baseDir) +// filesFrom converts a path->content map into GeneratedFile records. +func (a *Adapter) filesFrom(generated map[string]string, resourceID string) []adapter.GeneratedFile { + files := make([]adapter.GeneratedFile, 0, len(generated)) + for path, content := range generated { + files = append(files, adapter.GeneratedFile{ + Path: path, + Content: []byte(content), + Mode: fileMode, + Adapter: a.Name(), + ResourceID: resourceID, + }) + } + return files } // TargetPaths returns paths this adapter writes to. @@ -62,8 +93,3 @@ func (a *Adapter) TargetPaths() []string { filepath.Join(a.baseDir, "commands"), } } - -// MergeFile returns nil — Claude Code uses one file per agent/command. -func (a *Adapter) MergeFile(path string, existing []byte, fragments map[string]any) ([]byte, error) { - return nil, nil -} diff --git a/internal/adapter/claude/adapter_test.go b/internal/adapter/claude/adapter_test.go index 0ad2f86..266af34 100644 --- a/internal/adapter/claude/adapter_test.go +++ b/internal/adapter/claude/adapter_test.go @@ -375,17 +375,6 @@ func TestPromptFile(t *testing.T) { } } -func TestAdapterMergeFileReturnsNil(t *testing.T) { - a := claude.NewAdapter() - out, err := a.MergeFile("agents/build.md", []byte("x"), map[string]any{"a": 1}) - if err != nil { - t.Fatal(err) - } - if out != nil { - t.Errorf("expected nil, got %q", out) - } -} - func TestValidateAgent(t *testing.T) { a := claude.NewAdapter() err := a.ValidateAgent(pivot.AgentDefinition{ID: "x", Mode: "invalid"}) diff --git a/internal/adapter/codex/adapter.go b/internal/adapter/codex/adapter.go index 87febb7..dc3441d 100644 --- a/internal/adapter/codex/adapter.go +++ b/internal/adapter/codex/adapter.go @@ -7,16 +7,18 @@ import ( "strconv" "strings" + "github.com/S1933/Shenron/internal/adapter" "github.com/S1933/Shenron/internal/fsutil" "github.com/S1933/Shenron/internal/pivot" "github.com/pelletier/go-toml/v2" ) +const fileMode = 0o644 + // Adapter renders Shenron definitions into Codex custom-agent and custom-prompt files. type Adapter struct { - baseDir string - pivotDir string - nativeNames map[string]string + baseDir string + pivotDir string } type agentFile struct { @@ -34,14 +36,13 @@ type agentFile struct { func NewAdapter() *Adapter { return NewAdapterWithBaseDir(fsutil.CodexPath(), "") } func NewAdapterWithBaseDir(baseDir, pivotDir string) *Adapter { - return &Adapter{baseDir: baseDir, pivotDir: pivotDir, nativeNames: map[string]string{}} + return &Adapter{baseDir: baseDir, pivotDir: pivotDir} } func (a *Adapter) Name() string { return "codex" } func (a *Adapter) SetPivotDir(dir string) { a.pivotDir = dir - a.nativeNames = map[string]string{} } func (a *Adapter) ValidateAgent(agent pivot.AgentDefinition) error { @@ -51,53 +52,74 @@ func (a *Adapter) ValidateAgent(agent pivot.AgentDefinition) error { return nil } -func (a *Adapter) GenerateAgent(agent pivot.AgentDefinition) (map[string]string, error) { - if err := a.ValidateAgent(agent); err != nil { - return nil, err - } - nativeName := codexName(agent.ID, agent.Extensions) - a.nativeNames[agent.ID] = nativeName - instructions, err := resolveInstructions(agent, a.pivotDir) - if err != nil { - return nil, err - } +// Generate renders custom-agent TOML files and custom-prompt Markdown files. +// The pivot-id -> native-name map is built locally so a command's delegation +// line can reference the referenced agent's resolved Codex name. +func (a *Adapter) Generate(pf *pivot.PivotFile) (adapter.GenerationResult, error) { + var files []adapter.GeneratedFile + nativeNames := make(map[string]string, len(pf.Agents)) - native := agentFile{ - Name: nativeName, Description: agent.Description, Model: resolveModel(agent), - ModelReasoningEffort: codexString(agent.Extensions, "modelReasoningEffort"), - SandboxMode: resolveSandbox(agent), ApprovalPolicy: resolveApproval(agent), - WebSearch: resolveWebSearch(agent), NicknameCandidates: codexStrings(agent.Extensions, "nicknameCandidates"), - DeveloperInstructions: instructions, - } - data, err := toml.Marshal(native) - if err != nil { - return nil, fmt.Errorf("marshal Codex agent: %w", err) - } - return map[string]string{filepath.Join(a.baseDir, "agents", nativeName+".toml"): string(data)}, nil -} + for _, ag := range pf.Agents { + if err := a.ValidateAgent(ag); err != nil { + return adapter.GenerationResult{}, err + } + nativeName := codexName(ag.ID, ag.Extensions) + nativeNames[ag.ID] = nativeName + instructions, err := resolveInstructions(ag, a.pivotDir) + if err != nil { + return adapter.GenerationResult{}, fmt.Errorf("generate agent %q: %w", ag.ID, err) + } -func (a *Adapter) GenerateCommand(cmd pivot.CommandDefinition) (map[string]string, error) { - nativeAgent := cmd.Agent - if name, ok := a.nativeNames[cmd.Agent]; ok { - nativeAgent = name + native := agentFile{ + Name: nativeName, Description: ag.Description, Model: resolveModel(ag), + ModelReasoningEffort: codexString(ag.Extensions, "modelReasoningEffort"), + SandboxMode: resolveSandbox(ag), ApprovalPolicy: resolveApproval(ag), + WebSearch: resolveWebSearch(ag), NicknameCandidates: codexStrings(ag.Extensions, "nicknameCandidates"), + DeveloperInstructions: instructions, + } + data, err := toml.Marshal(native) + if err != nil { + return adapter.GenerationResult{}, fmt.Errorf("marshal Codex agent %q: %w", ag.ID, err) + } + files = append(files, adapter.GeneratedFile{ + Path: filepath.Join(a.baseDir, "agents", nativeName+".toml"), + Content: data, + Mode: fileMode, + Adapter: a.Name(), + ResourceID: ag.ID, + }) } - var content strings.Builder - content.WriteString("---\ndescription: ") - content.WriteString(strconv.Quote(cmd.Description)) - content.WriteString("\n---\n\n") - if nativeAgent != "" { - content.WriteString("Delegate this task to the `") - content.WriteString(nativeAgent) - content.WriteString("` custom agent.\n\n") + + for _, cmd := range pf.Commands { + nativeAgent := cmd.Agent + if name, ok := nativeNames[cmd.Agent]; ok { + nativeAgent = name + } + var content strings.Builder + content.WriteString("---\ndescription: ") + content.WriteString(strconv.Quote(cmd.Description)) + content.WriteString("\n---\n\n") + if nativeAgent != "" { + content.WriteString("Delegate this task to the `") + content.WriteString(nativeAgent) + content.WriteString("` custom agent.\n\n") + } + content.WriteString(cmd.Template) + files = append(files, adapter.GeneratedFile{ + Path: filepath.Join(a.baseDir, "prompts", codexName(cmd.ID, cmd.Extensions)+".md"), + Content: []byte(content.String()), + Mode: fileMode, + Adapter: a.Name(), + ResourceID: cmd.ID, + }) } - content.WriteString(cmd.Template) - return map[string]string{filepath.Join(a.baseDir, "prompts", codexName(cmd.ID, cmd.Extensions)+".md"): content.String()}, nil + + return adapter.GenerationResult{Files: files}, nil } func (a *Adapter) TargetPaths() []string { return []string{filepath.Join(a.baseDir, "agents"), filepath.Join(a.baseDir, "prompts")} } -func (a *Adapter) MergeFile(string, []byte, map[string]any) ([]byte, error) { return nil, nil } func resolveInstructions(agent pivot.AgentDefinition, pivotDir string) (string, error) { instructions := agent.SystemPrompt diff --git a/internal/adapter/codex/adapter_test.go b/internal/adapter/codex/adapter_test.go index 23a6387..55a9fef 100644 --- a/internal/adapter/codex/adapter_test.go +++ b/internal/adapter/codex/adapter_test.go @@ -5,11 +5,22 @@ import ( "strings" "testing" + "github.com/S1933/Shenron/internal/adapter" "github.com/S1933/Shenron/internal/adapter/codex" "github.com/S1933/Shenron/internal/pivot" "github.com/pelletier/go-toml/v2" ) +// fileContent returns the body of the generated file at path, or "" if absent. +func fileContent(files []adapter.GeneratedFile, path string) string { + for _, f := range files { + if f.Path == path { + return string(f.Content) + } + } + return "" +} + func TestGenerateAgentUsesCodexNativeFields(t *testing.T) { baseDir := t.TempDir() a := codex.NewAdapterWithBaseDir(baseDir, "") @@ -29,11 +40,11 @@ func TestGenerateAgentUsesCodexNativeFields(t *testing.T) { }, } - files, err := a.GenerateAgent(agent) + result, err := a.Generate(&pivot.PivotFile{Agents: []pivot.AgentDefinition{agent}}) if err != nil { t.Fatal(err) } - content := files[filepath.Join(baseDir, "agents", "build.toml")] + content := fileContent(result.Files, filepath.Join(baseDir, "agents", "build.toml")) for _, want := range []string{ "name = 'build'", "description = 'Build approved changes.'", @@ -59,20 +70,19 @@ func TestGenerateAgentUsesCodexNativeFields(t *testing.T) { func TestGenerateCommandDelegatesToCodexAgentName(t *testing.T) { baseDir := t.TempDir() a := codex.NewAdapterWithBaseDir(baseDir, "") - if _, err := a.GenerateAgent(pivot.AgentDefinition{ - ID: "code-review", Description: "Review code.", Mode: "subagent", - Extensions: map[string]any{"codex": map[string]any{"name": "code_reviewer"}}, - }); err != nil { - t.Fatal(err) - } - - files, err := a.GenerateCommand(pivot.CommandDefinition{ - ID: "review", Description: "Review the current change.", Agent: "code-review", Template: "Find defects.", + result, err := a.Generate(&pivot.PivotFile{ + Agents: []pivot.AgentDefinition{{ + ID: "code-review", Description: "Review code.", Mode: "subagent", + Extensions: map[string]any{"codex": map[string]any{"name": "code_reviewer"}}, + }}, + Commands: []pivot.CommandDefinition{{ + ID: "review", Description: "Review the current change.", Agent: "code-review", Template: "Find defects.", + }}, }) if err != nil { t.Fatal(err) } - content := files[filepath.Join(baseDir, "prompts", "review.md")] + content := fileContent(result.Files, filepath.Join(baseDir, "prompts", "review.md")) for _, want := range []string{ `description: "Review the current change."`, "Delegate this task to the `code_reviewer` custom agent.", diff --git a/internal/adapter/opencode/adapter.go b/internal/adapter/opencode/adapter.go index 1a5fb47..2f7c403 100644 --- a/internal/adapter/opencode/adapter.go +++ b/internal/adapter/opencode/adapter.go @@ -8,34 +8,28 @@ import ( "sort" "strings" + "github.com/S1933/Shenron/internal/adapter" "github.com/S1933/Shenron/internal/fsutil" "github.com/S1933/Shenron/internal/pivot" ) const configFileName = "opencode.json" +const fileMode = 0o644 // Adapter implements the OpenCode target adapter. type Adapter struct { - baseDir string - pivotDir string - fragments map[string]any + baseDir string + pivotDir string } // NewAdapter creates an OpenCode adapter writing to the default config directory. func NewAdapter() *Adapter { - return &Adapter{ - baseDir: fsutil.OpenCodePath(), - fragments: make(map[string]any), - } + return &Adapter{baseDir: fsutil.OpenCodePath()} } // NewAdapterWithBaseDir creates an adapter with a custom base directory (for tests). func NewAdapterWithBaseDir(baseDir, pivotDir string) *Adapter { - return &Adapter{ - baseDir: baseDir, - pivotDir: pivotDir, - fragments: make(map[string]any), - } + return &Adapter{baseDir: baseDir, pivotDir: pivotDir} } // SetPivotDir sets the pivot directory for promptFile resolution. @@ -56,48 +50,50 @@ func (a *Adapter) ValidateAgent(agent pivot.AgentDefinition) error { return nil } -// Fragments returns accumulated JSON fragments for opencode.json merge. -func (a *Adapter) Fragments() map[string]any { - return a.fragments -} - -// ResetFragments clears accumulated fragments before a new generation pass. -func (a *Adapter) ResetFragments() { - a.fragments = make(map[string]any) -} - -// GenerateAgent produces prompt files and accumulates the JSON fragment. -func (a *Adapter) GenerateAgent(agent pivot.AgentDefinition) (map[string]string, error) { - if err := a.ValidateAgent(agent); err != nil { - return nil, err - } - - fragment, promptRel, promptContent, err := GenerateAgentFragment(agent, a.pivotDir) - if err != nil { - return nil, err - } - - a.fragments["agent."+agent.ID] = fragment - - files := map[string]string{} - if promptContent != "" || agent.SystemPrompt != "" || agent.PromptFile != "" { - files[filepath.Join(a.baseDir, promptRel)] = promptContent +// Generate produces prompt/command body files and accumulates the JSON +// fragments that a later MergeFile/PruneManaged folds into opencode.json. +// Fragments are collected in a local map, so the adapter holds no state +// between calls. +func (a *Adapter) Generate(pf *pivot.PivotFile) (adapter.GenerationResult, error) { + var files []adapter.GeneratedFile + fragments := make(map[string]any) + + for _, ag := range pf.Agents { + if err := a.ValidateAgent(ag); err != nil { + return adapter.GenerationResult{}, err + } + fragment, promptRel, promptContent, err := GenerateAgentFragment(ag, a.pivotDir) + if err != nil { + return adapter.GenerationResult{}, fmt.Errorf("generate agent %q: %w", ag.ID, err) + } + fragments["agent."+ag.ID] = fragment + if promptContent != "" || ag.SystemPrompt != "" || ag.PromptFile != "" { + files = append(files, adapter.GeneratedFile{ + Path: filepath.Join(a.baseDir, promptRel), + Content: []byte(promptContent), + Mode: fileMode, + Adapter: a.Name(), + ResourceID: ag.ID, + }) + } } - return files, nil -} -// GenerateCommand produces command template files and accumulates the JSON fragment. -func (a *Adapter) GenerateCommand(cmd pivot.CommandDefinition) (map[string]string, error) { - fragment, cmdRel, cmdContent, err := GenerateCommandFragment(cmd) - if err != nil { - return nil, err + for _, cmd := range pf.Commands { + fragment, cmdRel, cmdContent, err := GenerateCommandFragment(cmd) + if err != nil { + return adapter.GenerationResult{}, fmt.Errorf("generate command %q: %w", cmd.ID, err) + } + fragments["command."+cmd.ID] = fragment + files = append(files, adapter.GeneratedFile{ + Path: filepath.Join(a.baseDir, cmdRel), + Content: []byte(cmdContent), + Mode: fileMode, + Adapter: a.Name(), + ResourceID: cmd.ID, + }) } - a.fragments["command."+cmd.ID] = fragment - - return map[string]string{ - filepath.Join(a.baseDir, cmdRel): cmdContent, - }, nil + return adapter.GenerationResult{Files: files, Fragments: fragments}, nil } // TargetPaths returns paths this adapter writes to. diff --git a/internal/adapter/opencode/adapter_test.go b/internal/adapter/opencode/adapter_test.go index e156bd5..f81f60f 100644 --- a/internal/adapter/opencode/adapter_test.go +++ b/internal/adapter/opencode/adapter_test.go @@ -438,20 +438,43 @@ func TestAdapterGenerateAgentIntegration(t *testing.T) { pf, pivotDir := testPivot(t) a := opencode.NewAdapterWithBaseDir(t.TempDir(), pivotDir) - files, err := a.GenerateAgent(pf.Agents[0]) + result, err := a.Generate(&pivot.PivotFile{Agents: []pivot.AgentDefinition{pf.Agents[0]}}) if err != nil { t.Fatal(err) } - if len(files) != 1 { - t.Fatalf("expected 1 file, got %d", len(files)) + if len(result.Files) != 1 { + t.Fatalf("expected 1 file, got %d", len(result.Files)) } - fragments := a.Fragments() - if _, ok := fragments["agent.build"]; !ok { + if _, ok := result.Fragments["agent.build"]; !ok { t.Error("missing agent.build fragment") } } +// TestGenerateIsReentrant guards the removal of the adapter's mutable fragment +// state: generating twice on the same instance must yield identical results, +// with no fragments accumulating across calls. +func TestGenerateIsReentrant(t *testing.T) { + pf, pivotDir := testPivot(t) + a := opencode.NewAdapterWithBaseDir(t.TempDir(), pivotDir) + + first, err := a.Generate(pf) + if err != nil { + t.Fatal(err) + } + second, err := a.Generate(pf) + if err != nil { + t.Fatal(err) + } + + if len(first.Fragments) != len(second.Fragments) { + t.Fatalf("fragments accumulated across calls: %d then %d", len(first.Fragments), len(second.Fragments)) + } + if len(first.Files) != len(second.Files) { + t.Fatalf("file count differs across calls: %d then %d", len(first.Files), len(second.Files)) + } +} + func TestEmptyPermissionOverrideFallsBackToRead(t *testing.T) { agent := pivot.AgentDefinition{ ID: "test", diff --git a/internal/cli/package_apply.go b/internal/cli/package_apply.go index 4fc830d..6291c6e 100644 --- a/internal/cli/package_apply.go +++ b/internal/cli/package_apply.go @@ -101,7 +101,7 @@ func RunPackagePush(opts PackagePushOptions) error { return fmt.Errorf("%w for %s@%s: %s; rerun with --allow-permissions", ErrPackagePermissions, installed.Name, installed.Revision, strings.Join(grants, ", ")) } - preflight := func(generated map[string]map[string]string, state *diff.StateFile, adapters map[string]adapter.Adapter) error { + preflight := func(generated map[string][]adapter.GeneratedFile, state *diff.StateFile, adapters map[string]adapter.Adapter) error { if err := rejectForeignPackageCollisions(pkg.Pivot, generated, state); err != nil { return err } @@ -115,7 +115,7 @@ func RunPackagePush(opts PackagePushOptions) error { } return nil } - postflight := func(generated map[string]map[string]string, state *diff.StateFile) error { + postflight := func(generated map[string][]adapter.GeneratedFile, state *diff.StateFile) error { return nil } return runPushAt(filepath.Join(installed.Root, shenronpackage.PivotFileName), opts.Target, opts.Force, opts.Adapters, store.StateDir(installed.Name), preflight, postflight, output, os.Stderr) @@ -299,16 +299,16 @@ func savePackageApproval(store *shenronpackage.Store, installed *shenronpackage. return nil } -func rejectForeignPackageCollisions(pf *pivot.PivotFile, generated map[string]map[string]string, state *diff.StateFile) error { +func rejectForeignPackageCollisions(pf *pivot.PivotFile, generated map[string][]adapter.GeneratedFile, state *diff.StateFile) error { for target, files := range generated { - for path := range files { - if target == "opencode" && filepath.Base(path) == "opencode.json" { - if err := rejectForeignOpenCodeCollisions(path, pf, state); err != nil { + for _, f := range files { + if target == "opencode" && filepath.Base(f.Path) == "opencode.json" { + if err := rejectForeignOpenCodeCollisions(f.Path, pf, state); err != nil { return err } continue } - if err := rejectForeignFileCollision(path, state); err != nil { + if err := rejectForeignFileCollision(f.Path, state); err != nil { return err } } @@ -358,10 +358,10 @@ func rejectForeignOpenCodeCollisions(path string, pf *pivot.PivotFile, state *di return nil } -func recordPackageOpenCodeOwnership(pf *pivot.PivotFile, files map[string]string, state *diff.StateFile) { - for path := range files { - if filepath.Base(path) == "opencode.json" { - state.SetManaged(path, packageOpenCodeManaged(pf)) +func recordPackageOpenCodeOwnership(pf *pivot.PivotFile, files []adapter.GeneratedFile, state *diff.StateFile) { + for _, f := range files { + if filepath.Base(f.Path) == "opencode.json" { + state.SetManaged(f.Path, packageOpenCodeManaged(pf)) } } } diff --git a/internal/cli/sync.go b/internal/cli/sync.go index 247a6b3..a7593c5 100644 --- a/internal/cli/sync.go +++ b/internal/cli/sync.go @@ -12,78 +12,33 @@ import ( "github.com/S1933/Shenron/internal/pivot" ) -type pivotDirSetter interface { - SetPivotDir(string) -} - -type fragmentAccumulator interface { - ResetFragments() - Fragments() map[string]any - ConfigPath() string -} +const configFileMode = 0o644 -type managedPruner interface { - PruneManaged(path string, existing []byte, managed map[string][]string, fragments map[string]any) ([]byte, error) -} - -// Generate produces the file map for each adapter from a parsed pivot file. -// state may be nil; when set, adapters that implement managedPruner use it to -// prune leaves they previously managed but the pivot no longer generates. -func Generate(pf *pivot.PivotFile, pivotDir string, adapters map[string]adapter.Adapter, state *diff.StateFile) (map[string]map[string]string, error) { - out := make(map[string]map[string]string, len(adapters)) +// Generate produces the generated files for each adapter from a parsed pivot +// file. state may be nil; when set, adapters that implement adapter.ManagedPruner +// use it to prune leaves they previously managed but the pivot no longer +// generates. +func Generate(pf *pivot.PivotFile, pivotDir string, adapters map[string]adapter.Adapter, state *diff.StateFile) (map[string][]adapter.GeneratedFile, error) { + out := make(map[string][]adapter.GeneratedFile, len(adapters)) for name, adpt := range adapters { - if setter, ok := adpt.(pivotDirSetter); ok { + if setter, ok := adpt.(adapter.PivotDirectoryAware); ok { setter.SetPivotDir(pivotDir) } - if acc, ok := adpt.(fragmentAccumulator); ok { - acc.ResetFragments() - } - - files := make(map[string]string) - - for _, agent := range pf.Agents { - agentFiles, err := adpt.GenerateAgent(agent) - if err != nil { - return nil, fmt.Errorf("%s: generate agent %q: %w", name, agent.ID, err) - } - for path, content := range agentFiles { - files[path] = content - } - } - for _, cmd := range pf.Commands { - cmdFiles, err := adpt.GenerateCommand(cmd) - if err != nil { - return nil, fmt.Errorf("%s: generate command %q: %w", name, cmd.ID, err) - } - for path, content := range cmdFiles { - files[path] = content - } + result, err := adpt.Generate(pf) + if err != nil { + return nil, fmt.Errorf("%s: %w", name, err) } + files := result.Files - if acc, ok := adpt.(fragmentAccumulator); ok { - configPath := acc.ConfigPath() - var existing []byte - data, err := os.ReadFile(configPath) - if err != nil { - if !os.IsNotExist(err) { - return nil, fmt.Errorf("%s: read %s: %w", name, filepath.Base(configPath), err) - } - } else { - existing = data - } - var merged []byte - if pruner, ok := adpt.(managedPruner); ok && state != nil { - merged, err = pruner.PruneManaged(configPath, existing, state.Managed(configPath), acc.Fragments()) - } else { - merged, err = adpt.MergeFile(configPath, existing, acc.Fragments()) - } + if merger, ok := adpt.(adapter.MergingAdapter); ok { + configFile, err := mergeConfig(name, merger, adpt, result.Fragments, state) if err != nil { - return nil, fmt.Errorf("%s: merge %s: %w", name, filepath.Base(configPath), err) + return nil, err } - if merged != nil { - files[configPath] = string(merged) + if configFile != nil { + files = append(files, *configFile) } } @@ -93,10 +48,46 @@ func Generate(pf *pivot.PivotFile, pivotDir string, adapters map[string]adapter. return out, nil } -// Ensure opencode.Adapter satisfies optional interfaces at compile time. +// mergeConfig folds accumulated fragments into the adapter's shared config file, +// pruning previously-managed leaves first when the adapter and state support it. +func mergeConfig(name string, merger adapter.MergingAdapter, adpt adapter.Adapter, fragments map[string]any, state *diff.StateFile) (*adapter.GeneratedFile, error) { + configPath := merger.ConfigPath() + + var existing []byte + data, err := os.ReadFile(configPath) + if err != nil { + if !os.IsNotExist(err) { + return nil, fmt.Errorf("%s: read %s: %w", name, filepath.Base(configPath), err) + } + } else { + existing = data + } + + var merged []byte + if pruner, ok := adpt.(adapter.ManagedPruner); ok && state != nil { + merged, err = pruner.PruneManaged(configPath, existing, state.Managed(configPath), fragments) + } else { + merged, err = merger.MergeFile(configPath, existing, fragments) + } + if err != nil { + return nil, fmt.Errorf("%s: merge %s: %w", name, filepath.Base(configPath), err) + } + if merged == nil { + return nil, nil + } + + return &adapter.GeneratedFile{ + Path: configPath, + Content: merged, + Mode: configFileMode, + Adapter: name, + }, nil +} + +// Ensure adapters satisfy the optional capability interfaces at compile time. var ( - _ pivotDirSetter = (*claude.Adapter)(nil) - _ pivotDirSetter = (*opencode.Adapter)(nil) - _ fragmentAccumulator = (*opencode.Adapter)(nil) - _ managedPruner = (*opencode.Adapter)(nil) + _ adapter.PivotDirectoryAware = (*claude.Adapter)(nil) + _ adapter.PivotDirectoryAware = (*opencode.Adapter)(nil) + _ adapter.MergingAdapter = (*opencode.Adapter)(nil) + _ adapter.ManagedPruner = (*opencode.Adapter)(nil) ) diff --git a/internal/cli/sync_runtime.go b/internal/cli/sync_runtime.go index 4ab5e6e..fc52270 100644 --- a/internal/cli/sync_runtime.go +++ b/internal/cli/sync_runtime.go @@ -75,8 +75,8 @@ func RunPush(opts PushOptions) error { return runPushAt(opts.ConfigPath, opts.Target, opts.Force, opts.Adapters, "", nil, nil, os.Stdout, os.Stderr) } -type pushPreflight func(generated map[string]map[string]string, state *diff.StateFile, adapters map[string]adapter.Adapter) error -type pushPostflight func(generated map[string]map[string]string, state *diff.StateFile) error +type pushPreflight func(generated map[string][]adapter.GeneratedFile, state *diff.StateFile, adapters map[string]adapter.Adapter) error +type pushPostflight func(generated map[string][]adapter.GeneratedFile, state *diff.StateFile) error // runDiffAt and runPushAt are the library entry points used by both the // public Go API and the package flow. They accept an explicit stateDir so the @@ -101,7 +101,7 @@ func runDiffAt(configPath, target string, adapters map[string]adapter.Adapter, s for _, name := range sortedAdapterNames(generated) { files := generated[name] - results, err := diff.ComputeDiffs(files, state, scope) + results, err := diff.ComputeDiffs(contentMap(files), state, scope) if err != nil { return err } @@ -189,7 +189,8 @@ func runPushAt(configPath, target string, force bool, adapters map[string]adapte wroteAny := false for _, name := range sortedAdapterNames(generated) { files := generated[name] - adapterResults, err := diff.ComputeDiffs(files, state, scope) + byPath := indexByPath(files) + adapterResults, err := diff.ComputeDiffs(contentMap(files), state, scope) if err != nil { return err } @@ -198,11 +199,11 @@ func runPushAt(configPath, target string, force bool, adapters map[string]adapte for _, r := range adapterResults { switch r.Status { case diff.StatusCreated, diff.StatusModified, diff.StatusManuallyModified: - content := files[r.Path] - if err := fsutil.WriteFileAtomic(r.Path, []byte(content), 0o644); err != nil { + gf := byPath[r.Path] + if err := fsutil.WriteFileAtomic(r.Path, gf.Content, gf.Mode); err != nil { return fmt.Errorf("write %s: %w", r.Path, err) } - state.SetFile(r.Path, name, []byte(content)) + state.SetFile(r.Path, name, gf.Content) fmt.Fprintf(stdout, "[%s] wrote %s (%s)\n", name, r.Path, diffStatusName(r.Status)) wroteAny = true case diff.StatusUnchanged: @@ -250,17 +251,39 @@ func printOrphanWarnings(stderr io.Writer, results []diff.DiffResult) { } } -func mergeGenerated(generated map[string]map[string]string) map[string]string { +// mergeGenerated flattens every adapter's files into a single path->content map +// for whole-tree diff and orphan detection. +func mergeGenerated(generated map[string][]adapter.GeneratedFile) map[string]string { merged := make(map[string]string) for _, files := range generated { - for path, content := range files { - merged[path] = content + for _, f := range files { + merged[f.Path] = string(f.Content) } } return merged } -func prepareSyncAt(configPath, target string, adapters map[string]adapter.Adapter, stateDir string) (pivotDir string, generated map[string]map[string]string, state *diff.StateFile, resolved map[string]adapter.Adapter, err error) { +// contentMap projects a slice of generated files onto a path->content map for +// the diff engine. +func contentMap(files []adapter.GeneratedFile) map[string]string { + out := make(map[string]string, len(files)) + for _, f := range files { + out[f.Path] = string(f.Content) + } + return out +} + +// indexByPath keys generated files by their destination path so the write loop +// can recover each file's mode and content. +func indexByPath(files []adapter.GeneratedFile) map[string]adapter.GeneratedFile { + out := make(map[string]adapter.GeneratedFile, len(files)) + for _, f := range files { + out[f.Path] = f + } + return out +} + +func prepareSyncAt(configPath, target string, adapters map[string]adapter.Adapter, stateDir string) (pivotDir string, generated map[string][]adapter.GeneratedFile, state *diff.StateFile, resolved map[string]adapter.Adapter, err error) { path, err := pivot.Discover(configPath) if err != nil { return "", nil, nil, nil, err @@ -314,7 +337,7 @@ func buildOrphanScope(adapters map[string]adapter.Adapter) *diff.OrphanScope { return scope } -func sortedAdapterNames(generated map[string]map[string]string) []string { +func sortedAdapterNames(generated map[string][]adapter.GeneratedFile) []string { names := make([]string, 0, len(generated)) for name := range generated { names = append(names, name) diff --git a/internal/cli/sync_test.go b/internal/cli/sync_test.go index ad185d5..b013cb0 100644 --- a/internal/cli/sync_test.go +++ b/internal/cli/sync_test.go @@ -40,8 +40,8 @@ func TestGenerateOpenCode(t *testing.T) { } foundConfig := false - for path := range files { - if filepath.Base(path) == "opencode.json" { + for _, f := range files { + if filepath.Base(f.Path) == "opencode.json" { foundConfig = true } } From e0c2437d37696f0b8c8f70b4e959112dadf96117 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Philippe=20D=C3=A9=C3=AFs=20Nuel?= Date: Mon, 13 Jul 2026 23:05:22 +0200 Subject: [PATCH 3/6] feat(fsutil): journalled push transaction with roll-forward recovery (P1.4) Make the multi-file push atomic and crash-recoverable: - Add fsutil.Transaction: Stage writes each file to a temp file in its destination directory; Commit journals the pending renames to .shenron-journal.json, performs them, then removes the journal. If a rename fails partway, the journal is left for recovery. - Add fsutil.RecoverTransaction: replays a leftover journal's pending renames (roll-forward) and removes it. runPushAt runs recovery at startup, before reading on-disk state, so a push interrupted after its journal was written completes on the next run. - runPushAt now stages every changed file and commits the batch as one transaction instead of renaming file-by-file, so a failure mid-batch leaves nothing applied. The write batch is now all-or-nothing. Updated the interrupt test to assert atomic semantics (no file committed when the batch fails) while keeping its core invariant: Managed is persisted before writes, so a re-push never collides on the package's own opencode.json entries. Co-Authored-By: Claude Opus 4.8 --- internal/cli/package_test.go | 23 +++-- internal/cli/sync_runtime.go | 44 ++++++++- internal/fsutil/journal.go | 156 ++++++++++++++++++++++++++++++++ internal/fsutil/journal_test.go | 123 +++++++++++++++++++++++++ 4 files changed, 334 insertions(+), 12 deletions(-) create mode 100644 internal/fsutil/journal.go create mode 100644 internal/fsutil/journal_test.go diff --git a/internal/cli/package_test.go b/internal/cli/package_test.go index 1809a2a..094c79e 100644 --- a/internal/cli/package_test.go +++ b/internal/cli/package_test.go @@ -315,10 +315,11 @@ commands: [] base := filepath.Join(t.TempDir(), "opencode") adapters := map[string]adapter.Adapter{"opencode": opencode.NewAdapterWithBaseDir(base, "")} - // Make the prompts directory unwritable so the push writes opencode.json - // (sorts first) and then fails writing the prompt file, simulating a - // crash mid-push: Managed must already be persisted by then, before - // postflight or the final SaveState ever run. + // Make the prompts directory unwritable so staging the prompt file fails, + // simulating a crash mid-push. Two invariants must hold: (1) Managed is + // persisted before any native write, so a re-push does not collide on the + // package's own opencode.json entries; (2) the write batch is atomic — a + // failure staging one file leaves none of the batch on disk. promptsDir := filepath.Join(base, "prompts") if err := os.MkdirAll(promptsDir, 0o755); err != nil { t.Fatal(err) @@ -331,18 +332,26 @@ commands: [] if err := cli.RunPackagePush(cli.PackagePushOptions{Store: store, Name: "acme-reviewers", Adapters: adapters}); err == nil { t.Fatal("expected first push to fail writing the read-only prompts directory") } - if _, err := os.Stat(filepath.Join(base, "opencode.json")); err != nil { - t.Fatalf("opencode.json should have been written before the interruption: %v", err) + // Atomicity: opencode.json was staged but never committed, so nothing lands. + if _, err := os.Stat(filepath.Join(base, "opencode.json")); !os.IsNotExist(err) { + t.Fatalf("no file should be committed when the batch fails, got: %v", err) } if err := os.Chmod(promptsDir, 0o755); err != nil { t.Fatal(err) } - // Re-push must NOT raise ErrPackageCollision on its own opencode.json entries. + // Re-push must NOT raise ErrPackageCollision on its own opencode.json entries, + // and must complete the write this time. if err := cli.RunPackagePush(cli.PackagePushOptions{Store: store, Name: "acme-reviewers", Adapters: adapters}); err != nil { t.Fatalf("re-push after simulated interrupt should succeed, got: %v", err) } + if _, err := os.Stat(filepath.Join(base, "opencode.json")); err != nil { + t.Fatalf("opencode.json should exist after successful re-push: %v", err) + } + if _, err := os.Stat(filepath.Join(base, "prompts", "build.md")); err != nil { + t.Fatalf("prompt file should exist after successful re-push: %v", err) + } } func TestRunPackagePushPrunesRemovedAgent(t *testing.T) { diff --git a/internal/cli/sync_runtime.go b/internal/cli/sync_runtime.go index fc52270..196b8c2 100644 --- a/internal/cli/sync_runtime.go +++ b/internal/cli/sync_runtime.go @@ -146,6 +146,20 @@ func runPushAt(configPath, target string, force bool, adapters map[string]adapte stderr = os.Stderr } + // Complete any push interrupted after its journal was written, before we + // read current on-disk state for diffing (roll-forward recovery). + recoverDir := stateDir + if recoverDir == "" { + if path, derr := pivot.Discover(configPath); derr == nil { + recoverDir = filepath.Dir(path) + } + } + if recoverDir != "" { + if err := fsutil.RecoverTransaction(recoverDir); err != nil { + return fmt.Errorf("recover interrupted push: %w", err) + } + } + pivotDir, generated, state, adapters, err := prepareSyncAt(configPath, target, adapters, stateDir) if err != nil { return err @@ -186,12 +200,16 @@ func runPushAt(configPath, target string, force bool, adapters map[string]adapte printOrphanWarnings(stderr, diff.OrphanedOnly(results)) - wroteAny := false + // Stage every changed file, then commit the batch through a journalled + // transaction so a crash mid-write can be completed on the next push. + tx := fsutil.NewTransaction(stateDir) + var logs []writeLog for _, name := range sortedAdapterNames(generated) { files := generated[name] byPath := indexByPath(files) adapterResults, err := diff.ComputeDiffs(contentMap(files), state, scope) if err != nil { + tx.Discard() return err } adapterResults = diff.FilterOrphaned(adapterResults) @@ -200,17 +218,25 @@ func runPushAt(configPath, target string, force bool, adapters map[string]adapte switch r.Status { case diff.StatusCreated, diff.StatusModified, diff.StatusManuallyModified: gf := byPath[r.Path] - if err := fsutil.WriteFileAtomic(r.Path, gf.Content, gf.Mode); err != nil { - return fmt.Errorf("write %s: %w", r.Path, err) + if err := tx.Stage(gf.Path, gf.Content, gf.Mode); err != nil { + tx.Discard() + return fmt.Errorf("stage %s: %w", r.Path, err) } state.SetFile(r.Path, name, gf.Content) - fmt.Fprintf(stdout, "[%s] wrote %s (%s)\n", name, r.Path, diffStatusName(r.Status)) - wroteAny = true + logs = append(logs, writeLog{name: name, path: r.Path, status: r.Status}) case diff.StatusUnchanged: state.SetFile(r.Path, name, []byte(r.NewContent)) } } } + + if err := tx.Commit(); err != nil { + return fmt.Errorf("commit push: %w", err) + } + for _, l := range logs { + fmt.Fprintf(stdout, "[%s] wrote %s (%s)\n", l.name, l.path, diffStatusName(l.status)) + } + wroteAny := len(logs) > 0 if postflight != nil { if err := postflight(generated, state); err != nil { return err @@ -230,6 +256,14 @@ func runPushAt(configPath, target string, force bool, adapters map[string]adapte return nil } +// writeLog records a staged write so its confirmation line can be printed only +// after the transaction commits. +type writeLog struct { + name string + path string + status diff.DiffStatus +} + func diffStatusName(status diff.DiffStatus) string { switch status { case diff.StatusCreated: diff --git a/internal/fsutil/journal.go b/internal/fsutil/journal.go new file mode 100644 index 0000000..805ec18 --- /dev/null +++ b/internal/fsutil/journal.go @@ -0,0 +1,156 @@ +package fsutil + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" +) + +const journalFileName = ".shenron-journal.json" +const journalVersion = "1" + +type journalEntry struct { + Final string `json:"final"` + Temp string `json:"temp"` +} + +type journalDoc struct { + Version string `json:"version"` + Entries []journalEntry `json:"entries"` +} + +// Transaction stages file writes as temp files and commits them as a batch: it +// records a journal listing every pending rename, renames each staged temp file +// into place, then removes the journal. A crash after the journal is written is +// repaired by RecoverTransaction, which replays the pending renames +// (roll-forward). Individual renames are atomic; the journal makes the batch +// recoverable so a mid-push crash never leaves a half-applied set with no way to +// finish it. +// +// A Transaction is not safe for concurrent use. +type Transaction struct { + dir string + entries []journalEntry +} + +// NewTransaction creates a transaction whose journal lives in dir (typically the +// state directory beside .shenron-state.json). +func NewTransaction(dir string) *Transaction { + return &Transaction{dir: dir} +} + +// Stage writes data to a temp file in the destination directory and records the +// pending rename. The file is not visible at its final path until Commit. +func (t *Transaction) Stage(path string, data []byte, perm os.FileMode) error { + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, 0o755); err != nil { + return fmt.Errorf("create parent directories: %w", err) + } + + tmp, err := os.CreateTemp(dir, ".shenron-*") + if err != nil { + return fmt.Errorf("create temp file: %w", err) + } + tmpPath := tmp.Name() + + if _, err := tmp.Write(data); err != nil { + _ = tmp.Close() + _ = os.Remove(tmpPath) + return fmt.Errorf("write temp file: %w", err) + } + if err := tmp.Chmod(perm); err != nil { + _ = tmp.Close() + _ = os.Remove(tmpPath) + return fmt.Errorf("chmod temp file: %w", err) + } + if err := tmp.Close(); err != nil { + _ = os.Remove(tmpPath) + return fmt.Errorf("close temp file: %w", err) + } + + t.entries = append(t.entries, journalEntry{Final: path, Temp: tmpPath}) + return nil +} + +// Commit journals the pending renames, performs them, then removes the journal. +// With nothing staged it is a no-op. If a rename fails partway, the journal is +// left in place so RecoverTransaction can finish the batch on the next run. +func (t *Transaction) Commit() error { + if len(t.entries) == 0 { + return nil + } + if err := t.writeJournal(); err != nil { + t.Discard() + return err + } + for _, e := range t.entries { + if err := os.Rename(e.Temp, e.Final); err != nil { + return fmt.Errorf("rename %s: %w", e.Final, err) + } + } + return t.removeJournal() +} + +// Discard removes any staged temp files without touching their destinations. +// Safe to call after a successful Commit (the temp files are already gone). +func (t *Transaction) Discard() { + for _, e := range t.entries { + _ = os.Remove(e.Temp) + } + t.entries = nil +} + +func (t *Transaction) journalPath() string { return filepath.Join(t.dir, journalFileName) } + +func (t *Transaction) writeJournal() error { + data, err := json.MarshalIndent(journalDoc{Version: journalVersion, Entries: t.entries}, "", " ") + if err != nil { + return fmt.Errorf("marshal journal: %w", err) + } + if err := WriteFileAtomic(t.journalPath(), append(data, '\n'), 0o644); err != nil { + return fmt.Errorf("write journal: %w", err) + } + return nil +} + +func (t *Transaction) removeJournal() error { + if err := os.Remove(t.journalPath()); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("remove journal: %w", err) + } + return nil +} + +// RecoverTransaction completes a push interrupted after its journal was written: +// it replays the pending renames (roll-forward) and removes the journal. Entries +// whose temp file is already gone (rename completed before the crash) are +// skipped. It is a no-op when no journal is present. +func RecoverTransaction(dir string) error { + path := filepath.Join(dir, journalFileName) + data, err := os.ReadFile(path) + if os.IsNotExist(err) { + return nil + } + if err != nil { + return fmt.Errorf("read journal: %w", err) + } + + var doc journalDoc + if err := json.Unmarshal(data, &doc); err != nil { + return fmt.Errorf("parse journal: %w", err) + } + + for _, e := range doc.Entries { + if _, err := os.Stat(e.Temp); err != nil { + continue // already renamed (or gone) — nothing to complete + } + if err := os.Rename(e.Temp, e.Final); err != nil { + return fmt.Errorf("recover rename %s: %w", e.Final, err) + } + } + + if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("remove journal after recovery: %w", err) + } + return nil +} diff --git a/internal/fsutil/journal_test.go b/internal/fsutil/journal_test.go new file mode 100644 index 0000000..ba02863 --- /dev/null +++ b/internal/fsutil/journal_test.go @@ -0,0 +1,123 @@ +package fsutil + +import ( + "os" + "path/filepath" + "testing" +) + +func TestTransactionCommitWritesFilesAndRemovesJournal(t *testing.T) { + dir := t.TempDir() + pathA := filepath.Join(dir, "a", "one.txt") + pathB := filepath.Join(dir, "b", "two.txt") + + tx := NewTransaction(dir) + if err := tx.Stage(pathA, []byte("alpha"), 0o644); err != nil { + t.Fatal(err) + } + if err := tx.Stage(pathB, []byte("beta"), 0o600); err != nil { + t.Fatal(err) + } + + // Nothing is visible before commit. + if _, err := os.Stat(pathA); !os.IsNotExist(err) { + t.Fatalf("file A visible before commit: %v", err) + } + + if err := tx.Commit(); err != nil { + t.Fatal(err) + } + + assertFile(t, pathA, "alpha", 0o644) + assertFile(t, pathB, "beta", 0o600) + + if _, err := os.Stat(filepath.Join(dir, journalFileName)); !os.IsNotExist(err) { + t.Errorf("journal not removed after commit: %v", err) + } +} + +// TestRecoverCompletesInterruptedRenames simulates a crash after the journal was +// written but before any rename ran: the staged temp files and journal are on +// disk, the finals are not. RecoverTransaction must roll the batch forward. +func TestRecoverCompletesInterruptedRenames(t *testing.T) { + dir := t.TempDir() + pathA := filepath.Join(dir, "one.txt") + pathB := filepath.Join(dir, "sub", "two.txt") + + tx := NewTransaction(dir) + if err := tx.Stage(pathA, []byte("alpha"), 0o644); err != nil { + t.Fatal(err) + } + if err := tx.Stage(pathB, []byte("beta"), 0o644); err != nil { + t.Fatal(err) + } + // Write the journal but stop before renaming — the crash point. + if err := tx.writeJournal(); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(pathA); !os.IsNotExist(err) { + t.Fatalf("final should not exist before recovery: %v", err) + } + + // A fresh process would only see the journal and temp files. + if err := RecoverTransaction(dir); err != nil { + t.Fatal(err) + } + + assertFile(t, pathA, "alpha", 0o644) + assertFile(t, pathB, "beta", 0o644) + if _, err := os.Stat(filepath.Join(dir, journalFileName)); !os.IsNotExist(err) { + t.Errorf("journal not removed after recovery: %v", err) + } +} + +func TestRecoverPartialRenamesIsResumable(t *testing.T) { + dir := t.TempDir() + pathA := filepath.Join(dir, "one.txt") + pathB := filepath.Join(dir, "two.txt") + + tx := NewTransaction(dir) + if err := tx.Stage(pathA, []byte("alpha"), 0o644); err != nil { + t.Fatal(err) + } + if err := tx.Stage(pathB, []byte("beta"), 0o644); err != nil { + t.Fatal(err) + } + if err := tx.writeJournal(); err != nil { + t.Fatal(err) + } + // Simulate a crash after the first rename completed but before the second. + if err := os.Rename(tx.entries[0].Temp, tx.entries[0].Final); err != nil { + t.Fatal(err) + } + + if err := RecoverTransaction(dir); err != nil { + t.Fatal(err) + } + assertFile(t, pathA, "alpha", 0o644) + assertFile(t, pathB, "beta", 0o644) +} + +func TestRecoverNoJournalIsNoop(t *testing.T) { + if err := RecoverTransaction(t.TempDir()); err != nil { + t.Fatalf("recover with no journal should be a no-op, got %v", err) + } +} + +func assertFile(t *testing.T, path, want string, perm os.FileMode) { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + if string(data) != want { + t.Errorf("%s = %q, want %q", path, data, want) + } + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if info.Mode().Perm() != perm { + t.Errorf("%s mode = %o, want %o", path, info.Mode().Perm(), perm) + } +} From b94e35643ff3cf7c74b09300f1b81127de7ab317 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Philippe=20D=C3=A9=C3=AFs=20Nuel?= Date: Mon, 13 Jul 2026 23:12:06 +0200 Subject: [PATCH 4/6] feat(cli): structured JSON output for diff and push (P1.5) Add `--output json` to the diff and push commands (default text). The runtime separates computation from rendering: - report.go defines DiffReport / PushReport with per-file {path, adapter, status, resourceId} entries plus orphaned paths, and builds them from the existing diff results. - runDiffAt/runPushAt take an output format; the text path is unchanged, the JSON path emits a report to stdout. - In JSON mode, stdout carries only the report; permission/skill preambles and orphan warnings are routed to stderr so the stream stays parseable. Adds end-to-end tests for both reports and for rejecting an unknown format, and documents the flag in the README. Co-Authored-By: Claude Opus 4.8 --- README.md | 3 + internal/cli/package_apply.go | 38 +++++++-- internal/cli/report.go | 151 ++++++++++++++++++++++++++++++++++ internal/cli/sync_runtime.go | 42 ++++++++-- internal/integration_test.go | 49 +++++++++++ 5 files changed, 267 insertions(+), 16 deletions(-) create mode 100644 internal/cli/report.go diff --git a/README.md b/README.md index ef3af5e..327f336 100644 --- a/README.md +++ b/README.md @@ -103,6 +103,9 @@ Common flags: - `push --allow-permissions` approves the package revision's declared permission grants; the approval is bound to the installed revision and its permission digest. +- `diff --output json` and `push --output json` emit a machine-readable + report on stdout (files with their adapter, status, and resource id, plus + any orphaned paths); human diagnostics stay on stderr. ## Pivot file diff --git a/internal/cli/package_apply.go b/internal/cli/package_apply.go index 6291c6e..1b78b0f 100644 --- a/internal/cli/package_apply.go +++ b/internal/cli/package_apply.go @@ -36,6 +36,7 @@ type PackageDiffOptions struct { Target string Adapters map[string]adapter.Adapter SkillsDir string + Format string // "text" (default) or "json" Output io.Writer } @@ -48,6 +49,7 @@ type PackagePushOptions struct { AllowPermissions bool Adapters map[string]adapter.Adapter SkillsDir string + Format string // "text" (default) or "json" Output io.Writer } @@ -59,31 +61,49 @@ type packageApproval struct { // RunPackageDiff shows a package's target diff and reports every permission // grant and missing package skill without modifying native configuration. func RunPackageDiff(opts PackageDiffOptions) error { + format, err := parseOutputFormat(opts.Format) + if err != nil { + return err + } installed, pkg, err := packageStore(opts.Store).Load(opts.Name) if err != nil { return err } output := packageOutput(opts.Output) + // Keep stdout pure JSON: send the human requirements preamble to stderr. + preamble := output + if format == formatJSON { + preamble = os.Stderr + } required, optional := missingPackageSkills(pkg.Manifest.Skills, opts.SkillsDir) - if err := printPackageRequirements(output, permissionGrants(pkg.Pivot), required, optional); err != nil { + if err := printPackageRequirements(preamble, permissionGrants(pkg.Pivot), required, optional); err != nil { return err } - return runDiffAt(filepath.Join(installed.Root, shenronpackage.PivotFileName), opts.Target, opts.Adapters, packageStore(opts.Store).StateDir(installed.Name), output, os.Stderr) + return runDiffAt(filepath.Join(installed.Root, shenronpackage.PivotFileName), opts.Target, opts.Adapters, packageStore(opts.Store).StateDir(installed.Name), format, output, os.Stderr) } // RunPackagePush applies exactly one installed package. Package state and // permission approvals are deliberately stored alongside the package cache, // never inside its immutable pivot snapshot. func RunPackagePush(opts PackagePushOptions) error { + format, err := parseOutputFormat(opts.Format) + if err != nil { + return err + } store := packageStore(opts.Store) installed, pkg, err := store.Load(opts.Name) if err != nil { return err } output := packageOutput(opts.Output) + // Keep stdout pure JSON: warnings go to stderr in JSON mode. + warnOut := output + if format == formatJSON { + warnOut = os.Stderr + } required, optional := missingPackageSkills(pkg.Manifest.Skills, opts.SkillsDir) if len(optional) > 0 { - if _, err := fmt.Fprintf(output, "warning: optional package skills unavailable: %s\n", strings.Join(optional, ", ")); err != nil { + if _, err := fmt.Fprintf(warnOut, "warning: optional package skills unavailable: %s\n", strings.Join(optional, ", ")); err != nil { return err } } @@ -118,28 +138,29 @@ func RunPackagePush(opts PackagePushOptions) error { postflight := func(generated map[string][]adapter.GeneratedFile, state *diff.StateFile) error { return nil } - return runPushAt(filepath.Join(installed.Root, shenronpackage.PivotFileName), opts.Target, opts.Force, opts.Adapters, store.StateDir(installed.Name), preflight, postflight, output, os.Stderr) + return runPushAt(filepath.Join(installed.Root, shenronpackage.PivotFileName), opts.Target, opts.Force, opts.Adapters, store.StateDir(installed.Name), format, preflight, postflight, output, os.Stderr) } // NewDiffCmd builds the top-level `diff` command. func NewDiffCmd(store func() *shenronpackage.Store) *cobra.Command { - var target string + var target, output string cmd := &cobra.Command{ Use: "diff ", Short: "Show differences for an installed configuration package", Args: cobra.ExactArgs(1), SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { - return RunPackageDiff(PackageDiffOptions{Store: store(), Name: args[0], Target: target, Output: cmd.OutOrStdout()}) + return RunPackageDiff(PackageDiffOptions{Store: store(), Name: args[0], Target: target, Format: output, Output: cmd.OutOrStdout()}) }, } cmd.Flags().StringVar(&target, "target", "", "limit to a single CLI target (e.g. opencode)") + cmd.Flags().StringVar(&output, "output", "text", "output format: text or json") return cmd } // NewPushCmd builds the top-level `push` command. func NewPushCmd(store func() *shenronpackage.Store) *cobra.Command { - var target string + var target, output string var force, allowPermissions bool cmd := &cobra.Command{ Use: "push ", @@ -147,10 +168,11 @@ func NewPushCmd(store func() *shenronpackage.Store) *cobra.Command { Args: cobra.ExactArgs(1), SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { - return RunPackagePush(PackagePushOptions{Store: store(), Name: args[0], Target: target, Force: force, AllowPermissions: allowPermissions, Output: cmd.OutOrStdout()}) + return RunPackagePush(PackagePushOptions{Store: store(), Name: args[0], Target: target, Force: force, AllowPermissions: allowPermissions, Format: output, Output: cmd.OutOrStdout()}) }, } cmd.Flags().StringVar(&target, "target", "", "limit to a single CLI target (e.g. opencode)") + cmd.Flags().StringVar(&output, "output", "text", "output format: text or json") cmd.Flags().BoolVar(&force, "force", false, "overwrite manually edited package-owned native files") cmd.Flags().BoolVar(&allowPermissions, "allow-permissions", false, "approve this package revision's declared permissions") return cmd diff --git a/internal/cli/report.go b/internal/cli/report.go new file mode 100644 index 0000000..ecff82d --- /dev/null +++ b/internal/cli/report.go @@ -0,0 +1,151 @@ +package cli + +import ( + "encoding/json" + "fmt" + "io" + "sort" + + "github.com/S1933/Shenron/internal/adapter" + "github.com/S1933/Shenron/internal/diff" +) + +// outputFormat selects how diff/push results are rendered. +type outputFormat string + +const ( + formatText outputFormat = "text" + formatJSON outputFormat = "json" +) + +// parseOutputFormat validates a user-supplied --output value. +func parseOutputFormat(s string) (outputFormat, error) { + switch s { + case "", "text": + return formatText, nil + case "json": + return formatJSON, nil + default: + return "", fmt.Errorf("unknown output format %q (want text or json)", s) + } +} + +// FileReport is one file's entry in a structured diff/push report. +type FileReport struct { + Path string `json:"path"` + Adapter string `json:"adapter"` + Status string `json:"status"` + ResourceID string `json:"resourceId,omitempty"` +} + +// DiffReport is the JSON shape emitted by `diff --output json`. +type DiffReport struct { + Files []FileReport `json:"files"` + Orphaned []string `json:"orphaned,omitempty"` + HasChanges bool `json:"hasChanges"` +} + +// PushReport is the JSON shape emitted by `push --output json`. +type PushReport struct { + Written []FileReport `json:"written"` + Orphaned []string `json:"orphaned,omitempty"` + Wrote bool `json:"wrote"` +} + +// diffStatusJSON maps a diff status onto a stable machine-readable token. +func diffStatusJSON(status diff.DiffStatus) string { + switch status { + case diff.StatusCreated: + return "created" + case diff.StatusModified: + return "modified" + case diff.StatusManuallyModified: + return "manually-modified" + case diff.StatusOrphaned: + return "orphaned" + default: + return "unchanged" + } +} + +// resourceIDByPath indexes generated files so a report can annotate each path +// with the pivot resource it came from. +func resourceIDByPath(generated map[string][]adapter.GeneratedFile) map[string]string { + out := map[string]string{} + for _, files := range generated { + for _, f := range files { + out[f.Path] = f.ResourceID + } + } + return out +} + +// writeJSON marshals v as indented JSON with a trailing newline. +func writeJSON(w io.Writer, v any) error { + data, err := json.MarshalIndent(v, "", " ") + if err != nil { + return fmt.Errorf("marshal report: %w", err) + } + if _, err := w.Write(append(data, '\n')); err != nil { + return fmt.Errorf("write report: %w", err) + } + return nil +} + +// buildDiffReport computes the full structured diff across every adapter. +func buildDiffReport(generated map[string][]adapter.GeneratedFile, state *diff.StateFile, scope *diff.OrphanScope) (DiffReport, error) { + ids := resourceIDByPath(generated) + report := DiffReport{} + + for _, name := range sortedAdapterNames(generated) { + results, err := diff.ComputeDiffs(contentMap(generated[name]), state, scope) + if err != nil { + return DiffReport{}, err + } + results = diff.FilterOrphaned(results) + if diff.HasChanges(results) { + report.HasChanges = true + } + for _, r := range results { + report.Files = append(report.Files, FileReport{ + Path: r.Path, + Adapter: name, + Status: diffStatusJSON(r.Status), + ResourceID: ids[r.Path], + }) + } + } + + allResults, err := diff.ComputeDiffs(mergeGenerated(generated), state, scope) + if err != nil { + return DiffReport{}, err + } + for _, r := range diff.OrphanedOnly(allResults) { + report.Orphaned = append(report.Orphaned, r.Path) + } + sort.Strings(report.Orphaned) + if len(report.Orphaned) > 0 { + report.HasChanges = true + } + + return report, nil +} + +// buildPushReport assembles the structured summary of a completed push. +func buildPushReport(logs []writeLog, orphans []diff.DiffResult, generated map[string][]adapter.GeneratedFile) PushReport { + ids := resourceIDByPath(generated) + report := PushReport{Wrote: len(logs) > 0} + for _, l := range logs { + report.Written = append(report.Written, FileReport{ + Path: l.path, + Adapter: l.name, + Status: diffStatusJSON(l.status), + ResourceID: ids[l.path], + }) + } + for _, r := range orphans { + report.Orphaned = append(report.Orphaned, r.Path) + } + sort.Strings(report.Orphaned) + return report +} diff --git a/internal/cli/sync_runtime.go b/internal/cli/sync_runtime.go index 196b8c2..b8ae7af 100644 --- a/internal/cli/sync_runtime.go +++ b/internal/cli/sync_runtime.go @@ -29,11 +29,16 @@ type DiffOptions struct { ConfigPath string Target string Adapters map[string]adapter.Adapter + Format string // "text" (default) or "json" } // RunDiff shows differences between pivot and native configs. func RunDiff(opts DiffOptions) error { - return runDiffAt(opts.ConfigPath, opts.Target, opts.Adapters, "", os.Stdout, os.Stderr) + format, err := parseOutputFormat(opts.Format) + if err != nil { + return err + } + return runDiffAt(opts.ConfigPath, opts.Target, opts.Adapters, "", format, os.Stdout, os.Stderr) } // CaptureOutput runs fn while capturing stdout and stderr separately. @@ -68,11 +73,16 @@ type PushOptions struct { Target string Force bool Adapters map[string]adapter.Adapter + Format string // "text" (default) or "json" } // RunPush pushes pivot config to native CLI configs. func RunPush(opts PushOptions) error { - return runPushAt(opts.ConfigPath, opts.Target, opts.Force, opts.Adapters, "", nil, nil, os.Stdout, os.Stderr) + format, err := parseOutputFormat(opts.Format) + if err != nil { + return err + } + return runPushAt(opts.ConfigPath, opts.Target, opts.Force, opts.Adapters, "", format, nil, nil, os.Stdout, os.Stderr) } type pushPreflight func(generated map[string][]adapter.GeneratedFile, state *diff.StateFile, adapters map[string]adapter.Adapter) error @@ -82,7 +92,7 @@ type pushPostflight func(generated map[string][]adapter.GeneratedFile, state *di // public Go API and the package flow. They accept an explicit stateDir so the // package flow can keep its state under ~/.shenron/packages/state//. -func runDiffAt(configPath, target string, adapters map[string]adapter.Adapter, stateDir string, stdout, stderr io.Writer) error { +func runDiffAt(configPath, target string, adapters map[string]adapter.Adapter, stateDir string, format outputFormat, stdout, stderr io.Writer) error { if stdout == nil { stdout = os.Stdout } @@ -96,6 +106,15 @@ func runDiffAt(configPath, target string, adapters map[string]adapter.Adapter, s } scope := buildOrphanScope(resolved) + + if format == formatJSON { + report, err := buildDiffReport(generated, state, scope) + if err != nil { + return err + } + return writeJSON(stdout, report) + } + colored := diff.SupportsColor() hasChanges := false @@ -138,7 +157,7 @@ func runDiffAt(configPath, target string, adapters map[string]adapter.Adapter, s return nil } -func runPushAt(configPath, target string, force bool, adapters map[string]adapter.Adapter, stateDir string, preflight pushPreflight, postflight pushPostflight, stdout, stderr io.Writer) error { +func runPushAt(configPath, target string, force bool, adapters map[string]adapter.Adapter, stateDir string, format outputFormat, preflight pushPreflight, postflight pushPostflight, stdout, stderr io.Writer) error { if stdout == nil { stdout = os.Stdout } @@ -198,7 +217,10 @@ func runPushAt(configPath, target string, force bool, adapters map[string]adapte return fmt.Errorf("%w: %s", ErrManualEdits, strings.TrimSpace(b.String())) } - printOrphanWarnings(stderr, diff.OrphanedOnly(results)) + orphans := diff.OrphanedOnly(results) + if format != formatJSON { + printOrphanWarnings(stderr, orphans) + } // Stage every changed file, then commit the batch through a journalled // transaction so a crash mid-write can be completed on the next push. @@ -233,9 +255,6 @@ func runPushAt(configPath, target string, force bool, adapters map[string]adapte if err := tx.Commit(); err != nil { return fmt.Errorf("commit push: %w", err) } - for _, l := range logs { - fmt.Fprintf(stdout, "[%s] wrote %s (%s)\n", l.name, l.path, diffStatusName(l.status)) - } wroteAny := len(logs) > 0 if postflight != nil { if err := postflight(generated, state); err != nil { @@ -247,6 +266,13 @@ func runPushAt(configPath, target string, force bool, adapters map[string]adapte return err } + if format == formatJSON { + return writeJSON(stdout, buildPushReport(logs, orphans, generated)) + } + + for _, l := range logs { + fmt.Fprintf(stdout, "[%s] wrote %s (%s)\n", l.name, l.path, diffStatusName(l.status)) + } if !wroteAny { fmt.Fprintln(stdout, "No changes") } else { diff --git a/internal/integration_test.go b/internal/integration_test.go index d7ef880..4f9fceb 100644 --- a/internal/integration_test.go +++ b/internal/integration_test.go @@ -371,6 +371,55 @@ func TestEndToEnd_PushAllTargetsIdempotent(t *testing.T) { } } +func TestEndToEnd_JSONOutput(t *testing.T) { + env := newIntegrationEnv(t) + + pushOpts := env.pushOpts("") + pushOpts.Format = "json" + out, _, err := cli.CaptureOutput(func() error { return cli.RunPush(pushOpts) }) + if err != nil { + t.Fatalf("push json: %v", err) + } + var pushReport cli.PushReport + if err := json.Unmarshal([]byte(out), &pushReport); err != nil { + t.Fatalf("push output is not valid JSON: %v\n%s", err, out) + } + if !pushReport.Wrote || len(pushReport.Written) == 0 { + t.Fatalf("expected written files, got %+v", pushReport) + } + for _, f := range pushReport.Written { + if f.Adapter == "" || f.Path == "" || f.Status == "" { + t.Errorf("incomplete file report: %+v", f) + } + } + + diffOpts := env.diffOpts("") + diffOpts.Format = "json" + out, _, err = cli.CaptureOutput(func() error { return cli.RunDiff(diffOpts) }) + if err != nil { + t.Fatalf("diff json: %v", err) + } + var diffReport cli.DiffReport + if err := json.Unmarshal([]byte(out), &diffReport); err != nil { + t.Fatalf("diff output is not valid JSON: %v\n%s", err, out) + } + if diffReport.HasChanges { + t.Errorf("expected no changes after push, got %+v", diffReport) + } + if len(diffReport.Files) == 0 { + t.Error("expected files in diff report") + } +} + +func TestEndToEnd_JSONInvalidFormat(t *testing.T) { + env := newIntegrationEnv(t) + opts := env.diffOpts("") + opts.Format = "yaml" + if err := cli.RunDiff(opts); err == nil { + t.Fatal("expected error for unknown output format") + } +} + func TestEndToEnd_PermissionMapping(t *testing.T) { env := newIntegrationEnv(t) From ff8a21481273306b81f30d30efe8e47adf153943 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Philippe=20D=C3=A9=C3=AFs=20Nuel?= Date: Mon, 13 Jul 2026 23:17:24 +0200 Subject: [PATCH 5/6] feat(cli): add `shenron doctor` health check (P2.3) Introduce a read-only `doctor` command that inspects the environment and every installed package: - target adapter config paths, flagging any whose nearest existing ancestor is not writable; - each installed package's snapshot digest (cache integrity, via Load), its sync-state file (must parse), and its permission-approval status (pending approval is a warning). Supports --output json (DoctorReport) alongside the default text; exits non-zero (ErrDoctorFailed) when any check fails. Warnings do not fail the run. Documented in the README command table. Co-Authored-By: Claude Opus 4.8 --- README.md | 1 + internal/cli/commands_test.go | 4 +- internal/cli/doctor.go | 178 ++++++++++++++++++++++++++++++++++ internal/cli/doctor_test.go | 92 ++++++++++++++++++ internal/cli/package.go | 17 ++++ 5 files changed, 290 insertions(+), 2 deletions(-) create mode 100644 internal/cli/doctor.go create mode 100644 internal/cli/doctor_test.go diff --git a/README.md b/README.md index 327f336..b18fff6 100644 --- a/README.md +++ b/README.md @@ -89,6 +89,7 @@ reports `No changes` for each synchronized target. | `shenron update ` | Validate and replace an installed snapshot from a new source or ref. | | `shenron diff ` | Show a package's native diff plus its permission grants and missing skills. | | `shenron push ` | Generate and atomically write a package's native files, then update its state. | +| `shenron doctor` | Check tool paths, snapshot-cache integrity, sync state, and pending permission approvals. | Common flags: diff --git a/internal/cli/commands_test.go b/internal/cli/commands_test.go index 40c21df..7d19af0 100644 --- a/internal/cli/commands_test.go +++ b/internal/cli/commands_test.go @@ -11,11 +11,11 @@ import ( ) // TestRootSubcommandNames asserts the root command advertises exactly the -// five top-level subcommands and no "package" parent. +// six top-level subcommands and no "package" parent. func TestRootSubcommandNames(t *testing.T) { root := cli.NewRootCmd() - want := []string{"diff", "install", "list", "push", "update"} + want := []string{"diff", "doctor", "install", "list", "push", "update"} var got []string for _, c := range root.Commands() { got = append(got, c.Name()) diff --git a/internal/cli/doctor.go b/internal/cli/doctor.go new file mode 100644 index 0000000..63481fd --- /dev/null +++ b/internal/cli/doctor.go @@ -0,0 +1,178 @@ +package cli + +import ( + "errors" + "fmt" + "io" + "os" + "path/filepath" + + "github.com/S1933/Shenron/internal/diff" + "github.com/S1933/Shenron/internal/fsutil" + shenronpackage "github.com/S1933/Shenron/internal/package" +) + +// ErrDoctorFailed is returned when at least one health check fails, so the +// process exits non-zero. +var ErrDoctorFailed = errors.New("doctor found problems") + +// Check status tokens. +const ( + statusOK = "ok" + statusWarn = "warn" + statusFail = "fail" +) + +// CheckResult is one health-check outcome. +type CheckResult struct { + Name string `json:"name"` + Status string `json:"status"` // ok | warn | fail + Detail string `json:"detail,omitempty"` +} + +// DoctorReport is the aggregate health report (also the JSON shape). +type DoctorReport struct { + Checks []CheckResult `json:"checks"` + OK bool `json:"ok"` // false if any check failed (warnings do not count) +} + +// DoctorOptions configures the doctor command for tests and embedding. +type DoctorOptions struct { + Store *shenronpackage.Store + Format string // "text" (default) or "json" + Output io.Writer +} + +// RunDoctor inspects the environment and every installed package, reporting +// tool paths, snapshot-cache integrity, sync state, and pending permission +// approvals. It returns ErrDoctorFailed if any check has status "fail". +func RunDoctor(opts DoctorOptions) error { + format, err := parseOutputFormat(opts.Format) + if err != nil { + return err + } + store := packageStore(opts.Store) + output := packageOutput(opts.Output) + + var checks []CheckResult + checks = append(checks, checkTargetPaths()...) + checks = append(checks, checkPackages(store)...) + + report := DoctorReport{Checks: checks, OK: true} + for _, c := range checks { + if c.Status == statusFail { + report.OK = false + } + } + + if format == formatJSON { + if err := writeJSON(output, report); err != nil { + return err + } + } else { + for _, c := range report.Checks { + if _, err := fmt.Fprintf(output, "[%s] %s: %s\n", c.Status, c.Name, c.Detail); err != nil { + return err + } + } + } + + if !report.OK { + return ErrDoctorFailed + } + return nil +} + +// checkTargetPaths reports each adapter's config directory and whether its +// nearest existing ancestor is writable. A missing directory is fine (push +// creates it); an unwritable ancestor is a warning. +func checkTargetPaths() []CheckResult { + targets := []struct { + name string + path string + }{ + {"claude-code", fsutil.ClaudePath()}, + {"codex", fsutil.CodexPath()}, + {"opencode", fsutil.OpenCodePath()}, + } + + checks := make([]CheckResult, 0, len(targets)) + for _, t := range targets { + name := "target " + t.name + if writableAncestor(t.path) { + checks = append(checks, CheckResult{Name: name, Status: statusOK, Detail: t.path}) + } else { + checks = append(checks, CheckResult{Name: name, Status: statusWarn, Detail: t.path + " (not writable)"}) + } + } + return checks +} + +// writableAncestor reports whether the nearest existing ancestor of path can be +// written to, by staging and removing a temp entry there. +func writableAncestor(path string) bool { + dir := path + for { + if _, err := os.Stat(dir); err == nil { + break + } + parent := filepath.Dir(dir) + if parent == dir { + return false + } + dir = parent + } + tmp, err := os.MkdirTemp(dir, ".shenron-doctor-*") + if err != nil { + return false + } + _ = os.Remove(tmp) + return true +} + +// checkPackages validates every installed package's snapshot, state, and +// permission-approval status. +func checkPackages(store *shenronpackage.Store) []CheckResult { + installed, err := store.List() + if err != nil { + return []CheckResult{{Name: "packages", Status: statusFail, Detail: err.Error()}} + } + if len(installed) == 0 { + return []CheckResult{{Name: "packages", Status: statusOK, Detail: "no packages installed"}} + } + + var checks []CheckResult + for _, p := range installed { + checks = append(checks, checkPackage(store, p)) + } + return checks +} + +// checkPackage runs the per-package checks and folds them into a single result: +// the snapshot digest is rechecked (cache integrity), the state file must parse, +// and declared permissions must be approved for the installed revision. +func checkPackage(store *shenronpackage.Store, p shenronpackage.InstalledPackage) CheckResult { + name := fmt.Sprintf("package %s@%s", p.Name, p.Version) + + installed, pkg, err := store.Load(p.Name) + if err != nil { + return CheckResult{Name: name, Status: statusFail, Detail: "snapshot invalid: " + err.Error()} + } + + if _, err := diff.LoadState(store.StateDir(p.Name)); err != nil { + return CheckResult{Name: name, Status: statusFail, Detail: "state unreadable: " + err.Error()} + } + + grants := permissionGrants(pkg.Pivot) + if len(grants) > 0 { + approved, err := packagePermissionsApproved(store, installed, permissionDigest(grants)) + if err != nil { + return CheckResult{Name: name, Status: statusFail, Detail: "approval unreadable: " + err.Error()} + } + if !approved { + return CheckResult{Name: name, Status: statusWarn, Detail: "permission approval pending; push requires --allow-permissions"} + } + } + + return CheckResult{Name: name, Status: statusOK, Detail: "snapshot verified, state and permissions ok"} +} diff --git a/internal/cli/doctor_test.go b/internal/cli/doctor_test.go new file mode 100644 index 0000000..2e7fed5 --- /dev/null +++ b/internal/cli/doctor_test.go @@ -0,0 +1,92 @@ +package cli_test + +import ( + "bytes" + "encoding/json" + "errors" + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/S1933/Shenron/internal/cli" + shenronpackage "github.com/S1933/Shenron/internal/package" +) + +func TestDoctorEmptyStore(t *testing.T) { + store := shenronpackage.NewStore(filepath.Join(t.TempDir(), "cache")) + var buf bytes.Buffer + if err := cli.RunDoctor(cli.DoctorOptions{Store: store, Output: &buf}); err != nil { + t.Fatalf("doctor on empty store should pass: %v", err) + } + out := buf.String() + if !strings.Contains(out, "no packages installed") { + t.Errorf("expected no-packages note, got:\n%s", out) + } + for _, target := range []string{"claude-code", "codex", "opencode"} { + if !strings.Contains(out, "target "+target) { + t.Errorf("expected target %q check, got:\n%s", target, out) + } + } +} + +func TestDoctorInstalledPackageJSON(t *testing.T) { + source := writeCLIPackage(t, "1.2.3") + store := shenronpackage.NewStore(filepath.Join(t.TempDir(), "cache")) + if err := cli.RunPackageInstall(cli.PackageInstallOptions{Store: store, Source: source, Output: io.Discard}); err != nil { + t.Fatal(err) + } + + var buf bytes.Buffer + if err := cli.RunDoctor(cli.DoctorOptions{Store: store, Format: "json", Output: &buf}); err != nil { + t.Fatalf("doctor: %v", err) + } + + var report cli.DoctorReport + if err := json.Unmarshal(buf.Bytes(), &report); err != nil { + t.Fatalf("doctor output is not valid JSON: %v\n%s", err, buf.String()) + } + if !report.OK { + t.Fatalf("expected healthy report, got %+v", report) + } + found := false + for _, c := range report.Checks { + if strings.HasPrefix(c.Name, "package acme-reviewers@1.2.3") { + found = true + if c.Status != "ok" { + t.Errorf("package check status = %q, want ok: %+v", c.Status, c) + } + } + } + if !found { + t.Errorf("expected a check for the installed package, got %+v", report.Checks) + } +} + +func TestDoctorDetectsCorruptSnapshot(t *testing.T) { + source := writeCLIPackage(t, "1.2.3") + store := shenronpackage.NewStore(filepath.Join(t.TempDir(), "cache")) + if err := cli.RunPackageInstall(cli.PackageInstallOptions{Store: store, Source: source, Output: io.Discard}); err != nil { + t.Fatal(err) + } + + installed, err := store.List() + if err != nil || len(installed) != 1 { + t.Fatalf("list: %v (%d packages)", err, len(installed)) + } + // Corrupt the immutable snapshot so its digest no longer matches. + pivotPath := filepath.Join(installed[0].Root, shenronpackage.PivotFileName) + if err := os.WriteFile(pivotPath, []byte("version: \"1\"\nagents: []\n# tampered\n"), 0o644); err != nil { + t.Fatal(err) + } + + var buf bytes.Buffer + err = cli.RunDoctor(cli.DoctorOptions{Store: store, Output: &buf}) + if !errors.Is(err, cli.ErrDoctorFailed) { + t.Fatalf("expected ErrDoctorFailed on corrupt snapshot, got: %v", err) + } + if !strings.Contains(buf.String(), "snapshot invalid") { + t.Errorf("expected snapshot-invalid detail, got:\n%s", buf.String()) + } +} diff --git a/internal/cli/package.go b/internal/cli/package.go index 3c57a3d..dce79bb 100644 --- a/internal/cli/package.go +++ b/internal/cli/package.go @@ -121,10 +121,27 @@ func NewRootCmd() *cobra.Command { NewUpdateCmd(resolver), NewDiffCmd(resolver), NewPushCmd(resolver), + NewDoctorCmd(resolver), ) return cmd } +// NewDoctorCmd builds the top-level `doctor` command. +func NewDoctorCmd(store func() *shenronpackage.Store) *cobra.Command { + var output string + cmd := &cobra.Command{ + Use: "doctor", + Short: "Check tool paths, snapshot cache, state, and permission approvals", + Args: cobra.NoArgs, + SilenceUsage: true, + RunE: func(cmd *cobra.Command, _ []string) error { + return RunDoctor(DoctorOptions{Store: store(), Format: output, Output: cmd.OutOrStdout()}) + }, + } + cmd.Flags().StringVar(&output, "output", "text", "output format: text or json") + return cmd +} + // NewInstallCmd builds the top-level `install` command. func NewInstallCmd(store func() *shenronpackage.Store) *cobra.Command { var ref string From 625a05759393e76ab38cb269618b4b1f51e89be7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Philippe=20D=C3=A9=C3=AFs=20Nuel?= Date: Mon, 13 Jul 2026 23:20:31 +0200 Subject: [PATCH 6/6] feat(cli): add `shenron explain` translation preview (P2.4) Introduce a read-only `explain --target ` command that shows the native files a package translates into for one target, computed from the pivot alone (ignoring on-disk state) so it answers "what does this package become for target X?". - Runs the target adapter's Generate over the package pivot; for merging adapters it also folds the fragments into a fresh config (empty host), so the opencode.json translation is shown with no foreign entries. - Each entry is labelled with the pivot resource it came from (GeneratedFile.ResourceID), which P1.1 now carries end to end. - Supports --output json (ExplainReport) alongside text. Requires --target; rejects unknown targets. Documented in the README command table. Co-Authored-By: Claude Opus 4.8 --- README.md | 1 + internal/cli/commands_test.go | 4 +- internal/cli/explain.go | 126 ++++++++++++++++++++++++++++++++++ internal/cli/explain_test.go | 89 ++++++++++++++++++++++++ internal/cli/package.go | 18 +++++ 5 files changed, 236 insertions(+), 2 deletions(-) create mode 100644 internal/cli/explain.go create mode 100644 internal/cli/explain_test.go diff --git a/README.md b/README.md index b18fff6..f48b129 100644 --- a/README.md +++ b/README.md @@ -90,6 +90,7 @@ reports `No changes` for each synchronized target. | `shenron diff ` | Show a package's native diff plus its permission grants and missing skills. | | `shenron push ` | Generate and atomically write a package's native files, then update its state. | | `shenron doctor` | Check tool paths, snapshot-cache integrity, sync state, and pending permission approvals. | +| `shenron explain --target ` | Preview the native files a package translates into for one target, without writing. | Common flags: diff --git a/internal/cli/commands_test.go b/internal/cli/commands_test.go index 7d19af0..adb492d 100644 --- a/internal/cli/commands_test.go +++ b/internal/cli/commands_test.go @@ -11,11 +11,11 @@ import ( ) // TestRootSubcommandNames asserts the root command advertises exactly the -// six top-level subcommands and no "package" parent. +// seven top-level subcommands and no "package" parent. func TestRootSubcommandNames(t *testing.T) { root := cli.NewRootCmd() - want := []string{"diff", "doctor", "install", "list", "push", "update"} + want := []string{"diff", "doctor", "explain", "install", "list", "push", "update"} var got []string for _, c := range root.Commands() { got = append(got, c.Name()) diff --git a/internal/cli/explain.go b/internal/cli/explain.go new file mode 100644 index 0000000..32c79b6 --- /dev/null +++ b/internal/cli/explain.go @@ -0,0 +1,126 @@ +package cli + +import ( + "fmt" + "io" + "sort" + + "github.com/S1933/Shenron/internal/adapter" + shenronpackage "github.com/S1933/Shenron/internal/package" +) + +// ExplainedFile is one native file a package translates into for a target. +type ExplainedFile struct { + ResourceID string `json:"resourceId,omitempty"` + Adapter string `json:"adapter"` + Path string `json:"path"` + Content string `json:"content"` +} + +// ExplainReport is the structured output of `explain` (also the JSON shape). +type ExplainReport struct { + Package string `json:"package"` + Target string `json:"target"` + Files []ExplainedFile `json:"files"` +} + +// ExplainOptions configures the explain command for tests and embedding. +type ExplainOptions struct { + Store *shenronpackage.Store + Name string + Target string // required: claude-code | codex | opencode + Adapters map[string]adapter.Adapter + Format string // "text" (default) or "json" + Output io.Writer +} + +// RunExplain shows, for one target, the native files an installed package +// translates into. It is a pure preview: the translation is computed from the +// pivot alone, ignoring whatever is currently on disk, so it answers "what does +// this package become for target X?". +func RunExplain(opts ExplainOptions) error { + if opts.Target == "" { + return fmt.Errorf("explain requires --target (claude-code, codex, or opencode)") + } + format, err := parseOutputFormat(opts.Format) + if err != nil { + return err + } + + store := packageStore(opts.Store) + installed, pkg, err := store.Load(opts.Name) + if err != nil { + return err + } + output := packageOutput(opts.Output) + + adapters := opts.Adapters + if adapters == nil { + adapters, err = ResolveTargets(opts.Target) + if err != nil { + return err + } + } + adpt, ok := adapters[opts.Target] + if !ok { + return errUnknownTarget(opts.Target) + } + + if setter, ok := adpt.(adapter.PivotDirectoryAware); ok { + setter.SetPivotDir(installed.Root) + } + + result, err := adpt.Generate(pkg.Pivot) + if err != nil { + return fmt.Errorf("%s: %w", opts.Target, err) + } + files := result.Files + + // For merging adapters, fold the fragments into a fresh config so the + // preview shows the opencode.json translation with no host entries. + if merger, ok := adpt.(adapter.MergingAdapter); ok { + merged, err := merger.MergeFile(merger.ConfigPath(), nil, result.Fragments) + if err != nil { + return fmt.Errorf("%s: merge preview: %w", opts.Target, err) + } + if merged != nil { + files = append(files, adapter.GeneratedFile{ + Path: merger.ConfigPath(), + Content: merged, + Adapter: opts.Target, + }) + } + } + + report := ExplainReport{Package: installed.Name, Target: opts.Target} + for _, f := range files { + report.Files = append(report.Files, ExplainedFile{ + ResourceID: f.ResourceID, + Adapter: f.Adapter, + Path: f.Path, + Content: string(f.Content), + }) + } + sort.Slice(report.Files, func(i, j int) bool { return report.Files[i].Path < report.Files[j].Path }) + + if format == formatJSON { + return writeJSON(output, report) + } + return writeExplainText(output, report) +} + +func writeExplainText(w io.Writer, report ExplainReport) error { + if _, err := fmt.Fprintf(w, "%s -> %s\n", report.Package, report.Target); err != nil { + return err + } + for _, f := range report.Files { + label := f.Path + if f.ResourceID != "" { + label = fmt.Sprintf("%s (from %s)", f.Path, f.ResourceID) + } + if _, err := fmt.Fprintf(w, "\n--- %s ---\n%s\n", label, f.Content); err != nil { + return err + } + } + return nil +} diff --git a/internal/cli/explain_test.go b/internal/cli/explain_test.go new file mode 100644 index 0000000..f59dca5 --- /dev/null +++ b/internal/cli/explain_test.go @@ -0,0 +1,89 @@ +package cli_test + +import ( + "bytes" + "encoding/json" + "io" + "path/filepath" + "strings" + "testing" + + "github.com/S1933/Shenron/internal/cli" + shenronpackage "github.com/S1933/Shenron/internal/package" +) + +func installExplainPackage(t *testing.T) *shenronpackage.Store { + t.Helper() + source := writeCLIPackage(t, "1.0.0") + writeCLIFile(t, filepath.Join(source, shenronpackage.PivotFileName), `version: "1" +agents: + - id: build + description: Build the project. + mode: subagent + systemPrompt: Build carefully. +commands: [] +`) + store := shenronpackage.NewStore(filepath.Join(t.TempDir(), "cache")) + if err := cli.RunPackageInstall(cli.PackageInstallOptions{Store: store, Source: source, Output: io.Discard}); err != nil { + t.Fatal(err) + } + return store +} + +func TestExplainRequiresTarget(t *testing.T) { + store := installExplainPackage(t) + if err := cli.RunExplain(cli.ExplainOptions{Store: store, Name: "acme-reviewers", Output: io.Discard}); err == nil { + t.Fatal("expected error when --target is missing") + } +} + +func TestExplainCodexJSON(t *testing.T) { + store := installExplainPackage(t) + var buf bytes.Buffer + if err := cli.RunExplain(cli.ExplainOptions{Store: store, Name: "acme-reviewers", Target: "codex", Format: "json", Output: &buf}); err != nil { + t.Fatalf("explain: %v", err) + } + + var report cli.ExplainReport + if err := json.Unmarshal(buf.Bytes(), &report); err != nil { + t.Fatalf("explain output is not valid JSON: %v\n%s", err, buf.String()) + } + if report.Target != "codex" || report.Package != "acme-reviewers" { + t.Fatalf("unexpected report header: %+v", report) + } + + var found bool + for _, f := range report.Files { + if f.ResourceID == "build" { + found = true + if !strings.Contains(f.Content, "name = 'build'") { + t.Errorf("codex agent content missing native name:\n%s", f.Content) + } + } + } + if !found { + t.Errorf("expected a file translated from the build agent, got %+v", report.Files) + } +} + +func TestExplainOpenCodeIncludesConfig(t *testing.T) { + store := installExplainPackage(t) + var buf bytes.Buffer + if err := cli.RunExplain(cli.ExplainOptions{Store: store, Name: "acme-reviewers", Target: "opencode", Output: &buf}); err != nil { + t.Fatalf("explain: %v", err) + } + out := buf.String() + if !strings.Contains(out, "opencode.json") { + t.Errorf("expected merged opencode.json in preview, got:\n%s", out) + } + if !strings.Contains(out, "\"build\"") { + t.Errorf("expected the build agent entry in the config preview, got:\n%s", out) + } +} + +func TestExplainUnknownTarget(t *testing.T) { + store := installExplainPackage(t) + if err := cli.RunExplain(cli.ExplainOptions{Store: store, Name: "acme-reviewers", Target: "emacs", Output: io.Discard}); err == nil { + t.Fatal("expected error for unknown target") + } +} diff --git a/internal/cli/package.go b/internal/cli/package.go index dce79bb..6b48eb2 100644 --- a/internal/cli/package.go +++ b/internal/cli/package.go @@ -122,10 +122,28 @@ func NewRootCmd() *cobra.Command { NewDiffCmd(resolver), NewPushCmd(resolver), NewDoctorCmd(resolver), + NewExplainCmd(resolver), ) return cmd } +// NewExplainCmd builds the top-level `explain` command. +func NewExplainCmd(store func() *shenronpackage.Store) *cobra.Command { + var target, output string + cmd := &cobra.Command{ + Use: "explain ", + Short: "Preview the native files a package translates into for a target", + Args: cobra.ExactArgs(1), + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { + return RunExplain(ExplainOptions{Store: store(), Name: args[0], Target: target, Format: output, Output: cmd.OutOrStdout()}) + }, + } + cmd.Flags().StringVar(&target, "target", "", "CLI target to explain: claude-code, codex, or opencode (required)") + cmd.Flags().StringVar(&output, "output", "text", "output format: text or json") + return cmd +} + // NewDoctorCmd builds the top-level `doctor` command. func NewDoctorCmd(store func() *shenronpackage.Store) *cobra.Command { var output string