-
Notifications
You must be signed in to change notification settings - Fork 36
codex: allow-list --profile by subcommand; omit it where codex rejects it #184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -391,12 +391,84 @@ def _gpt_version_key(entry: tuple[str, tuple[int, int | None, int | None, str]]) | |
| return max(parsed, key=_gpt_version_key)[0] | ||
|
|
||
|
|
||
| # codex global options that consume the following token as their value. Used to | ||
| # skip past a leading global (e.g. ``--model X``) when locating the subcommand, | ||
| # so it isn't mistaken for the subcommand itself. Boolean flags take no value; | ||
| # ``--flag=value`` forms are self-contained and handled inline. | ||
| _CODEX_GLOBAL_VALUE_FLAGS = frozenset( | ||
| { | ||
| "-c", | ||
| "--config", | ||
| "-i", | ||
| "--image", | ||
| "-m", | ||
| "--model", | ||
| "-p", | ||
| "--profile", | ||
| "-s", | ||
| "--sandbox", | ||
| "-C", | ||
| "--cd", | ||
| "-a", | ||
| "--ask-for-approval", | ||
| "--enable", | ||
| "--disable", | ||
| "--remote", | ||
| "--remote-auth-token-env", | ||
| "--local-provider", | ||
| "--add-dir", | ||
| } | ||
| ) | ||
|
|
||
|
|
||
| def _codex_subcommand(tool_args: list[str]) -> str | None: | ||
| """Return the codex subcommand in *tool_args*, skipping global options. | ||
|
|
||
| ``None`` when there is no subcommand (the interactive TUI). | ||
| """ | ||
| i = 0 | ||
| while i < len(tool_args): | ||
| arg = tool_args[i] | ||
| if not arg.startswith("-"): | ||
| return arg | ||
| # A bare value flag (``--model X``) consumes the next token too; a | ||
| # ``--flag=value`` form or a boolean flag consumes only itself. | ||
| if "=" not in arg and arg in _CODEX_GLOBAL_VALUE_FLAGS: | ||
| i += 1 | ||
| i += 1 | ||
| return None | ||
|
|
||
|
|
||
| # codex accepts the global --profile ONLY on these runtime subcommands (plus the | ||
| # bare interactive TUI). Server-family subcommands (app-server, mcp-server, | ||
| # exec-server, remote-control) and utilities reject it: "--profile only applies | ||
| # to runtime commands and codex mcp". Verified against codex 0.137 + 0.141. | ||
| # ALLOW-LIST (not deny-list) so a future codex server subcommand defaults to the | ||
| # safe no-profile path instead of breaking. | ||
| _CODEX_PROFILE_SUBCOMMANDS = frozenset( | ||
| {"exec", "review", "resume", "archive", "delete", "unarchive", "fork", "mcp", "sandbox"} | ||
| ) | ||
|
Comment on lines
+448
to
+450
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will we also need to maintain this list ? it also will depend on the cli version of codex that the customer is on
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, I can't see how we can get around this. but problem statement is ucode unconditionally adds --profile to all codex commands when codex doesn't support --profile on all subcommands. do you have any ideas? |
||
|
|
||
|
|
||
| def _codex_accepts_profile(tool_args: list[str]) -> bool: | ||
| sub = _codex_subcommand(tool_args) | ||
| return sub is None or sub in _CODEX_PROFILE_SUBCOMMANDS | ||
|
|
||
|
|
||
| def launch(state: dict, tool_args: list[str]) -> None: | ||
| binary = SPEC["binary"] | ||
| workspace = state.get("workspace") | ||
| if workspace: | ||
| os.environ["OAUTH_TOKEN"] = get_databricks_token(workspace, state.get("profile")) | ||
| exec_or_spawn([binary, "--profile", CODEX_PROFILE_NAME, *tool_args]) | ||
| if _codex_accepts_profile(tool_args): | ||
| exec_or_spawn([binary, "--profile", CODEX_PROFILE_NAME, *tool_args]) | ||
| else: | ||
| # codex rejects the global --profile on these subcommands (app-server, | ||
| # mcp-server, exec-server, remote-control, ...). They are caller- | ||
| # configured — e.g. omnigent runs `codex app-server` with its own | ||
| # CODEX_HOME + config — so ucode simply omits --profile and stays out of | ||
| # the way instead of injecting a named profile codex won't accept. | ||
| exec_or_spawn([binary, *tool_args]) | ||
|
|
||
|
|
||
| def validate_cmd(binary: str) -> list[str]: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will we need to maintain this list whenever codex updates their cli ?