host
be41ad2c0a
- test_skill_appendix_sync: all 9 appendix chapters 00..08 in order, body role-matrix set == appendix role-title set, authority statement on both sides, GitHub guarantee in body + named appendix chapters 00/04/07, seeded copy byte-identical with full appendix. Previously only 00/08 existence was asserted. - test_skill_seed_pinning: only SKILL.md copied (pack extras never land, never resurrected), tenant extras left alone, tenant-modified SKILL.md never overwritten across repeats, identical = noop, deleted SKILL.md re-seeded (documented boundary, not deletion preservation). - skill_seed.py: evaluation note only (why whole-dir copy is not done); no code change.
160 lines
7.2 KiB
Python
160 lines
7.2 KiB
Python
"""Seed ChatGPT-Codex-alignment skills into a tenant codex-home.
|
|
|
|
Mirrors the websearch MCP seeding pattern in cloud/websearch.py,
|
|
but for skills, not MCP config:
|
|
|
|
- Source is cloud/skills/<name>/SKILL.md (the batch-1 skill pack).
|
|
- Destination is <codex-home>/skills/<name>/SKILL.md.
|
|
Why this path: verified live on 2026-10-30 with the real
|
|
Codex binary (host.py skills-list against a temp codex-home
|
|
on the production host, production untouched). The first
|
|
attempt -- <codex-home>/.agents/skills/ -- was NOT discovered
|
|
(skills/list returned only the 5 system skills). Copying the
|
|
same SKILL.md files to <codex-home>/skills/ made them appear
|
|
as scope=user (5 system + 3 ours, count 8). System skills
|
|
themselves live at <codex-home>/skills/.system/, which is
|
|
the tell that this is the user-skills directory for a
|
|
CODEX_HOME. A Docker tenant task must not pollute its task
|
|
repo/worktree (seeding into the worktree would show up in
|
|
the task diff), so the tenant codex-home -- the per-project
|
|
persistent home that is mounted into the container at
|
|
/var/lib/codex-app-server/codex-home -- is the right channel.
|
|
This is the same channel websearch MCP seeding uses.
|
|
- Idempotent and non-destructive by design:
|
|
- a missing destination file is copied;
|
|
- an identical destination file is left alone;
|
|
- a differing destination file is NEVER overwritten (that is
|
|
a tenant-modified skill) -- it is reported in
|
|
``skills_skipped_differ`` and left alone;
|
|
- a skill directory without SKILL.md is skipped, never invented.
|
|
- No config.toml change, no key, no network: skills are plain
|
|
instruction files, and the helper never writes secrets.
|
|
|
|
Honest boundary (see docs/chatgpt-codex-alignment.md): discovery
|
|
itself is now live-verified (skills/list, scope=user, on a temp
|
|
home with the real Codex binary). What is NOT yet verified is
|
|
model invocation inside a real Docker tenant task (an explicit
|
|
$live-info call that actually runs and returns a result, and
|
|
whether the relay model honours it). Until a task-level probe
|
|
passes, do not claim the skills are live in production tasks --
|
|
this helper only guarantees the files land in the tenant home,
|
|
idempotently, without clobbering tenant edits, and that
|
|
skills/list sees them there.
|
|
|
|
Whole-directory seeding evaluation (2026-10-30, conclusion: NOT DONE):
|
|
copying a whole skill directory instead of only SKILL.md was evaluated
|
|
and rejected, so the behaviour above is unchanged. A per-file
|
|
skip-if-differ rule would preserve a tenant-modified file's content,
|
|
but it cannot preserve a tenant's *deletion* (a missing destination
|
|
file is recopied, resurrecting a file the tenant removed on purpose)
|
|
and it would auto-seed every future pack file into tenant homes
|
|
without review -- new surface, no benefit today, since every pack
|
|
directory currently contains only SKILL.md. The hard line stands:
|
|
a tenant-modified file is never overwritten, and whole-directory
|
|
copying is not adopted unless a future evaluation proves both
|
|
content-preservation AND deletion-preservation. Until then, only
|
|
SKILL.md is seeded; extra files in a pack directory are ignored
|
|
(pinned by test_merge_gate_protectskill.py).
|
|
|
|
Evaluation note (2026-10-31, skill-engineering convergence line;
|
|
comment only, behaviour unchanged -- see
|
|
cloud/tests/test_skill_seed_pinning.py, which pins it):
|
|
whole-directory copy is explicitly NOT done, for three reasons.
|
|
(1) The pack today ships exactly one file per skill (SKILL.md;
|
|
verified: cloud/skills/*/ contains nothing else), so copying a
|
|
whole directory has zero benefit today and would silently start
|
|
copying future extras (templates/, README) without review.
|
|
(2) Per-file skip-if-differ cannot preserve a tenant deletion:
|
|
a missing destination is copied by design, so whole-dir copy
|
|
would resurrect tenant-deleted auxiliary files on the next run.
|
|
SKILL.md-only copy bounds that resurrection surface to the single
|
|
SKILL.md file itself (auxiliary deletions are preserved trivially
|
|
because those files never landed in the tenant home).
|
|
(3) Seeding never prunes the tenant directory, so tenant-added
|
|
files in a skill directory are left alone by the same rule.
|
|
Boundary, stated plainly so it is not misread: a tenant-deleted
|
|
SKILL.md IS re-seeded (missing = copied); only the differ-guard
|
|
preserves edits. Preserving a SKILL.md deletion would need a
|
|
tombstone, which is new behaviour, not a convergence fix.
|
|
"""
|
|
|
|
import shutil
|
|
from pathlib import Path
|
|
|
|
# cloud/skills/<name>/SKILL.md, next to this module.
|
|
SKILLS_DIR = Path(__file__).resolve().parent / "skills"
|
|
# User-level skills inside a codex-home (live-verified 2026-10-30:
|
|
# .agents/skills inside a codex-home is NOT discovered).
|
|
DEST_SUBDIR = Path("skills")
|
|
|
|
|
|
def skill_source_dir(skills_dir=None):
|
|
"""Skill pack directory: explicit override or cloud/skills/."""
|
|
if skills_dir is not None:
|
|
return Path(skills_dir)
|
|
return SKILLS_DIR
|
|
|
|
|
|
def list_skills(skills_dir=None):
|
|
"""[(name, SKILL.md path)] for every skill in the pack."""
|
|
src = skill_source_dir(skills_dir)
|
|
out = []
|
|
if not src.is_dir():
|
|
return out
|
|
for entry in sorted(src.iterdir()):
|
|
if not entry.is_dir():
|
|
continue
|
|
skill_file = entry / "SKILL.md"
|
|
if skill_file.is_file():
|
|
out.append((entry.name, skill_file))
|
|
return out
|
|
|
|
|
|
def ensure_codex_home_skills(codex_home_dir, skills_dir=None):
|
|
"""Copy the skill pack into a tenant codex-home.
|
|
|
|
Returns {"dest_dir", "skills_copied", "skills_skipped_differ",
|
|
"skills_skipped_missing"}: copied = names newly written,
|
|
skipped_differ = names whose destination SKILL.md differs
|
|
from the pack (tenant-modified, left alone), skipped_missing
|
|
= skill directories in the pack without a SKILL.md file
|
|
(should not happen; the batch-1 test guards it).
|
|
A missing source directory yields all-empty lists, never
|
|
an exception: skills are optional, a missing pack must not
|
|
block a task the way a missing websearch config would.
|
|
"""
|
|
home = Path(codex_home_dir)
|
|
dest_root = home / DEST_SUBDIR
|
|
dest_root.mkdir(parents=True, exist_ok=True)
|
|
copied = []
|
|
skipped_differ = []
|
|
skipped_missing = []
|
|
src = skill_source_dir(skills_dir)
|
|
if not src.is_dir():
|
|
return {"dest_dir": str(dest_root), "skills_copied": copied,
|
|
"skills_skipped_differ": skipped_differ,
|
|
"skills_skipped_missing": skipped_missing}
|
|
# Directories in the pack, including ones without SKILL.md,
|
|
# so a broken pack is reported instead of silently dropped.
|
|
for entry in sorted(src.iterdir()):
|
|
if not entry.is_dir():
|
|
continue
|
|
skill_file = entry / "SKILL.md"
|
|
if not skill_file.is_file():
|
|
skipped_missing.append(entry.name)
|
|
continue
|
|
dest_dir = dest_root / entry.name
|
|
dest_file = dest_dir / "SKILL.md"
|
|
src_bytes = skill_file.read_bytes()
|
|
if dest_file.is_file():
|
|
if dest_file.read_bytes() == src_bytes:
|
|
continue
|
|
skipped_differ.append(entry.name)
|
|
continue
|
|
dest_dir.mkdir(parents=True, exist_ok=True)
|
|
shutil.copyfile(skill_file, dest_file)
|
|
copied.append(entry.name)
|
|
return {"dest_dir": str(dest_root), "skills_copied": copied,
|
|
"skills_skipped_differ": skipped_differ,
|
|
"skills_skipped_missing": skipped_missing}
|