Fix the build, the 404 consent URL, and put AI/ML API first - #3
Merged
Merged
Conversation
The hand-written AimlapiProvider was replaced by the declarative provider
definition and its file deleted, but the module declaration and the
registry entry were left behind:
crates/goose/src/providers/mod.rs pub mod aimlapi;
crates/goose/src/providers/init.rs aimlapi::AimlapiProvider
registry.register::<AimlapiProvider>(false)
There is no aimlapi.rs and no aimlapi/ directory for them to resolve to,
so the crate does not compile at all. This removes the two leftovers.
The provider docs row also advertised AIMLAPI_HOST. The declarative
definition has a fixed base_url and reads only AIMLAPI_API_KEY, so the
extra parameter never did anything.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… the tests Two things, both found by actually running the flow. The consent URL 404'd. verificationBaseUrl is handed to the server, which returns it with "/agent/authorize" appended; the web app is served under an "/app/" base path, so the default produced https://aimlapi.com/agent/authorize 404 https://aimlapi.com/app/agent/authorize 200 and every sign-in died on its first step. The default now carries /app. Fixing it here rather than server-side keeps the change off the shared default that other clients already depend on. The tests in server.rs had never compiled. CallbackQuery gained a `state` field when state verification went in, and the initializer in its test module was not updated, so cargo test skipped the whole module and the two HTML-escaping tests silently never ran. Adds a tests module mirroring signup_openrouter: the challenge really is the S256 hash of the verifier, challenge and state are url-safe and unpadded, separate flows share nothing, the partner id matches the shape the gateway attributes, the API and consent hosts stay distinct, and the consent base keeps its /app so a future tidy-up of that URL fails a test instead of a user. Verified: 8 tests pass, and a live sign-in against production mints a key that goose then uses for inference with no environment variables set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fork-only placement: AI/ML API moves to the top of the setup picker and carries the same "(Recommended)" label. OpenRouter's own label is left exactly as it was, and no other entry changes. This is a deliberate choice for our build and is not meant for upstream — the menu is a hand-ordered list in someone else's repository, and nothing in the provider schema makes "recommended" a claimable flag. Keep it out of any upstream branch. The manual provider list is unaffected: it sorts by display_name, where "AI/ML API" already lands first on its own. Also replaces literal newlines inside the sign-in println! strings with escapes, so rustfmt leaves the block alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three commits, deliberately kept separate so the first two can go upstream later and the third cannot.
1.
maindoes not compilePR #2 deleted
crates/goose/src/providers/aimlapi.rswhen the hand-written provider was replaced by the declarative definition, but left both references behind:Nothing under
crates/goose/src/providers/resolves those — noaimlapi.rs, noaimlapi/directory — so the crate fails to build onmaintoday. This removes the leftovers.The provider docs row also advertised
AIMLAPI_HOST; the declarative definition has a fixedbase_urland reads onlyAIMLAPI_API_KEY, so that parameter never did anything.2. The consent URL 404'd, and the tests were never running
verificationBaseUrlis handed to the server, which returns it with/agent/authorizeappended. The web app is served under an/app/base path, so the default produced:https://aimlapi.com/agent/authorizehttps://aimlapi.com/app/agent/authorizeEvery sign-in died on its first step. Fixed on this side rather than server-side, to keep the change off the shared default other clients already depend on.
Separately, the test module in
server.rshad never compiled:CallbackQuerygained astatefield when state verification went in and the test's initializer was not updated, socargo testskipped the module entirely and the two HTML-escaping tests silently never ran.Adds a
testsmodule mirroringsignup_openrouter:part_<alnum>)/app, so a future tidy-up of that URL fails a test instead of a user3. Sign-in menu placement — fork only, do not take upstream
AI/ML API moves to the top of the setup picker with the same
(Recommended)label. OpenRouter's own label is untouched and no other entry changes.This is a deliberate choice for our build. The menu is a hand-ordered list in someone else's repository and nothing in the provider schema makes "recommended" a claimable flag. The manual provider list is unaffected — it sorts by
display_name, whereAI/ML APIalready lands first on its own merit, 1st of 46.Verification
cargo build -p goose-cli --no-default-features --features "code-mode,aws-providers,telemetry,nostr,otel,rustls-tls,system-keyring,update"— succeeds. (The defaultlocal-inferencefeature needs libclang for bindgen, which is why this tree previously would not build locally; it is unrelated to these changes.)cargo test -p goose --lib signup_aimlapi— 8 passed, including the two inserver.rsthat compile here for the first time.cargo fmt --check— clean on both edited crates.goose configure→ AI/ML API sign-in → browser consent → loopback redirect → code+verifier exchange → key stored. Then, with no environment variables set at all,goose runanswers onaimlapi anthropic/claude-sonnet-5. The declarative provider was also verified separately onopenai/gpt-5-5.