Skip to content

feat(ci): add ci_checks.py sanity gate and a worktree-safe hook installer#75

Open
marcoaperez wants to merge 2 commits into
petrowsky:mainfrom
Taiko-Solutions:feat/ci-checks-and-worktree-hook-fix
Open

feat(ci): add ci_checks.py sanity gate and a worktree-safe hook installer#75
marcoaperez wants to merge 2 commits into
petrowsky:mainfrom
Taiko-Solutions:feat/ci-checks-and-worktree-hook-fix

Conversation

@marcoaperez

Copy link
Copy Markdown
Contributor

Motivated by a real incident: a single missing comma silently turned step-catalog-en.json into invalid JSON, and nothing caught it before it reached collaborators — every tool that loads the catalog broke at once.

What this adds

  • agent/scripts/ci_checks.py validates that every agent/catalogs/*.json parses, then runs the converter tests and the fmlint test suite. Exit 0/1, --quick for the catalog-only fast path. Runnable by hand, from a git hook, or from CI.
  • agent/scripts/hooks/pre-push runs ci_checks.py automatically whenever a push touches a critical artifact (a catalog, the SaXML/HR converters, or fmlint itself), and blocks the push if anything fails. Emergency bypass via git push --no-verify.
  • agent/scripts/install-hooks.sh installs the hooks, resolving the hooks directory via git rev-parse --git-common-dir instead of assuming .git/hooks — a plain .git assumption fails inside a git worktree, where .git is a pointer file and the real hooks directory lives in the main checkout.

Test plan

  • ci_checks.py --quick correctly flags the catalog JSON bug currently on main (see feat(fmlint): add param-fidelity rules to catch silent-discard params (X001-X003) #72) as FAIL
  • ci_checks.py (full) runs converter + fmlint test suites
  • install-hooks.sh run from inside a git worktree correctly installs into the main checkout's .git/hooks/, not a worktree-local path
  • pre-push correctly no-ops when the push doesn't touch a critical path, and triggers ci_checks.py when it does

@agentic-fm agentic-fm added the needs-changes Changes requested before merge label Jul 15, 2026

@agentic-fm agentic-fm left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for this, @marcoaperez — the intent is exactly right, and I confirmed the value: check_catalogs() catches invalid catalog JSON before it reaches collaborators, which is precisely the failure that recently broke the linter for everyone. I want this in.

One structural change before merge — placement:

  • agent/scripts/ is reserved for agent/LLM-operation scripts (e.g. analyze.py, clipboard.py, companion_server.py, deploy.py). Repo-level CI / developer tooling like this doesn't belong there.
  • Please move all three into a new top-level scripts/ folder at the repo root:
    • scripts/ci_checks.py
    • scripts/hooks/pre-push
    • scripts/install-hooks.sh
  • Add a scripts/AGENTS.md that documents what each script does and, importantly, how to wire the hooks upinstall-hooks.sh usage, what pre-push gates on, and the --no-verify emergency bypass. Please write it as actionable setup instructions: in most cases it will be an agent wiring this up in a collaborator's clone, and without that doc the scripts just sit there unused. (Folder-scoped AGENTS.md is picked up by AGENTS.md-aware tools; note the repo-root AGENTS.md is a symlink to the canonical instructions, so keep this one specific to the scripts/ folder.)

Once relocated with that AGENTS.md, this is good to merge. Appreciate you closing the gap that bit us. 🙏

marcoaperez and others added 2 commits July 16, 2026 17:56
…ller

Motivated by a real incident: a single missing comma silently turned
step-catalog-en.json into invalid JSON, and nothing caught it before it
reached collaborators — every tool that loads the catalog broke at once.

- agent/scripts/ci_checks.py validates that every agent/catalogs/*.json
  parses, then runs the converter tests and the fmlint test suite. Exit
  0/1, --quick for the catalog-only fast path. Runnable by hand, from a
  git hook, or from CI.
- agent/scripts/hooks/pre-push runs ci_checks.py automatically whenever a
  push touches a critical artifact (a catalog, the SaXML/HR converters, or
  fmlint itself), and blocks the push if anything fails.
- agent/scripts/install-hooks.sh installs the hooks, resolving the
  directory via `git rev-parse --git-common-dir` instead of assuming
  `.git/hooks` — a plain `.git` assumption fails inside a git worktree,
  where `.git` is a pointer file and the real hooks directory lives in the
  main checkout.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
…NTS.md

Per maintainer review: agent/scripts/ is reserved for agent/LLM-operation
scripts (companion_server.py, deploy.py, clipboard.py); repo-level CI and
developer tooling doesn't belong there. Moves:

  agent/scripts/ci_checks.py       → scripts/ci_checks.py
  agent/scripts/hooks/pre-push     → scripts/hooks/pre-push
  agent/scripts/install-hooks.sh   → scripts/install-hooks.sh

Updates every internal path reference accordingly (REPO_ROOT depth in
ci_checks.py, the hook's own self-reference, install-hooks.sh's
HOOKS_SRC). Adds scripts/AGENTS.md with actionable setup instructions —
what each file does, the one-command install, what the hook gates on,
and the emergency bypass — written for an agent wiring this up in a
collaborator's clone, per the review request.

Also hardens ci_checks.py while verifying it against a real upstream/main
checkout: the "converter tests" and "fmlint tests" steps referenced
agent/scripts/test_fm_xml_to_snippet.py and agent/fmlint/tests/, neither
of which exists on main yet (the fmlint suite ships in petrowsky#72, still open;
the converter test file was never proposed upstream). Both steps now
check for their target path first and report SKIP instead of a hard FAIL
when it's absent, so the checks stay usable standalone rather than
red out of the box on a checkout that doesn't have those suites yet.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
@marcoaperez
marcoaperez force-pushed the feat/ci-checks-and-worktree-hook-fix branch from ec703fc to 53b8253 Compare July 16, 2026 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-changes Changes requested before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants