Skip to content

[codex] Require Tessl auth for skill review#17

Merged
fernandezbaptiste merged 1 commit into
mainfrom
fix/auth-token-input
Jun 30, 2026
Merged

[codex] Require Tessl auth for skill review#17
fernandezbaptiste merged 1 commit into
mainfrom
fix/auth-token-input

Conversation

@fernandezbaptiste

@fernandezbaptiste fernandezbaptiste commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add a tessl-token input and pass that one configured secret through setup-tessl@v2 and the review runtime.
  • Treat Tessl login/401 errors as authentication failures even when fail-threshold is 0.
  • Update README examples to show the required secret.

Why

skill review now requires Tessl authentication. Existing workflows could appear successful when review failed under fail-threshold: 0; this makes missing or invalid auth fail the check clearly.

Validation

  • bun test
  • bun run lint
  • git diff --check

Copilot AI left a comment

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.

Pull request overview

This PR updates the GitHub Action to require Tessl authentication for tessl skill review, ensuring missing/invalid auth fails the check even when fail-threshold: 0 (which should only disable score gating).

Changes:

  • Add tessl-token input and wire it through setup-tessl@v2 and runtime env vars.
  • Detect Tessl auth/login/401 failures and treat them as action failures regardless of threshold.
  • Update documentation and examples to show the required secret.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/skill-review.ts Adds auth-error detection and adjusts failure handling to avoid “passing” auth failures when threshold is 0.
src/main.ts Fails the action on detected auth failures before applying score-threshold gating.
src/main.test.ts Adds unit tests for auth error detection and threshold-0 auth failure behavior.
README.md Updates usage and inputs docs to require tessl-token.
action.yml Adds tessl-token input, upgrades to setup-tessl@v2, and passes token to setup/runtime.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/skill-review.ts
Comment on lines 138 to 149
if (exitCode !== 0) {
const error = stderr || stdout || `Process exited with code ${exitCode}`;
console.warn(
`tessl skill review failed for ${skillFilePath} (exit code ${exitCode}): ${stderr}`,
`tessl skill review failed for ${skillFilePath} (exit code ${exitCode}): ${error}`,
);
return {
skillPath: skillFilePath,
passed: threshold === 0,
passed: threshold === 0 && !isAuthErrorMessage(error),
score: -1,
output: '',
error: stderr || `Process exited with code ${exitCode}`,
error,
};
Comment thread action.yml
Comment on lines +16 to +18
tessl-token:
description: 'Tessl API token used to authenticate review requests. Store this as a GitHub secret, for example secrets.TESSL_TOKEN.'
required: false
Comment thread src/main.ts
Comment on lines +56 to +65
// 5. Auth failures are infrastructure failures, not score failures.
// `fail-threshold: 0` disables score gating only; it must not hide missing auth.
const authFailures = results.filter((r) => isAuthErrorMessage(r.error));
if (authFailures.length > 0) {
const summary = authFailures.map((r) => ` ${r.skillPath}`).join('\n');
core.setFailed(
`Tessl authentication failed for ${authFailures.length} skill(s). Configure the tessl-token input with a Tessl API token stored in a GitHub secret.\n${summary}`,
);
return;
}
@popey

popey commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Thanks for this, Bap — pulled the branch down and gave it a review. bun test (39 pass) and bun run lint are both green, and the change does what it says on the tin. A few things I'd love your thoughts on:

Comment still posts on an auth failure (main thing I'd fix before merge)

In main.ts the order is: post PR comment → detect auth failures → setFailed. So when auth fails, every changed skill becomes an error result and we render a ⚠️ Error: ✘ 401 Unauthorized block per skill on the PR before the check fails. That's a lot of noise for what's really a single repo-config problem, and it slightly contradicts the new code comment ("auth failures are infrastructure failures, not score failures").

Could we detect authFailures before the comment step and either skip commenting or post one concise "Tessl auth not configured — set the tessl-token input" notice instead of N identical stderr dumps? Keeps the actionable guidance front and centre.

Token passed via three channels — nice to confirm which is canonical

action.yml now feeds the token to setup-tessl@v2 via with.token and exports both TESSL_API_TOKEN and TESSL_TOKEN to the runtime. If setup-tessl@v2 already persists auth for the spawned tessl skill review, the two env vars might be redundant. Worth trimming to whatever the CLI actually reads, with a one-line comment so it doesn't rot. Harmless as-is, just belt-and-suspenders.

setup-tessl v1 → v2 — just double-checking v2 is released/stable and that its token input name matches, since CI here won't catch a tag that doesn't exist.

Nits, non-blocking:

  • required: false while the token is functionally required is the right call 👍 — an unset token flows through to a clear auth-failure message rather than a confusing "missing required input".
  • Heads up that this reverses the old README "No authentication required" promise, so previously-passing fail-threshold: 0 workflows will now fail until a secret is added. Expected given the PR's intent, just flagging the user-facing behaviour change.

Approvable from my side — I'd just sort the comment-on-auth-failure ordering, then happy to 🚀

@fernandezbaptiste
fernandezbaptiste marked this pull request as ready for review June 30, 2026 11:03

@popey popey left a comment

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.

Approving — re-ran bun test (39 pass) and bun run lint (clean) on the branch. The core change is right: this flips the 401-under-fail-threshold:0 case from a silent green check to a clear red failure with actionable guidance, which is exactly what we need for the accounts still on the unauthenticated setup.

✅ from me to land this now. Two notes, neither blocking the merge:

  1. Coordinate the merge with outreach. Since consumers pin @main and there are no tags, merging immediately turns affected repos' checks red until they add a TESSL_TOKEN secret + the with: tessl-token: line. I'm prepping the copy-paste snippet for sales so the merge and the "here's how to fix it" message go out together rather than leaving Adobe/Samsara staring at a red X.
  2. Comment-on-auth-failure ordering (from my earlier review) — still worth a quick follow-up so we don't post N identical 401 blocks, but don't let it hold up the urgent merge.

This is auth-only; the tessl skill reviewtessl review command migration is tracked as the separate 2–3 week piece.

@fernandezbaptiste
fernandezbaptiste merged commit c9357d3 into main Jun 30, 2026
1 check 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.

3 participants