fix(context): relativize project_instructions and constitution source labels

[qiuYliangM]

Summary

Split out of #54 as requested in review. The <project_instructions source="..."> label carried the absolute path of the loaded AGENTS.md inside the pinned system prompt. On this line the pinned prompt is never rewritten: workspace drift reaches the model as a bounded <context_update> history append. Relativizing the label therefore buys three smaller, honest things:

1. No spurious <context_update> append (token and attention noise) when a single flat instructions file (no ancestor chain, no project rules) is recased on a case-insensitive filesystem while the content is unchanged. Narrower than an earlier draft claimed: a directory move inside a session re-pins the header rather than emitting a <context_update> — there the relativized label keeps the absolute path out of the new pinned header. Repos whose block body embeds absolute paths (ancestor chain, project rules) still drift; see the v0.8.8 stabilization — sub-agent caps, mutex contention, RLM polish, CI cleanup Hmbown/Codewhale#514 note below.
2. Absolute project paths stay out of provider-bound prompt labels (prompt hygiene).
3. One shared helper renders the label for both the system prompt and the context report, so the two cannot drift.

The label is an origin tag, not a locator; directory identity stays discoverable via the shell.

Scope

• The repo constitution block (<codewhale_repo_constitution source="...">) sits in the same pinned region and previously still carried the absolute path. The source handed to the renderer is a runtime-canonicalized absolute path, but its final segment is the compile-time constant .codewhale/constitution.json, so the base name is stable — this PR applies the same shared helper there. No locator is lost (constitution_source_path in the report and /constitution still show it).
• Ancestor-chain (<!-- scoped instructions: {abs} -->) and <project_rule source="{abs}"> keep their absolute spellings: filename-only would genuinely collide for chain segments, so relativizing them needs a repo-relative decision. Their bodies also embed absolute paths, which relativizing the labels alone would not fix. Tracked in the parent repo: CodeWhale follow-up: relativize scoped-instructions chain and project_rule prompt source labels (repo-relative) pinvou-agent#514.
• The <!-- global: {path} --> label in merge_global_and_project_instructions is deliberately untouched: the home path does not drift with the workspace.
• Out of scope, same mechanism: <codewhale_user_constitution source="{abs}"> (crates/config/src/user_constitution.rs) still embeds the absolute home path, and the core fragment path (crates/core/src/fragments.rs) uses workspace-relative / caller-named labels instead of this helper. No consumer parses the source attribute, so these conventions coexist without collisions, but they are not unified.

Tests

• Byte-identity of the instructions block for identical content loaded from two different directories (forkguard_-prefixed regression).
• The constitution block renders source="constitution.json" with no absolute path.
• The helper's "project" fallback has a unit test.

Non-goals / notes

• The context report still displays the absolute source path for operators; only the prompt label is relativized (the shared helper keeps the prompt-side token derivation in sync with the model-visible label).

No-Issue: review-requested split from #54; no separate tracking issue.

Signed-off-by: qiuYliangM 185303122+qiuYliangM@users.noreply.github.com

[github-actions]

Thanks @qiuYliangM for taking the time to contribute.

This repository is observing a maintainer-managed PR intake gate in dry-run mode, so this pull request is staying open. This note helps maintainers prepare the allowlist before any enforcement is considered.

Please read CONTRIBUTING.md for the expected contribution shape. A maintainer can grant recurring PR access by commenting /lgtm on a pull request.

[asto18089]

Fresh round-1 review of the split-out PR — full pass over premise/mechanism, completeness, collisions, test quality, and commit hygiene, with local compile and red-green test verification at afd91a21b.

Verdict: Request changes

The code itself is minimal, correct, and clean — I verified the regression test fails on base and passes here, the "project" fallback semantics are preserved byte-for-byte, there are no label collisions with operational impact, and there is nothing to split out (scope, DCO, and base alignment are exact). But the PR's stated root cause does not exist on this branch, and the same emitted system-prompt block still contains three absolute-path labels the PR leaves behind — one of which needs no new decision and was already flagged in the #54 round-2 review that motivated this split. As written, the wrong cache-invalidation story is enshrined in the body, both commit messages, the helper doc comment, and the test comment.

Major 1 — the claimed KV-prefix-bust mechanism does not exist on pinvou3-clean

The body and commit messages claim that moving a project directory "rewrote the label and invalidated the provider KV prefix cache for the entire request, including conversation history". On this line that cannot happen:

• The pinned-header policy re-pins only on explicit-input changes (model, mode, goal, route, …); workspace drift is converted into a bounded <context_update> user-role history append (crates/tui/src/core/engine.rs:7345-7390, applied at engine.rs:5624-5633).
• prefix_cache.rs:736-744 states the design outright: "The pinned system prompt is never rewritten; this is how workspace, instruction, skills, memory, and goal drift reaches the model as a normal history append." The delta is emitted once per change (context_update_baseline absorbs it, engine.rs:7388), so the provider byte prefix is preserved throughout.

That rationale was true on the branch line this commit was ported from — there the verbatim project_instructions_section reinjection re-emitted the label mid-conversation (commit 2's own message records that this reinjection no longer exists here) — but it does not transfer. The honest benefits on this branch are smaller and should be stated as such:

1. It removes a one-time spurious <context_update> append on move/recase (token + attention noise).
2. It stops leaking the absolute project path to the provider inside prompt labels (prompt hygiene).
3. It unifies the two label renderings behind one helper.

Please rewrite the mechanism story in the PR body, both commit messages, types.rs:126-132, and the test comment (amend/squash is cheap per CONTRIBUTING; otherwise the wrong claim lands in the merge message).

Major 2 — the byte-stability invariant is declared on a block where it does not hold

as_system_block emits ONE block = constitution + <project_instructions> + rules (types.rs:88-123). The PR fixes the label of the middle part only. Still inside the very same cache-stable block:

• <codewhale_repo_constitution source="{abs}"> — project_context/constitution.rs:247-252, prepended at types.rs:105-107. This was already flagged in the #54 round-2 review that requested this split, and unlike the two follow-ups below it needs no decision: the basename is the compile-time constant "constitution.json" (constitution.rs:14-15), so project_instructions_source_label(Some(source)) applies verbatim, and the locator stays available to operators via constitution_source_path in the report and /constitution.
• <!-- scoped instructions: {abs} --> for ancestor AGENTS.md chain segments — project_context.rs:945-956. Filename-only would genuinely collide there (that is exactly what the comment disambiguates), so repo-relative is a real design decision — a legitimate follow-up.
• <project_rule source="{abs}"> — project_context.rs:519-524. Same class; needs a workspace-relative decision — a legitimate follow-up.

Consequence: for any repo with a constitution, rules, or a parent AGENTS.md chain — including this repository itself, which ships .codewhale/constitution.json — a directory move/recase still rewrites bytes inside the block the new test claims to pin as move-stable. The test exercises two flat tempdirs, i.e. precisely the only configuration where the invariant holds.

Requested: fix the constitution label in this PR (one line plus a test case) — or open and link a concrete follow-up PR and soften every byte-stability claim (helper doc, test comment) to single-file scope; and register the scoped-instructions / project_rule follow-ups explicitly, noting the repo-relative direction.

Should-fix — PR body claims vs reality

1. "The context report renders the same file-name-only label" (listed under Tests): no such test exists, and the report never renders the label — SourceEntry keeps only a token estimate from the block text, while the entry still displays the full absolute path in its source_path field (context_report.rs:465-471). The base_source_entries change is behaviorally inert except for the token count, so "so operators see the same label the model sees" is inaccurate as stated. Sharing the helper is still right for drift prevention — the claim, not the code, is the problem.
2. "Regression tests renamed with the forkguard_ prefix" (plural): exactly one rename, of a test this PR's own first commit added three files earlier. No pre-existing test was renamed.

Nits (non-blocking)

• "block 2" (types.rs:128 and the test comment): the block is SystemBlock 0; "2" is only the composition-stage section numbering (prompts.rs:1190). Suggest dropping the number.
• The helper's "project" fallback branch has no unit test (types.rs:133-138).
• The new comment at context_report.rs:452-453 ("不泄漏绝对路径") is misleading: the entry it annotates exposes the absolute path a few lines below (context_report.rs:468-470).
• Missing blank line between the new test and test_empty_file_warning (project_context.rs:1419-1420); rustfmt-tolerated.

Verified clean (for the record)

• Red-green: the new test inserted verbatim into a clean pinvou3-clean worktree fails on base (source="/tmp/.tmp…/AGENTS.md"), passes on the branch; the project_context (65) and context_report (15) test groups are green locally.
• Fallback semantics preserved byte-for-byte at both call sites; source_path is always absolute in production; file_name() degenerate cases are unreachable via the loaders.
• No collision impact: nothing in the report machinery sorts, dedupes, or compares by the source string, and the report retains the absolute path for operators; nested chain segments merge into a single instructions string, so no coexisting identical labels.
• No stale consumers, tests, or docs; /context, /constitution, and doctor displays keep absolute paths by design.
• Helper extraction is idiomatic (mirrors the existing pub(crate) use self::… re-export pattern in project_context.rs) and deletes the duplicated chains commit 1 introduced — no wheel reinvented.
• Scope: exactly the 3 files serving the stated purpose, no #54 dependencies, based exactly on the current pinvou3-clean tip, DCO complete on both commits, commit messages match repo convention.

[asto18089]

Round-2 review of head 388dbd8 (fresh full pass with parallel verification: premise/mechanism, per-hunk correctness, red-green proof on base vs head, completeness, commit hygiene, fork-policy/CI, side-effect sweep).

Verdict: Request changes — two cheap items, everything else passes

The round-1 majors are genuinely fixed and independently re-verified: the mechanism story is now accurate and honest for this branch (pinned prompt never rewritten; drift is a bounded <context_update> history append, engine.rs:7346-7395), the constitution label is fixed by commit 3 (dynamically proven move-stable), the report claim is corrected, and the nits are addressed. Red-green is proven with real output: the new tests spliced into base ae7e3fb fail for exactly the absolute-path label (FAILED. 75 passed; 2 failed); on the head, project_context 78 passed / context_report 18 passed, cargo fmt --check clean, clippy 0 warnings. No unrelated hunks, DCO 3/3, base exactly on tip, no fingerprint/golden/snapshot consumer breaks, CI fully green.

Two items remain before merge — both are cheap:

1. "Registered as follow-ups" is not verifiable — please make the registration real

The body claims the ancestor-chain (<!-- scoped instructions: {abs} -->, project_context.rs:946-954) and <project_rule source="{abs}"> (project_context.rs:521-525) relativizations are "Registered as follow-ups." I checked every plausible registry: this repo has issues disabled; no open PR addresses them; nothing in docs/; no registration reply on this thread. Round 1 explicitly asked for explicit registration noting the repo-relative direction — that has not happened.

The deferral itself is defensible (filename-only genuinely collides for chain segments, so repo-relative is a real design decision), but a deferral that is tracked nowhere is a silent gap: the root cause this PR articulates — spurious <context_update> and path leakage on move — still fires verbatim for any repo with rules files, a parent AGENTS.md chain, or work from a subdirectory. I confirmed with a production-loader probe that such a repo's pinned block is still not byte-identical after a move.

Requested: add a clickable tracking reference to the body (parent-repo issue, a docs/ follow-up entry, or your tracking PR), stating the repo-relative direction — or delete the claim.

2. The byte-stability test's assert message overstates its scope

forkguard_project_instructions_source_is_file_name_not_absolute_path uses two flat tempdirs with a lone AGENTS.md — precisely the only configuration where the invariant holds (without a .git dir the chain loader stops at the workspace, and no rules/constitution paths are exercised). The inline comment states this scope honestly, but the assert message "a directory move must not change the project instructions block" asserts more than the test proves. Please scope the message to what is actually pinned (single instructions file, no chain/rules/constitution path dependence).

Nits (non-blocking)

• Commit 3's message doesn't mention the fallback unit test and the comment translations included in the same commit.
• constitution.rs:249-253 comment: render_block receives the run-time-discovered absolute path; what's constant is only the file_name() (constitution.json). The conclusion holds; the wording is slightly loose.
• merge_contexts (types.rs:143, dead code, no production callers) would emit duplicate source="AGENTS.md" labels — the one place the collision rationale actually applies; unreachable, noting only.

Merge coordination note (not a PR defect)

Per fork-policy, the parent-repo companion PR must land promptly after this merges: verify-public-submodule.sh turns parent CI red the moment pinvou3-clean advances past the pinned gitlink, and it must carry the docs/fork-modifications.md update plus fork-guard.sh EXPECTED_HEAD (ae7e3fb→388dbd85) / EXPECTED_COMMITS (28→31). All existing fork-guard fingerprints survive this diff — verified one by one.

[asto18089]

Approved. Re-reviewed from scratch after the round-2 split; the two Major items from my earlier CHANGES_REQUESTED are resolved, and independent verification confirms the rest.

Root cause (real, now stated honestly). Verified against engine.rs (refresh_pinned_header_for_turn) and prefix_cache.rs: the pinned prompt is never rewritten, and the absolute-path label did leak into provider-bound text and could feed a spurious bounded <context_update> on a case-only rename of a flat instructions file. The summary now states the narrowed scope accurately (a move re-pins the header instead; ancestor-chain/rule bodies still embed absolute paths — tracked in Pinvou/pinvou-agent#514, which should also cover those content-embedded paths).

Elegance. The helper deletes a verbatim duplicate rather than adding one; the closest existing helper (workspace_basename) has different sentinel semantics and is correctly not reused. Upstream (Hmbown/CodeWhale) still renders the absolute path with no fix or issue, so there is nothing to align with; this remains a good upstreamization candidate later.

Scope. No unrelated code across the three commits; the constitution change is the same defect in the same pinned region and was requested in the previous review round. DCO and conventional-commit format are clean.

Defects. None functional. Verified independently: red/green (the forkguard test fails on the base with the exact label diff, passes here), codewhale-tui checks clean, no consumer parses the source attribute, and the operator-facing locators (SourceEntry.source_path, /constitution) keep absolute paths.

I pushed one commit (30ada031e) correcting the misleading "compile-time constant" comment in constitution.rs (the rendered source is a runtime-canonicalized absolute path; only its final segment is constant) and updated the PR title/summary to match the final scope.

Merge conditions / follow-ups:

1. Registration in the parent repo must land with the next gitlink-advancing PR there: fork-modifications list (zh + en), a new fingerprint for project_instructions_source_label, forkguard test count 54→55, and the commit-sequence table.
2. Pinvou/pinvou-agent#514 should be expanded with the content-embedded absolute paths (ancestor-chain and project-rule bodies), which label-only relativization will not fix.
3. Optional follow-up: an engine-level test asserting same-content rebuild in a moved directory emits no <context_update>, so the headline benefit is pinned at the integration layer rather than inferred from block byte-identity.