Skip to content

fix(ai-engineer): distinguish REST 5xx from invalid gh credentials #2438

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
cursor wants to merge 3 commits into
base: main
Choose a base branch
from
+52 −9
Draft

fix(ai-engineer): distinguish REST 5xx from invalid gh credentials #2438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9cde15d
chore: claim #2206
cursoragent Jul 20, 2026
2ae87d6
fix(ai-engineer): distinguish REST 5xx from invalid gh credentials
cursoragent Jul 20, 2026
bd40976
fix(ai-engineer): treat rate-limit 403 as service degradation
cursoragent Jul 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 29 additions & 1 deletion .claude/scripts/maintainer-preflight.test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,37 @@ grep -Fq 'classify the saved login as indeterminate' "${run_loop}" ||
grep -Fq 'authentication verification unavailable' "${run_loop}" ||
fail "missing the unavailable host-verification classification"

grep -Fq 'Only an explicit credential rejection from that host-level check proves the saved login invalid.' "${run_loop}" ||
grep -Fq 'Only an explicit credential rejection' "${run_loop}" ||
fail "missing the host-confirmed invalid classification"

grep -Fq 'GitHub service degraded' "${run_loop}" ||
fail "missing the REST 5xx / service-degraded classification"

grep -Fq "gh api graphql --hostname github.com -f query='{viewer{login}}'" "${run_loop}" ||
fail "missing the authenticated GraphQL viewer.login fallback pinned to github.com"

grep -Fq 'rate-limited 403/429' "${run_loop}" ||
fail "missing the rate-limit-as-service-degradation classification"

grep -Fq 'same host and credential context' "${run_loop}" ||
fail "GraphQL fallback does not preserve the failing probe's credential context"

grep -Fq 'HTTP **401**' "${run_loop}" ||
fail "missing the explicit HTTP 401 authentication-rejection criterion"

grep -Fq 'non-rate-limit' "${run_loop}" ||
fail "missing the non-rate-limit 403 credential-rejection criterion"

grep -Fq 'A REST 5xx (or' "${run_loop}" ||
fail "missing the REST-503-plus-GraphQL-success regression rule"

grep -Fq 'recommend' "${run_loop}" && grep -Fq 'gh auth login' "${run_loop}" ||
fail "missing the gh-auth-login-only-on-confirmed-rejection handoff rule"

# The handoff must be gated on confirmed rejection — not on every auth-status failure.
grep -Fq 'and **only then** recommend' "${run_loop}" ||
fail "missing the confirmed-rejection gate before recommending gh auth login"

Comment on lines +47 to +71

@coderabbitai coderabbitai Bot Aug 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add a contract assertion for the deployment-scoped fallback identity.

The test checks the GraphQL command and credential context, but it does not check which viewer.login value is accepted. It can pass while the skill rejects the documented cloud identity app/cursor. Assert the deployment-scoped identity rule and the wrong GitHub identity classification.

🧰 Tools
🪛 Shellcheck (0.11.0)

[info] 65-65: Note that A && B || C is not if-then-else. C may run when A is true.

(SC2015)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.claude/scripts/maintainer-preflight.test.sh around lines 47 - 71, Extend
the assertions in the maintainer preflight contract checks to require the
documented deployment-scoped viewer.login identity rule for app/cursor, and
verify the corresponding “wrong GitHub identity” classification. Anchor these
checks to the existing run_loop grep assertions without changing the
credential-context or rejection-gating checks.

grep -Fq 'record only these gate classifications in durable memory' "${run_loop}" ||
fail "missing the credential-safe memory rule"

