Skip to content

feat: Add repository (project) token support OD-489 - #37

Merged
alerizzo merged 5 commits into
mainfrom
cloud-cli-add-support-for-project-tokens-od-489
Aug 11, 2026
Merged

feat: Add repository (project) token support OD-489#37
alerizzo merged 5 commits into
mainfrom
cloud-cli-add-support-for-project-tokens-od-489

Conversation

@alerizzo

Copy link
Copy Markdown
Collaborator

Summary

  • Adds --repository-token <token> (and CODACY_PROJECT_TOKEN) to every command, sent as the project-token header. CI no longer needs a personal account token that reaches every org and repo the user can see.
  • Precedence matches analysis-cli#104 exactly:--repository-token > CODACY_PROJECT_TOKEN > CODACY_API_TOKEN > stored login. An explicit flag wins outright, so a scoped run is never silently widened. Auth is now a RemoteAuth discriminated union carrying the token kind and its source, so refusals can name where the token came from.
  • Codacy honours repository tokens on only 13 operations, so commands outside that set refuse before sending anything — and before the git auto-detection line — rather than surfacing a bare Unauthorized. Verified: refusals return in ~115 ms against a black-holed base URL; supported commands still reach the network.
  • codacy repository skips its two non-whitelisted calls (listRepositoryPullRequests, listCoverageReports) and marks them in JSON as "unavailable": ["pullRequests"], keeping pullRequests an iterable [] so existing jq consumers don't break. Under an account token the payload is byte-identical to before.
  • tool, patterns, and pattern needed no logic changes — every endpoint they touch was already whitelisted.

Also fixes two pre-existing bugs found along the way:

  • codacy repository lost the entire dashboard when the pull-request lookup failed (its three sibling calls were already .catch()-guarded).
  • codacy login --token <repo-token> reported "Invalid API token. Check that it is correct and not expired" — wrong advice for a token that's fine but the wrong kind, since /user rejects repository tokens by design.

New docs: SPECS/repository-tokens.md (whitelist + per-command support matrix, re-verify on every npm run update-api) and SPECS/missing-endpoints.md (ranked gaps, as candidate follow-up tasks — listRepositoryPullRequests is the standout, being the only reason repository degrades at all).

Closes OD-489.

Test plan

  • npm test — 606 pass (40 new), including 11 refusal tests asserting the guarded service was never called
  • npm run build && npx tsc --noEmit
  • Header on the wire (verified against a local listener): CODACY_PROJECT_TOKENproject-token; CODACY_API_TOKENapi-token; never both; no empty api-token on repository-token runs
  • Fail-fast: with CODACY_API_BASE_URL=http://127.0.0.1:9 and only a repository token, each of info, repositories, ls, directories, pull-request(s), issue, findings, finding, repo --add/--remove/--follow/--unfollow/--link-standard/--unlink-standard, issues --ignore/--ignored exits 1 in well under a second
  • Account-token regression: codacy repo -o json unchanged (no unavailable key, real pullRequests); info/ls/pull-requests/issues --ignored still work
  • End-to-end: run the configure-codacy-cloud skill against a real repo with onlyCODACY_PROJECT_TOKEN set — the acceptance criterion for OD-489. Full manual script in SPECS/repository-tokens.md.

🤖 Generated with Claude Code

Connecting CI or the auto-configuration agent to Codacy required a personal
account API token that acts as the full user — every organization, every
repository, and no expiry. Repository tokens now work on the operations the
API whitelists for them, so CI can authenticate with a credential whose blast
radius is one repository.
Adds `--repository-token <token>` (and `CODACY_PROJECT_TOKEN`) on every
command, sent as the `project-token` header. Precedence matches the Codacy
Analysis CLI exactly: flag > CODACY_PROJECT_TOKEN > CODACY_API_TOKEN > stored
login. Auth is now a `RemoteAuth` discriminated union carrying both the token
kind and its source, so "exactly one token, and we know which" is a
compile-time property and refusals can name where the token came from.
Codacy honours repository tokens on only 13 operations, so commands whose
endpoints are outside that set refuse up front — before any request, and
before the git auto-detection line — instead of surfacing a bare
`Unauthorized`. `codacy repository` skips its two non-whitelisted calls and
marks them in JSON as `unavailable`, keeping `pullRequests` an iterable empty
array so existing `jq` consumers keep working; under an account token the
payload is byte-identical to before.
Also fixes two pre-existing issues found along the way: the dashboard lost
entirely when the pull-request lookup failed (its three sibling calls were
already guarded), and `login` reporting a repository token as "invalid" when
it is rejected by /user by design.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codacy-production

codacy-productionBot commented Aug 11, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics34 complexity · 210 duplication

MetricResults
Complexity34
Duplication210

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

Run reviewer

TIP This summary will be updated as you push new changes.

@codacy-productioncodacy-productionBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

The repository token support implementation is functionally complete and correctly adheres to the specified precedence model (flag > env > stored) and header usage (project-token).

However, the PR is currently 'not up to standards' according to Codacy. Key concerns include:

  • Complexity: src/commands/login.ts has grown too complex due to identity resolution and error handling. src/commands/repository-token-refusals.test.ts (complexity: 22) and src/commands/repository.ts (complexity: 10) also show high complexity scores.
  • Cloning: There is a substantial delta in code clones (+206), which should be investigated to ensure the test suites are not duplicating logic unnecessarily.
  • File Length: src/commands/pattern.test.ts has exceeded the 500-line threshold.

All functional test scenarios, including fail-fast guards and the degraded 'repository' dashboard, are reported as found.

About this PR

  • The PR introduces 206 new clones and increases complexity in core command files (notably login.ts and repository.ts). This systemic increase suggests that the logic for handling token-based branching and fail-fast guards may need more abstract, reusable patterns rather than inline condition checks.
1 comment outside of the diff
src/commands/pattern.test.ts

line 1 ⚪ LOW RISK
Consider splitting this test file into smaller, focused modules (e.g., pattern-info.test.ts for read operations and pattern-config.test.ts for modification logic) to stay within file length guidelines.

Test suggestions

  • Token precedence order (flag > project env > api env > stored login)
  • Header selection: account token uses 'api-token', repo token uses 'project-token'
  • Fail-fast guards: verify account-only commands (e.g., info, findings) exit early with descriptive messages
  • Mixed-mode 'repository' dashboard: skips PR/coverage calls but renders metrics under a repo token
  • Mixed-mode 'repository' JSON: verify 'unavailable' field and presence of empty pullRequests array
  • Import guard: 'tools --import --force' refusal when organization-level unlinking is required
  • Login/Logout warnings: verify warnings are issued only for explicit flag usage, not ambient env vars
  • Regression: Account token continues to see full dashboard and PR listings

TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback

Comment threadAGENTS.md
- account-only end to end → `resolveAccountAuth(this, "<why a repository token can't do it>")`
- fully whitelisted → `resolveAuth(this)`
- mixed → `resolveAuth(this)` plus `requireAccountToken(auth, "<operation>", "<why>")` per unsupported flag, or `fetchIfAccountToken(...)` to skip an unsupported sub-call
- **Guards must run before any request**, and before `resolveRepoArgs()` — that shells out to git and prints an auto-detection line, which is misleading ahead of a refusal.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MEDIUM RISK

Document a clear exception or escalation process alongside this absolute rule. Include criteria for when an exception might apply and who can authorize it.

See Issue in Codacy

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Fair — the rule is absolute but the exception was left implicit. Added two sub-bullets: commands whose endpoints are all whitelisted need no guard at all (tool, patterns, pattern are the live examples), and when an operation's scope is genuinely unclear the escalation is to confirm against the API owners and record it in SPECS/repository-tokens.md rather than guess a guard.

🤖 Generated by /pr-fixup command

Comment threadsrc/commands/login.ts
pass them per command with --repository-token, or set CODACY_PROJECT_TOKEN.`,
)
.action(async (options) => {
.action(async function (this: Command, options) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MEDIUM RISK

The action handler logic is doing too much. Extract the user identity resolution and the detailed 401/403 error handling into a dedicated helper function (e.g., resolveAndValidateUser) to improve maintainability and reduce cyclomatic complexity.

Try running the following prompt in your IDE agent:

Refactor the .action handler in src/commands/login.ts to extract the authentication validation and error handling (lines 78-103) into a separate helper function.

See Issue in Codacy
See Issue in Codacy

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Good catch — fixed. Extracted resolveAndValidateUser() (validation + the 401/403 branches) and acquireToken() (flag vs. interactive prompt). The action handler is now CCN 2 / 18 NLOC, down from 9 / 61; both helpers are well under the limits too.

Worth noting the CCN 9 was pre-existing on main — it surfaced here only because this PR touched that function — so this is a real cleanup rather than a regression being papered over.

🤖 Generated by /pr-fixup command

Comment threadAGENTS.md Outdated
- fully whitelisted → `resolveAuth(this)`
- mixed → `resolveAuth(this)` plus `requireAccountToken(auth, "<operation>", "<why>")` per unsupported flag, or `fetchIfAccountToken(...)` to skip an unsupported sub-call
- **Guards must run before any request**, and before `resolveRepoArgs()` — that shells out to git and prints an auto-detection line, which is misleading ahead of a refusal.
- The whitelist is hardcoded in these guards. **Re-verify it after every `npm run update-api`.**

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚪ LOW RISK

Suggestion: Split this into separate bullet points to ensure both actions are clearly visible to the reader.

Suggested change
- The whitelist is hardcoded in these guards.**Re-verify it after every `npm run update-api`.**
- The whitelist is hardcoded in these guards.
-**Re-verify it after every `npm run update-api`.**

See Issue in Codacy

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Agreed — split into two bullets.

🤖 Generated by /pr-fixup command

Comment threadAGENTS.md Outdated
- **API base URL:** `https://app.codacy.com/api/v3` (configured in `src/index.ts` via `OpenAPI.BASE`)
- **Auth mechanism:** `CODACY_API_TOKEN` environment variable, sent as `api-token` header
- **Authentication — two token kinds.** Read `SPECS/repository-tokens.md` before touching auth or adding a command.
- An **account token** (`api-token` header) reaches everything its owner can see. A **repository token** (`project-token` header) is scoped to one repository and is accepted only on a fixed whitelist of 13 operations; everywhere else Codacy rejects it as if no token had been sent.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚪ LOW RISK

Suggestion: Split this instruction into separate points for account tokens and repository tokens to improve clarity.

Suggested change
- An **account token** (`api-token` header) reaches everything its owner can see. A **repository token** (`project-token` header) is scoped to one repository and is accepted only on a fixed whitelist of 13 operations; everywhere else Codacy rejects it as if no token had been sent.
- An **account token** (`api-token` header) reaches everything its owner can see.
- A **repository token** (`project-token` header) is scoped to one repository and is accepted only on a fixed whitelist of 13 operations; everywhere else Codacy rejects it as if no token had been sent.

See Issue in Codacy

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Agreed — split into one bullet per token kind, as suggested.

🤖 Generated by /pr-fixup command

alerizzoand others added 2 commits August 11, 2026 10:57
Extract `acquireToken()` and `resolveAndValidateUser()` out of login's action
handler, which was doing token acquisition, API validation and three error
branches in one function. Brings it to CCN 2 / 18 NLOC (was 9 / 61). The
complexity predated this branch but surfaced as new because the PR touched
that function.
Split two compound bullets in AGENTS.md's authentication section, and give
the "guards must run before any request" rule the exception it was missing:
commands whose endpoints are all whitelisted need no guard, and an unclear
operation scope should be confirmed with the API owners rather than guessed.
Add .codacy/instructions/review.md with the project-specific context behind
this round's false positives — long test files are deliberate, Lizard's TS
parser merges adjacent function spans, the token whitelist mirrors a
server-side allowlist and is hardcoded on purpose, and cross-references are
often added in the same PR as their target. `.codacy/` was ignored wholesale,
so the ignore rule is narrowed to `.codacy/*` to let authored files under
`instructions/` be tracked while all local CLI state stays ignored.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The escape hatch added for the previous review round packed three actions
into one bullet, tripping the compound-instruction rule. One action per line.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@alerizzo

Copy link
Copy Markdown
CollaboratorAuthor

Review round 1 — resolved

All 4 inline comments addressed (replies inline). Codacy's new-issue count went 5 → 0:

IssueResolution
login.ts CCN 9 / 61 NLOCFixed — extracted acquireToken() + resolveAndValidateUser(); handler now CCN 2 / 18 NLOC
AGENTS.md sentence-complexityFixed — split into one bullet per token kind
AGENTS.md compound-instructionFixed — split into separate bullets
AGENTS.md escape-hatch-missingFixed — documented the exception (all-whitelisted commands need no guard) and the escalation path
AGENTS.md compound-instruction (2nd)Dismissed — false positive, see below
pattern.test.ts file-nloc 515Dismissed — long test files are deliberate here; fixtures are written out in full so each test reads standalone
AGENTS.md outdated-cross-referenceDismissed — false positive; SPECS/repository-tokens.md is added in this same PR

On the dismissed compound-instruction: it flags - **Re-verify the whitelist after every npm run update-api.** — a single action. The rule seems to count actions across the parent bullet's nested children rather than the flagged bullet, so splitting the block twice only walked the flag further down it. It also pulls against clarity_escape-hatch-missing on the same block, which asked for more detail. Contorting the doc further would make it worse to read, so it's marked false-positive with that reasoning.

Also added .codacy/instructions/review.md capturing the project-specific context behind this round's false positives, so future reviews don't repeat them (long test files are intentional; Lizard's TS parser merges adjacent function spans, inflating reported complexity; the token whitelist is hardcoded on purpose because it mirrors a server-side allowlist the client can't query). .codacy/ was gitignored wholesale, so the rule is narrowed to .codacy/* — all local CLI state stays ignored.

🤖 Generated by /pr-fixup command

Adversarial review found a real footgun: `--repository-token "$CODACY_PROJECT_TOKEN"`
with the secret unset or misspelled passed an empty string, which resolved as
"no flag" and fell through to an ambient CODACY_API_TOKEN or stored login. A
CI job asking for a repository-scoped run silently authenticated with a full
account token — the exact widening the precedence rule promises can't happen.
Verified against a local listener before and after: it sent
`api-token=<account>`; it now errors.
An explicitly-passed empty flag is now refused; empty *env vars* keep meaning
"unset" (the test config depends on that). Flag and env values are trimmed, so
whitespace-only tokens get the CLI's own error rather than a server 401.
Also from the review:
- `unavailable` in `repository --output json` now lists `coverageReports`.
Skipping that call forces `expectsCoverage` false, silently suppressing the
"missing coverage reports" state, so a repo configured for coverage but
missing reports looked identical to a healthy one. README and SPECS claimed
coverage was marked when only pull requests were.
- Hoist the shared `X-Codacy-Origin` header into `BASE_HEADERS`, so the
wholesale `OpenAPI.HEADERS` assignment can't drop anything set at startup.
- Strengthen two dashboard tests that asserted only on the token-kind branch
and would still have passed with the skip optimization removed; add the
untested `CODACY_PROJECT_TOKEN` vs stored-credentials precedence case and the
account-token JSON failure path.
- Move `CODACY_PROJECT_TOKEN` cleanup into `afterEach` so a failing assertion
can't leak the highest-precedence token into later tests.
- Document the data-dependent guard exception in AGENTS.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@alerizzo

Copy link
Copy Markdown
CollaboratorAuthor

Adversarial review round (Sonnet 5 × 3 + /code-review)

Ran four independent review passes. One real bug surfaced, plus a doc/behaviour mismatch and some test-strength gaps. All fixed in 489c132.

🔴 Empty --repository-token silently widened scope to an account token.

--repository-token "$CODACY_PROJECT_TOKEN" with the secret unset or misspelled passed an empty string, which resolved as "no flag" and fell through to an ambient CODACY_API_TOKEN or a stored login. A CI job asking for a repository-scoped run authenticated with a full account token — the exact widening the precedence rule promises cannot happen. Confirmed against a local listener, before and after:

before: --repository-token "" → sent: api-token=SECRET-ACCOUNT-TOKEN
after: --repository-token "" → Error: --repository-token was given an empty value…

An explicitly-passed empty flag is now refused. Empty env vars keep meaning "unset" (the asymmetry is deliberate — the test config depends on it). Flag and env values are trimmed, so whitespace-only tokens get the CLI's own error instead of a server 401. Controls re-verified on the wire: real token → project-token, padded → trimmed, no flag → api-token, whitespace-only env → falls through.

🟠 Coverage was skipped silently while the docs claimed it was marked. Skipping listCoverageReports forces expectsCoverage false, which suppresses the "waiting for / missing coverage reports" state — so a repo configured for coverage but missing reports rendered identically to a healthy one, with nothing in JSON to say otherwise. unavailable now lists coverageReports alongside pullRequests; README and SPECS/repository-tokens.md corrected.

🟡 Two dashboard tests gave false confidence. Mutation testing (revert the skip, re-run) proved they asserted only on the token-kind branch and passed with the optimization removed. They now assert the calls were genuinely never made. Also added the untested CODACY_PROJECT_TOKEN vs stored-credentials precedence case, the account-token JSON failure path, and moved CODACY_PROJECT_TOKEN cleanup into afterEach so a failing assertion can't leak the highest-precedence token into later tests.

Clean under adversarial scrutiny: the 13-operation whitelist (every "supported" path walked against the generated services — no missed call), guard ordering, the prsUnavailable write inside Promise.all (no race — Promise.all can't resolve until the .catch handler completes), pickDeep genuinely omitting unavailable, and optsWithGlobals behaviour verified against the installed commander source rather than assumed.

610 tests pass (+4).

🤖 Generated by /pr-fixup command

The bullet added in the previous commit ran to 41 words. Split into four
short instructions, one per point.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@alerizzo
alerizzo merged commit 402edd8 into mainAug 11, 2026
4 checks passed
@alerizzo
alerizzo deleted the cloud-cli-add-support-for-project-tokens-od-489 branch August 11, 2026 12:15
@github-actionsgithub-actionsBot mentioned this pull request Aug 11, 2026
Sign up for freeto 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

@alerizzo