weiqingy commented on code in PR #969:
URL: https://github.com/apache/flink-agents/pull/969#discussion_r3742124644
##########
.github/workflows/ci.yml:
##########
@@ -49,6 +49,8 @@ jobs:
run: ./tools/lint.sh -c
- 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
Review 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?
##########
tools/check-skill-schema.py:
##########
@@ -0,0 +1,101 @@
+#!/usr/bin/env python3
+################################################################################
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#################################################################################
+"""Assert the coding-agent skill still bundles the current YAML schema.
+
+The skill ships a copy of docs/yaml-schema.json so it can answer YAML questions
+offline, and records in yaml-contracts.yaml the git blob SHA that copy was
taken
+from. Nothing regenerates either one, so both drift silently the moment the
+schema is re-exported. A stale copy teaches agents a schema the repository no
+longer has, and a stale blob SHA misreports which revision the copy describes.
+
+Only the unversioned "main" contract is checked. The versioned schemas beside
it
+pin released refs, so they are expected to differ from the working tree.
+
+Regex-only and dependency-free so it runs on any Python 3 without a build step.
+"""
+
+import hashlib
+import re
+import sys
+from pathlib import Path
+
+REPO_ROOT = Path(__file__).resolve().parent.parent
+DOCS_SCHEMA = REPO_ROOT / "docs" / "yaml-schema.json"
+ASSETS = REPO_ROOT / "dev" / "agent-skills" / "flink-agents-dev" / "assets"
+BUNDLED_SCHEMA = ASSETS / "yaml-schema.json"
+MANIFEST = ASSETS / "yaml-contracts.yaml"
+
+
+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()
Review 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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
