Skip to content

updated github action for model changes#2564

Merged
tim-inkeep merged 1 commit intomainfrom
bugfix/model-sync-no-date
Mar 6, 2026
Merged

updated github action for model changes#2564
tim-inkeep merged 1 commit intomainfrom
bugfix/model-sync-no-date

Conversation

@tim-inkeep
Copy link
Copy Markdown
Contributor

No description provided.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Mar 6, 2026

⚠️ No Changeset found

Latest commit: c2ef55a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 6, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agents-api Ready Ready Preview, Comment Mar 6, 2026 3:51am
agents-docs Ready Ready Preview, Comment Mar 6, 2026 3:51am
agents-manage-ui Ready Ready Preview, Comment Mar 6, 2026 3:51am

Request Review

@pullfrog
Copy link
Copy Markdown
Contributor

pullfrog Bot commented Mar 6, 2026

Removes the date-based lookback filter from the model-sync GitHub Action, replacing it with ID-pattern heuristics so the detection script considers all gateway models regardless of release date.

  • .github/scripts/detect-model-changes.ts — removes DAYS env var, released field, and cutoffUnixSeconds helper; adds ID-based exclusion filters (-chat suffix, oss- prefix/infix); adds a random slug to the changeset filename to avoid collisions; expands the Claude Code prompt with explicit ADD/SKIP rules for model triage
  • .github/workflows/model-sync.yml — removes the days workflow_dispatch input and DAYS env passthrough

Pullfrog  | View workflow run | Using Claude Code | Triggered by Pullfrogpullfrog.com𝕏

Copy link
Copy Markdown
Contributor

@pullfrog pullfrog Bot left a comment

Choose a reason for hiding this comment

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

Low urgency. The shift from date-based to keyword-based filtering is a reasonable simplification. Three suggestions below — the most impactful is moving more skip-logic into code rather than relying entirely on the Claude prompt to filter models that are explicitly listed under "New models to add."

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow runpullfrog.com𝕏

Comment on lines +69 to +71
const rawId = m.id.slice(slashIndex + 1);
if (rawId.endsWith('-chat')) return false;
if (rawId.includes('-oss-') || rawId.startsWith('oss-')) return false;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The code-level filters only cover -chat and oss, but the prompt's SKIP rules cover a much broader set (instruct, embedding, tts, dall-e, realtime, audio, -image, dated snapshots, etc.). This means models matching those keywords still pass through to newModels and appear in the prompt's "New models to add" section — then Claude is asked to ignore them.

Consider filtering more of these at the code level (at minimum -image/-image-preview suffixes and instruct/embedding keywords). This reduces noise in the prompt and avoids relying on Claude to correctly skip models that are explicitly presented to it as new.

---

Add new models: ${modelSummary}
Add new models: <comma-separated list of provider/model-id for each model you added>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The changeset description is now a literal template Add new models: <comma-separated list ...> that Claude fills in. Since Claude is also told to skip certain models from the "New models to add" list, the generated description must exclude skipped models. If Claude misreads this and includes the angle-bracket placeholder literally, or lists all detected models including skipped ones, the changeset will be wrong.

Consider generating the description dynamically in code (as modelSummary previously did) and instructing Claude to adjust it only if it skips models, rather than generating it from scratch.

const modelList = newModels.map((m) => `- ${m.provider}/${m.id}`).join('\n');
const modelSummary = newModels.map((m) => `${m.provider}/${m.id}`).join(', ');
const today = new Date().toISOString().split('T')[0];
const slug = Math.random().toString(36).slice(2, 8);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: Math.random().toString(36).slice(2, 8) is fine for collision avoidance, but since the workflow's concurrency: { group: model-sync, cancel-in-progress: false } guarantees only one run at a time and each run already includes today's date — collisions can't actually happen. The slug is harmless but a brief comment explaining why it's there (future-proofing if concurrency changes) would help maintainers.

Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

PR Review Summary

(0) Total Issues | Risk: Low

This PR simplifies the model-sync GitHub Action by removing date-based filtering (DAYS parameter and released timestamp checks) in favor of ID-pattern-based filtering. The changes are well-scoped to internal CI/CD tooling with no customer-facing impact.

🔴❗ Critical (0) ❗🔴

None.

🟠⚠️ Major (0) 🟠⚠️

None.

🟡 Minor (0) 🟡

None.

💭 Consider (1) 💭

💭 1) model-sync.yml Reduced manual override flexibility

Issue: The days workflow_dispatch input has been removed, so operators can no longer manually trigger with a custom lookback window (e.g., DAYS=90 for backfills after outages).

Why: This is a reasonable architectural simplification since the new approach compares against the full gateway model list. If backfill capability is ever needed, a different mechanism would be required.

Refs: model-sync.yml:6


✅ APPROVE

Summary: Clean PR that simplifies the model detection logic. The code is correct, the filtering approach is sound, and the changeset slug addition prevents filename collisions. The security posture (SHA-pinned actions, minimal permissions) remains strong. Good to merge! 🚀

Discarded (4)
Location Issue Reason Discarded
detect-model-changes.ts:168 Changeset filename collision prevention Positive observation — not an issue
detect-model-changes.ts:69-72 ID-pattern filtering replaces date-based filtering Informational observation — intentional design
model-sync.yml Actions properly pinned to SHAs Positive observation — not an issue
model-sync.yml:16-18 Workflow permissions follow least-privilege Positive observation — not an issue
Reviewers (2)
Reviewer Returned Main Findings Consider While You're Here Inline Comments Pending Recs Discarded
pr-review-devops 5 0 1 0 0 0 4
pr-review-standards 0 0 0 0 0 0 0
Total 5 0 1 0 0 0 4

