[ci] Guard the coding-agent skill's bundled YAML schema against drift - #969
Conversation
The flink-agents-dev skill ships a copy of docs/yaml-schema.json and records the blob SHA it was taken from in yaml-contracts.yaml. Nothing regenerates either, so both go stale the moment the schema is re-exported: agents are then taught a schema the repository no longer has, under a SHA that misreports which revision it describes. Add tools/check-skill-schema.py, run from the existing lint job, asserting the bundled copy and the recorded SHA both match docs/yaml-schema.json. The versioned schemas beside it pin released refs and are deliberately excluded.
github-actions
Bot
added
doc-included
| - name: Check AGENTS.md freshness | ||
| run: python3 tools/check-agents-md.py | ||
| - name: Check bundled YAML schema freshness | ||
| run: python3 tools/check-skill-schema.py |
There was a problem hiding this comment.
This workflow also runs for release-* branches, but the new step always validates the contract whose source.ref is main. If a schema change is backported to a release branch, the check either leaves that branch's CI red or asks us to record the release branch's blob SHA as main provenance. This is a realistic path: release-0.3 has already received schema-changing backports. Could we restrict this step to main pushes and PRs targeting main, or make contract selection branch-aware?
There was a problem hiding this comment.
Good catch. Scoped it to main: if: github.ref == 'refs/heads/main' || github.base_ref == 'main'.
One thing I found while checking, in case it changes your read. Release branches carry their own ci.yml, and this step only exists on main, so nothing runs there today. release-0.3 doesn't even have the older Check AGENTS.md freshness step. The skill directory isn't on release-0.3 at all either, so a backported step would just fail with does not exist.
Your premise still holds though. Those backports are real (0c8da869 and 0bb16280 both touch docs/yaml-schema.json on release-0.3), so this would bite as soon as someone syncs the workflow across branches. Seemed worth guarding either way.
I skipped the branch-aware option since the skill only ships one unversioned schema, so there's nothing to select between. Is that what you had in mind, or were you picturing the manifest carrying per-branch entries at some point?
| def blob_sha(path: Path) -> str: | ||
| """Return the git blob SHA of a file, matching `git hash-object <path>`.""" | ||
| data = path.read_bytes() | ||
| return hashlib.sha1(b"blob %d\0" % len(data) + data).hexdigest() |
There was a problem hiding this comment.
Hashing read_bytes() gives a worktree-content hash, which is not always the Git blob SHA. With core.autocrlf=true and no .gitattributes, these JSON files are checked out as CRLF, so this reports the recorded LF blob SHA as stale and suggests a SHA that Linux CI will reject. Could we hash through Git's clean filters (for example, git hash-object --stdin --path=docs/yaml-schema.json) or enforce LF for these files?
There was a problem hiding this comment.
You're right, thanks. Reproduced it in a scratch repo with core.autocrlf=true: the byte hash comes out 4ca505... while the real blob is 8d6b85..., so the check calls a good pin stale and then hands you a SHA that CI rejects.
blob_sha() now runs git hash-object -- <path>, which applies whatever clean filter the path's attributes select. Nothing moves on an LF checkout, both files still hash to 183cc7ac... and the mutation cases behave the same.
I went with hashing through git rather than a .gitattributes pin, mostly because it covers any filter and not just line endings, and it keeps line-ending policy out of this PR. Would you want the .gitattributes too, or is going through git enough on its own?
…main Hashing the worktree bytes is only the git blob SHA where no clean filter applies. Under core.autocrlf the schema files are checked out CRLF, so the recorded LF SHA read as stale and the printed remediation prescribed a SHA git would never store. blob_sha() now delegates to git hash-object, which applies whatever filter the path's attributes select. The bundled skill exists only on main, so the workflow step is now scoped to main pushes and PRs targeting main rather than running wherever the workflow triggers.
github-actions
Bot
added
doc-included
wenjin272
left a comment
wenjin272
left a comment
There was a problem hiding this comment.
Thanks for addressing my comments. LGTM.
2 participants
Linked issue: #968
Purpose of change
The
flink-agents-devskill ships a copy ofdocs/yaml-schema.jsonatdev/agent-skills/flink-agents-dev/assets/yaml-schema.json, and records the blob SHA it was taken from inassets/yaml-contracts.yaml. Nothing regenerates either one, and the two existing schema checks (test_specs.py,SchemaParityTest) stop atdocs/yaml-schema.json, so re-exporting the schema leaves both stale with no failing test.The stale copy teaches agents a schema the repository no longer has. The stale
blob_shais the quieter half:SKILL.mdandreferences/local-development.mdboth instruct agents to trust the bundled schema only when it describes the same revision as the checkout, and that comparison is decided by exactly this value.tools/check-skill-schema.pyasserts the bundled copy and the recorded SHA both matchdocs/yaml-schema.json. It runs from the existinglintjob next toCheck AGENTS.md freshness, scoped tomainpushes and PRs targetingmainsince the bundled skill only exists onmain. Its Python side is regex-only and stdlib-only for the same reasoncheck-agents-md.pyis: that job has no Python environment beyond the interpreter.The SHA itself comes from
git hash-objectrather than from hashing the file's bytes, so it goes through git's clean filters. A worktree-content hash is only the blob SHA where no filter applies: undercore.autocrlfthese JSON files are checked out CRLF, and the check would then report a current pin as stale and prescribe a SHA that the Linux CI rejects.The versioned schemas beside it (
assets/yaml-schemas/release-0.3.0.json) pin released refs and are expected to differ, so only the unversionedmaincontract is checked.docs/content/docs/development/yaml.mddocuments the regeneration command, so it now also says to refresh the bundled copy. Without that, following the documented steps literally leaves CI red.Tests
No unit test. The check is itself the test, so it was verified by mutation instead: each pin was desynced independently and the check confirmed red, then restored.
docs/yaml-schema.jsonedited, both copies left behindblob_shaedited alonemaincontract loses itsblob_sha,0.3.0keeps one'main' contract has no blob_sharelease-0.3.0.jsoneditedThe fifth row is why the manifest is read with a bounded match rather than a document-wide search for
blob_sha. An unbounded search falls through to the0.3.0contract's SHA and the guard passes on a manifest that has lost the value it is supposed to check.The hashing path was checked separately in a scratch repository with
core.autocrlf=true, committing LF content and checking it back out: the byte hash and the blob SHA diverge there, and onlygit hash-objectreturns the committed blob. All seven rows above were re-run afterwards and are unchanged. Two further cases were covered by hand, since the check now depends on git being present: run outside a git repository it still reports the correct SHA, and with git offPATHit exits with a plain message rather than a traceback.Also run locally:
tools/check-license.sh(RAT passed),python3 tools/check-agents-md.py.API
No public API change. New file is a repository tooling script.
Documentation
doc-neededdoc-not-neededdoc-included