Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0fcf081ff
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let resolved = if path.is_absolute() { | ||
| path.to_path_buf() | ||
| } else { | ||
| working_dir.join(path) | ||
| }; |
There was a problem hiding this comment.
Canonicalize tracked directories before boundary checks
When a tool argument contains .. (for example cat ../shared/file.md), resolve_to_parent_dir stores a path like /repo/project/../shared without normalization. The later starts_with(working_dir) checks are lexical, so these escaped paths are treated as in-project and load_hints_from_directory can read .goosehints/AGENTS.md from parent directories outside the session root, injecting unintended instructions into the system prompt.
Useful? React with 👍 / 👎.
crates/goose/src/agents/agent.rs
Outdated
| if !tools_updated { | ||
| (tools, toolshim_tools, system_prompt) = | ||
| self.prepare_tools_and_prompt(&session_config.id, &session.working_dir).await?; |
There was a problem hiding this comment.
Rebuild system prompt whenever new hints are added
If tools_updated is true and subdirectory hints are discovered in the same turn, the prompt is rebuilt before hints are inserted, then skipped afterward because of if !tools_updated. In that case the newly added hint extras are never reflected in system_prompt for subsequent turns unless some unrelated later path triggers another rebuild, so the feature silently fails in mixed "tools changed + new subdir hint" iterations.
Useful? React with 👍 / 👎.
| hints_filenames: Vec<String>, | ||
| } | ||
|
|
||
| impl SubdirectoryHintTracker { |
There was a problem hiding this comment.
can we add this to the prompt manager? right now it ends up being part of the agent struct and it feels like the agent should really not need to worry about this.
e0fcf08 to
03a4c13
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03a4c138d9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if token.starts_with('-') { | ||
| continue; | ||
| } | ||
| if token.contains(std::path::MAIN_SEPARATOR) || token.contains('.') { |
There was a problem hiding this comment.
Record plain directory tokens from shell commands
The command-path heuristic only accepts tokens containing MAIN_SEPARATOR or .; this misses common directory navigation patterns like cd nested && cat doc.md. In that flow, nested is skipped, doc.md resolves to the session root, and SubdirectoryHintTracker never queues nested, so .goosehints/AGENTS.md in that subdirectory are not loaded even though the tool operated there.
Useful? React with 👍 / 👎.
* main: (270 commits) test(acp): align provider and server test parity (#7822) fix(acp): register MCP extensions when resuming a session (#7806) fix(goose): load .gitignore in prompt_manager for hint file filtering (#7795) fix: remap max_completion_tokens to max_tokens for OpenAI-compatible providers (#7765) fix(openai): preserve Responses API tool call/output linkage (#7759) chore(deps): bump @hono/node-server from 1.19.9 to 1.19.11 in /evals/open-model-gym/mcp-harness (#7687) fix: return ContextLengthExceeded when prompt exceeds effective KV cache size (#7815) feat: MCP Roots support (#7790) fix(google): use `includeThoughts/part.thought` for thinking handling (#7593) refactor: simplify tokenizer initialization — remove unnecessary Result wrapper (#7744) Fix model selector showing wrong model in tabs (#7784) Stop collecting goosed stderr after startup (#7814) fix: avoid word splitting by space for windows shell commands (#7781) (#7810) Simplify and make it not break on linux (#7813) Add preferred microphone selection (#7805) Remove dependency on posthog-rs (#7811) feat: load hints in nested subdirs (#7772) feat(acp): add read tool and delegate filesystem I/O to ACP clients (#7668) Support secret interpolation in streamable HTTP extension URLs (#7782) More logging for command injection classifier model training (#7779) ...
fixes #5840 - a new version of #5759 that can also work when code mode is on
verified
.goosehintssays to "say bonjour at the end of responses"nested/.goosehintssays to "say adieu at the end of responses"