Make spec-detected auth methods immutable in the add flow#1188
Merged
Conversation
Two additions to the e2e setup sharing one idea: the interfaces you develop
the product with are the same ones the generated tests drive, and every run is
watchable.
Authoring loop (src/journey/, scripts/cli.ts):
- `bun run cli browse <target> <step>` drives the live web UI one step at a
time (real logged-in Chromium), each step replaying the whole flow and
printing the page's controls plus a screenshot. Steps span the browser, a
terminal (`run`), and HTTP (`request`), interleaved in one session.
- `bun run cli promote <target> "<name>"` turns the recorded journey into a
committed scenario() and runs it. One Step DSL is the source of truth for
both live execution and codegen, so the generated test drives the same
surfaces the exploration drove and cannot quietly diverge.
Cross-OS packaged desktop (setup/desktop-*, desktop-vm/, src/vm/desktop.ts):
- desktop-macos / desktop-linux / desktop-windows run the real electron-builder
bundle inside a guest, drive it over a CDP tunnel, and film the console into
runs/<target>/ alongside test.ts and step screenshots. One shared scenario and
driver; only launch and capture differ per OS:
macOS: autologin Aqua session, launchctl asuser, screencapture
linux: Xvfb + openbox, xdotool window resize, ffmpeg x11grab
windows: dockur (QEMU) interactive session, QEMU screendump
- macOS and Linux auto-provision a tart guest and build the bundle locally (the
executor binary cross-compiles via BUN_TARGET); Windows attaches to a dockur
host configured through E2E_DESKTOP_WIN_* env. The desktop targets are not in
the default test chain and skip honestly without a guest.
Also:
- Force tart SSH to password-only (PubkeyAuthentication=no, IdentitiesOnly=yes)
so a loaded SSH agent does not exhaust the guest's MaxAuthTries, an
intermittent failure the existing cli-{os} lanes also hit.
- build-sidecar keys the executable-bit chmod on the build target, not the
host, so a windows-target cross-build no longer fails looking for a unix
executor binary.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
executor-marketing | 456f125 | Commit Preview URL Branch Preview URL |
Jun 28 2026, 07:43 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
executor-cloud | 456f125 | Jun 28 2026, 07:43 PM |
Contributor
Cloudflare previewTorn down — the PR is closed. |
Greptile SummaryThis PR makes spec/probe-detected auth methods immutable in the
Confidence Score: 5/5Safe to merge; the change narrows user choice at the UI layer without touching any auth submission or connection logic. The functional change is confined to how detected auth rows are rendered — no auth submission path, storage, or connection wiring is touched. The UI correctly prevents users from editing detected rows, and the new e2e scenarios with in-PR recordings validate both the MCP and OpenAPI surfaces. The two findings are cosmetic edge cases that do not affect any current user path. packages/react/src/components/auth-method-list-editor.tsx — minor edge cases in DetectedMethodSummary for empty OAuth fields and the unguarded setRowAt. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Seeds["AuthMethodSeed[]\n(from spec/probe detection)"] -->|useAuthMethodList| Rows["AuthMethodRow[]\nseeded: true | false"]
Rows --> RenderCheck{row.seeded?}
RenderCheck -->|"true (detected)"| DetectedSummary["DetectedMethodSummary\nread-only, aria-disabled\nlock icon + kind label\nPulled from spec hint"]
RenderCheck -->|"false (user-added)"| EditableEditor["AuthTemplateEditor\nfull kind selector\neditable fields"]
DetectedSummary --> KindCheck{value.kind}
KindCheck -->|none| NoneText["No credential text"]
KindCheck -->|apikey| PlacementLines["PlacementLine[] (or null)"]
KindCheck -->|oauth| OAuthCheck{oauthMetadata}
OAuthCheck -->|discovered| DiscoveryText["Discovery text"]
OAuthCheck -->|other| SpecFields["authorizationUrl / tokenUrl / scopes"]
AddRow["addRow()"] -->|seeded: false| Rows
RemoveRow["removeRowAt(i)"] -->|removes row| Rows
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
Seeds["AuthMethodSeed[]\n(from spec/probe detection)"] -->|useAuthMethodList| Rows["AuthMethodRow[]\nseeded: true | false"]
Rows --> RenderCheck{row.seeded?}
RenderCheck -->|"true (detected)"| DetectedSummary["DetectedMethodSummary\nread-only, aria-disabled\nlock icon + kind label\nPulled from spec hint"]
RenderCheck -->|"false (user-added)"| EditableEditor["AuthTemplateEditor\nfull kind selector\neditable fields"]
DetectedSummary --> KindCheck{value.kind}
KindCheck -->|none| NoneText["No credential text"]
KindCheck -->|apikey| PlacementLines["PlacementLine[] (or null)"]
KindCheck -->|oauth| OAuthCheck{oauthMetadata}
OAuthCheck -->|discovered| DiscoveryText["Discovery text"]
OAuthCheck -->|other| SpecFields["authorizationUrl / tokenUrl / scopes"]
AddRow["addRow()"] -->|seeded: false| Rows
RemoveRow["removeRowAt(i)"] -->|removes row| Rows
Reviews (2): Last reviewed commit: "Make spec-detected auth methods immutabl..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+48
to
+51
| // Restore the unix executable bit — keyed on the TARGET, not the host. A | ||
| // windows-target cross-build (BUN_TARGET=bun-windows-x64 on macOS/linux) stages | ||
| // `executor.exe`, which needs no bit; chmod'ing a non-existent `executor` there | ||
| // would ENOENT. |
There was a problem hiding this comment.
AGENTS.md prohibits em-dashes (—) in all contexts: "prose, docs, code comments, commit messages, or PRs. Use commas, colons, parentheses, or separate sentences instead." Line 48 uses one (// Restore the unix executable bit — keyed on the TARGET), as do comments in e2e/AGENTS.md ("the real test, not a transcript of one — develop the flow"), e2e/setup/desktop-linux.globalsetup.ts ("no launchctl — just background processes"), e2e/scripts/cli.ts, and several other new files throughout this PR.
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
@executor-js/cli
@executor-js/config
@executor-js/execution
@executor-js/sdk
@executor-js/codemode-core
@executor-js/runtime-quickjs
@executor-js/plugin-file-secrets
@executor-js/plugin-graphql
@executor-js/plugin-keychain
@executor-js/plugin-mcp
@executor-js/plugin-onepassword
@executor-js/plugin-openapi
executor
commit: |
RhysSullivan
force-pushed
the
lock-detected-auth-tabs
branch
from
c9bb8ea to
b9be1ff
Compare
A method seeded from spec/probe detection rendered the full None/API key/ OAuth selector and editable fields, so a user could silently switch a detected method (e.g. a Bearer-token API) to OAuth with empty endpoints and end up with a broken integration. Detected methods now render read-only in a disabled state: a lock icon, the auth kind named explicitly (OAuth / API key / No auth, since MCP seeds a detected method as just "Detected"), the declared config shown read-only, and "Pulled from spec. Remove to override." The only action is to remove the row and add a custom one; hand-added methods stay fully editable. This lives in the shared AuthMethodListEditor, so the OpenAPI and MCP add flows both get it. Detection is keyed off an explicit row flag rather than the seed slug, since MCP seeds a detected method without one. Also fix PlacementLine: its inline-flex container trimmed the whitespace at child edges, dropping the space after "Authorization:" and the trailing space in a "Bearer " prefix and rendering "Authorization:Bearer••••••". Render it as plain inline with preserved whitespace.
RhysSullivan
force-pushed
the
lock-detected-auth-tabs
branch
from
b9be1ff to
456f125
Compare
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
1 participant
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.
Problem
In the add-integration flow, an auth method seeded from spec/probe detection rendered the full
None / API key / OAuthselector with editable fields. Nothing stopped a user from switching a detected method (say a Bearer-token API) to OAuth, which blanks the fields to emptyprovider.example.comendpoints. The result is a broken integration whose auth no longer matches what the provider actually supports. The detected type came from the spec, so it should not be a free-form choice.Change
Spec/probe-detected methods are now immutable in the add flow. They render as a disabled, read-only summary:
aria-disabled, non-selectable,cursor-not-allowed) block,The only way to change a detected method is to remove the row (the header's X) and add a custom one. Methods a user adds by hand keep the full editable selector.
This lives in the shared
AuthMethodListEditor, so the OpenAPI and MCP add flows both pick it up. Detection is keyed off an explicit per-row flag rather than the seed slug, because MCP seeds a detected method with a label but no slug.Also fixes
PlacementLine: itsinline-flexcontainer trimmed the whitespace at child edges, dropping the space afterAuthorization:and the trailing space in aBearerprefix, so the preview readAuthorization:Bearer••••••. It now renders as plain inline with preserved whitespace (Authorization: Bearer ••••••). This also corrects the same preview in the connect modal and the editable placement editor.Recordings
MCP add flow (OAuth-detected server): detected method locked and named, a hand-added method stays editable.
OpenAPI add flow (spec with both API key and OAuth): both detected methods locked and named, a hand-added method stays editable.
Tests
New selfhost browser scenario
e2e/selfhost/detected-auth-immutable-ui.test.tscovers both surfaces: it asserts detected methods are read-only, named, with no kind selector and the remove-to-override hint, and that a hand-added method exposes the full selector.