Skip to content

ci(update-models): sign commits with bot identity, add semantic catalog diff - #4110

Merged
aheritier merged 1 commit into
mainfrom
chore/update-models-workflow-signed-commits
Sep 2, 2026
Merged

ci(update-models): sign commits with bot identity, add semantic catalog diff#4110
aheritier merged 1 commit into
mainfrom
chore/update-models-workflow-signed-commits

Conversation

@aheritier

@aheritier aheritier commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

🤖 Automated implementer agentthis comment was posted by the implementer bot from Docker Agentic Platform, not by a human developer

Two independent improvements to the weekly update-models.yml refresh (root-cause analysis and the full three-phase design were scoped directly with the requester in a design discussion prior to this PR; there is no filed GitHub issue to link). Phase 3 (AI-generated PR summary via docker/docker-agent-action) is explicitly deferred to a follow-up PR — no agent/LLM step, no .github/agents/*.yaml, no new secrets here.

Signed commits

  • Create pull request (peter-evans/create-pull-request) now passes sign-commits: true with token: ${{ github.token }} — this builds the commit through GitHub's API instead of a local git push, so it's Verified regardless of which token creates it. On the default GITHUB_TOKEN, the commit author/committer is consistently github-actions[bot], rather than whoever happened to trigger the run.
  • No App-token dependency: an earlier iteration of this PR used docker/docker-agent-action/setup-credentials to mint a docker-read-write[bot] App token, buying bot branding and CI auto-triggering on the created PR. That was deliberately dropped — not worth the added dependency for those two benefits alone. Accepted trade-off: commits show as github-actions[bot], and the pre-existing weekly-manual-CI-rerun tax (GitHub's recursion guard doesn't fire workflows off PRs created by GITHUB_TOKEN) returns.
  • Job permissions are contents: write + pull-requests: write (needed for the default GITHUB_TOKEN to do the signed-commit-via-API + PR creation itself). persist-credentials: false on checkout. A concurrency group and a canonical-repo job guard prevent an overlapping workflow_dispatch from racing the weekly schedule.
  • The on: block (workflow_dispatch + weekly schedule) is untouched — byte-identical to main.

Semantic catalog diff, no AI

  • .github/scripts/models-delta.jq — structural added/removed/changed diff between two snapshot.json files.
  • .github/scripts/models-delta-render.jq — renders that diff to markdown tables, with a safe()/safe_diff() sanitiser clamping every third-party (models.dev) string to a short, bounded, control-character-free charset before it reaches the PR body, since that body flows into this repo's automated PR-review pipeline.
  • scripts/models-delta.sh — the single caller of both scripts; task models-delta (Taskfile) and the workflow both shell out to it, so the jq logic exists in exactly one place. With no args it diffs HEAD's snapshot.json against the working tree (what a local task update-models would change); --json-out FILE also emits the structural JSON.
  • The workflow runs the diff after task update-models and uses the rendered markdown as the PR body/title/commit message. The delta step is continue-on-error: true with a static-body fallback, so a jq/shell failure degrades the PR text instead of silently skipping the weekly refresh.
  • A refresh still opens a PR even when only snapshot_date.txt changed (no catalog content diff) — conservative choice, preserves today's behavior and doesn't interact with check-models-fresh/TestSnapshotDateIsFresh.
  • scripts/models-delta-test.sh is a golden-fixture regression test for the two jq scripts. .github/workflows/models-delta-lint.yml runs it (plus shellcheck) as its own path-filtered workflow — mirrors this repo's existing docs-lint.yml pattern — rather than unconditionally on every PR via ci.yml's blanket lint job.

Testing

  • task build, task test, task lint all pass (one pre-existing, environment-flaky pkg/cache test unrelated to this diff, reproduced identically on unmodified main).
  • ./scripts/workflow-lint.sh, actionlint -shellcheck=shellcheck, and shellcheck scripts/models-delta.sh scripts/models-delta-test.sh all clean.
  • Self-diff of the real pkg/modelsdev/snapshot.json via task models-delta yields empty added/removed/changed.
  • Reviewed by the reviewer sub-agent across many passes (verdict: approve) plus multiple rounds of human security review — see PR comments for the full history of findings addressed, including the deliberate App-token removal and the CI lint path-scoping.

Branch history was squashed into a single commit per request; the incremental "address review round N" commits are not individually preserved.

@aheritier
aheritier marked this pull request as ready for review September 2, 2026 07:13
@aheritier
aheritier requested a review from a team as a code owner September 2, 2026 07:13
trungutt
trungutt previously approved these changes Sep 2, 2026
@aheritier aheritier added area/ci CI/CD workflows and pipeline kind/chore Maintenance, deps, CI, tooling (maps to chore: commit prefix) labels Sep 2, 2026
aheritier

This comment was marked as resolved.

@aheritier

This comment was marked as resolved.

@aheritier
aheritier marked this pull request as ready for review September 2, 2026 09:47
aheritier

This comment was marked as resolved.

@aheritier

This comment was marked as resolved.

Comment thread .github/workflows/update-models.yml Outdated
Comment thread Taskfile.yml Outdated
trungutt
trungutt previously approved these changes Sep 2, 2026
Comment thread .github/workflows/ci.yml Outdated
Two independent improvements to the weekly models.dev snapshot refresh
(.github/workflows/update-models.yml), which previously opened PRs
with an unsigned commit attributed to whoever happened to trigger the
run, and a static, uninformative PR body.

Signed commits
---------------
Add `sign-commits: true` to the existing `peter-evans/create-pull-request`
step. This builds the commit through GitHub's API instead of a local
`git push`, so it is Verified regardless of which token creates it.
On the default `GITHUB_TOKEN` (used explicitly via `token: github.token`
for clarity) the commit author/committer becomes `github-actions[bot]`
consistently, rather than the human who happened to run the workflow.

`persist-credentials: false` is set on checkout as a minor hardening
measure, since `create-pull-request` is given its token explicitly and
never needs checkout's persisted git credential.

Semantic catalog diff, no AI
-----------------------------
`pkg/modelsdev/snapshot.json` is a single-line, multi-megabyte JSON
file, so its raw git diff is one useless line — this is why the PR
body was static. Instead:

- `.github/scripts/models-delta.jq` computes a structural
  added/removed/changed diff between two snapshot.json files.
- `.github/scripts/models-delta-render.jq` renders that diff to
  markdown tables, with a `safe()`/`safe_diff()` sanitiser clamping
  every third-party (models.dev) string to a short, bounded,
  control-character-free charset before it reaches the PR body — that
  body is consumed by this repo's automated PR-review pipeline, so
  untrusted text needs bounding before it gets there.
- `scripts/models-delta.sh` is the single wrapper around both jq
  scripts; a new `task models-delta` target and the workflow's
  "Compute catalog delta" step both call it, so the diff logic exists
  in exactly one place. With no arguments it diffs HEAD's
  pkg/modelsdev/snapshot.json against the working tree — the same
  comparison the workflow makes after `task update-models`, so the
  local and CI code paths never drift apart.
- The workflow uses the rendered markdown as the PR body/title/commit
  subject. The delta step is `continue-on-error: true` with a
  static-body fallback step, so a jq/shell bug degrades the PR text
  instead of silently skipping the weekly refresh. A refresh still
  opens a PR even when only snapshot_date.txt changed (no catalog
  content diff), preserving today's behaviour and avoiding any
  interaction with the check-models-fresh staleness test.
- `scripts/models-delta-test.sh` is a golden-fixture regression test
  for the two jq scripts (fixtures under .github/scripts/testdata/),
  since they have no Go-test equivalent and only run for real,
  unattended, once a week. `.github/workflows/models-delta-lint.yml`
  runs it (plus shellcheck) as its own path-filtered workflow — mirrors
  the repo's existing docs-lint.yml pattern — rather than unconditionally
  on every PR via ci.yml's blanket lint job.
- A concurrency group and a canonical-repo job guard prevent an
  overlapping workflow_dispatch from racing the weekly schedule run.

Phase 3 of the original proposal (an LLM-written PR summary via
docker/docker-agent-action) is deferred to a future change.
@aheritier
aheritier force-pushed the chore/update-models-workflow-signed-commits branch from f756ea2 to f78c6dd Compare September 2, 2026 15:17
@aheritier

This comment was marked as resolved.

@aheritier
aheritier merged commit eac39b7 into main Sep 2, 2026
30 of 32 checks passed
@aheritier
aheritier deleted the chore/update-models-workflow-signed-commits branch September 2, 2026 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/ci CI/CD workflows and pipeline kind/chore Maintenance, deps, CI, tooling (maps to chore: commit prefix)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants