Skip to content

feat(opencode): adopt the Codex full-mirror skill-sync model - #171

Open
omergk28 wants to merge 6 commits into
ActiveMemory:mainfrom
omergk28:feat/opencode-mirror-sync
Open

feat(opencode): adopt the Codex full-mirror skill-sync model#171
omergk28 wants to merge 6 commits into
ActiveMemory:mainfrom
omergk28:feat/opencode-mirror-sync

Conversation

@omergk28

@omergk28 omergk28 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

feat(opencode): adopt the Codex full-mirror skill-sync model

Branch: feat/opencode-mirror-syncmain (rebased onto main after #160 merged)
Commits: cd60161d, a04c0637, 8f234e11, 249b6308 · Spec: specs/opencode-mirror-sync.md

What

Follow-up to #160, adopting what #163's Codex integration did better —
and giving Codex what this branch does better. Each integration
inherits the other's strength:

OpenCode adopts the Codex mirror model:

Before (#160) After
Opt-in by directory presence — new canonical skills silently don't ship Default-include mirror: 54 canonical − 4 Claude-only = 50 skills, matching Codex
Orphaned directories blessed by the parity test's exemption rule Sync removes orphans; test flags them
references/ files not shipped (latent 404s) references/ mirrored, embedded, and deployed by ctx setup opencode --write

Excluded (same four as Codex, same rationale): ctx-permission-sanitize,
ctx-plan-import, ctx-dream, ctx-skill-create.

Codex gains CI-level enforcement: TestSyncedSkillParity now
byte-checks all three generated trees and holds the two mirror trees
(OpenCode, Codex) to completeness, no-orphan, and reference-parity
assertions — 135 skills + 4 reference files verified in go test,
which CI runs (make audit's freshness checks only run on developer
machines).

Also: check-opencode-skills restore hardened to full-replace (a
mirror sync adds/removes directories); docs/home/opencode.md follows
the Codex page's presentation. The regenerated search.json indexes
docs/home/codex.md (committed in #163 without its site page), so
site/home/codex/index.html rides along in the site commit — shipping
the index without the page would put dead codex entries in site search.

Verification

  • make audit green end-to-end: all three skill trees in sync.
  • Full test suite green, including the OpenCode setup deploy tests.
  • shellcheck clean on the rewritten sync script.

Notes

🤖 Generated with Claude Code

https://claude.ai/code/session_01VAiL9PRdXX38aUA3z7XSD9

The opt-in-by-directory-presence model from ActiveMemory#158 re-drifts by
default: every future canonical skill silently does not ship to
OpenCode, orphaned directories are blessed by the parity test's
exemption rule, and references/ files skills cite are not shipped.

Mirror the canonical tree the way hack/sync-codex-skills.sh does:
default-include with the same four Claude-only exclusions, mirror
references/ directories, remove orphans. The tree grows from 17
curated skills to the full 50. Deployment ships references via
agent.OpenCodeSkillReferences and a deployReferences step in
ctx setup opencode, mirroring the Codex deploy path.

In return, extend CI-level enforcement to Codex: the parity test
now byte-checks all three generated trees and holds mirror trees
(OpenCode, Codex) to completeness, no-orphan, and reference-parity
assertions — coverage make audit alone gave Codex, since CI never
runs make audit. check-opencode-skills restore is hardened to
full-replace, since a mirror sync adds and removes directories.

Spec: specs/opencode-mirror-sync.md
Signed-off-by: Omer Kocaoglu <omergk28@gmail.com>
Regenerates the OpenCode page for the mirror-model slash-command
section and search.json for both pages. The codex page rides along:
docs/home/codex.md landed in ActiveMemory#163 without its generated site page,
and search.json regeneration inevitably indexes committed docs —
shipping the index without the page would put dead codex entries in
site search. Remaining full-site entity-encoding churn (122 files)
restored, per the 2026-08-19 site-churn learning.

Spec: specs/opencode-mirror-sync.md
Signed-off-by: Omer Kocaoglu <omergk28@gmail.com>
Self-review hardening: deployReferences shipped with no direct test,
against the contributing guide's add-tests-for-new-functionality
rule. Cover the fresh-deploy path (every embedded reference lands
byte-exact) and the skill-skipped path (an unchanged SKILL.md must
still refresh a stale reference — the wiring a reference-only
upstream change depends on).

Record in the spec why ctx-serendipity ships although its companion
ctx-dream is excluded: the single /ctx-dream reference is a passing
provenance mention, not an instruction, and the dream/review split
is a real cross-tool workflow. The Codex mirror ships it under the
same reasoning.

Spec: specs/opencode-mirror-sync.md
Signed-off-by: Omer Kocaoglu <omergk28@gmail.com>
Adversarial-review findings, each reproduced before fixing:

1. Empty EXCLUDE array aborts both mirror sync scripts on stock
   macOS /bin/bash 3.2 under set -u (latent today; the class is a
   recorded must-fix per LEARNINGS 2026-08-19). Guard the expansion
   with the established idiom from lint-drift.sh. The codex sibling
   carried the identical pre-existing pattern and gets the same fix.

2. Wrong-cwd invocation of sync-opencode-skills.sh silently printed
   a 0/0/0 success line and created a junk skills tree via the
   unconditional mkdir -p — a regression vs the old presence-model
   script, and worse than the codex sibling, which at least dies at
   its dual-manifest cp. Self-anchor both scripts to the repo root
   the way eight other hack scripts already do.

3. A YAML block-form allowed-tools: key would be corrupted by the
   sed transform (orphaned sequence items) and pass every existing
   check: the parity test replicates the transform, and the
   frontmatter guard sees one in-frontmatter key line. Teach
   TestAllowedToolsConfinedToFrontmatter to reject the block form.

Spec: specs/opencode-mirror-sync.md
Signed-off-by: Omer Kocaoglu <omergk28@gmail.com>
PR ActiveMemory#161 landed while this branch was open and froze the Pi and
OpenCode skill trees byte-for-byte (TestPiSkillsMirrorOpenCode),
with specs/pi-cli-integration.md defining the Pi set as "the same
bundled skill set as OpenCode". Growing OpenCode from 19 curated
skills to the full canonical 50 therefore breaks that freeze — the
two PRs collide semantically, not just textually in embed.go.

Resolve by making the derivation structural instead of manual:
hack/sync-pi-skills.sh copies the generated OpenCode tree verbatim,
so the freeze holds by construction and Pi inherits the mirror
model — full canonical coverage, orphan removal, references — for
free. Wired into make build and the audit chain alongside its
siblings.

Pi also gains reference deployment (PiSkillReferences + the
deployReferences step, mirroring the OpenCode and Codex paths):
without it Pi would embed reference files it never writes, leaving
deployed skill bodies citing paths that do not exist.

The Pi ordering test now matches its SKILL.md line specifically; a
skill's reference files deploy immediately after it and share its
directory prefix, which double-counted under the old substring
match.

Spec: specs/opencode-mirror-sync.md
Signed-off-by: Omer Kocaoglu <omergk28@gmail.com>
@omergk28
omergk28 force-pushed the feat/opencode-mirror-sync branch from 249b630 to 93f4e86 Compare September 11, 2026 03:49
@omergk28

Copy link
Copy Markdown
Contributor Author

Rebased onto current main after #161 (Pi CLI integration) merged. Two collisions, both resolved:

Textualinternal/assets/embed.go: both integrations add //go:embed lines at the same spot. Resolved as a union.

Semantic — the interesting one. #161 froze the Pi and OpenCode skill trees byte-for-byte (TestPiSkillsMirrorOpenCode), and specs/pi-cli-integration.md defines the Pi set as "the same bundled skill set as OpenCode". Growing OpenCode from 19 curated skills to the full canonical 50 therefore broke that freeze.

Resolved by making the derivation structural rather than manual: hack/sync-pi-skills.sh copies the generated OpenCode tree verbatim, so the freeze holds by construction and Pi inherits the mirror model — full canonical coverage, orphan removal, references/ — for free. Wired into make build and the audit chain alongside its siblings.

Pi also gains reference deployment (PiSkillReferences + a deployReferences step, mirroring the OpenCode and Codex paths); without it Pi would embed reference files it never writes, leaving deployed skill bodies citing paths that don't exist. Pi's ordering test now matches its SKILL.md line specifically, since a skill's reference files deploy right after it and share its directory prefix.

Happy to split the Pi portion into its own PR if you'd rather keep this one narrower — flagging it because it does widen the blast radius beyond OpenCode, though main can't stay green without it.

All 8 checks green (including Typecheck Pi); make audit clean locally.

@bilersan bilersan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed commit 93f4e860206bb1c70bdfb9fec81e23e5a18bc38a.

The full-mirror approach and the Pi scope are justified by #161's existing Pi/OpenCode parity contract. I do not think the Pi portion needs to be split out. The Pi TypeScript extension/hooks are unchanged, and the new reference deployment works in the tested fresh, stale-reference, and non-regular-target cases.

I am requesting changes for one P2 blocker: check-pi-skills can return success after its sync script fails and can discard the backup of a partially replaced tree. Details and the minimum error-propagation fix are inline.

The missing OpenCode-to-Pi build dependency is also confirmed, but I regard it as P3/non-blocking because it requires a parallel developer build; committed tree drift is caught by CI parity. The one-line dependency fix is inline.

Non-blocking follow-ups: add the analogous OpenCode fresh/stale-reference assertions to Pi's committed deployment tests, and update the Pi docs/spec for the 50-skill set and reference files.

Verification: generated skill/reference parity, targeted asset/deployment tests, and a temporary Pi reference overlay test passed. An independent Claude review confirmed both build/check findings and reran the relevant probes. All eight CI checks were green on this head. The wider local Windows run was not fully green, but the same CRLF/path test failures reproduced on the exact base. The parallel-sync reproduction used the real scripts with a deterministic interleaving, not a native make -j build run; native GNU make and a live Pi LLM round-trip were not tested here.

@josealekhine, could you also review the sync/check changes and the Pi integration compatibility?

Comment thread Makefile Outdated
rm -rf "$$TMPDIR"; \
exit 1; \
fi; \
rm -rf "$$TMPDIR"; \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 (blocking): keep failure propagation through cleanup and the success message.

If backup creation or sync-pi-skills.sh fails, the && list skips the if, but these two semicolon-separated commands still run. The check deletes its backup, prints Pi skills are in sync., and returns exit 0. This is particularly risky here because the sync script removes the Pi tree before copying its replacement.

I reproduced the exact recipe in a temporary fixture without the OpenCode source tree: the real sync script printed FAIL: ... not found, while the check printed Pi skills are in sync. and exited 0. An independent review also reproduced a partial-copy failure where the damaged tree remained and the backup was deleted.

The minimum error-propagation fix is to replace the trailing semicolons at lines 424-425 with && (fi && and guarded cleanup), so a sync failure returns nonzero, retains the backup, and cannot print success. This leaves the temporary backup for recovery; it does not automatically restore a partially copied tree on that failure path.

Comment thread Makefile Outdated
@./hack/sync-opencode-skills.sh

## sync-pi-skills: Mirror Pi skills from the generated OpenCode tree
sync-pi-skills:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 (non-blocking): declare the producer dependency.

build lists OpenCode sync and Pi sync as sibling prerequisites. Their written order does not serialize them under make -j or parallel MAKEFLAGS, so Pi can copy a SKILL.md while the OpenCode generator has truncated it, or copy an incomplete references directory.

Using the actual scripts with a deterministic interleaving, both returned exit 0 while the final OpenCode SKILL.md had 46 bytes and the Pi copy had 0 bytes. This was a script-level reproduction of a schedule allowed by parallel make, not a native make -j build run on this Windows host.

Please declare sync-pi-skills: sync-opencode-skills. That orders the producer before its consumer and also makes standalone make sync-pi-skills refresh its source first.

Review findings from ActiveMemory#171.

P2: check-pi-skills could exit 0 after its sync script failed. The
recipe's &&-chain skips the diff when the backup copy or the sync
script fails, but the semicolon-separated cleanup still ran — so the
gate deleted its backup, printed "Pi skills are in sync." and
returned success. Worse here than elsewhere because sync-pi-skills.sh
removes the tree before copying its replacement. Chain the cleanup and
the success message with && so a failure returns nonzero and retains
the backup for recovery.

The three sibling gates (copilot, codex, opencode) carry the identical
recipe shape, so all four are fixed together per the reviewer guidance
on ActiveMemory#160 not to let them diverge.

P3: declare sync-pi-skills: sync-opencode-skills. As sibling
prerequisites of build their written order does not serialize them
under make -j, so Pi could copy a SKILL.md the OpenCode generator had
truncated. The dependency also makes a standalone make sync-pi-skills
refresh its source first.

Follow-ups from the same review: port the OpenCode fresh-deploy and
stale-reference assertions to Pi's committed tests, and update the Pi
spec and docs for the generated 50-skill set and its references files.

Spec: specs/opencode-mirror-sync.md
Signed-off-by: Omer Kocaoglu <omergk28@gmail.com>
@omergk28

Copy link
Copy Markdown
Contributor Author

Thanks @bilersan — all four points addressed in 9a045816.

P2 (blocking) — fixed. You're right, and the failure mode was exactly as you described. Chained the cleanup and the success message with && so a failed backup or sync returns nonzero, retains the backup, and cannot print success.

Reproduced your fixture against the fixed recipe (sync script run with no OpenCode source tree):

FAIL: internal/assets/integrations/opencode/skills not found — run sync-opencode-skills first
make: *** [check-pi-skills] Error 1        # exit 2, no "in sync" line, backup retained

One scope note: the three sibling gates (copilot, codex, opencode) carry the identical recipe shape and the identical bug. @josealekhine's guidance on #160 was explicit — "if you tighten it, tighten both siblings together; don't let them diverge over this" — so all four are fixed in the same commit rather than leaving three silently-passing gates on main. Happy to narrow it to Pi if you'd both rather keep this PR tighter.

P3 — fixed. Declared sync-pi-skills: sync-opencode-skills. Agreed on your severity read; the ordering matters for make -j and it also makes standalone make sync-pi-skills refresh its source first.

Follow-ups — both done. Ported the OpenCode fresh-deploy and stale-reference assertions to Pi's committed tests (TestDeploySkills_DeploysReferences, TestDeploySkills_RefreshesStaleReferenceWhenSkillUnchanged), and updated specs/pi-cli-integration.md and docs/home/pi.md for the generated 50-skill set, its references/ files, and the four Claude-only exclusions. The spec's enumerated 19-skill tree listing is replaced with a description of the generated set, since enumerating it would just go stale again.

make audit green locally.

@bilersan

Copy link
Copy Markdown
Contributor

Thanks @omergk28 for addressing the review feedback in 9a045816! Both findings are resolved, and the Pi reference tests and documentation follow-ups are covered. Keeping the fix consistent across all four sibling gates makes sense.

I rechecked the changes: the targeted tests and gate probes passed, and all eight CI checks are green.

@josealekhine, my earlier review concerns are resolved. This is ready to merge from our side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants