Files
codex-app-server/cloud/skill_seed.py
T
host d553f9549c feat: configurable merge gate (branch protection + directory isolation) and skill body/appendix checks
- cloud/merge_gate.py: pure gate; directory check reuses the v0.3
  git diff --name-only merge-base command; protection modes
  record (default, explicit confirm required when unprotected),
  enforce (always blocks when unprotected), off (explicit only).
  Confirm covers protection risk only, never directory violations.
  Protection status is caller/env-declared only; unknown is a
  recorded risk, not a claim of protection.
- server POST /tasks/{id}/merge: applies the gate only when gate
  evidence is supplied (or env declares protection); no-evidence
  calls behave as before plus a merge_gate field in the response.
  Blocked merges return 409 without changing status; confirmed
  unprotected merges write a merge_gate_risk audit entry.
- skill_seed.py: whole-directory seeding evaluated and rejected
  (cannot preserve tenant deletions; no benefit while the pack
  holds only SKILL.md); SKILL.md-only behaviour unchanged.
- software-engineering SKILL.md: gate paragraph added to the body
  and to the coordinator + client appendix sections (v0.2 text on
  this base; port to v0.3 text on rebase after #199 merges).
- tests: 16 new tests covering gate modes, directory reuse,
  body/appendix bidirectional assertions naming the missing side,
  SKILL.md-only seeding and tenant-edit preservation.
2026-11-03 02:48:55 +00:00

139 lines
6.0 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).
"""
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}