Expand Down
31 changes: 23 additions & 8 deletions .claude/skills/portfolio-maintenance/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,29 @@ card.
If only the host-level saved-login check succeeds, run every subsequent `gh` command through that
approved host-level execution path.
Clearing the injected tokens does not make a sandboxed macOS Keychain readable.
Only an explicit credential rejection from that host-level check proves the saved login invalid.
If the host-level check instead authenticates a different account, hard-block as `wrong GitHub identity`
without describing the credential as invalid.
If the host-level check cannot run or fails
for a transport reason, hard-block as `authentication verification unavailable` instead of instructing
the maintainer to replace a credential that was never tested. Keep the injected-token result, saved-login
result, and `git fetch` result as separate gates, because repository reachability cannot prove GitHub API
identity (and vice versa); record only these gate classifications in durable memory, never credential output.
**Distinguish authentication rejection from GitHub service degradation** (monorepo#2206): a REST
`/user` (or `gh auth status`) probe that returns HTTP 5xx, HTML, other non-JSON service noise, **or
a rate-limited 403/429** (GitHub may return either status when the limit is exceeded — check
`x-ratelimit-*` headers or a rate-limit message body) is **not** proof the credential is bad —
classify that outcome as `GitHub service degraded` and run a bounded authenticated GraphQL
fallback against the **same host and credential context** as the failing probe:
`gh api graphql --hostname github.com -f query='{viewer{login}}'`. Prefix with
`env -u GH_TOKEN -u GITHUB_TOKEN` **only when the failing probe itself was the cleared-env
saved-login check**; otherwise keep the injected `GH_TOKEN`/`GITHUB_TOKEN` so a transient REST
failure cannot be misread as a bad keychain login. Always pass `--hostname github.com` so
`GH_HOST` cannot redirect the fallback to an unrelated enterprise host. Accept the identity when
GraphQL returns `devantler`.
Only an explicit credential rejection proves the login invalid — meaning HTTP **401**, a **non-rate-limit**
403 that is clearly a credential/permission rejection (never a rate-limit 403), or `gh` reporting
the token rejected / not logged in — and **only then** recommend `gh auth login`. A REST 5xx (or
rate-limit) with a successful GraphQL `viewer.login` must never be reported as an invalid saved
Comment on lines +60 to +75

@coderabbitai coderabbitai Bot Aug 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Apply authentication-status precedence before service-noise handling.

Line [61] classifies any HTML or non-JSON response as GitHub service degraded, without limiting that rule to non-authentication statuses. Line [72] separately says HTTP 401 and a confirmed non-rate-limit 403 prove credential rejection. State the precedence explicitly: classify 401 and confirmed non-rate-limit 403 as credential rejection first; apply the non-JSON service-degradation rule only to other statuses. Otherwise, an HTML 401 response can skip the required gh auth login recommendation.

🧰 Tools
🪛 SkillSpector (2.4.4)

[error] 40: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.

Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.

(Privilege Escalation (PE3))

[error] 49: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.

Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.

(Privilege Escalation (PE3))

[error] 59: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.

Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.

(Privilege Escalation (PE3))

[error] 69: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.

Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.

(Privilege Escalation (PE3))

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.claude/skills/portfolio-maintenance/SKILL.md around lines 60 - 75, Update
the authentication decision rules in the documented GitHub probe flow so
explicit credential rejection takes precedence: classify HTTP 401 and confirmed
non-rate-limit 403 as invalid credentials before evaluating response format.
Restrict the HTML/non-JSON service-degradation classification to other statuses,
preserving the required gh auth login recommendation for authentication
failures.

login. If the host-level check instead authenticates a different account, hard-block as
`wrong GitHub identity` without describing the credential as invalid.
If the host-level check cannot run or fails for a transport reason (and the GraphQL fallback is
likewise unreachable), hard-block as `authentication verification unavailable` instead of
instructing the maintainer to replace a credential that was never tested. Keep the injected-token
Comment on lines +70 to +80

@coderabbitai coderabbitai Bot Aug 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Make the GraphQL identity contract deployment-aware and test it.

The skill defines deployment-specific identities but hardcodes devantler for the fallback. The contract test does not detect this mismatch.

  • .claude/skills/portfolio-maintenance/SKILL.md#L70-L80: compare viewer.login with the deployment's expected identity and classify a mismatch as wrong GitHub identity.
  • .claude/scripts/maintainer-preflight.test.sh#L47-L71: add assertions for the deployment-scoped identity and wrong-identity handling.
🧰 Tools
🪛 SkillSpector (2.4.4)

[error] 40: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.

Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.

(Privilege Escalation (PE3))

[error] 49: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.

Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.

(Privilege Escalation (PE3))

[error] 59: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.

Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.

(Privilege Escalation (PE3))

[error] 69: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.

Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.

(Privilege Escalation (PE3))

📍 Affects 2 files
  • .claude/skills/portfolio-maintenance/SKILL.md#L70-L80 (this comment)
  • .claude/scripts/maintainer-preflight.test.sh#L47-L71
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.claude/skills/portfolio-maintenance/SKILL.md around lines 70 - 80, Update
the GraphQL fallback contract in .claude/skills/portfolio-maintenance/SKILL.md,
around the host-level authentication guidance at lines 70-80, to compare
viewer.login against the deployment’s expected identity rather than hardcoding
devantler; classify any mismatch as wrong GitHub identity. Extend
.claude/scripts/maintainer-preflight.test.sh lines 47-71 with assertions
covering the deployment-scoped identity and wrong-identity handling.

result, saved-login result, and `git fetch` result as separate gates, because repository
reachability cannot prove GitHub API identity (and vice versa); record only these gate classifications in durable memory, never credential output.
3. **Check the boot memory surface fits in one read — BEFORE you read it.** A boot-loaded file past the Read cap is
**truncated silently**: the run continues on a partial cursor with no signal that carry-forwards,
stand-down notes, or `HANDS-OFF` records beyond the cut are missing (the 2026-06-05 blinding;
Expand Down
Loading