Feat/hardening p0 p2 - #9
Merged
Merged
Conversation
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 <noreply@anthropic.com>
…-1.3)
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 <noreply@anthropic.com>
…(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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Introduce a read-only `explain <name> --target <tool>` 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 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (30)
📝 WalkthroughWalkthroughThe change unifies adapter generation around generated-file records, adds ChangesSync and CLI evolution
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
doctorhealth checks for installation, permissions, snapshots, and target directories.explainto preview generated files for a selected target.diffandpush.Documentation
Bug Fixes