Repository navigation
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55bb886253
ℹ️ 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".
auth agent connect refused every machine output mode, because the link, the code and the wait line all went to stdout. Whatever drives the ceremony for a person (a one-click Connect, a sandbox install, an agent relaying it) had to scrape prose to learn the link and the code. Under --json (and --agent) stdout now carries two JSON values: a verification line, written as soon as Basecamp answers the intake (link, user code, expiry), then the result envelope (profile, account, scope, client id). The person's half moves to stderr and the browser opens as it would. Neither stream carries the client secret or the device code. --jq, --quiet, --ids-only and --count are still refused: none of them can carry the verification line.
…ut to its end The line's link was the raw URL the server named, which url.Parse lets carry Unicode C1 controls, and the JSON encoder writes those through as the bytes a terminal acts on. It is now the stripped copy the terminal shows, as the code already was; the browser is still sent to the URL itself. The test read stdout while More(), which at the top level stops quietly at a stray closing delimiter. It now decodes to the end.
55bb886 to
1eb0a25
Compare
|
@codex review |
|
@cubic-dev-ai review |
@jeremy I have started the AI code review. It will take a few minutes to complete. |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
Under --json the line is how the link and the code reach whoever shows them to the person. A stdout that refused it was ignored, and the ceremony went on to open the browser, poll and store a credential that nobody reading the data had been asked to approve. The intake callback can now fail, and a failed write ends the ceremony before anything waits. The test that reads stdout at the poll now hands what it saw across the server's goroutine under a lock.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f01878a93
ℹ️ 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".
|
@codex review |
|
@cubic-dev-ai review |
@jeremy I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f01878a93
ℹ️ 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".
The expiry handed to OnIntake, the one the operator is told and the one the poll stops at were each taken from the clock separately, so a slow callback left the poll running past what the verification line said, or opening the browser on a code already dead. ConnectAgent now fixes one deadline as the intake answers and everything counts down to it; a callback that outlived the code ends the ceremony as expired.
… nothing The help called the second value the result envelope in every mode, but --agent prints the data alone. The output-mode refusals claimed to stop before the server was asked anything while only mints were counted; the intake is counted now too.
|
@codex review |
|
@cubic-dev-ai review |
@jeremy I have started the AI code review. It will take a few minutes to complete. |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Rebased onto main (943dbb5) with no conflicts. The earlier CI failure on 55bb886 was Review threads: 11 resolved (9 fixed, 2 declined), 1 open for a decision.
Declined:
Open for a decision:
Codex reported on 3a4a795 with no major issues, and cubic found no issues. Copilot can't review because the requester's quota is exhausted. |
The skill ran auth agent connect plainly and scraped the terminal for the link and the code, and its first-time-setup eval forbade --json on the grounds that the connection refuses it, which this branch makes untrue. The skill now runs it with --json and relays the verification line's link, code and expiry, which are already stripped of control characters; auth login and profile create still refuse --json and are run plainly. The eval requires --json, mocks the two JSON values the command writes, and rejects only the output modes the connection still refuses. --agent is no longer rejected: the CLI takes it, and those rules reject only what it refuses.
|
@codex review |
|
@cubic-dev-ai review |
@jeremy I have started the AI code review. It will take a few minutes to complete. |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
The open decision is settled. Jeremy decided the skill thread: the Review threads: 12 resolved (10 fixed, 2 declined), 0 open. The declines are listed in the summary above. The full |
Builds the machine-readable connection ceremony that the "Bring your agents to Basecamp" cards keep waiting on. CLI additions every GUI path needs lists it ("
basecamp auth agent connectrefuses--jsontoday"), and the one-click Connect on My > Agent (D6) depends on it. The Distribution doc gives the shape: "Emit the verification URL, the code and the expiry as a first line, then the result."Problem
auth agent connectrefuses every machine output mode, because the link, the code and the wait line all go to stdout. Anything that drives the ceremony for a person has to scrape prose to find the link and the code. That covers a Connect button, the agent sandbox's install, or a coding agent relaying the ceremony.Change
Under
--json, stdout carries two JSON values.{"type":"verification","verification_uri":"https://…/oauth/agent_connection_verification?user_code=WDJB-MJHT","user_code":"WDJB-MJHT","expires_at":"2026-10-06T18:10:00Z","expires_in":600}{"ok":true,"data":{"profile":"agent","account_id":"999","base_url":"…","source":"agent_connection","oauth_type":"agent","scope":"full","client_id":"bc-agent-42","profile_created":true,"default":true},"summary":"Connected profile \"agent\" to a Basecamp agent"}Other behavior:
--json.--agentis accepted the same way. It prints the result's data alone, after the same line.--jq(it filters one envelope, and this writes two values), and--quiet,--ids-onlyand--count, which would throw the verification line away.BASECAMP_NONINTERACTIVEstill refuses the ceremony, because approval still needs a person.connect setup's use of the ceremony is untouched.The ceremony gets an
OnIntakehook inauth.AgentConnectOptions. It is called once, after the intake has been validated and before the browser opens, with only the link, the code and the expiry.Who uses it
basecamp auth agent connect -P agent --software-name agent-sandbox --json, shows the link and code, and reads the result without scraping.Tests
TestAuthAgentConnectJSONWritesTheVerificationThenTheResult. The mock server snapshots stdout at the poll, which proves the verification line is out, newline-terminated, before approval. The test also checks the result envelope, that stderr carries the code, and that neither stream carries the secret or the device code.TestAuthAgentConnectJSONOnADeclineWritesOnlyTheVerification: a decline leaves only the verification line, mints nothing and stores nothing.TestAuthAgentConnectRefusesOutputModesThatCannotCarryIt:--jq,--quiet,--ids-onlyand--count. This replaces the old blanket refusal test. It is a regression guard and passes on main too.The first two fail on main.
bin/ciis green on linux.Summary by cubic
basecamp auth agent connectnow reads the connection ceremony as data when run with--json(or--agent), so a Connect button, the agent sandbox's install, or a coding agent can drive it without scraping prose.--json.--agentprints the result's data alone after the same verification line.--jq,--quiet,--ids-only, and--countare still refused, since none can carry the verification line.BASECAMP_NONINTERACTIVEstill refuses the ceremony, andconnect setupis unchanged.--jsonand relays the verification line's link, code, and expiry;auth loginandprofile createstill refuse--jsonand are run plainly.Written for commit d6a6dfe. Summary will update on new commits.