diff --git a/cmd/gortex/instructions.go b/cmd/gortex/instructions.go index 838d305e9..e16c43fc5 100644 --- a/cmd/gortex/instructions.go +++ b/cmd/gortex/instructions.go @@ -9,9 +9,19 @@ import ( "github.com/zzet/gortex/internal/agents" "github.com/zzet/gortex/internal/agents/claudecode" + "github.com/zzet/gortex/internal/agents/codex" "github.com/zzet/gortex/internal/profiles" ) +// fileContains reports whether path exists and holds needle. Used to +// decide whether an agent's rule block is installed at all; an +// unreadable file is treated as "not installed" so a switch never +// creates a surface `gortex install` did not. +func fileContains(path, needle string) bool { + data, err := os.ReadFile(path) + return err == nil && strings.Contains(string(data), needle) +} + // instructions.go is the `gortex instructions` command tree — the CLI // front end for instruction profiles (internal/profiles): named // bundles of instructions body + MCP tool preset + skills subset + @@ -136,6 +146,19 @@ func runInstructionsSwitch(cmd *cobra.Command, args []string) error { if md, err := os.ReadFile(claudecode.UserClaudeMdPath(home)); err != nil || !strings.Contains(string(md), agents.GlobalRulesStartMarker) { cmd.Printf("Note: no Gortex rule block found in ~/.claude/CLAUDE.md — run `gortex install` to wire the @-include pointer.\n") } + + // Codex has no @-include: its rule block is a copy of the active + // profile body, so the switch has to rewrite it. Refresh only + // when a block is already installed — creation stays with + // `gortex install`, which is what decides Codex is set up at all. + if path := codex.GlobalInstructionsPath(home); fileContains(path, agents.GlobalRulesStartMarker) { + if _, err := agents.UpsertMarkedBlock(nil, path, agents.GlobalInlineBody(dir), + agents.GlobalRulesStartMarker, agents.GlobalRulesEndMarker, agents.ApplyOpts{}); err != nil { + fmt.Fprintf(cmd.ErrOrStderr(), "warning: could not refresh %s: %v\n", path, err) + } else { + cmd.Printf("Refreshed the Gortex rule block in %s.\n", path) + } + } } cmd.Printf("\nTakes effect for NEW sessions only: the instructions @-include, the MCP tools/list, and skills are all loaded at session start — running sessions keep their current surface.\n") diff --git a/docs/agents.md b/docs/agents.md index d3f598af7..9a6c0ac7f 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -24,7 +24,7 @@ commands accept `--agents=` to constrain setup and | `aider` | `.aiderignore` block, `CONVENTIONS.md` communities block | project | https://aider.chat/docs/config/aider_conf.html | | `antigravity` | `~/.gemini/antigravity/mcp_config.json` + Knowledge Item | user | https://antigravity.google/docs/mcp | | `cline` | `cline_mcp_settings.json` (per VS Code / Cursor globalStorage), `.clinerules/gortex-communities.md` | both | https://docs.cline.bot/mcp/mcp-overview | -| `codex` | `~/.codex/config.toml` (`[mcp_servers.gortex]` + `SessionStart`, `UserPromptSubmit`, Bash/Gortex-read `PreToolUse`, and Bash/`apply_patch` `PostToolUse` hooks), `AGENTS.md` communities block | both | https://developers.openai.com/codex/mcp | +| `codex` | `~/.codex/config.toml` (`[mcp_servers.gortex]` + `SessionStart`, `UserPromptSubmit`, Bash/Gortex-read `PreToolUse`, and Bash/`apply_patch` `PostToolUse` hooks), `~/.codex/AGENTS.md` rule block, repo `AGENTS.md` communities block | both | https://developers.openai.com/codex/mcp | | `continue` | `.continue/mcpServers/gortex.json`, `.continue/rules/gortex-communities.md` | project | https://docs.continue.dev/customize/deep-dives/mcp | | `cursor` | `.cursor/mcp.json` (project) or `~/.cursor/mcp.json`, `.cursor/rules/gortex-communities.mdc` | both | https://docs.cursor.com/en/context/mcp | | `gemini` | `.gemini/settings.json` or `~/.gemini/settings.json`, `GEMINI.md` communities block | both | https://geminicli.com/docs/tools/mcp-server/ | @@ -227,7 +227,27 @@ directory that exists. Auto-approval field is `alwaysAllow` (not ### codex OpenAI Codex CLI stores config in `~/.codex/config.toml`. We -upsert a `[mcp_servers.gortex]` table there. When hooks are enabled +upsert a `[mcp_servers.gortex]` table there. `gortex install` also merges the +active instruction profile into `~/.codex/AGENTS.md` between +`` / `` markers — Codex +loads that file into every session, so it is the Codex analogue of +`~/.claude/CLAUDE.md`. The body is inlined rather than `@`-included because +Codex reads AGENTS.md as literal markdown; `gortex instructions switch` +refreshes the copy in place. Skip it with `--no-claude-md`. If +`~/.codex/AGENTS.override.md` exists, Codex reads that instead and the +installer says so. + +> **Trust the hooks or they do nothing.** Codex records trust against each +> non-managed hook's current hash and **skips new or changed hooks until you +> review them**. A fresh `gortex install` therefore leaves the Gortex hooks +> configured but inert, and nothing in `config.toml` distinguishes the two +> states. Run `/hooks` inside Codex, review the gortex entries, and trust +> them — and again after any upgrade that changes a hook definition (the +> installer prints a reminder whenever it writes or changes one). Until then +> `SessionStart` never fires, which is the surface that puts the Gortex rule +> in front of the model. + +When hooks are enabled (the default), Codex receives user-level hooks. The default posture remains advisory. A team can opt into `deny`, `rewrite`, or `suppress` by setting `GORTEX_CODEX_HOOK_MODE` while running `gortex init --hooks-only`; the selected diff --git a/internal/agents/agents.go b/internal/agents/agents.go index e3562dad8..0efb29ad4 100644 --- a/internal/agents/agents.go +++ b/internal/agents/agents.go @@ -89,7 +89,8 @@ type Env struct { HookMode string // InstallGlobalInstructions toggles whether `gortex install` - // merges the rule block into ~/.claude/CLAUDE.md. Only honoured + // merges the rule block into each agent's user-level instructions + // file (~/.claude/CLAUDE.md, ~/.codex/AGENTS.md). Only honoured // in ModeGlobal; ignored elsewhere. Default true so a fresh // install delivers full enforcement; set false by --no-claude-md. InstallGlobalInstructions bool diff --git a/internal/agents/claudecode/adapter.go b/internal/agents/claudecode/adapter.go index 84e338568..6890943b1 100644 --- a/internal/agents/claudecode/adapter.go +++ b/internal/agents/claudecode/adapter.go @@ -246,7 +246,7 @@ func (a *Adapter) applyGlobal(env agents.Env, opts agents.ApplyOpts, res *agents // block, the rule only surfaces at deny-time (PreToolUse) which // is late: the agent has already wasted a turn on a forbidden // tool. - insDir := instructionsDir(env) + insDir := agents.InstructionsDir(env) if env.InstallGlobalInstructions { insAction := agents.FileAction{Path: insDir, Action: agents.ActionMerge, Keys: []string{"instruction-profiles"}} if opts.DryRun { @@ -368,7 +368,7 @@ func (a *Adapter) RemoveGlobal(env agents.Env, opts agents.ApplyOpts) (removed i // 6. Generated instruction profiles — gortex-owned generated // files (plus the tiny active-state record), safe to delete. - insDir := instructionsDir(env) + insDir := agents.InstructionsDir(env) if _, err := os.Stat(insDir); err == nil { if opts.DryRun { removed++ @@ -613,16 +613,6 @@ func installPermissions(w io.Writer, settingsPath string, opts agents.ApplyOpts) }, opts) } -// instructionsDir resolves where the generated instruction profiles -// live for this install run: the Env override (tests) or the machine -// default shared with the daemon and the `gortex instructions` verb. -func instructionsDir(env agents.Env) string { - if env.InstructionsDir != "" { - return env.InstructionsDir - } - return profiles.DefaultDir() -} - // SyncGlobalSkills reconciles ~/.claude/skills/gortex-* with the // allowed subset (nil = every shipped skill): // diff --git a/internal/agents/codex/adapter.go b/internal/agents/codex/adapter.go index c33417e4c..8f636465c 100644 --- a/internal/agents/codex/adapter.go +++ b/internal/agents/codex/adapter.go @@ -42,6 +42,16 @@ const ( const codexSessionStartMatcher = "startup|resume|clear|compact" +// codexHookTrustNotice is surfaced whenever this run wrote or changed a Codex +// lifecycle hook. Codex records trust against each non-managed hook's current +// hash and skips new or changed hooks until the user reviews them in `/hooks`, +// so a freshly installed hook set is inert — and inert silently: the config +// file looks identical whether the hooks are trusted or skipped. Since +// SessionStart is what puts the Gortex rule in front of a Codex session at +// all, an untrusted hook set reads to the user as "Gortex configured, Gortex +// ignored". Nothing on disk says so, which is why the installer has to. +const codexHookTrustNotice = "Codex skips new or changed hooks until they are trusted — run `/hooks` inside Codex, review the gortex entries, and trust them" + // v060CodexSessionStart* fingerprints the static hook shipped by gortex // v0.60.0 so an upgrade replaces it instead of installing a duplicate. The // concrete retirement gate is documented in docs/versioning.md. @@ -54,6 +64,10 @@ const ( codexPostToolUseMatcher = "^(Bash|apply_patch)$" codexHookTimeoutSeconds = 5 codexHookModeEnvVar = "GORTEX_CODEX_HOOK_MODE" + // Codex merges its home instructions file into every session ahead of + // the repo's own AGENTS.md, preferring the override name when present. + codexGlobalInstructionsFile = "AGENTS.md" + codexGlobalInstructionsOverrideFile = "AGENTS.override.md" ) type Adapter struct{} @@ -89,6 +103,12 @@ func (a *Adapter) Plan(env agents.Env) (*agents.Plan, error) { Keys: keys, }) } + if env.Mode == agents.ModeGlobal && env.InstallGlobalInstructions && env.Home != "" { + p.Files = append(p.Files, agents.FileAction{ + Path: GlobalInstructionsPath(env.Home), Action: agents.ActionWouldMerge, + Keys: []string{"gortex-rules-block"}, + }) + } if env.Mode != agents.ModeGlobal && env.SkillsRouting != "" { p.Files = append(p.Files, agents.FileAction{ Path: filepath.Join(env.Root, "AGENTS.md"), Action: agents.ActionWouldMerge, @@ -98,6 +118,49 @@ func (a *Adapter) Plan(env agents.Env) (*agents.Plan, error) { return p, nil } +// GlobalInstructionsPath is Codex's user-level instructions file. Codex +// merges it into every session ahead of the repo's own AGENTS.md, which +// makes it the Codex analogue of ~/.claude/CLAUDE.md. +func GlobalInstructionsPath(home string) string { + return filepath.Join(home, ".codex", codexGlobalInstructionsFile) +} + +// upsertGlobalInstructions writes the machine-wide Gortex rule block into +// ~/.codex/AGENTS.md. Without it a Codex session carries no standing rule: +// the MCP server's `instructions` field is not guaranteed to reach the +// model, and the lifecycle hooks only re-surface guidance when the prompt +// probe returns graph hits — so most turns arrive with nothing and the +// model falls back to shell reads and greps. Claude Code gets a thin +// @-include pointer at the active profile; Codex cannot resolve one, so +// the profile body is inlined and refreshed in place on every install and +// every `gortex instructions switch`. +func upsertGlobalInstructions(env agents.Env, opts agents.ApplyOpts) (agents.FileAction, error) { + path := GlobalInstructionsPath(env.Home) + // Codex prefers AGENTS.override.md in its home when that file exists, + // and never reads AGENTS.md alongside it. Write ours either way — the + // override is the user's file to own — but say so, otherwise the block + // lands somewhere Codex silently ignores. + if _, err := os.Stat(filepath.Join(env.Home, ".codex", codexGlobalInstructionsOverrideFile)); err == nil { + internalutil.Warnf(env.Stderr, "Codex reads %s instead of %s; copy the Gortex block there or delete the override", + codexGlobalInstructionsOverrideFile, codexGlobalInstructionsFile) + } + action, err := agents.UpsertMarkedBlock(nil, path, agents.GlobalInlineBody(agents.InstructionsDir(env)), + agents.GlobalRulesStartMarker, agents.GlobalRulesEndMarker, opts) + if err != nil { + return agents.FileAction{}, err + } + // UpsertMarkedBlock is shared with the per-repo communities block, so + // it labels every action with "communities-block". Relabel here so the + // install report distinguishes the two. + if action.Keys != nil { + action.Keys = []string{"gortex-rules-block"} + } + if !opts.DryRun && action.Action != agents.ActionSkip { + internalutil.Logf(env.Stderr, "[gortex install] wrote rule block to %s", path) + } + return action, nil +} + func (a *Adapter) Apply(env agents.Env, opts agents.ApplyOpts) (*agents.Result, error) { res := &agents.Result{Name: Name, DocsURL: DocsURL} detected, _ := a.Detect(env) @@ -112,6 +175,7 @@ func (a *Adapter) Apply(env agents.Env, opts agents.ApplyOpts) (*agents.Result, internalutil.Logf(env.Stderr, "[gortex init] setting up OpenAI Codex CLI integration...") path := filepath.Join(env.Home, ".codex", "config.toml") + hooksChanged := false action, err := agents.MergeTOML(env.Stderr, path, func(root map[string]any, _ bool) (bool, error) { changed := upsertCodexMCPServer(root, opts) if supported, detectedVersion := codexSupportsDirectToolNamespaces(); supported || codexHasDirectToolNamespaces(root) { @@ -125,6 +189,7 @@ func (a *Adapter) Apply(env agents.Env, opts agents.ApplyOpts) (*agents.Result, if env.InstallHooks { if upsertCodexHooks(root, env, opts) { changed = true + hooksChanged = true } } return changed, nil @@ -133,6 +198,20 @@ func (a *Adapter) Apply(env agents.Env, opts agents.ApplyOpts) (*agents.Result, return res, err } res.Files = append(res.Files, action) + if hooksChanged { + res.Warnings = append(res.Warnings, codexHookTrustNotice) + } + + // User-level instructions → ~/.codex/AGENTS.md. This is the surface + // that makes Codex reach for the graph tools on every turn; see + // upsertGlobalInstructions for why the hooks alone do not cover it. + if env.Mode == agents.ModeGlobal && env.InstallGlobalInstructions { + insAction, err := upsertGlobalInstructions(env, opts) + if err != nil { + return res, fmt.Errorf("codex global instructions: %w", err) + } + res.Files = append(res.Files, insAction) + } // Repo-local community routing → AGENTS.md (also read by // OpenCode; both adapters upsert the same marker-guarded block, diff --git a/internal/agents/codex/adapter_test.go b/internal/agents/codex/adapter_test.go index a8b880aab..25c8b33a8 100644 --- a/internal/agents/codex/adapter_test.go +++ b/internal/agents/codex/adapter_test.go @@ -1154,3 +1154,189 @@ func hasHookCommand(t *testing.T, cfg map[string]any, event string, command stri } return false } + +// TestCodexInstallWritesGlobalInstructions covers the surface that makes a +// Codex session reach for Gortex at all. Codex merges ~/.codex/AGENTS.md into +// every session; without a block there, the only Gortex guidance a session can +// get is the SessionStart hook — which Codex skips until the user trusts it. +func TestCodexInstallWritesGlobalInstructions(t *testing.T) { + env := codexGlobalEnv(t) + env.InstallGlobalInstructions = true + a := New() + + res, err := a.Apply(env, agents.ApplyOpts{}) + if err != nil { + t.Fatalf("apply: %v", err) + } + + path := GlobalInstructionsPath(env.Home) + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + got := string(data) + if !strings.Contains(got, agents.InstructionsSentinel) { + t.Fatalf("expected the mandatory-rule sentinel in %s:\n%s", path, got) + } + if !strings.Contains(got, agents.GlobalRulesStartMarker) || !strings.Contains(got, agents.GlobalRulesEndMarker) { + t.Fatalf("expected a marker-fenced block in %s:\n%s", path, got) + } + // Codex reads AGENTS.md as literal markdown — an @-include line is prose + // to it, so the profile body must be inlined, not pointed at. + if strings.Contains(got, "@"+filepath.Join(env.InstructionsDir, "active.md")) { + t.Fatalf("expected an inlined body, got an @-include pointer:\n%s", got) + } + if !strings.Contains(got, "explore") { + t.Fatalf("expected the active profile body to be inlined:\n%s", got) + } + + var reported bool + for _, f := range res.Files { + if f.Path == path { + reported = true + } + } + if !reported { + t.Fatalf("apply did not report %s in its file actions: %#v", path, res.Files) + } +} + +// TestCodexGlobalInstructionsIdempotent asserts a re-run leaves exactly one +// block. `gortex install` is re-run after every upgrade, so an appending +// writer would grow the file Codex loads on every session without bound. +func TestCodexGlobalInstructionsIdempotent(t *testing.T) { + env := codexGlobalEnv(t) + env.InstallGlobalInstructions = true + a := New() + + for i := 0; i < 3; i++ { + if _, err := a.Apply(env, agents.ApplyOpts{}); err != nil { + t.Fatalf("apply %d: %v", i, err) + } + } + data, err := os.ReadFile(GlobalInstructionsPath(env.Home)) + if err != nil { + t.Fatal(err) + } + if n := strings.Count(string(data), agents.GlobalRulesStartMarker); n != 1 { + t.Fatalf("expected exactly one rule block after 3 applies, got %d", n) + } +} + +// TestCodexGlobalInstructionsPreservesUserContent asserts the block merges +// into a personal AGENTS.md instead of replacing it. +func TestCodexGlobalInstructionsPreservesUserContent(t *testing.T) { + env := codexGlobalEnv(t) + env.InstallGlobalInstructions = true + path := GlobalInstructionsPath(env.Home) + if err := os.WriteFile(path, []byte("# My rules\n\nAlways run gofmt.\n"), 0o644); err != nil { + t.Fatal(err) + } + + if _, err := New().Apply(env, agents.ApplyOpts{}); err != nil { + t.Fatalf("apply: %v", err) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + got := string(data) + if !strings.Contains(got, "Always run gofmt.") { + t.Fatalf("user content was clobbered:\n%s", got) + } + if !strings.Contains(got, agents.InstructionsSentinel) { + t.Fatalf("rule block missing after merge:\n%s", got) + } +} + +// TestCodexGlobalInstructionsOptOutAndScope pins the two cases that must not +// write the file: --no-claude-md (InstallGlobalInstructions=false) and project +// mode, where `gortex init` has no business writing user-level rules. +func TestCodexGlobalInstructionsOptOutAndScope(t *testing.T) { + t.Run("opt-out", func(t *testing.T) { + env := codexGlobalEnv(t) + env.InstallGlobalInstructions = false + if _, err := New().Apply(env, agents.ApplyOpts{}); err != nil { + t.Fatalf("apply: %v", err) + } + if _, err := os.Stat(GlobalInstructionsPath(env.Home)); !os.IsNotExist(err) { + t.Fatalf("--no-claude-md should not write %s (stat err=%v)", GlobalInstructionsPath(env.Home), err) + } + }) + t.Run("project-mode", func(t *testing.T) { + env := codexGlobalEnv(t) + env.Mode = agents.ModeProject + env.InstallGlobalInstructions = true + if _, err := New().Apply(env, agents.ApplyOpts{}); err != nil { + t.Fatalf("apply: %v", err) + } + if _, err := os.Stat(GlobalInstructionsPath(env.Home)); !os.IsNotExist(err) { + t.Fatalf("project mode should not write user-level rules (stat err=%v)", err) + } + }) +} + +// TestCodexGlobalInstructionsDryRun asserts --dry-run plans the write without +// touching disk, and that Plan agrees with what Apply would do. +func TestCodexGlobalInstructionsDryRun(t *testing.T) { + env := codexGlobalEnv(t) + env.InstallGlobalInstructions = true + a := New() + + plan, err := a.Plan(env) + if err != nil { + t.Fatalf("plan: %v", err) + } + var planned bool + for _, f := range plan.Files { + if f.Path == GlobalInstructionsPath(env.Home) { + planned = true + } + } + if !planned { + t.Fatalf("plan omitted %s: %#v", GlobalInstructionsPath(env.Home), plan.Files) + } + + if _, err := a.Apply(env, agents.ApplyOpts{DryRun: true}); err != nil { + t.Fatalf("apply: %v", err) + } + if _, err := os.Stat(GlobalInstructionsPath(env.Home)); !os.IsNotExist(err) { + t.Fatalf("dry-run wrote %s (stat err=%v)", GlobalInstructionsPath(env.Home), err) + } +} + +// TestCodexHookInstallWarnsAboutTrust pins the notice that makes an otherwise +// silent failure visible: Codex hashes each non-managed hook and skips new or +// changed ones until they are trusted in `/hooks`, so writing the hook set is +// only half the job. The notice must not repeat once the hooks are unchanged. +func TestCodexHookInstallWarnsAboutTrust(t *testing.T) { + env := codexGlobalEnv(t) + a := New() + + res, err := a.Apply(env, agents.ApplyOpts{}) + if err != nil { + t.Fatalf("apply: %v", err) + } + if len(res.Warnings) == 0 { + t.Fatal("expected a hook-trust notice on first install") + } + found := false + for _, w := range res.Warnings { + if strings.Contains(w, "/hooks") { + found = true + } + } + if !found { + t.Fatalf("hook-trust notice should name /hooks: %#v", res.Warnings) + } + + // Second run changes nothing, so the trust hashes still match and there + // is nothing for the user to re-approve. + res2, err := a.Apply(env, agents.ApplyOpts{}) + if err != nil { + t.Fatalf("re-apply: %v", err) + } + if len(res2.Warnings) != 0 { + t.Fatalf("unchanged hooks should not re-warn: %#v", res2.Warnings) + } +} diff --git a/internal/agents/instructions.go b/internal/agents/instructions.go index 0683ac406..b8cb24b14 100644 --- a/internal/agents/instructions.go +++ b/internal/agents/instructions.go @@ -67,7 +67,48 @@ func GlobalPointerBody(instructionsDir string) string { return "## MANDATORY: Use Gortex MCP tools instead of Read/Grep/Glob\n\n" + "The machine-wide Gortex rules load from the active instruction profile, imported below:\n\n" + "@" + active + "\n\n" + - "Switch guidance depth with `gortex instructions switch ` (`list` shows all) — applies to NEW sessions only.\n" + switchDepthLine +} + +// switchDepthLine is the profile-discovery footer the pointer body +// carries, so a machine that switched guidance depth can always find its +// way back. The generated profile bodies end with their own switch-back +// bullet; this is for the surfaces that do not embed one. +const switchDepthLine = "Switch guidance depth with `gortex instructions switch ` (`list` shows all) — applies to NEW sessions only.\n" + +// GlobalInlineBody renders the machine-level rule block for agents whose +// instructions file is consumed as literal markdown, with no @-include +// mechanism to follow. Claude Code gets GlobalPointerBody — a thin +// pointer at /active.md, so `gortex instructions +// switch` never has to rewrite CLAUDE.md. Codex reads its AGENTS.md +// verbatim (an @path line is prose to it), so it gets a copy of the +// active profile body inlined instead. The block is marker-fenced, so +// `gortex install` and `gortex instructions switch` both refresh the +// copy in place rather than appending a second one. +// +// No switch-back footer is appended: every generated profile body +// already ends with one, and the file Codex loads on every session +// should not pay for the same line twice. +func GlobalInlineBody(instructionsDir string) string { + body := strings.TrimRight(profiles.Active(instructionsDir).Body(), "\n") + if body == "" { + // No profile row resolved (unknown name, empty table row): fall + // back to the shared agent-neutral block plus the switch footer, + // so the agent still gets the rule rather than an empty fence. + return strings.TrimRight(InstructionsBody, "\n") + "\n\n" + switchDepthLine + } + return body + "\n" +} + +// InstructionsDir resolves where the generated instruction profiles live +// for an install run: the Env override (tests pin a temp dir) or the +// machine default shared with the daemon and the `gortex instructions` +// verb. +func InstructionsDir(env Env) string { + if env.InstructionsDir != "" { + return env.InstructionsDir + } + return profiles.DefaultDir() } // InstructionsBody is the shared, agent-neutral rule block every doc-aware