@github-actions github-actions Bot deleted a comment from claude Bot Mar 6, 2026
@tim-inkeep tim-inkeep added this pull request to the merge queue Mar 6, 2026
Merged via the queue into main with commit 79828ea Mar 6, 2026
19 checks passed
@tim-inkeep tim-inkeep deleted the bugfix/model-sync-no-date branch March 6, 2026 03:59
@itoqa
Copy link
Copy Markdown

itoqa Bot commented Mar 6, 2026

Ito Test Report ❌

12 test cases ran. 11 passed, 1 failed.

✅ Most verification checks for workflow configuration, model filtering behavior, idempotency safeguards, and unauthorized-access boundaries behaved as expected. 🔍 One security-relevant defect was confirmed: prompt content is built from untrusted model IDs without escaping, allowing injected markdown/control text to break prompt structure.

✅ Passed (11)
Test Case Summary Timestamp Screenshot
ROUTE-2 Validated from workflow run logs that Detect model changes runs successfully without DAYS env and without lookback contract errors. 0:00 ROUTE-2_0-00.png
ROUTE-5 Verified run history integrity using completed Model Sync runs #9 and #8; each run resolves to its own run_id/job_id and includes expected Detect model changes step without cross-run log mix-up. 5:34 ROUTE-5_5-34.png
LOGIC-1 detect-model-changes.ts has no DAYS lookback logic and retains explicit tracked-provider, language-type, and exclusion filters for -chat and oss variants. 8:31 LOGIC-1_8-31.png
LOGIC-2 model-sync.yml no longer defines workflow_dispatch days input and does not pass DAYS into the detect step environment. 8:31 LOGIC-2_8-31.png
LOGIC-3 Validated the no-change branch using a deterministic harness response built from current constants; script returned has_changes=false with empty prompt. 11:49 LOGIC-3_11-49.png
LOGIC-4 GitHub search returned an open model-sync PR and the detection script set has_changes=false, preventing duplicate automation in harness execution. 11:49 LOGIC-4_11-49.png
LOGIC-5 Anthropic ID normalization in detect-model-changes.ts converts dotted minors to dash format and matches anthropic model IDs stored in constants. 8:31 LOGIC-5_8-31.png
EDGE-3 Verified unauthorized access is blocked: workflow page required sign-in and exposed no manual dispatch controls. 14:46 EDGE-3_14-46.png
ADV-1 Verified model-sync config remains drift-free: detect script has no DAYS/cutoff/released gate, workflow has no workflow_dispatch days input or DAYS env, and workflow UI controls exposed no legacy days parameter. 19:08 ADV-1_19-08.png
ADV-2 Captured baseline unauthenticated gateway response and verified explicit failure behavior under simulated 429 conditions with no silent continuation. 11:49 ADV-2_11-49.png
ADV-3 Reproduced non-200 search response and confirmed fail-safe behavior where detection warns and forces has_changes=false to avoid duplicate mutation. 11:49 ADV-3_11-49.png
❌ Failed (1)
Test Case Summary Timestamp Screenshot
ADV-4 Dynamic injected model ID containing newline and fenced markdown was interpolated directly into the generated prompt, demonstrating structural prompt-break vulnerability. 20:25 ADV-4_20-25.png
Prompt injection resilience from model IDs – Failed
  • Where: .github/scripts/detect-model-changes.ts prompt construction for the model-sync automation handoff.

  • Steps to reproduce: Return a model ID from the gateway containing newline/backtick markdown payload; run detection; inspect the generated prompt output.

  • What failed: Untrusted model ID text is inserted into the prompt body verbatim, so injected markdown fences/instructions alter prompt structure instead of being treated as inert data.

  • Code analysis: Reviewed filtering, model mapping, and prompt assembly in the production detection script. The code enforces provider/type/exclusion constraints but does not sanitize control characters in model IDs before joining into markdown bullet lines and embedding into the final prompt template.

  • Relevant code:

    .github/scripts/detect-model-changes.ts (lines 64-72)

    .filter((m) => {
      const slashIndex = m.id.indexOf('/');
      if (slashIndex === -1) return false;
      if (!TRACKED_PROVIDERS.has(m.id.slice(0, slashIndex))) return false;
      if (m.type !== 'language') return false;
      const rawId = m.id.slice(slashIndex + 1);
      if (rawId.endsWith('-chat')) return false;
      if (rawId.includes('-oss-') || rawId.startsWith('oss-')) return false;
      return true;
    })

    .github/scripts/detect-model-changes.ts (lines 166-174)

    const modelList = newModels.map((m) => `- ${m.provider}/${m.id}`).join('\n');
    const today = new Date().toISOString().split('T')[0];
    const slug = Math.random().toString(36).slice(2, 8);
    
    const prompt = `New AI models have been detected from Vercel AI Gateway that are not yet in our static model list.

New models to add:
${modelList}


- **Why this is likely a bug:** Production code directly interpolates externally sourced model IDs into an instruction prompt without escaping, enabling prompt-structure injection by crafted IDs.
- **Introduced by this PR:** No – pre-existing bug (code not changed in this PR).
- **Timestamp:** 20:25

</details>

<details>
<summary>📋 View Recording</summary>

[Screen Recording](https://api.ito.ai/media/112bea0b-ab66-4b63-8daf-e4e6faa20b83.mp4?sig=3cd95b94629d67d5415b5a0d5a18561cf0a4d8552220fb30c74a0c0555292a26)

</details>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant