Skip to content

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

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

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

Conversation

@ammar-agent
Copy link
Copy Markdown
Collaborator

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

Summary

Sub-project workspaces previously gave the agent a deep absolute cwd, called the sub-project directory "a git repository clone" (it isn't — the parent is), and concatenated parent + sub-project AGENTS.md segments via a doubled-path lookup that effectively only ever returned the sub-project's AGENTS.md. The agent had no anchor for relative-path usage and tended to repeat the absolute cwd prefix in tool calls. This PR makes sub-project workspaces appear to the agent as regular single-project workspaces rooted at the sub-project directory — same <environment> framing, same bash tool description — while still concatenating parent + sub-project AGENTS.md, with a small H1 path heading at the start of each segment + an inner HTML comment after each scoped section so the agent can disambiguate which root path references resolve against, even for guidance authored under ## Tool: / ## Model: headings. As a side fix it also aligns extractModelSection with extractToolSection's multi-match semantics so sub-project (and multi-project) model overrides actually compose with parent guidance instead of being silently dropped.

Background

Reported as: "When using sub-projects, the agent seems to prefer using the full path of the repo instead of relative paths." An earlier iteration of this PR threaded a SubProjectExecutionContext through prompts and tool descriptions to call out "the packages/api sub-project of the git repository clone at /workspace" with an explicit relative-paths nudge. The user pushed back on that direction — preferring the agent see sub-projects as regular projects instead of as a special branch — but also wanted AGENTS.md concatenation with the parent preserved, plus a ../AGENTS.md-style note so the agent can tell which root each segment was authored against. This revision implements that: neutral framing in prompts and tool descriptions, parent + sub-project AGENTS.md concatenated under the hood, each segment opening with a lightweight path-as-heading note, plus an inner HTML comment that travels into per-tool/per-model extraction.

Implementation

  • buildEnvironmentContext — no subProject parameter, no sub-project branch. The cwd (already pointed at the sub-project directory by resolveWorkspaceExecutionPath) is presented through the same per-runtime description and guardrail lines as a regular project at that path.
  • readSingleProjectContextInstructions — derives the parent root and a relative-path hint via deriveSubProjectLayout, which strips the recorded sub-project relative segment off the cwd and computes how many ../ levels separate the cwd from the parent root. Reads AGENTS.md from [parentRoot, cwd], wraps each non-empty segment with tagAgentsSegment, and concatenates the wrapped segments with \n\n.
  • tagAgentsSegment — wraps a segment with two complementary path markers:
    • A visible H1 heading like # `../../AGENTS.md` (or # `./AGENTS.md` for the sub-project segment) at the top. The H1 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. Without this bound, a parent's ## Model: section would extend across the segment join and stripScopedInstructionSections would delete the sub-project's narrative along with it.
    • A markdown-invisible HTML comment <!-- ../../AGENTS.md --> injected right after every ATX ## Tool: / ## Model: heading inside the segment. The H1 above doesn't survive scoped extraction (extractToolSection returns the section body, not the surrounding H1), 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. an AGENTS.md authored with a markdown example showing how to structure scoped sections) do NOT receive an injected comment, matching the downstream markdown parser's fenced-content semantics.
    • The visible H1 is deliberately just a path note (# `../../AGENTS.md`), not the v1 framing # Project context (root: …). It tells the agent which directory the segment was authored against without dressing the sub-project up as a special structural feature.
  • extractModelSection (markdown.ts) — aligned with extractToolSection's multi-match semantics: collects every matching ## Model: … section in source order and concatenates them, instead of returning only the first match. Codex caught that the singular behavior silently dropped sub-project model overrides whenever the parent AGENTS.md also defined a matching ## Model: section — only the parent's body landed in the per-model section, never the sub-project's intended override. The new behavior parallels how tool overrides already compose for multi-project workspaces. The unused internal extractSectionByHeading helper is removed.
  • Boundary properties:
    • Parent's AGENTS.md is read from the workspace's own checkout (derived from workspacePath), not from metadata.projectPath, so worktree/SSH/Docker workspaces see parent guidance from their own branch rather than the user's local checkout.
    • Stale-metadata fallback: if metadata.subProjectPath doesn't sit cleanly under metadata.projectPath, or the cwd doesn't end with the expected suffix, the helper returns null and we degrade to reading just the cwd's AGENTS.md with no path-source markers.
    • Regular non-sub-project workspaces emit no path-source markers (one source = no ambiguity), preserving exact prompt bytes for single-project workspaces.
    • Replaces the earlier doubled-path lookup that 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> (a path that never existed).
  • buildBashToolDescription / ToolConfiguration / aiService.ts / runtimeHelpers.ts — reverted to baseline. No SubProjectExecutionContext, resolveSubProjectContext, or describeRuntimeRoot plumbing anywhere.

Sub-project plumbing in the runtime layer is unchanged: resolveWorkspaceExecutionPath still appends the sub-project relative path so the agent's cwd is the sub-project directory. The pivot is purely about what the agent sees in prompts.

Validation

  • make static-check — green (lint, typecheck, prettier, doc snapshot, code-to-docs links).
  • bun test src/node/utils/main/markdown.test.ts src/node/services/systemMessage.test.ts — 47/47 pass, including 11 sub-project cases and 4 new extractModelSection unit tests.
  • 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 — 152/152 pass.

New / updated test cases in systemMessage.test.ts > sub-project workspaces look like regular projects:

  • <environment> for a sub-project workspace is byte-equal to one produced for a regular single-project workspace at the same cwd.
  • <environment> does not contain the strings sub-project or Prefer paths relative to.
  • AGENTS.md is concatenated parent → sub-project with # `../../AGENTS.md` and # `./AGENTS.md` H1 headings, each preceding its corresponding content; no # Project context (root: …) / # Sub-project context (root: …) verbose headings.
  • Depth tracks nesting: a single-segment api sub-project produces # `../AGENTS.md`, not # `../../AGENTS.md`.
  • Only-parent and only-sub-project AGENTS.md cases each load the available file alone with the matching path-source heading.
  • Regular non-sub-project workspaces emit no path-source markers.
  • Scoped section provenance (Codex regression guard Better authentication UX #1): when both parent and sub-project AGENTS.md define matching ## Tool: and ## Model: sections, the per-tool extraction includes both bodies with their respective <!-- ../../AGENTS.md --> and <!-- ./AGENTS.md --> comments preceding each rule, AND the per-model section also includes both bodies in parent → sub order. The <custom-instructions> block strips both scoped sections (including the inner comments) but keeps the top-level H1 path headings.
  • Fenced-code documentation safety (Codex regression guard ipc: use consistent error propogation #2): a fenced markdown example containing a ## Tool: bash line inside the fence does NOT receive an injected provenance comment. The fenced region in <custom-instructions> is preserved verbatim, the real (non-fenced) ## Tool: bash section still gets the provenance comment in the per-tool extraction, and the example body does not leak into the tool extraction.
  • Stale subProjectPath (not a descendant of projectPath) degrades to reading just the cwd's AGENTS.md with no path-source markers.

New unit tests in markdown.test.ts > extractModelSection:

  • Returns all matching ## Model: sections in source order, joined with \n\n (parallel to extractToolSection).
  • Supports regex-style heading patterns and still collects all matches.
  • Returns null when no ## Model: heading matches the active model.
  • Skips empty matching sections while keeping later non-empty content.

Risks

Behavior change is scoped to sub-project workspaces and the (rare) multi-match ## Model: case:

  1. The <environment> block and bash tool description for sub-project workspaces no longer mention the sub-project at all — they look identical to a regular project at that cwd. The cwd was already the sub-project directory.
  2. Parent + sub-project AGENTS.md are now both visible to the agent. Previously only the sub-project's AGENTS.md was loaded (the doubled-path lookup never found the parent's). Anyone whose parent AGENTS.md previously had no effect on sub-project workspaces will start seeing it influence agent behavior.
  3. Sub-project workspaces emit two new H1 headings inside <custom-instructions> — a small visible structural change. The headings are minimal (just a backticked path) and serve double duty as a path note and a section boundary.
  4. extractModelSection now collects all matching ## Model: sections in source order rather than returning only the first match. This affects any AGENTS.md (single-project, multi-project, or sub-project) that has multiple matching ## Model: regex sections. I checked the repo and didn't find any authoring patterns relying on first-match semantics — the change essentially only takes effect in the new sub-project / existing multi-project flows.
  5. Non-sub-project single-project workspaces (no subProjectPath) are unaffected — deriveSubProjectLayout returns null, the function collapses to a single readInstructionSetFromRuntime(runtime, workspacePath) call, and no path-source markers are emitted.

Pains

  • The earlier direction had been Codex-approved and merged into the PR loop, so this revision needed a soft-reset and squash to express a single clean intent. Tests had to be flipped from "asserts the new sub-project framing is present" to "asserts the cwd looks like a regular project, but with parent inheritance preserved and lightweight path-source markers."
  • Codex caught three subtle bugs in earlier iterations of this PR: (1) scoped section bodies were extracted into per-tool/per-model contexts without the path-source marker, fixed by injecting an HTML comment after each scoped heading; (2) sub-project model overrides were dropped because extractModelSection was singular while extractToolSection was plural, fixed by aligning the two; and (3) the regex-based injection naively matched scoped-looking lines inside fenced code examples, fixed by switching to a line-by-line scanner that tracks fence state. While investigating (1) also discovered that scoped sections in the parent segment could span across the segment join into the sub-project's narrative (since neither HTML comments nor plain text bound a section), prompting the move from a top-level HTML comment to a top-level H1 heading that does bound scoped sections.
  • The previous code's doubled-path bug in readSingleProjectContextInstructions was silently producing sub-project-only AGENTS.md, which made the bug invisible in dogfooding but a tripwire for anyone trying to reason about the inheritance semantics from the source. Replacing it with the explicit suffix-strip helper removes that confusion and unlocks correct parent inheritance for sub-projects.

Generated with mux • Model: anthropic:claude-opus-4-7 • Thinking: max • Cost: $40.78

@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
@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
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
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".

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