Skip to content

Request: collaborator access to contribute the #126 BYOK auth-header fix (basil-k-aji-dev) #195

Closed
Closed
Request: collaborator access to contribute the #126 BYOK auth-header fix (basil-k-aji-dev)#195
Labels
contribution-requestRequest for contribution access or a scoped contribution path; requires a 48h decisionquestionUsage or project-policy question

Description

@basil-k-aji-dev
basil-k-aji-dev
opened on Sep 19, 2026

Summary

Requesting repository collaborator access for basil-k-aji-dev.

CONTRIBUTING.md accepts code and documentation contributions from collaborators only, so this issue is the route the policy points to. I have read @Frank-zhu0404's #156 and @Eswar809's #174 and @hetaoBackend's replies to both — I understand this is a separate maintainer decision, that an issue does not grant access, and that the policy stands regardless of what is in this request.

What is already prepared

One scoped fix, complete and public rather than promised:

#126 — BYOK minimax_api cannot reach a third-party OpenAI/Anthropic-compatible relay, because the key always goes out as x-api-key and parseMinimaxApiConfig accepts no way to say otherwise. @tim-korso's root-cause analysis is accurate against main (cc00e01); I worked from it and have credited it.

Branch: basil-k-aji-dev/minimax-code → fix/minimax-api-custom-auth-headers
Details and checks: #126 (comment)

The approach reuses the headers escape hatch custom_provider already has rather than adding a parallel auth vocabulary, folds headers into the connection-test fingerprint so a stale green badge cannot survive an edit, and reports header names without values in the settings view.

Checks run: tsc -p tsconfig.standalone.json --noEmit clean; vitest run packages/local-runtime-v2/src/service/model-system/ packages/config/ 488 passed, 1 skipped; new tests registered in test/vitest-suites.json per AGENTS.md; three of them fail against unpatched src/.

Not run: no third-party relay here, so the end-to-end path in #126 is unverified on my side. The capability suite has failures in 11 untouched files that reproduce identically on a clean tree, so I could not produce a green full-suite run on this machine and am not claiming one.

If the answer is no

That is a reasonable answer and needs no explanation. The branch stays public and a maintainer is welcome to take it, adapt it, or discard it, with or without credit — the same route @hetaoBackend described for #143 and #158. I would rather the bug were fixed than that I were the one to fix it.

Background

Recent merged work elsewhere, offered only as evidence of how I contribute: alibaba/open-code-review #1228 and #1232, jingyaogong/minimind #854, #855, #859, JustVugg/colibri #1549, deeplethe/utopia #786.

GitHub user: basil-k-aji-dev

Activity

  1. hetaoBackend commented on Sep 19, 2026

    @hetaoBackend
    hetaoBackend
    on Sep 19, 2026
    Collaborator

    Thanks for providing the patch and documenting the validation limits. I reviewed the fork at feaa79fca9f42c5dca4b7e676e504aa7007cd196 against current public main and opened #200, with credit to your proposal and @tim-korso's report.

    The parser/probe/fingerprint approach is useful, but the fork leaves a runtime gap: planMinimaxApiResolution does not forward the headers to actual conversations, and the Anthropic SDK still adds its default x-api-key. #200 carries the headers through both resolver paths and the SDK, preserves explicitly requested dual auth, invalidates stale connection verdicts, and exposes header names without values. The new regression suite had 7 failures before the source fix; the focused Model System suite now passes all 458 tests. All 14 pnpm verify gates passed on clean commit c2796f8 on macOS arm64 / Node 26.4.0.

    This is an open PR, not a shipped fix. Real relay and Desktop/mobile end-to-end behavior remain unverified, and the mobile model-selection part of #126 is outside this patch.

    Collaborator access remains a separate maintainer decision under CONTRIBUTING.md. This technical follow-up does not grant access or imply approval of the request.

  2. hetaoBackend
    added a commit that references this issue on Sep 19, 2026

    fix: honor MiniMax API custom authentication headers

    1ed488c

  3. basil-k-aji-dev commented on Sep 19, 2026

    @basil-k-aji-dev
    basil-k-aji-dev
    on Sep 19, 2026
    Author

    @hetaoBackend you are right about the runtime gap, and it is worth being precise about how bad it was.

    I checked my own branch rather than take it on faith:

    $ git show --stat --format="" feaa79f
 packages/config/src/byok-config.ts                                    | 15 +
 packages/config/src/config.ts                                         | 11 +
 .../model-system/catalog/config-fingerprint.ts                        |  5 +
 .../model-system/catalog/provider-views.ts                            |  5 +
 .../model-system/connectivity/provider-request-auth.test.ts           | 75 +
 .../model-system/connectivity/provider-request.ts                     | 13 +
 .../model-system/contracts.ts                                         |  2 +
 .../model-system/management/service-context.test.ts                   | 48 +
 .../model-system/management/service-context.ts                        |  2 +
 packages/local-runtime/src/config/types.ts                            |  2 +
 packages/local-runtime/src/model-provider/provider-request.ts         | 13 +
 test/vitest-suites.json                                               |  1 +
 12 files changed, 192 insertions(+)

$ git show feaa79f | grep configHeaders
(no output)

    model-resolver-byok.ts is not in that list, and configHeaders never appears. planMinimaxApiResolution returns its plan without it, while planCustomProviderResolution — in the same file, about thirty lines down — sets configHeaders: mergeProviderHeaders(...). I had read the custom-provider path closely enough to copy its config shape and still missed that the conversation path reads the headers from somewhere my patch never wrote to.

    The consequence is worse than an incomplete fix. Everything I wired up was on the probe and fingerprint side, so a user would have configured Authorization: Bearer, seen the connection test turn green, and then had real conversations fail exactly as before — with the badge now asserting the opposite. Someone would reasonably have concluded the relay was at fault. A patch that makes the diagnostic lie is worse than no patch, and my own test suite did not catch it because every test I wrote lived on the side I had changed.

    So #200 is not my patch plus a detail; the part it adds is the part that fixes the reported bug. Thank you for reviewing the fork carefully enough to find that rather than taking the checks at face value.

    Two notes that may still be useful, both small:

    • The x-api-key suppression needs to stay conditional on the header being a default. An earlier draft of mine deleted it unconditionally and discover-models.test.ts failed, because model discovery deliberately probes an unknown endpoint under both schemes. Your description of fix: honor MiniMax API custom authentication headers #200 already preserves an explicitly configured x-api-key, so this is confirmation rather than a suggestion.
    • Strings-only parsing matters for the same reason headers reach the wire verbatim: a number or nested object either throws at request time or serialises into something the endpoint never meant to receive.

    On collaborator access: understood, and no follow-up needed from me. The boundary in CONTRIBUTING.md is clear and #200 shows the report can be fixed without it. Happy for this issue to stay open for the mobile model-selection scope, which my patch did not address either.

  4. SaladDay
    added
    needs-triageAwaiting maintainer assessment
    questionUsage or project-policy question
    repository
    and removed
    repository
    on Sep 19, 2026

  5. basil-k-aji-dev commented on Sep 19, 2026

    @basil-k-aji-dev
    basil-k-aji-dev
    on Sep 19, 2026
    Author

    Closing this.

    I opened it to contribute the #126 auth-header fix. @yujiachen-y has since decided that minimax_api will not become header-configurable, because it is the official endpoint and third-party BYOK belongs in custom_provider — which already supports custom headers and Bearer auth. That is a reasonable line, and it leaves this request with nothing to enable.

    No action needed from anyone. Thanks to @hetaoBackend for the review, which found a real gap in my branch, and to @tim-korso for a report precise enough to work from.

  6. basil-k-aji-dev
    closed this as completedon Sep 19, 2026
  7. hetaoBackend
    added
    contribution-requestRequest for contribution access or a scoped contribution path; requires a 48h decision
    and removed
    needs-triageAwaiting maintainer assessment
    on Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    contribution-requestRequest for contribution access or a scoped contribution path; requires a 48h decisionquestionUsage or project-policy question

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions