Skip to content

Probe an agent's token through its person record in auth status and doctor - #785

Merged
jeremy merged 2 commits into
fix-me-agent-profilefrom
fix-agent-auth-probes
Sep 30, 2026
Merged

jeremy merged 2 commits into
fix-me-agent-profilefrom
fix-agent-auth-probes

Conversation

@jeremy

@jeremy jeremy commented Sep 22, 2026 •

Copy link
Copy Markdown
Member

Stacked on #784 (base: fix-me-agent-profile). Follow-up to the "Not changed here" note there, from this card.

What was wrong

basecamp auth status --check and doctor's "API Connectivity" check both ask /authorization.json whether the server accepts the token. Basecamp answers 401 to every agent self-token there, because the token has no identity behind it (see #784). So a working agent profile was reported as:

  • auth status --check: "valid": false
  • doctor: API Connectivity: fail — Cannot connect to Basecamp API: Authorization failed: invalid or expired token

Both are confirmed by the new tests, which fail on fix-me-agent-profile with exactly those results and a request to /authorization.json.

What changed

  • agentProfile(app) (in people.go) is the one check for "requests go out on a stored agent credential": BASECAMP_TOKEN unset and stored oauth_type is agent. me now uses it instead of its inline condition.
  • checkWithServer (auth status --check): for an agent profile, it sends the same freshly produced token to /{account}/my/profile.json instead of /authorization.json. The verdict logic after the request is unchanged, so an auth-class refusal there still reads as "rejected".
  • checkAPIConnectivity (doctor): for an agent profile, it reads the person record through the account client.
  • An agent profile with no account configured gets the usual "Account ID required" error (auth status) or a failed check with that hint (doctor), since there is nothing to probe without the account.
  • Non-agent profiles and anything under BASECAMP_TOKEN are unchanged.

Tests

agent_probe_test.go: TestAuthStatusCheckAcceptsAValidAgentToken and TestDoctorAPIConnectivityPassesForAValidAgentToken. Each stores a valid agent credential against a fake server that refuses /authorization.json. They assert the probe passes and that only /555/my/profile.json was requested.


Summary by cubic

auth status --check and doctor's API Connectivity check now validate agent tokens through the agent's person record instead of /authorization.json.

Basecamp refuses every agent self-token at /authorization.json because the token has no identity behind it, so a working agent profile was reported as "valid": false and as "Cannot connect to Basecamp API". Both checks now probe /{account}/my/profile.json for agent profiles, matching the path me takes.

  • Extracts the agent-profile detection into a shared agentProfile helper used by me, auth status --check, and doctor.
  • Agent profiles without an account configured now get the "Account ID required" error before any token is minted.
  • Non-agent profiles and BASECAMP_TOKEN usage are unchanged.
  • Adds tests asserting a valid agent token passes both checks and only hits /555/my/profile.json, plus a test for the missing-account case.

Written for commit 8424aa9. Summary will update on new commits.

Review in cubic

…octor

`auth status --check` and doctor's API Connectivity check both asked
/authorization.json whether the server accepts the token. Basecamp
refuses every agent self-token there, since it has no identity behind
it, so a working agent profile was reported as rejected ("valid": false)
and as "Cannot connect to Basecamp API".

Both now take the path `me` takes for an agent profile: its person
record in the account it is bound to. The detection moves into one
helper, agentProfile, shared by all three.
Copilot AI balanced review requested due to automatic review settings September 22, 2026 22:28
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-22T22:45:04.776591Z 8424aa9 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions github-actions Bot added commands CLI command implementations tests Tests (unit and e2e) labels Sep 22, 2026

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.

Copilot review overview

🟡 Changes recommended

Account validation ordering and authorization-header coverage must be fixed; the help text also needs updating.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity · 1 Low severity

Open (2)
What changed in this PR

Updates agent-token validation to use the account-scoped person record instead of /authorization.json.

Changes:

  • Centralizes agent-profile detection.
  • Updates auth status and doctor connectivity probes.
  • Adds regression tests for agent-token probes.
File Review
internal/​commands/​people.go Adds shared agent-profile detection.
internal/​commands/​doctor.go Probes agent connectivity through the person record.
internal/​commands/​auth.go Adds the account-scoped probe, but validates the account too late and leaves help text inaccurate.
internal/​commands/​agent_probe_test.go Covers probe paths, but does not verify the bearer token.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread internal/commands/auth.go Outdated
Comment thread internal/commands/auth.go Outdated
--check on an agent profile with no account now says so before any
token is minted, and its help names the person-record probe. The probe
tests also require the agent's own bearer token.
@jeremy

jeremy commented Sep 22, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: 8424aa918d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

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.

Copilot review overview

🟢 Approved

The reviewed changes have focused test coverage and no unresolved issues.

Review effort: Balanced
Findings: None

Resolved since last review (2)

@jeremy
jeremy added this pull request to stack #801 September 30, 2026 03:32
@jeremy
jeremy merged commit 4668434 into main Sep 30, 2026
40 checks passed
@jeremy
jeremy deleted the fix-agent-auth-probes branch September 30, 2026 03:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commands CLI command implementations tests Tests (unit and e2e)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants