Repository navigation
[SSH] Pass Codex context via developer_instructions - #7197
Conversation
| if err != nil { | ||
| return nil, fmt.Errorf("failed to encode agent context: %w", err) | ||
| } | ||
| return []string{"-c", strings.TrimSpace(string(kv))}, nil |
There was a problem hiding this comment.
Blocking: this silently drops the user's own developer_instructions.
The old ContextHomeFile path deliberately didn't clobber (// already present — don't clobber), and a test covered that. A -c override is Codex's highest-priority config layer and it replaces the key, so any developer_instructions the user set in ~/.codex/config.toml (or a project .codex/config.toml) vanishes once they launch through the shim. No error or warning. This PR also removes the "does not clobber an existing file" test without a replacement.
Repro with a real codex (0.164.0), using the exact arg this function produces (arg below):
D=$(mktemp -d); W=$(mktemp -d)
printf 'developer_instructions = "USER-OWN-INSTRUCTIONS"\n' > $D/config.toml
cd $W && git init -q
CODEX_HOME=$D codex debug prompt-input | grep -c USER-OWN-INSTRUCTIONS # 1
CODEX_HOME=$D codex -c "$(cat arg)" debug prompt-input | grep -c USER-OWN-INSTRUCTIONS # 0The second run shows only the Databricks context. Could we combine this with the user's existing value, or use a mechanism that adds rather than replaces, and bring back a test that the user's own instructions survive?
There was a problem hiding this comment.
@anton-107 This was intentional (for now at least) since I don't want to reimplement Codex's config resolution in order to read the existing value (if it exists).
I thought this was a reasonable trade off since these are agents running in a managed setting (ug configured in the SSH tunnel) and users can use AGENTS.md (global or project scope) to add additional context for their projects.
If this ends up being an issue in practice we can always add the config resolution to merge our instructions with the user's
There was a problem hiding this comment.
Fair, I'm withdrawing "blocking". Merging would mean reimplementing Codex's layered config resolution, and AGENTS.md still gets loaded alongside the override (I checked), so users have a way to add context. The old path also dropped our context completely whenever the user had their own AGENTS.md, so this is a net improvement. One optional nit: a line in the codexContextArg comment saying the override replaces any user-set developer_instructions, and why that's acceptable, so nobody "fixes" it later.
Integration test reportCommit: 9f28a77
Top 6 slowest tests (at least 2 minutes):
|
rugpanov
left a comment
rugpanov
left a comment
There was a problem hiding this comment.
[nice to have] This PR changes how the released Codex shim gets its context, so users can see the change. Codex now gets the context even when the user has their own ~/.codex/AGENTS.md. The rule in .agents/rules/changelog.md asks for a fragment for each user-visible change. Add a file under .nextchanges/cli/, like ssh-agents-dedicated.md.
| return []string{"--append-system-prompt-file", f}, nil | ||
| } | ||
|
|
||
| func codexContextArg(home, systemContext string) ([]string, error) { |
There was a problem hiding this comment.
[blocking] ug codex -- -c developer_instructions=... <args> always gives ug two or more passthrough args. At the pinned ugCommit, _smart_routing_launch_shape in src/ucode/cli.py treats a Codex launch as interactive only when it has no args or one explicit prompt. So _disable_smart_routing_for_subcommand turns off smart routing for every Codex session that starts through the shim, also for workspaces that turned it on. The tool_args[:1] == ["update"] checks in src/ucode/agents/codex.py also no longer match codex update. Agree with the Unity Gateway team on a supported way to add developer instructions, or make ug skip leading -c overrides when it picks the launch shape. Then add a Codex case to TestPrepareAgentLaunch.
There was a problem hiding this comment.
At the pinned ugCommit, _smart_routing_launch_shape in src/ucode/cli.py treats a Codex launch as interactive only when it has no args or one explicit prompt. So _disable_smart_routing_for_subcommand turns off smart routing for every Codex session that starts through the shim, also for workspaces that turned it on
Yeah this is a bad heuristic. There are plenty of flags a normal user would pass to an interactive session, and with this they are opted out of smart routing.
The tool_args[:1] == ["update"] checks in src/ucode/agents/codex.py also no longer match codex update
Will fix 👍
Integration test reportCommit: 1d80643
Top 50 slowest tests (at least 2 minutes):
|
Changes
Use
-c developer_instructions="..."instead ofAGENTS.mdto pass system context to CodexWhy
This avoids issues with stale/user defined
AGENT.mdfilesTests
tests updated