Skip to content

fix(skills): stop ambient env leaking into generated SKILL.md output - #15948

Merged
diegosouzapw merged 20 commits into
cheaper-inference:release/v3.8.52from
BenjaminAronsson:fix/skills-generator-determinism
Oct 18, 2026
Merged

diegosouzapw merged 20 commits into
cheaper-inference:release/v3.8.52from
BenjaminAronsson:fix/skills-generator-determinism

Conversation

@BenjaminAronsson

@BenjaminAronsson BenjaminAronsson commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Current checkpoint — 2026-10-17

Head: f59763c185395a5a4be88bcc58d565b2ac5bb87d; reconciled base: 906fb59a05ee28607dfe4a31a7764125df53fdef. The live release subsequently advanced to 7ff5f6e9b6af688ce0f9e4ccbcb51e3abc845d97; these are scoped receipts, not acceptance of a newer combined candidate.

The current net scope keeps generated examples deterministic across six ambient URL variables (including NEXT_PUBLIC_BASE_URL), retains explicit caller-selected hosts, and adds no published-skill changes. Base PR #16890 already fixes the two stale generated references previously reported below.

Original contributor history, Benjamin's authorship and the human maintainer's co-authorship are preserved. The original fork branch was updated by a verified fast-forward, without force. Independent Standards and Spec reviews found zero issues at this exact head. No operational skill was changed.

VPS .15 exact-head scoped checks, 04:23:21–04:24:32 UTC:

  • 35 Node regression tests PASS, zero failures/skips/cancellations.
  • Generated skills: 0 pending / 46 unchanged, PASS.
  • Changelog integrity PASS.
  • Full check:docs-all FAIL: endpoint /api/balance not found in src/app/api/ in docs/reference/ENVIRONMENT.md.
  • The same strict fabricated-docs failure was separately reproduced at the exact 906 base, then again at 7ff. It is being repaired in a separate documentation-only PR, without a checker/allowlist change.
  • Clean source before/after; Node 24.20.0 / npm 11.15.0 and matching installed lockfile. Overall receipt remains FAIL, not release acceptance.

Admission: HOLD for the documentation repair, unresolved strict docs i18n, all applicable required current-candidate checks, and fresh base/freeze readback. Preserve the reviewed #15945 → #15948 order. Inherited failure is not a merge exemption; no merge/deploy is represented here.

Earlier checkpoints and original description (historical, not current-head acceptance)

Current checkpoint — 2026-10-16

Head: e012a91ba65e3aa2d288054f3532b12723457ac3; reconciled base: 8526008921e232eda3ede6bc67c2985f411a9831. Original contributor history and Benjamin's authorship are preserved; no force push. The misplaced #15945 changelog fragment was removed only from this PR and remains in #15945.

VPS .15 exact-head scoped checks, 21:29:00–21:29:43 UTC:

  • 35 Node regression tests PASS, covering all six ambient URL variables, including NEXT_PUBLIC_BASE_URL, and explicit caller-selected hosts.
  • Changelog integrity and docs-all PASS.
  • Generated-skills synchronization FAIL: 2 pending /44 unchanged (omni-providers and omni-inference), matching the exact852 base signature. fix(skills): regenerate the 20 SKILL.md files drifted since #14381 #15945 repairs those generated artifacts.
  • Clean source fingerprints; Node24.20.0/npm11.15.0 and matching installed lockfile.

The overall receipt is FAIL, not release acceptance. Preserve order #15945 then #15948, refresh the live base, and require all applicable gates; inherited failure is not a merge exemption. Historical 20-file and five-variable descriptions below do not certify the new head.

Historical description and previous evidence (not current-head acceptance)

Summary

  • Makes the SKILL.md generator's output a pure function of its inputs. fix(skills): regenerate the 20 SKILL.md files drifted since #14381 #15945 regenerates the 20 drifted files; this PR stops them drifting again.
  • apiOperationExample.ts / cliConfigurationExample.ts called resolveOmniRouteBaseUrl(), which reads process.env. Their only consumer is the generator, whose output is committed and then diff-gated by check:agent-skills-sync — so the committed artifact was a function of whoever last ran the generator. Anyone with PORT, API_PORT, DASHBOARD_PORT, BASE_URL or OMNIROUTE_BASE_URL set baked their own host into the repo; CI, which sets none of them, then reported drift. Because Merge integrity is .mergify.yml's always-on merge anchor, that drift blocked every PR into the branch.
  • baseUrl is now an explicit argument defaulting to DEFAULT_OMNIROUTE_BASE_URL, so regeneration is reproducible on any machine.
  • fix(docs): resolve generated skill curl examples to the configured loopback port #15778's capability is preserved, not reverted — a caller that wants the operator's configured host passes resolveOmniRouteBaseUrl() as that argument. What changes is that it must be asked for rather than picked up from the ambient environment.

Related Issues

Validation

  • Change type: other — documentation generator (src/lib/agentSkills/**), no request-path or DB code
  • Focused tests and category gates from the golden path — the contract suite plus both other generator suites; TDD red→green recorded below
  • npm run lint — exit 0
  • Reconciled with the current active release base — worktree cut fresh from the release/v3.8.52 tip (61e07fb7e0); focused checks rerun afterward
  • Production-code changes include a new or updated automated test in this PR
check result
tests/unit/cli-configuration-consumer-contract.test.ts 5/5
agentSkills-generator + cli-api-generator-ref-params 30/30
typecheck:core exit 0
npm run lint exit 0
generated output vs #15945's committed files byte-identical (git diff empty)

TDD record (Hard Rule #18). The new determinism test was confirmed RED against the previous code before the fix was restored:

AssertionError [ERR_ASSERTION]: env {"PORT":"37128"} leaked into the example
ℹ pass 0
ℹ fail 2

Tests Added Or Updated

  • tests/unit/cli-configuration-consumer-contract.test.ts (updated) — three changes:
    • New: generated examples ignore ambient base-url env so committed output is reproducible — asserts the default output is identical across all five env vars (PORT, API_PORT, DASHBOARD_PORT, BASE_URL, OMNIROUTE_BASE_URL). This is the regression guard, and the one confirmed red above.
    • New: configuration-API examples are reproducible and still accept an explicit host — pins the /api/cli-tools/* branch, which routes through cliConfigurationExample.ts, on both paths.
    • Rewritten: fix(docs): resolve generated skill curl examples to the configured loopback port #15778's configuration examples track the configured loopback port… → …when given one. Same assertion (http://localhost:37128, no 20128), now via the explicit argument, so fix(docs): resolve generated skill curl examples to the configured loopback port #15778's behaviour stays covered rather than being silently dropped.
    • Added a withBaseUrlEnv() helper so each test restores all five vars in a finally, replacing the previous three-variable save/restore block.

Coverage Notes

  • Touched production files are src/lib/agentSkills/apiOperationExample.ts and src/lib/agentSkills/cliConfigurationExample.ts. Both are fully exercised by cli-configuration-consumer-contract.test.ts, which covers every branch: bearer, dashboard-session (login / mutation / read), and the two /api/cli-tools/* paths — each now on both the default and explicit-baseUrl paths, so the new parameter adds covered branches rather than uncovered ones.
  • No coverage moves down; the change is a signature change plus removal of two process.env reads.

Reviewer Notes

…nference#14381

`check:agent-skills-sync` has been failing on the release/v3.8.52 tip,
reporting 20 drifted skills. It blocks every PR into the branch: the
`Merge integrity (changelog + generated skills)` job is named in
.mergify.yml as the always-on anchor that must be an affirmative success.

Root cause: cheaper-inference#14381 (4df9cea) extracted the curl examples out of
generator.ts into apiOperationExample.ts and swapped the hardcoded
`https://localhost:20128` for `resolveOmniRouteBaseUrl()`, whose default is
`http://localhost:20128`. The committed SKILL.md files were never
regenerated, so every generated example has disagreed with the generator
since that commit.

This is the mechanical output of `node --import tsx/esm
scripts/skills/generate-agent-skills.mjs --apply` with no base-url env vars
set, i.e. the documented default — the same environment CI runs in. No
hand edits, no instruction text touched. The gate goes 20 drifted -> 0.
The curl-example builders called resolveOmniRouteBaseUrl(), which reads
process.env. Their only consumer is the SKILL.md generator, whose output is
committed and then diff-gated by check:agent-skills-sync — so the committed
artifact was a function of whoever last ran the generator. Anyone with PORT,
API_PORT, DASHBOARD_PORT, BASE_URL or OMNIROUTE_BASE_URL set wrote their own
host into the files, and CI, which sets none of them, reported drift. That is
the trap that left all 20 skills drifted and blocked every PR into the branch
(the Merge integrity job is .mergify.yml's always-on merge anchor).

baseUrl is now an explicit argument defaulting to DEFAULT_OMNIROUTE_BASE_URL,
so regeneration is reproducible on any machine.

cheaper-inference#15778's capability is preserved, not dropped: a caller that wants the
operator's configured host passes resolveOmniRouteBaseUrl() as that argument,
and its test now asserts exactly that. What changes is that it must be asked
for rather than picked up from the environment.

Verified byte-identical to the regenerated output in cheaper-inference#15945, so the two land
in either order without conflict. The determinism test was confirmed RED
against the previous code ('env {"PORT":"37128"} leaked into the example')
before the fix was restored. 5/5 in the contract suite, 30/30 across
agentSkills-generator and cli-api-generator-ref-params, typecheck and lint 0.
BenjaminAronsson and others added 2 commits October 10, 2026 08:34
…ontracts

Refresh against the current release base, preserve canonical generator defaults and explicit host support, and remove an inaccurate reference to a generator CLI flag.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
… metadata docs

Preserve the original generated-skills contribution and merge release base 7510677. Regenerate the owner-approved PUT /api/provider-models reference from canonical inputs.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Hi @BenjaminAronsson — closing this pull request to clear the open queue. The branch fix/skills-generator-determinism is kept, so the commits stay. Comment here if it should be reopened.

Retain contributor history and current release changes. Additional new-base generated instruction changes remain preview-only until separately approved.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Owner has explicitly requested reopening and merging the campaign PRs. This PR was already reopened when checked. The relay-15948 reconciliation worktree belongs to another session, so this campaign will not overwrite or merge over that session. A fresh read-only VPS probe on generated-output candidate 6718424 confirms the independent determinism defect: PORT=37128 makes sync exit 2 (19 stale / 27 unchanged). This PR remains necessary unless a new current-base execution proves supersession. Fresh required checks are pending; no inherited failure or pending result is being treated as PASS.

BenjaminAronsson and others added 2 commits October 10, 2026 19:45
Preserve deterministic examples and contributor history while refreshing the exact release base and correcting the changelog fragment format.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Merge release base 7a0bb87 without losing contributor work or release changes. Focused generator and consumer tests pass 33/33 with matching-lock dependencies. Two additional instruction hunks remain preview-only pending explicit approval.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Reopened at the repository owner's explicit request, retaining the original contribution. Thank you @BenjaminAronsson for fixing environment-dependent generated examples.

Normal fast-forward update: head b9b3c20f671005eb7a3efd841010e4be649a255b, tree 936fbfb66074c7dc2f1ce8cb2ac0c1dfe2f8060d, real merge of the previous contributor head and pinned release base 7a0bb8754d7432c9fc6b899464d88adbb63d0bd0. Benjamin remains author, maintainer co-author. The only additional correction was the required bullet marker in this PR's changelog fragment.

With dependencies matching the current lock: exact-base-helper RED 3 PASS / 2 FAIL; restored fix GREEN 35 PASS across the contract, skill-generator and CLI-reference-parameter suites. Focused lint, final format, test-masking, changelog, typecheck:core and normal hooks exit 0. The physical cache is from a completed clean npm ci; no separate clean install per worktree is claimed.

Admission remains HOLD until current-candidate applicable CI succeeds and the separately owned generated-content repair #15945 lands. This PR intentionally does not copy that contribution: its standalone generator still reports 20 pending / 26 unchanged / zero errors. No merge, bypass, force push or baseline increase occurred.

Regenerate the two explicitly approved instruction hunks with the canonical generator. Neutral full dry-run reports 0 pending, 46 unchanged and 0 errors; focused generator and consumer tests pass 33/33.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Delivery reconciliation — 2026-10-13

Thank you @BenjaminAronsson and @woodsonl for the selected fixes. The repository owner has explicitly authorized merging the original PRs #15945, #15948 and #15682, including the previously reviewed generated instruction hunks. That approval remains settled; no replacement PR or contributor-branch overwrite is planned.

Reconciled the current release base 7f3cbb046b3706bf0bbc047b7eaa0cbada8d9e50 and all three exact published heads into an isolated local interaction candidate 2336a6d2e1f39e38da8ab8e54c37e189224a867b, tree bea8bc1a7f4f1538dfe97a89f7295363df8fa5e7. Preserved the author's latest #15682 Stryker registrations (b6716df...). All selected heads and the current base are real ancestors; no conflicts.

Current scoped results: 96/96 focused native tests PASS, typecheck:core PASS, canonical neutral skill generator 0 pending / 46 unchanged / 0 errors, changelog integrity PASS. These are focused interaction receipts, not a full CI certificate or admission for different intermediate squash trees.

The earlier pending-workflow snapshot has been superseded by terminal results. #15945's Gate / Quality passed, but Gate / CI failed on Docs Sync (Strict); #15948 and #15682 also have failed required lanes. Reproduced the blocking i18n checker on both the exact pure base and the selected interaction candidate: each exits 1, 27 drifted sources / 132 drifted targets, with identical affected path sets. The 264 separately reported stale targets are advisory, not the blocking count. The selected contribution clears generated-skill drift, not this global translation drift.

The retained full Release-Green report for exact base 7f3cbb..., run https://github.com/diegosouzapw/OmniRoute/actions/runs/38292478149 , also verifies FAIL through the versioned report verifier. Other test/package/typecheck signatures still need their own causal diagnosis; the older #15306 issue body is not treated as a current certificate.

Integration remains HOLD on required validation, not on owner approval. A separate base-repair scope has been raised with the owner. No admin merge, force push, gate/hash/baseline bypass, release or deploy was performed. Original PRs remain open and contributor credit is preserved.

@diegosouzapw

Copy link
Copy Markdown
Collaborator

The repository owner explicitly transferred #15945/#15948 to this Video Bridge campaign on 2026-10-13 and requested completion. I am assuming coordination, preserving the published b9b3c20 head and the original contribution. Prior relay worktrees remain untouched. Fresh .15 checks will distinguish current candidate failures from current release-base defects; merge remains conditional on all applicable required gates, not the older focused receipts.

BenjaminAronsson and others added 2 commits October 13, 2026 08:09
No content changes. Refresh the original PR against the current release merge candidate after cheaper-inference#15840 restored translation synchronization.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
No content changes. Re-evaluate the original contributor PR against the current release merge candidate.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
BenjaminAronsson and others added 3 commits October 14, 2026 19:13
Preserve the original contribution and current generated outputs. The candidate delta against release 2c06e3c is only the existing changelog fragment.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Retain original contributor changes and the refreshed cheaper-inference#15945 dependency; validate this exact candidate without reusing old failing or passing checks.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Reconcile release 7f1be3b while preserving the original generator changes. Cover NEXT_PUBLIC_BASE_URL isolation and restoration after reproducing the environment-sensitive failure. Clean dependency install, 51 focused tests, canonical generation under ambient overrides and core typecheck passed.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Thanks @BenjaminAronsson. Your original PR has been reconciled with release 7f1be3bbf05475fd7e53461f4ec49ec2ae311d7f, preserving your generator changes, authorship and original commit ancestry. Updated head: 955c22ff52673fd45f6c44d15c720aa0491a53ec; tree: a04cd030a7fb324b74dc892a88e6b1525df911c8. The reconciliation keeps you as author and records the maintainer's adjustment as co-authorship.

Review found one additional test-isolation defect: NEXT_PUBLIC_BASE_URL was a supported resolver input but absent from the environment helper. Setting that variable reproduced 4 passing / 1 failing consumer-contract tests on the original PR. The helper now isolates and restores it, the configured-port test always exercises an ambient public URL, and the canonical-generation matrix covers all six supported environment inputs. Runtime configured-host behavior is unchanged; generated examples remain canonical unless callers explicitly supply a host.

Validation of this reconciliation:

  • Clean npm ci --no-audit --no-fund, with normal install scripts/hooks: PASS (2,526 packages, 22 minutes). No borrowed/symlinked dependency directory was used in this new worktree. Lockfile unchanged; Node 24.20.0 / npm 11.15.0.
  • Generator, registry parser, consumer contract and base-URL resolver tests with an ambient public URL: 51/51 PASS, zero skips/cancellations.
  • Full generator with ambient port/base/public-URL overrides: Generated 0 / Unchanged 46 / Errors 0.
  • npm run typecheck:core: PASS.
  • Focused ESLint/Prettier, diff checks and normal commit hooks: PASS.
  • Post-commit consumer regression with NEXT_PUBLIC_BASE_URL set: 5/5 PASS.
  • Post-hook scope audit: exactly five residual paths, no protected instruction-file changes; original head and pinned base are both parents of this merge commit.

These are scoped current-candidate receipts, not full merge/release acceptance. Required hosted CI/Quality and all applicable remaining lanes must still pass for the actual candidate. The generated sync-pull example remains separately tracked in #15945. No queue label, merge, PR closure, bypass, paid operation or deployment was performed.

Preserve release ad37e8f and explicit URL inputs. Focused tests: 51/51 under ambient NEXT_PUBLIC_BASE_URL. Full generation under ambient host and port overrides: Generated 0, Unchanged 46.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Relay continuation — scoped reconciliation, not merge acceptance

Thanks Benjamin. Reconciled this original PR with pinned release ad37e8f8f51eb2d505f88a3139e76912cf9598e2 and published 2723144a57c6d84740c38aacf5bbc84426aa13c5 through a normal fast-forward push. Original commits and contributor attribution are preserved.

51/51 focused tests PASS under ambient NEXT_PUBLIC_BASE_URL; full catalog Generated 0 / Unchanged 46 with ambient host/port overrides. Explicit configured URLs and runtime resolver behavior remain preserved.

Node 24.20.0 / npm 11.15.0; unchanged lockfile SHA-256 f2de65a4140bbe8f4f1ae78ea26e1ba06baa67850c297306d94e7b0793f7e3b4. Normal commit hooks ran. These are focused diagnostics, not the full current-candidate acceptance matrix. The release has advanced since the pinned comparison, and full mandatory checks must be refreshed before admission. No queue label, bypass, merge, closure or deployment was performed by this continuation.

BenjaminAronsson and others added 3 commits October 16, 2026 18:22
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw
diegosouzapw merged commit 6e1ff7d into cheaper-inference:release/v3.8.52 Oct 18, 2026
43 of 60 checks passed
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.

2 participants