[OPIK-7793] [FE] ci: run the frontend checks with the private ai-spend plugin staged

[andriidudar]

Details

The comet frontend image compiles comet-ml/opik-plugin-ai-spend, checked out at build time into src/plugins/ai-spend and type-checked against whatever this repo holds. Nothing on a pull request compiles it — the image build runs on push to main, on release, and on PRs labelled test-environment — so a breaking change to a shared surface the plugin imports passes every PR check and only fails after merge.

This stages the plugin the way the image stages it and runs typecheck, eslint and deps:validate against the result, so that class of break fails on the PR that causes it.

• All three run even when an earlier one fails, so a PR sees every problem in one go.
• eslint is scoped to the plugin sources. Staging the plugin cannot change core lint results, so linting core here would repeat the fe-eslint leg's verdict while making a red plugin check ambiguous. deps:validate is deliberately not scoped: it is a whole-graph analysis, and the plugin's edges into core can only be judged by cruising all of src — which is how the plugin's import cycle was found in the first place.
• The trigger covers this workflow file as well as apps/opik-frontend, so a change to the check runs the check.
• Staging asserts it copied something. If the plugin's src layout moves, the job fails loudly instead of leaving a check that passes vacuously.

Fork pull requests cannot read OPIK_PLUGIN_AI_SPEND_TOKEN, so the job concludes green with a ::notice rather than failing or hanging — it can be made required without blocking outside contributors. pull_request_target would give it secrets but would run fork code with our token on a public repo, so it is not used. The comet image build on push to main remains the backstop for a fork PR that merges without this having run.

Intentional breaking changes name the adapting plugin branch in the PR body and merge the plugin side first:

ai-spend-plugin-ref: someone/my-branch

There is deliberately no skip label — a green check that proves nothing is how the release broke.

Cost of the check: it means a frontend PR can be gated by a private repo. That is the trade being made, and it only bites once this is added to the ruleset as required.

Change checklist

• User facing
• Documentation update

Issues

• OPIK-7793

AI-WATERMARK

AI-WATERMARK: yes

• Tools: Claude Code
• Model(s): Claude Opus 5
• Scope: The workflow, and the diagnosis of why existing CI missed the break.
• Human verification: All three checks dry-run locally against this branch's base with plugin main staged the way the workflow stages it; the ref parser and the trigger gap were each tested rather than reasoned about. A negative control is prepared but not yet run (see Testing).

Testing

The three checks, dry-run locally in apps/opik-frontend on this branch's base, with plugin main (2df2230) copied into src/plugins/ai-spend exactly as the workflow copies it — macOS, local Node:

• npm run typecheck — exit 0
• npx eslint src/plugins/ai-spend --max-warnings=0 — clean
• npm run deps:validate — exit 0, 27 known violations matched, no ai-spend violation of any rule

Also confirmed clean with no plugin present and with the plugin symlinked (the dev-runner layout), so the job's staging step is the only thing that has to work.

The PR-body ref parser was run over five inputs: a plain override, a differently-cased key with extra whitespace, a body with no override, an empty body, and ../../evil; rm -rf /. First two resolve correctly, next two fall back to main, the last is rejected by the ref allowlist before reaching actions/checkout.

The trigger gap. The first version filtered on apps/opik-frontend/** only, which meant this PR — a workflow-only change — would not have run its own check. .github/workflows/frontend_private_plugin_checks.yml is now in paths, so this PR running the job green is itself the positive control.

Not yet run: the negative control. A green check does not prove a working check. andriid/NA-scratch-plugin-check-negative-control is prepared: it adds a required prop to LogsTabProps and passes it from the only in-repo caller, so the sole compile error is the plugin's LogsTab call — the shape of the change that broke two release runs. Locally it produces exactly one error, TS2741 on AiSpendSessionsPage.tsx. It will be opened as a throwaway draft PR carrying ai-spend-plugin-ref: main in the body, which exercises the parser against GitHub's real body encoding at the same time, and closed without merging.

Also not run: the fork path, which needs a real fork PR. The has_token branch is a single conditional and was read rather than executed.

Documentation

None in this PR. The plugin's local check (bash scripts/dev-runner.sh --lint-fe) and the notion page it is documented on already exist; the failure annotation points developers at that command. If this becomes a required check, the ruleset entry is a repo-settings change rather than a documentation one.

---

🤖 Generated with Claude Code

[github-actions]

⏱️ pre-commit per-hook timing

Hook	Description	Result	Duration
⚙️ actionlint — github workflows	Lint GitHub Actions workflows	✅	0.55s
🌈 zizmor — github workflows security	Security-scan GitHub Actions workflows	✅	0.05s
Total (2 ran)			0.60s

⏭️ 41 skipped (no matching files changed)

Hook	Description	Result
🐍 trim trailing whitespace — python sdk	Strip trailing whitespace	⏭️
🐍 fix end of files — python sdk	Ensure files end in a newline	⏭️
🐍 ruff — python sdk	Lint + autofix Python (ruff)	⏭️
🐍 ruff-format — python sdk	Format Python code (ruff)	⏭️
🐍 mypy — python sdk	Static type check	⏭️
🤖 trim trailing whitespace — optimizer	Strip trailing whitespace	⏭️
🤖 fix end of files — optimizer	Ensure files end in a newline	⏭️
🤖 check yaml — optimizer	Validate YAML syntax	⏭️
🤖 check json — optimizer	Validate JSON syntax	⏭️
🤖 check toml — optimizer	Validate TOML syntax	⏭️
🤖 check for added large files — optimizer	Block large files (>1MB)	⏭️
🔐 detect private key — optimizer	Block committed private keys	⏭️
🤖 check for merge conflicts — optimizer	Block merge-conflict markers	⏭️
🤖 check for case conflicts — optimizer	Block case-only name clashes	⏭️
🤖 pyupgrade — optimizer	Modernize Python syntax	⏭️
🤖 ruff — optimizer	Lint + autofix Python (ruff)	⏭️
🤖 ruff-format — optimizer	Format Python code (ruff)	⏭️
🤖 mypy — optimizer	Static type check	⏭️
📓 nbstripout — optimizer notebooks	Strip notebook output	⏭️
📝 markdownlint — optimizer	Lint Markdown	⏭️
🔤 codespell — optimizer	Fix common misspellings	⏭️
📊 radon cc — optimizer	Cyclomatic-complexity gate	⏭️
📊 radon raw — optimizer	Raw size metrics gate	⏭️
📊 xenon — optimizer	Fail on complexity thresholds	⏭️
📊 lizard — optimizer	Cyclomatic-complexity gate	⏭️
🧹 vulture — optimizer	Find dead code	⏭️
🛡️ trim trailing whitespace — guardrails	Strip trailing whitespace	⏭️
🛡️ fix end of files — guardrails	Ensure files end in a newline	⏭️
🛡️ ruff — guardrails	Lint + autofix Python (ruff)	⏭️
🛡️ ruff-format — guardrails	Format Python code (ruff)	⏭️
🛡️ mypy — guardrails	Static type check	⏭️
⚓ helm-docs	Regenerate Helm chart README	⏭️
block non-public FE plugins	Block non-public FE plugins	⏭️
☕ spotless — java backend	Format Java code	⏭️
🧪 pre-commit wrapper smoke tests	Self-test the wrapper scripts	⏭️
🧪 rebaseline script tests	Self-test the changelog re-baseline script	⏭️
🌐 eslint — frontend	Lint + autofix JS/TS	⏭️
🌐 typecheck — frontend	Whole-project tsc type check	⏭️
📘 eslint — typescript sdk	Lint + autofix JS/TS	⏭️
📘 typecheck — typescript sdk	Whole-project tsc type check	⏭️
🐳 hadolint — dockerfiles	Lint Dockerfiles	⏭️

[CometActions]

No test needed here.

No product surface in this PR — only docs, CI, tests, lockfiles or other non-product files.

Run

Advisory, from the QA test radar. Nothing here blocks this PR, and anything it proposes is a draft for review.

Re-checked after a push on 20 Aug 14:02 UTC.

[liyaka]

Reviewed the diff against the surrounding CI, the frontend's actual eslint/tsconfig/dependency-cruiser config, and the job log from the run on this PR.

The design is right and the write-up is honest. pull_request rather than pull_request_target, permissions: contents: read, persist-credentials: false on both checkouts, the PAT confined to one with: block and never exported into a run:, the PR body passed through env: instead of interpolated into shell — that is the correct shape for this. I also confirmed it is not vacuously green: the run staged 143 TypeScript files, found the manifest, and depcruise cruised 1982 modules. The body parser behaves as claimed against CRLF, fenced, unclosed-fence, inline-code and injection inputs.

Three changes requested.

1. types: [opened, synchronize, reopened, edited]

The only real defect. Default pull_request types are opened, synchronize, reopened — not edited. So the ai-spend-plugin-ref: escape hatch, which this PR documents in both the body and the workflow header, silently does not work: adding the line after opening the PR does not re-trigger, and "Re-run jobs" replays the original event payload, i.e. the stale body. Whoever first needs the escape hatch will discover this by pushing an empty commit.

2. Move the staging step to after npm ci

Current order is stage → setup-node → npm ci → checks, so the private plugin source is on disk while PR-controlled postinstall (patch-package, with patches/ from the PR) runs. Reordering costs nothing and takes install-time code execution out of the exposure window. It does not stop a malicious typecheck script — that is inherent to the approach — but there is no reason to leave the easy one open.

3. Accept both quote styles in the manifest grep

grep -qE "name:[[:space:]]*['\"]ai-spend['\"]" "${manifest}"

The manifest assertion itself is worth having — I confirmed PluginsStore really does select by manifest.name against VITE_FE_PLUGINS, and no in-repo plugin has a manifest.ts, so there is no public example keeping it honest. But as written it is a textual assertion on a private repo's formatting: a prettier config change there to single quotes turns every opik frontend PR red, and the reason is invisible to most reviewers.

---

Optional, not worth another round on their own:

• The "produced nothing" annotation is unreachable for the case it names. If the plugin's src/ moves, cp -R .ai-spend-plugin/src/. fails first under set -e and you get a raw cp error rather than the annotation. The message only fires when src/ exists but holds no .ts/.tsx. Test the directory before copying.
• tr -d '[:space:]' concatenates rather than truncates: ai-spend-plugin-ref: main rebased resolves to mainrebased, passes the ref allowlist, then fails at checkout with a confusing "couldn't find ref". Take the first whitespace-delimited field instead.
• Consider making the resolved ref a ::warning when it is not main. Two things become visible on the PR that are currently only in the raw log: that the PR is green against an unmerged plugin branch (nothing enforces the plugin side landed first), and that an unbalanced fence anywhere in the body made the awk toggle swallow the rest of it and silently fall back to main.

Out of scope for this PR, but worth tracking:

• Token scope. This makes write access to a public repo reach a private one on every frontend PR — a widening from the maintainer-gated push-to-main / release / test-environment paths where the token is used today, and since pull_request runs the workflow from the PR head, an author can edit this file directly. Same exposure typescript_sdk_e2e_tests.yml already has, so it is consistent with the repo. If OPIK_PLUGIN_AI_SPEND_TOKEN is a classic PAT, swapping it for an App token scoped to contents: read on opik-plugin-ai-spend alone bounds the blast radius to "read the plugin source", which anyone who can run this check can do anyway.
• The negative control. Worth running before this is relied on. A green check is not evidence that it fails when it should, and the draft PR exercises the body parser against GitHub's real encoding at the same time.
• tsc vs vite build. The image build runs npm run build (tsc && vite build) with VITE_FE_PLUGINS=comet,ai-spend; this runs tsc only. A break that surfaces at bundle time — unresolvable runtime import, alias/rollup failure, the import.meta.glob manifest wiring, the build's 8GB heap — still lands post-merge. tsc covers both breakages actually observed, so this is a conscious-narrowing question rather than a gap to fix now.

[andriidudar]

Fixed in d498baa — all three requested changes, plus the three optional one-liners (all cheap, all correct, no reason to leave them):

1. types: [opened, synchronize, reopened, edited] — confirmed the default types don't include edited before fixing; the ai-spend-plugin-ref override in the PR body would silently not retrigger.
2. Moved setup-node/npm ci ahead of the private plugin checkout and staging — postinstall now runs with no plugin source on disk.
3. Manifest grep now accepts both quote styles.
4. Stage step checks the source directory exists before cp -R — verified empirically that a missing dir dies under set -e with a raw cp error before reaching the friendly message, exactly as you said.
5. Ref parsing takes the first whitespace-delimited token instead of stripping all whitespace — "main rebased" no longer concatenates into "mainrebased".
6. Non-main resolved ref now emits a ::warning.

Re-verified against opik-plugin-ai-spend main (disposable worktree, so as not to disturb an unrelated in-progress checkout of the plugin repo): typecheck, eslint, deps:validate all exit 0 with the plugin staged. Ref parser re-tested against all prior cases plus "main rebased". zizmor --pedantic clean.

On the three out-of-scope items — agreed these are real and worth tracking separately, not blocking this PR:

• Token scope: a GitHub App token scoped to contents:read on opik-plugin-ai-spend alone is the right follow-up: bounds the blast radius to exactly what this check itself already needs.
• Negative control: already run, twice — a scratch branch with a required prop the plugin doesn't satisfy, dispatched via workflow_dispatch, confirmed it fails with the same TS2741 shape as the actual OPIK-7793 incident, then torn down. Happy to do it again on this exact commit if useful before merge.
• tsc vs vite build: agreed this is a conscious narrowing, not a gap — tsc covers both breakages actually observed (OPIK-7793's required prop, the CodeHighlighter constant move). Adding the full build would roughly match the real image's ~8GB-heap/multi-minute cost for coverage against failure modes not yet observed in practice. Open to revisiting if a build-time-only break ever slips through this check.

🤖 Reply posted via /address-github-pr-comments

[liyaka]

Re-reviewed d498baa against the previous round. All six items are addressed, and I verified each rather than taking the commit message for it.

Item	Status
1. types: [… edited]	✅
2. Staging after npm ci	✅ setup-node/npm ci ahead of the plugin checkout
3. Manifest grep, both quote styles	✅ regex correct through YAML + bash quoting
4. Test src/ before cp -R	✅
5. First whitespace token instead of tr -d	⚠️ regression — inline below
6. ::warning on a non-main ref	✅

The negative control is no longer outstanding. Run 32234674674 on 8ba73524 failed with exactly TS2741: Property 'demoRequiredProp' is missing … required in type 'LogsTabProps' on src/plugins/ai-spend/pages/AiSpendSessionsPage.tsx, after staging 143 files and finding the manifest. That was the strongest of my out-of-scope items — the check is now demonstrated to fail when it should, in the OPIK-7793 shape, not just to pass when nothing is wrong. Thanks for running it.

One regression, from the fix to my optional item 5. It is a one-line fix; details inline on the parser.

Non-blocking, no need for another round:

• edited re-runs the full job on every title/body edit of any frontend-touching PR. cancel-in-progress bounds it, and the correctness win is worth the runner time — just noting the cost is real and new.
• "Plugin staging produced nothing" is now the title on two different conditions (missing src/, and zero .ts files). The bodies distinguish them; the titles don't.

Everything else holds. The reorder is clean, and the security shape — pull_request rather than pull_request_target, permissions: contents: read, persist-credentials: false on both checkouts, the PAT confined to one with: block, the PR body reaching the shell through env: — is unchanged from what I reviewed last time.