Skip to content

🤖 fix: present sub-projects to the agent as regular projects#3244

Open
ammar-agent wants to merge 1 commit into
mainfrom
path-resolution-adew
Open

🤖 fix: present sub-projects to the agent as regular projects#3244
ammar-agent wants to merge 1 commit into
mainfrom
path-resolution-adew

Conversation

@ammar-agent
Copy link
Copy Markdown
Collaborator

@ammar-agent ammar-agent commented May 6, 2026

Summary

Present sub-project workspaces to the agent exactly like regular single-project workspaces rooted at the execution cwd, while still inheriting parent AGENTS.md guidance. The parent and sub-project instruction segments are concatenated parent → sub-project with lightweight path-source markers (for example # `../../AGENTS.md` and # `./AGENTS.md` ) so relative paths inside each segment stay anchored to the directory where that guidance was authored.

Background

The original path-confusion bug came from sub-project agents preferring absolute paths and seeing too much parent-repo framing. A rejected first pass made the prompt explicitly describe “the sub-project of …”; the desired behavior is the opposite: the agent should see a normal project at its cwd. The only parent context it should receive is inherited AGENTS.md content, with enough source-path provenance to interpret any relative-path guidance correctly.

Implementation

  • Keep <environment> and tool descriptions neutral for sub-project workspaces; no sub-project callout and no relative-path nudge.
  • Load parent and sub-project instruction sets from the workspace checkout (not the user's local project path), preserving the structured InstructionSet scopes added on main.
  • Add prompt-only wrappers to InstructionSet.combinedContent for sub-project context: visible H1 path headings bound scoped sections, while injected <!-- path --> comments inside ## Tool: / ## Model: sections preserve provenance after scoped extraction. The structured files[] payload stays as authored for the Instructions tab.
  • Use a fence-aware scanner for comment injection so scoped-looking headings inside fenced documentation examples are not modified; it follows CommonMark rules for closing fences with info strings and ATX headings with up to three leading spaces.
  • Align extractModelSection with extractToolSection by collecting every matching ## Model: section in source order, allowing later sub-project model guidance to layer after parent guidance.
  • Rebased onto latest origin/main, resolving the conflict with the new structured instruction-loading / Instructions-tab work.

Validation

  • bun test src/node/utils/main/markdown.test.ts src/node/services/systemMessage.test.ts
  • bun test src/node/services/streamContextBuilder.test.ts src/node/services/aiService.test.ts src/node/runtime/runtimeHelpers.test.ts src/node/services/tools/bash.test.ts
  • make static-check

Risks

  • Sub-project workspaces now inherit parent AGENTS.md; users whose parent guidance previously had no effect in sub-project workspaces will see it apply.
  • Sub-project prompt content gains small H1 path headings and scoped-section provenance comments. Non-sub-project single-project workspaces remain unwrapped.
  • extractModelSection now has multi-match semantics. This should match existing tool-section behavior, but it can affect AGENTS files with multiple matching model sections.
  • The Instructions tab continues to display raw file contents; the extra path markers are prompt-only wrappers on flattened context content.

Pains

  • The PR changed direction after an initially approved approach, requiring a squash to keep the final diff focused on the neutral-framing behavior.
  • Codex found several subtle markdown-layering bugs during review: scoped extraction losing provenance, singular model-section extraction, fenced-code corruption, nested fence handling, and indented ATX headings. Each now has a regression test.
  • Rebasing over the new Instructions-tab work required adapting the implementation from plain strings to structured InstructionSet content without leaking prompt-only markers into the panel's file display.

Generated with mux • Model: openai:gpt-5.5 • Thinking: xhigh • Cost: $44.21

@mintlify
Copy link
Copy Markdown

mintlify Bot commented May 6, 2026

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
Mux 🟢 Ready View Preview May 6, 2026, 5:24 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@ammar-agent
Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

@ammar-agent
Copy link
Copy Markdown
Collaborator Author

@codex review

Both P2s addressed in 2aadcf0:

  • Windows separatorsresolveSubProjectContext now normalizes the workspacePath to forward slashes for the suffix comparison so backslash-separated local Windows cwds still activate sub-project framing. Parent root is sliced from the original workspacePath by length so prompt strings keep native separators. Added a unit test that exercises the normalization with a synthetic C:\… cwd.
  • Worktree guardrails — Restructured buildEnvironmentContext so runtime-specific lines (gitCommonLines + the worktree isolation warning + parent-directory warning) are derived from the existing per-runtime switch first, and the sub-project branch then augments them: it overrides only the description and appends the relative-paths nudge. The worktree-specific "Do not modify or visit other worktrees" line and the parent-directory warning are preserved for sub-project worktree workspaces. Added a regression-guard test asserting both lines stay in the environment block.

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Another round soon, please!

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ammar-agent ammar-agent changed the title 🤖 fix: anchor sub-project paths in env block, bash desc, and AGENTS.md tagging 🤖 fix: present sub-projects to the agent as regular projects May 7, 2026
@ammar-agent ammar-agent force-pushed the path-resolution-adew branch from 2aadcf0 to 6150fe5 Compare May 7, 2026 15:36
@ammar-agent
Copy link
Copy Markdown
Collaborator Author

@codex review

Force-pushed a complete pivot. The previous direction (which you approved on the prior commit) made sub-projects a recognized branch with explicit "the X sub-project of Y at Z" framing and AGENTS.md tagging — the user rejected that direction in favor of having the agent see sub-projects as regular projects.

This revision:

  • Drops all SubProjectExecutionContext / resolveSubProjectContext / describeRuntimeRoot plumbing across systemMessage.ts, bash.ts, tools.ts, aiService.ts, and runtimeHelpers.ts (baseline-reverted).
  • Simplifies readSingleProjectContextInstructions to a single readInstructionSetFromRuntime(runtime, workspacePath) call. Sub-projects own their own AGENTS.md; the parent repo's AGENTS.md is not inherited. This also removes the previous doubled-path lookup that never matched anything in production.
  • Tests assert the <environment> block for a sub-project workspace is byte-equal to one for a regular single-project workspace at the same cwd, and that sub-project / "Prefer paths relative to" wording does not leak in.

Please flag anything that looks wrong with the new direction, especially around the AGENTS.md inheritance change.

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. What shall we delve into next?

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ammar-agent
Copy link
Copy Markdown
Collaborator Author

@codex review

Force-pushed an extension of the previous direction. The user wants sub-projects to look like regular projects to the agent (no "sub-project of …" framing, no relative-paths nudge in , no special bash tool description) but still wants parent AGENTS.md concatenated with the sub-project's so the agent inherits parent conventions transparently.

This revision keeps everything from the previous round (env block + bash desc neutral, all SubProjectExecutionContext plumbing reverted) and adds:

  • readSingleProjectContextInstructions derives the parent root by stripping the recorded sub-project relative segment off the cwd (forward-slash normalization for Windows).
  • Reads AGENTS.md from [parentRoot, cwd] and concatenates with \n\n — no segment headings, so the agent sees a single AGENTS.md just like a regular project's.
  • Stale-metadata fallback: if subProjectPath isn't a descendant of projectPath, or the cwd doesn't end with the expected suffix, degrade to reading just the cwd's AGENTS.md.
  • Tests cover concatenation order, only-parent / only-sub cases, no-segment-tagging contract, and the stale-metadata fallback.

Behavior delta the user should know: sub-project workspaces now actually inherit parent AGENTS.md (the previous doubled-path lookup never found it), so anyone whose parent AGENTS.md previously had no effect on sub-project workspaces will start seeing it influence agent behavior.

Please flag anything that looks wrong.

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Swish!

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ammar-agent
Copy link
Copy Markdown
Collaborator Author

@codex review

Force-pushed an extension of the previous round. The user asked: "Is it clear to the agent that the parent AGENTS.md came from the parent? E.g. a ../AGENTS.md note would probably help?"

This revision adds a small markdown-invisible HTML-comment hint before each AGENTS.md segment naming its source path relative to the cwd:

  • For a packages/api sub-project, the agent sees <!-- ../../AGENTS.md --> before the parent segment and <!-- ./AGENTS.md --> before the sub-project segment.
  • Depth of the ../ prefix tracks the actual sub-project nesting depth (single-segment sub-projects get <!-- ../AGENTS.md -->).
  • Regular non-sub-project workspaces emit no comment (one source = no ambiguity), preserving exact prompt bytes for single-project workspaces.
  • Stale-metadata fallback also emits no comment (degrades to plain cwd read).

HTML comments survive markdown rendering as no-ops but are visible to the model in the raw system prompt. We deliberately avoided the rejected v1 # Project context (root: ...) / # Sub-project context (root: ...) H1-heading framing — those revealed the sub-project structure in the visible prompt, which contradicted the "regular project" framing in <environment> and tool descriptions.

8 test cases cover: concatenation order, comment presence and ordering, depth computation across nesting levels, only-parent / only-sub fallbacks, regular-project no-op preservation, and stale-metadata degradation.

Please flag anything that looks wrong with the comment-based hint approach.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a62901b91b

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/systemMessage.ts Outdated
@ammar-agent ammar-agent force-pushed the path-resolution-adew branch from a62901b to b1e5205 Compare May 7, 2026 16:26
@ammar-agent
Copy link
Copy Markdown
Collaborator Author

@codex review

Force-pushed to address the previous round's P2 finding (good catch — scoped tool/model bodies lost the path-source marker because the top-level HTML comment didn't travel into the extracted section body).

Two changes this round:

  1. Inject the path-source comment after every ## Tool: / ## Model: heading. Now <!-- ../../AGENTS.md --> lands as the first line of the extracted bash/model body, so per-tool descriptions retain provenance even when the surrounding H1 is stripped by extraction.

  2. Switch the top-level marker from an HTML comment to an H1 heading containing the path (e.g. # `../../AGENTS.md`). The HTML comment didn't bound markdown sections, so a parent's ## Model: … section was spanning across the segment join into the sub-project's narrative — stripScopedInstructionSections then deleted the sub-project's narrative as part of the parent's model section. The H1 heading both names the segment's source AND bounds scoped sections to their own segment.

The H1 is deliberately just a path note (# `../AGENTS.md`), not the rejected v1 verbose framing (# Project context (root: …)). It's the smallest visible marker that survives markdown parsing as a heading boundary.

9 sub-project test cases now cover: byte-equal <environment>, H1 heading presence and ordering, depth tracking across nesting levels, only-parent / only-sub fallbacks, regular-project no-op preservation, scoped section provenance through extraction (the new regression guard), and stale-metadata degradation.

Please flag anything that still looks wrong.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b1e5205988

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/systemMessage.ts Outdated
@ammar-agent ammar-agent force-pushed the path-resolution-adew branch from b1e5205 to a6424bf Compare May 7, 2026 16:36
@ammar-agent
Copy link
Copy Markdown
Collaborator Author

@codex review

Force-pushed to address the previous round's P2 finding (great catch — the singular extractModelSection was dropping sub-project model overrides whenever the parent AGENTS.md had a matching ## Model: section).

Two changes this round:

  1. Aligned extractModelSection with extractToolSection — both now collect every matching scoped section in source order and concatenate. This is the principled fix because model overrides should compose just like tool overrides do, and parent → sub-project ordering naturally lets sub-project rules override parent rules via order of presentation. Removed the now-unused internal extractSectionByHeading helper.

  2. Extended the regression-guard test to also cover the model-override case: when both parent and sub-project AGENTS.md define matching ## Model: sections, the per-model section in the system message contains both bodies in parent → sub order with their respective <!-- ../../AGENTS.md --> and <!-- ./AGENTS.md --> provenance comments.

Also added 4 unit tests in markdown.test.ts > extractModelSection covering the new multi-match semantics, regex-style heading matching, the no-match null return, and skip-empty-matches behavior.

This is a small behavior change for the rare case where a single AGENTS.md has multiple matching ## Model: regex sections (previously: first match wins; now: all concatenated). I checked the repo and didn't find any such authoring patterns — the change is essentially observable only in the new sub-project + multi-project flows. Please flag if you think this needs a different signal to existing users.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a6424bf1fa

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/systemMessage.ts Outdated
@ammar-agent ammar-agent force-pushed the path-resolution-adew branch from a6424bf to e2bade5 Compare May 7, 2026 16:45
@ammar-agent
Copy link
Copy Markdown
Collaborator Author

@codex review

Force-pushed to address the previous round's P2 finding (sharp catch — the regex-based injection didn't respect fenced code blocks, so ## Tool: bash lines inside markdown documentation examples would have gotten a <!-- ../../AGENTS.md --> comment injected into them, corrupting the example).

Two changes this round:

  1. Switched tagAgentsSegment from regex String.replace to a line-by-line scan that tracks fence state. The scanner toggles openFence on ^ {0,3}(\{3,}|~{3,})lines (CommonMark fence syntax), with closing fences required to use the same character as the opener and at least as many delimiters. While inside a fence, scoped-looking heading lines do not receive injected comments. This mirrors the downstream markdown parser's fenced-content semantics —stripScopedInstructionSections/extractToolSection` correctly skip fenced content, so injecting there would only corrupt documented examples.

  2. Added a regression-guard test with an AGENTS.md that contains both a fenced markdown example and a real ## Tool: bash section. Asserts the fenced region in <custom-instructions> is uncorrupted (no <!-- between the fenced heading and the example body), the real section's body still gets the provenance comment in the per-tool extraction, and the example body does not leak into the tool extraction.

Total now: 11 sub-project test cases + 4 new extractModelSection unit tests + 21 existing buildSystemMessage tests, all green. make static-check passes.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e2bade5b4c

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/systemMessage.ts Outdated
@ammar-agent ammar-agent force-pushed the path-resolution-adew branch from e2bade5 to f48fae8 Compare May 7, 2026 16:52
@ammar-agent
Copy link
Copy Markdown
Collaborator Author

@codex review

Force-pushed to address the previous round's P2 finding (very nice catch — CommonMark §4.5 says a closing fence cannot have an info string, so ```ts inside an outer ```markdown fence does not close the outer fence; the previous scanner naively closed on any matching marker character/length).

Two changes this round:

  1. Tightened tagAgentsSegment's closing-fence detection to require an empty/whitespace-only trailing portion after the marker. The opening fence still accepts any info string. This matches markdown-it's behavior — ```ts is parsed as still-inside-the-fence content when the outer fence is open.

  2. Added a regression-guard test with an AGENTS.md that has an outer ````markdown fence containing both an inner ```ts ... ``` example AND ## Tool: foo lines after that inner fence. Asserts no provenance comment lands anywhere inside the outer fenced region (anchored between the outer ````markdown opener and its closing ````), the real ## Tool: bash section after the outer fence still carries the comment in the per-tool extraction, and example bodies don't leak into the tool extraction.

Total now: 12 sub-project test cases + 4 extractModelSection unit tests + 21 existing buildSystemMessage tests. make static-check passes.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f48fae86fa

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/systemMessage.ts Outdated
@ammar-agent
Copy link
Copy Markdown
Collaborator Author

@codex review

Force-pushed to address the previous round's P3 finding (good catch — CommonMark §4.2 lets ATX headings start with up to 3 leading spaces, and markdown-it parses those as headings, but my regex was anchored at column 0).

Two changes this round:

  1. Loosened SCOPED_HEADING_LINE_REGEX from ^#{1,6} to ^ {0,3}#{1,6} so the scanner matches the same indented-ATX cases markdown-it does. Without this, an indented ## Tool: bash heading would survive into the per-tool prompt without provenance.

  2. Added a regression-guard test with a parent AGENTS.md that uses both ## Tool: bash (2-space indent) and ## Model: … (3-space indent). Asserts both bodies appear in the per-tool / per-model extractions with their respective <!-- ../../AGENTS.md --> provenance comments preceding the rule.

Total now: 13 sub-project test cases + 4 extractModelSection unit tests + 21 existing buildSystemMessage tests. make static-check passes.

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Another round soon, please!

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Sub-project workspaces now look indistinguishable from a regular
single-project workspace rooted at the sub-project directory:

- The <environment> block uses the same per-runtime description and
  guardrails it would use for any project at that cwd. No "the X
  sub-project of the Y at Z" framing, no relative-paths preamble.
- Bash tool description is unchanged from the regular single-project
  case (same "Runs in <cwd> - no cd needed" form).
- AGENTS.md is concatenated parent → sub-project (parent first so
  general rules anchor before specific overrides). Each segment is
  wrapped with two complementary path markers:
    * A visible H1 heading naming the source path relative to the cwd
      (e.g. `` # `../../AGENTS.md` ``). The heading both names the
      segment's source AND bounds any scoped `## Tool:` / `## Model:`
      sections inside the segment so they can't span across the
      segment join into the next segment's narrative.
    * A markdown-invisible HTML comment with the same path injected
      after every ATX-style `## Tool:` / `## Model:` heading inside
      the segment. The H1 doesn't survive scoped extraction, so the
      inner comment carries the path provenance into per-tool/per-
      model contexts. Heading-line scanning is fence-aware: scoped-
      looking lines that sit inside a fenced code block (e.g. a
      `markdown` documentation example showing how to structure
      scoped sections) do NOT receive an injected comment, mirroring
      the downstream markdown parser's fenced-content semantics.
- The parent root is derived by stripping the recorded sub-project
  relative segment off the cwd, so worktree/SSH/Docker workspaces
  read the parent's AGENTS.md from their own branch's checkout (not
  from the user's local checkout, which may be on a different commit).
  Depth of the relative-path heading tracks the actual sub-project
  nesting depth.
- Stale-metadata fallback: if the recorded subProjectPath isn't a
  descendant of projectPath, or the cwd doesn't end with the expected
  suffix, we degrade to reading just the cwd's AGENTS.md verbatim
  with no path-source heading or comment — historical behavior,
  unchanged.
- Regular non-sub-project workspaces are unaffected: no path-source
  markers are emitted (one source = no ambiguity), preserving the
  exact prompt bytes for single-project workspaces.

Also align extractModelSection's multi-match semantics with
extractToolSection: collect every matching `## Model: …` section in
source order and concatenate, instead of returning only the first
match. Without this, sub-project model overrides were silently
dropped when the parent AGENTS.md also defined a matching `## Model:`
section — only the parent's body landed in the per-model section,
and the sub-project's intended override never reached the agent.
The change is principled (parallel to how tool overrides already
compose for multi-project workspaces) and removes the now-unused
internal extractSectionByHeading helper.

Replaces the earlier doubled-path lookup in
readSingleProjectContextInstructions (which always read the
sub-project's AGENTS.md as if it were the parent's, and tried to read
the sub-project's AGENTS.md from <cwd>/<rel>/<rel> which never
existed) with an inline helper that derives both the parent root and
the relative-path hint deterministically.
@ammar-agent ammar-agent force-pushed the path-resolution-adew branch from 3aadf83 to e570d57 Compare May 12, 2026 04:00
@ammar-agent
Copy link
Copy Markdown
Collaborator Author

@codex review

Rebased onto latest origin/main and resolved the structured instruction-loading conflict; local targeted tests and make static-check pass.

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant