Skip to content

fix: guide compile-only project installs (supersedes #2115, closes #2057)#2293

Merged
danielmeppiel merged 10 commits into
mainfrom
supersede/pr-2115
Jul 17, 2026
Merged

fix: guide compile-only project installs (supersedes #2115, closes #2057)#2293
danielmeppiel merged 10 commits into
mainfrom
supersede/pr-2115

Conversation

@danielmeppiel

@danielmeppiel danielmeppiel commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

fix(install): guide compile-only project installs

TL;DR

Project-scope installs now tell Gemini, Codex, OpenCode, and experimental Hermes users to run apm compile when installed dependency instructions still need root-context compilation. The hint is target-aware, no-op aware, dry-run safe, and emitted only when dependency instruction files exist. This superseding PR preserves Sergio Sisternes's original commits and folds the review-panel follow-ups: exact output names, path containment, target classification, regression evidence, docs, and changelog coverage.

Note

Supersedes #2115 and closes #2057. The compile pipeline already worked; the missing post-install next step caused the reported silent-success experience.

Problem (WHY)

  • apm install <local-gemini-package> could exit successfully while the package instructions remained under apm_modules/ until the user independently discovered apm compile ([BUG] Gemini instructions are not compiled into GEMINI.md when installing a local package #2057).
  • The existing finalize path emitted a compile hint for user-scope installs but had no project-scope counterpart.
  • The first review pass found that the new recursive scan could accept an instruction symlink resolving outside apm_modules/ on supported Python versions.
  • [!] The original hint said root context files, which did not name the files a user could inspect after compiling.
  • [!] The install guides described only the user-scope hint, leaving the new project-scope behavior undiscoverable.

These fixes keep the workflow explicit and verifiable. The implementation follows the PROSE rule that "Grounding outputs in deterministic tool execution transforms probabilistic generation into verifiable action." and the Agent Skills guidance to "Add what the agent lacks, omit what it knows".

Approach (WHAT)

# Fix Anchor
1 Route project-scope finalize calls through a dedicated compile-hint helper. "Favor small, chainable primitives over monolithic frameworks."
2 Gate the hint on real instruction files, compile-only targets, successful work, and non-dry-run execution. Issue #2057 reproduction
3 Resolve every scan candidate through ensure_path_within before accepting it. src/apm_cli/utils/path_security.py containment contract
4 Derive the exact applicable root-context outputs from canonical compile predicates and name them in the next-step message. Review-panel convergence
5 Prove both behavior and containment with mutation-break regression tests. "do the work, run a validator (a script, a reference checklist, or a self-check), fix any issues, and repeat until validation passes."

Implementation (HOW)

File Change
src/apm_cli/install/phases/finalize.py Adds the project-scope hint, conservatively scans all installed modules, rejects escaping candidates, names exact outputs, and excludes directory-native Claude and IntelliJ paths.
tests/unit/install/phases/test_finalize_user_compile.py Covers fire/no-fire routing, exact message text, multi-target output selection, unsupported targets, and a real escaping symlink.
tests/unit/install/test_finalize_phase.py Extends the finalize test context with the fields consumed by the new project path.
tests/integration/test_install_content_hash_roundtrip.py Exercises the ordinary dependency install path through the real CLI finalize phase.
docs/src/content/docs/reference/cli/install.md Adds the project-scope behavior to the command reference.
docs/src/content/docs/consumer/install-packages.md Explains the install-then-compile next step for consumers.
docs/src/content/docs/getting-started/first-package.md Distinguishes compile-only root-context targets from directory-native targets.
docs/src/content/docs/reference/targets-matrix.md Defines the canonical target classification and links the OpenAPM requirement.
packages/apm-guide/.apm/skills/apm-usage/commands.md Keeps the agent-facing command resource synchronized with the CLI behavior.
CHANGELOG.md Credits @sergio-sisternes-epam and records the user-visible fix.

Diagrams

Legend: the dashed nodes are the project-scope behavior added by this change; the containment check runs before a candidate can trigger output.

flowchart LR
    subgraph Finalize[Install finalize]
        S{Install scope}
        U[Existing user hint]
        P[Project hint checks]
    end
    subgraph Inspect[Dependency inspection]
        T{Compile-only target}
        R[Scan instruction files]
        C[Contain resolved path]
    end
    subgraph Output[User output]
        N[No hint]
        H[CommandLogger info hint]
    end
    S -->|USER| U
    S -->|PROJECT| P
    P --> T
    T -->|No| N
    T -->|Yes| R
    R --> C
    C -->|No safe match| N
    C -->|Safe match| H
    classDef new stroke-dasharray: 5 5;
    class P,T,R,C,H new;
Loading

Trade-offs

  • Hint instead of auto-compile. Chose an actionable next step; rejected implicit compilation because install and compile remain separate commands and users may intentionally control generated root files.
  • Conservative all-module scan. Chose to consider dependency instructions from earlier installs; rejected current-run-only state because the next compile must represent the full installed module tree.
  • Bounded recursive scan. Chose an early-return filesystem scan for this fix; rejected a cross-phase context refactor because the measured install-path cost is currently small and that redesign exceeds [BUG] Gemini instructions are not compiled into GEMINI.md when installing a local package #2057.
  • Exact output list. Chose canonical compile predicates over a shared static list so each hint names only the files the active targets will update.

Benefits

  1. A project-scope Gemini, Codex, OpenCode, or experimental Hermes install with dependency instructions emits one immediate apm compile next step.
  2. Claude, Copilot, Cursor, IntelliJ, Kiro, Windsurf, Antigravity, dry-run, and zero-install paths remain silent under the focused regression suite.
  3. An instruction symlink escaping apm_modules/ cannot trigger the hint.
  4. The command reference, consumer guide, first-package guide, and changelog now describe the same behavior.
  5. Sergio's two original commits retain their authorship; the follow-up commit credits Sergio and Copilot via trailers.

Validation

uv run --extra dev pytest -q tests/unit/install tests/integration/test_install_content_hash_roundtrip.py:

1856 passed in 35.87s

uv run --extra dev pytest -q tests/spec_conformance -p no:randomly:

143 passed, 2 skipped in 12.70s
Canonical lint mirror

uv run --extra dev ruff check src/ tests/:

All checks passed!

uv run --extra dev ruff format --check src/ tests/:

1542 files already formatted

uv run --extra dev python -m pylint --disable=all --enable=R0801 --min-similarity-lines=10 --fail-on=R0801 src/apm_cli/:

Your code has been rated at 10.00/10

bash scripts/lint-auth-signals.sh && bash scripts/lint-architecture-boundaries.sh:

[+] auth-signal lint clean
[+] architecture boundary lint clean

The YAML I/O, 2100-line file limit, and raw str(relative_to()) guards also exited 0.

Mutation-break evidence
  • Replacing the concrete root-file message with root context files made TestHintProjectCompileNeeded::test_hint_fires_for_compile_only_target_with_instruction_files fail; the message was restored.
  • Removing the ensure_path_within guard made TestHintProjectCompileNeeded::test_instruction_scan_ignores_symlink_outside_apm_modules fail; the guard was restored.
  • Removing the project-scope Claude exclusion made TestHintProjectCompileNeeded::test_hint_not_fired_for_claude_target fail; the exclusion was restored.
  • Mutations to exact output derivation, unsupported-target filtering, IntelliJ exclusion, unsafe-link diagnostics, the ordinary dependency finalize path, and req-tg-007 output text each broke their corresponding regression test and were restored.

Scenario Evidence

# Scenario (user promise) Principle(s) Test(s) proving it Type
1 After an ordinary project-scope Gemini dependency install, the user sees apm compile instead of silent success DevX, Multi-harness support tests/integration/test_install_content_hash_roundtrip.py::test_project_install_with_dependency_instructions_prints_compile_hint integration with fixture
2 The hint names the root files that compilation updates DevX tests/unit/install/phases/test_finalize_user_compile.py::TestHintProjectCompileNeeded::test_hint_fires_for_compile_only_target_with_instruction_files unit
3 An instruction symlink outside apm_modules/ cannot trigger the hint Secure by default tests/unit/install/phases/test_finalize_user_compile.py::TestHintProjectCompileNeeded::test_instruction_scan_ignores_symlink_outside_apm_modules unit
4 Directory-native, unsupported, dry-run, and no-op installs do not receive irrelevant guidance DevX, Multi-harness support tests/unit/install/phases/test_finalize_user_compile.py::TestHintProjectCompileNeeded unit

How to test

  • Install a local Gemini-target package containing .apm/instructions/*.instructions.md; expect one [i] line directing you to apm compile.
  • Run apm compile; expect the installed instructions to appear through AGENTS.md and GEMINI.md.
  • Repeat with Claude and Copilot target packages; expect no compile hint because install deploys those instructions directly.
  • Repeat with --dry-run; expect no compile hint and no root-context writes.
  • Run the install regression and spec-conformance commands above; expect 1856 passing install tests and 143 passing spec tests.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

Sergio Sisternes and others added 3 commits July 17, 2026 17:31
After 'apm install <local-gemini-package>' on project scope, AGENTS.md /
GEMINI.md never received the installed instructions because the user had
no signal that 'apm compile' was required.  User-scope installs already
had _hint_global_root_context(); project-scope had no equivalent.

Changes:
- Add _has_dep_instruction_files(ctx) -- lightweight scan of apm_modules/
  for *.instructions.md under .apm/instructions/ subtrees.
- Add _hint_project_compile_needed(ctx) -- fires on project-scope installs
  when a compile-only target family (gemini, agents, claude) is active and
  dep instruction files are present; emits a one-line hint pointing at
  'apm compile'.
- Wire both into run(): USER -> _hint_global_root_context, PROJECT ->
  _hint_project_compile_needed, None -> neither.
- Extend _FakeCtx in test_finalize_phase.py with dry_run, targets, and
  apm_modules_dir fields so existing tests continue to pass.
- Add 11 new unit tests in TestHintProjectCompileNeeded covering the
  fire/no-fire conditions, plus a run()-level routing test.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…aude family)

The previous message named only 'AGENTS.md / GEMINI.md' but the hint also
fires for the claude compile family (CLAUDE.md). Using the generic term
'root context files' matches the docstring and covers all three families
correctly.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Name the generated root files, reject escaping instruction symlinks, and document the project-scope compile step. Addresses the in-scope review-panel follow-ups for PR #2115.

Co-authored-by: Sergio Sisternes <sergio-sisternes-epam@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the apm install project-scope UX for compile-only targets (Gemini / agents-protocol / Claude) by emitting a targeted “run apm compile” hint when dependency instruction files are present, preventing “silent success” where instructions remain unapplied until a manual compile.

Changes:

  • Add a project-scope post-install compile hint and dependency-instruction detection to finalize.py.
  • Extend unit tests to cover the new hint routing, fire/no-fire conditions, and symlink-escape regression behavior.
  • Update docs and changelog to describe the new project-scope behavior.
Show a summary per file
File Description
src/apm_cli/install/phases/finalize.py Adds project-scope compile hint and scans apm_modules/ for dependency instruction files to decide whether to emit it.
tests/unit/install/phases/test_finalize_user_compile.py Adds/updates tests for project-scope hint routing and behavior, including symlink-escape coverage.
tests/unit/install/test_finalize_phase.py Extends the finalize-phase context stub with fields needed by the new project-scope logic.
docs/src/content/docs/reference/cli/install.md Documents the new project-scope “root context hint” behavior.
docs/src/content/docs/getting-started/first-package.md Clarifies install vs compile responsibilities for compile-only targets and mentions the hint.
docs/src/content/docs/consumer/install-packages.md Adds a consumer-facing explanation of the project-scope compile hint and what remains unchanged until compile.
CHANGELOG.md Adds a Fixed entry describing the user-visible change and credits the original author.

Review details

  • Files reviewed: 7/7 changed files
  • Comments generated: 2
  • Review effort level: Low

Comment on lines +47 to +65
for candidate in apm_modules.rglob("*.instructions.md"):
try:
candidate = ensure_path_within(candidate, apm_modules)
except PathTraversalError:
continue
if not candidate.is_file():
continue
# Only count files that live inside a ``.apm/instructions/`` subtree,
# which is where ``_copy_local_package`` and the bundle staging logic
# place compile-only instruction files.
parts = candidate.parts
try:
idx = parts.index(".apm")
except ValueError:
continue
if idx + 1 < len(parts) and parts[idx + 1] == "instructions":
return True

return False
Comment thread CHANGELOG.md Outdated
Comment on lines +33 to +36
- Project-scope installs for Gemini, agents-protocol, and Claude targets now
prompt users to run `apm compile` when dependency instructions need root
context compilation, instead of silently leaving those instructions unapplied
-- by @sergio-sisternes-epam (closes #2057; supersedes #2115).
@danielmeppiel

Copy link
Copy Markdown
Collaborator Author

APM Spec Guardian: fold_and_ship

Scope: editorial-patch; diff = +34/-4 lines across 1 file(s). Shocked-meter avg: 7.75/10.

The panel recommends shipping after the small req-tg-007 clarifications. Three panels independently asked for a citable per-target classification; the fold now points to the target support matrix, defines the unclassified-target default, clarifies full-tree scan scope, permits the complete output-name set, and reconciles the Claude companion entry.

Convergence

Panel Stance Shocked New B New R New N
Swagger / OpenAPI Editor ship_with_followups 8/10 0 1 1
OCI Distribution Editor ship 8/10 0 0 0
Package-Manager Registry-Contract Editor ship_with_followups 8/10 0 2 1
Web-Platform Architect ship_with_followups 7/10 0 1 2

B = highest-severity findings, R = recommended, N = nits.
Counts are signal strength. The maintainer decides whether to ship.

Convergent themes (flagged by 2+ panels)

  • T1 -- Per-target compile-mode classification needs a citable source (supporting: sw-rec-r1-1, pkg-rec-r1-1, tag-rec-r1-1)
  • T2 -- Compile-mode terminology needs consistent wording (supporting: sw-nit-r1-1, tag-nit-r1-1)

Fold now (4 item(s))

  1. [F1 / T1] req-tg-007 (Section 8.5.2) -- Point the requirement to the companion target support matrix as the source of the per-target post-install compilation classification.
    Success criterion: req-tg-007 links to targets-matrix.md#post-install-instruction-compilation.
  2. [F2 / T2] req-tg-007 (Section 8.5.2) -- Remove the ambiguous native-rule dichotomy and use the explicit post-install root-context compilation classification.
    Success criterion: Condition (c) names the companion classification without introducing a second per-target decision.
  3. [F3 / standalone] req-tg-007 clause (a) (Section 8.5.2) -- Separate the current operation's installed-package trigger from the full installed dependency-tree scan scope.
    Success criterion: The requirement explicitly covers packages installed during earlier operations.
  4. [F4 / standalone] req-tg-007 diagnostic text (Section 8.5.2) -- State that naming the complete canonical output set is conforming when only one target is active.
    Success criterion: The requirement contains that explicit allowance.
Defer to v0.1.1 (4 items)
  • [F5 / T1] Target support matrix -- Continue expanding per-target staging/materialization detail as new targets are registered.
  • [F6 / standalone] Target classification default -- Keep the fail-quiet default explicit for future unclassified targets.
  • [F7 / standalone] Installed-tree authority -- Revisit a machine-readable inventory source if compile-only instruction sources gain a canonical lockfile field. The current editorial note explains why deployed_files is not that field.
  • [F8 / T2] Terminology -- Promote the post-install compilation classification into the glossary if another requirement needs the same term.

Linter notes (2 check(s) failed)

  • [7] The checklist's simplified heading slugger reports existing dotted-heading links; the new req-tg-007 anchor and companion fragment resolve.
  • [11] This is a code PR and modifies Python files; the general APM review panel also applies.

Linter handoff: Re-run count reconciliation and conformance generation after the fold. The req-tg-007 count remains 102 total statements (97 MUST, 5 SHOULD).


Full per-panel findings

Swagger / OpenAPI Editor -- shocked_meter 8/10, confidence high

Summary: The fold is well anchored and mechanically aligned. The substantive gap was the uncited per-target compile-mode classification.

New recommended findings (1)

  • [sw-rec-r1-1] req-tg-007 -- The firing condition introduced a per-target classification without a citable spec-side source.
    Recommended fix: Point to the companion target support matrix.

New nit findings (1)

  • [sw-nit-r1-1] Adjacent requirements used inconsistent native-rule phrasing. -- fix: Use one post-install compilation term.

OCI Distribution Editor -- shocked_meter 8/10, confidence high

Summary: The diagnostic addition does not weaken distribution, extraction, or content-integrity guarantees.

Package-Manager Registry-Contract Editor -- shocked_meter 8/10, confidence high

Summary: The addition is correctly scoped but left the target classification and installed-tree interpretation under-specified.

New recommended findings (2)

  • [pkg-rec-r1-1] req-tg-007 -- The target classification needed a named companion source.
    Recommended fix: Bind it to the target support matrix and define an unclassified-target default.
  • [pkg-rec-r1-2] req-tg-007 -- Filesystem and lockfile-derived implementations could disagree under drift.
    Recommended fix: Document the chosen authority. The fold explains that compile-only sources are not necessarily deployed_files.

New nit findings (1)

  • [pkg-nit-r1-1] Clause (a) could imply that only packages installed in the current operation are scanned. -- fix: Name the full installed dependency tree.

Web-Platform Architect -- shocked_meter 7/10, confidence high

Summary: Counts and real code-level conformance coverage align; the target classification needed a citable source and clearer companion wording.

New recommended findings (1)

  • [tag-rec-r1-1] req-tg-007 / target support matrix -- The compile classification was not explicit and the Claude entry was ambiguous.
    Recommended fix: Cross-reference the companion and distinguish install-time staging from compile-time materialization.

New nit findings (2)

  • [tag-nit-r1-1] Compile-mode terms lacked a glossary entry. -- fix: Add entries if the terms become shared.
  • [tag-nit-r1-2] The fixed message lists all canonical outputs for one active target. -- fix: Explicitly allow the complete set.

This panel is advisory. The maintainer decides whether to ship. Re-apply the spec-review label after addressing feedback to re-run.

danielmeppiel and others added 4 commits July 17, 2026 18:00
Add req-tg-007, its manifest and conformance bindings, and the target-matrix classification required by the spec-conformance gate.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use the generated OpenAPM v0.1 route so the docs link checker resolves the requirement anchor.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Derive exact output names from canonical compile predicates, share target filtering, report unsafe links, exclude unsupported IntelliJ hints, and add ordinary-dependency coverage.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep the contributor credit and issue handoff on one line and end with the superseding PR number, addressing Copilot inline feedback.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel

Copy link
Copy Markdown
Collaborator Author

APM Review Panel: ship_with_followups

Eliminates a silent post-install dead end by printing an actionable compile hint for project-scope installs whose instructions require compilation.

cc @sergio-sisternes-epam -- a fresh advisory pass is ready for your review.

The panel converges strongly: the active panelists endorse replacing a silent-success trap with an actionable next step, and the test lens confirms all critical surfaces carry mutation-break regression traps. No architectural, logging, or security fault was identified.

One load-bearing correctness issue remains: the target matrix and hint logic classify Claude as requiring post-install compilation, but Claude project instructions deploy directly to .claude/rules/ during install and compile intentionally reports that no further action is needed. This is a surgical correction; the core hint infrastructure is sound for Gemini and the agents-family targets that genuinely need compilation.

Two panelists raised possible directory-symlink recursion in Path.rglob. Deterministic probes on CPython 3.10.16 and 3.12.10 showed no recursion through directory symlinks, and ensure_path_within still rejects escaping file candidates, so that concern is not elevated.

Dissent. The security and performance lenses raised directory-symlink traversal concerns. Deterministic CPython probes disprove the premise on supported versions, so empirical evidence outweighs those findings.

Aligned with: The hint respects per-target behavior once Claude direct deployment is excluded.; The actionable post-install message gives users the exact next command and outputs.; The superseding PR preserves the contributor authorship and community issue context.

Growth signal. The silent-success-to-actionable-guidance story is a strong release-note hook, and the preserved-authorship handoff is a reusable community contribution pattern.

Panel summary

Persona B R N Takeaway
Python Architect 0 0 1 Clean extraction of shared hint logic with correct canonical-owner delegation; no split-authority or architectural fault.
Cli Logging Expert 0 0 2 Compile hint follows CommandLogger lifecycle, correct traffic-light severity, proper gating; ship.
Devx Ux Expert 0 0 1 Project-scope compile hint is actionable, conservative, and well-documented; install UX is improved with no anti-patterns.
Supply Chain Security Expert 0 0 1 Path containment uses sanctioned ensure_path_within; read-only scan with fail-closed symlink rejection; no new attack surface introduced.
Oss Growth Hacker 0 0 3 Strong retention fix; eliminates a silent-failure trap, credits community contributor, and documents across all discovery surfaces.
Doc Writer 1 1 1 The docs are discoverable, but Claude is incorrectly described as requiring post-install compilation.
Test Coverage Expert 0 0 0 All four critical surfaces have regression traps at floor tier with mutation-break proof; ship.
Performance Expert 0 1 2 rglob scan is noise-tier on the network-dominated install path; early-exit chain and lazy imports are well-structured.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 4 follow-ups

  1. [Doc Writer] (blocking-severity) Exclude Claude from the project-scope compile hint and align the related documentation. -- Claude instructions deploy directly to .claude/rules/; telling those users to compile is incorrect.
  2. [Doc Writer] Name the concrete affected targets in the changelog. -- Canonical target names are clearer than the internal agents-family label.
  3. [Devx Ux Expert] Document the project-scope hint in the agent-facing command resource. -- The resource must stay synchronized with observable CLI behavior.
  4. [Doc Writer] Use a relative cross-page link for req-tg-007. -- Relative links preserve the documentation hosting contract.

Architecture

classDiagram
    class InstallContext
    class FinalizePhase
    class TargetDetection
    class PathSecurity
    FinalizePhase --> InstallContext : reads
    FinalizePhase --> TargetDetection : canonical predicates
    FinalizePhase --> PathSecurity : containment
    class FinalizePhase:::touched
    classDef touched fill:#fff3b0,stroke:#d47600
Loading
flowchart TD
    A[apm install finalize] --> B{Project scope?}
    B -->|No| Z[Other finalize path]
    B -->|Yes| C[Filter active targets]
    C --> D[Scan dependency instructions]
    D --> E[Validate containment]
    E --> F[Derive exact root outputs]
    F --> G[Print actionable compile hint]
Loading

Recommendation

The core hint is sound and well tested. Correct the Claude classification and align the related prose before merge; the remaining polish can follow separately.


Full per-persona findings

Python Architect

  • [nit] Consider guarding ctx.diagnostics access in _has_dep_instruction_files for defensive robustness at src/apm_cli/install/phases/finalize.py:92
    The scan pushes an unsafe-link warning through the collector; guarding diagnostics would make the helper more defensive, although the pipeline always provides it.
    Suggested: Add an if ctx.diagnostics guard before warn.

Cli Logging Expert

  • [nit] Project-scope hint verb installed is slightly ambiguous when prior-dependency instructions trigger it at src/apm_cli/install/phases/finalize.py:143
    The full-tree scan can find instructions from an earlier install, so present or require compilation would be more exact in that uncommon case.
  • [nit] Sibling global and project hint messages use different structures at src/apm_cli/install/phases/finalize.py:182
    The project message names concrete outputs while the older global message uses a generic phrase; future consistency could help.

Devx Ux Expert

  • [nit] Skill resource commands.md does not mention the new project-scope hint behavior at packages/apm-guide/.apm/skills/apm-usage/commands.md
    The command resource describes install/compile separation but not the new hint, while repository instructions require CLI resource synchronization.
    Suggested: Add a short note that eligible project installs print an info hint naming outputs.

Supply Chain Security Expert

  • [nit] Explicitly avoid directory-symlink recursion as defense in depth at src/apm_cli/install/phases/finalize.py:87
    The reviewer believed older pathlib versions recurse into directory symlinks; deterministic probes are needed because the current containment guard already rejects escaping candidates.

Oss Growth Hacker

  • [nit] Mine the changelog entry for the next release beat at CHANGELOG.md:33
    The silent-success to actionable-guidance story is a strong release-note hook.
  • [nit] The first-package target-matrix link creates a minor click-away risk at docs/src/content/docs/getting-started/first-package.md:234
    Inline examples could preserve one-glance orientation, though the canonical link is more maintainable.
  • [nit] Contributor credit pattern is a reusable community template
    Preserved authorship plus changelog credit demonstrates that community contributions are carried through superseding work.

Auth Expert -- inactive

PR #2293 touches install finalize logic, docs, conformance artifacts, and compile-hint tests; no auth, token, credential, or host-resolution surface changes.

Doc Writer

  • [blocking] Remove Claude from the post-install compile-only guidance. at docs/src/content/docs/reference/targets-matrix.md:49
    Claude instructions deploy directly to .claude/rules during install and compile omits those instructions from CLAUDE.md when deployed, so the new project hint and matrix conflict with runtime behavior and existing docs.
    Suggested: Exclude Claude from project hint classification and align changelog, guides, target matrix, and PR body.
  • [recommended] Name the actual affected targets in the changelog. at CHANGELOG.md:33
    agents-protocol is a compile family rather than a canonical target; list Gemini, Codex, OpenCode, and experimental Hermes.
  • [nit] Use a relative link for req-tg-007. at docs/src/content/docs/reference/targets-matrix.md:52
    The documentation contract requires relative cross-page links.

Test Coverage Expert

No findings.

Performance Expert

  • [recommended] Verify directory-symlink traversal semantics across supported Python versions. at src/apm_cli/install/phases/finalize.py:87
    The reviewer believed older pathlib rglob versions recurse into directory symlinks and could make scan cost unbounded; the install pipeline itself does not create directory symlinks.
  • [nit] Miss-case scan cost is linear in files under apm_modules. at src/apm_cli/install/phases/finalize.py:87
    Measured typical projects remain noise-tier; a package-targeted check could be a future optimization.
  • [nit] A directory-symlink regression test could document traversal semantics. at tests/unit/install/phases/test_finalize_user_compile.py:413
    Current coverage tests an escaping file symlink; a directory symlink probe would make version behavior explicit.

This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.

danielmeppiel and others added 2 commits July 17, 2026 18:41
Claude receives project instructions directly in .claude/rules, so prompting those users to compile is incorrect. Align runtime filtering, tests, docs, changelog, and the agent-facing command resource; addresses the final panel follow-ups.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
State explicitly that install writes Claude rules directly and compile deduplicates them, removing the last ambiguity raised by the terminal documentation review.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel

Copy link
Copy Markdown
Collaborator Author

APM Review Panel: ship_now

Adds actionable compile hints after install so users never face silent success with no next step, with full test and documentation coverage.

cc @sergio-sisternes-epam -- a fresh advisory pass is ready for your review.

All panelists converge on a clean ship signal. The active personas returned no blocking-severity or recommended findings; every open observation is a nit. Supply-chain security confirms containment rejects escaping file symlinks. Test coverage confirms the regression traps are load-bearing with mutation-break evidence. Architecture, CLI output, and documentation are consistent with the implementation.

The recurring quickstart observation is a deliberate trade-off: linking the canonical target matrix avoids stale duplicated lists as targets evolve. The narrow-helper and future-scaling observations cross this issue scope and do not warrant follow-up work. Exact-head CI is green.

This change closes a silent-success gap for new users while preserving the community contributor credit. No dissent remains to arbitrate.

Aligned with: Containment validation rejects escaping file symlinks before they can trigger guidance.; The post-install message is quiet when irrelevant and names the exact next command and files when needed.; Contributor authorship and issue context remain visible through the superseding handoff.

Growth signal. The silent-success-to-actionable-next-step fix is a strong release-note story, paired with preserved community contributor credit.

Panel summary

Persona B R N Takeaway
Python Architect 0 0 2 Shared target filtering, canonical compile predicates, and path containment preserve single-owner architecture; ship.
Cli Logging Expert 0 0 0 Compile hint uses CommandLogger.info with the correct symbol, actionable message, and proper guards; no logging concerns.
Devx Ux Expert 0 0 1 Hint follows the npm post-install pattern: actionable, quiet for irrelevant targets, names exact files, and has synchronized docs.
Supply Chain Security Expert 0 0 0 Containment rejects escaping file symlinks and deterministic probes confirm directory symlinks are not traversed; no supply-chain concern.
Oss Growth Hacker 0 0 1 Fixes a silent-success funnel killer; hint, docs coverage, and community credit are shaped for a release narrative.
Doc Writer 0 0 0 Documentation is consistent with the implemented target behavior and the supplied validation evidence.
Test Coverage Expert 0 0 0 All critical surfaces have regression traps; integration coverage proves the user promise and mutation-break evidence confirms the tests are load-bearing.
Performance Expert 0 0 2 Filesystem scan cost is noise-tier versus network-dominated install; no performance regression.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Architecture

classDiagram
    class InstallContext
    class FinalizePhase
    class TargetDetection
    class PathSecurity
    FinalizePhase --> InstallContext : reads
    FinalizePhase --> TargetDetection : canonical predicates
    FinalizePhase --> PathSecurity : containment
    class FinalizePhase:::touched
    classDef touched fill:#fff3b0,stroke:#d47600
Loading
flowchart TD
    A[apm install finalize] --> B{Project scope?}
    B -->|No| Z[Other finalize path]
    B -->|Yes| C[Filter active targets]
    C --> D[Scan dependency instructions]
    D --> E[Validate containment]
    E --> F[Derive exact root outputs]
    F --> G[Print actionable compile hint]
Loading

Recommendation

Exact-head CI is green, all substantive findings are folded, and the regression evidence is load-bearing. Ship with contributor credit in the next release note.


Full per-persona findings

Python Architect

  • [nit] The instruction scan accepts the full InstallContext rather than narrow parameters at src/apm_cli/install/phases/finalize.py:72
    Narrow parameters could document the helper dependency surface, but the current phase-local shape matches neighboring helpers and avoids premature refactoring.
  • [nit] Cheapest-first guard ordering is correct
    Dry-run, install count, and target checks all short-circuit before filesystem I/O; no change is needed.

Cli Logging Expert

No findings.

Devx Ux Expert

  • [nit] The first-package callout links to the canonical target list instead of duplicating it inline at docs/src/content/docs/getting-started/first-package.md:235
    An inline parenthetical could save a click for newcomers, while the current canonical link avoids stale target duplication and remains accurate.

Supply Chain Security Expert

No findings.

Oss Growth Hacker

  • [nit] The quickstart uses a canonical target-matrix link instead of an inline target list at docs/src/content/docs/getting-started/first-package.md:235
    Inline examples could reduce one click, although the canonical link is less likely to drift as targets evolve.

Auth Expert -- inactive

PR touches install finalize behavior, docs, tests, changelog, and conformance artifacts with no auth, token, credential, or host-resolution surface.

Doc Writer

No findings.

Test Coverage Expert

No findings.

Performance Expert

  • [nit] Full-scan cost remains linear when no instruction match exists at src/apm_cli/install/phases/finalize.py:89
    Measured worst-case cost is still noise-tier at current project sizes; monitor only if dependency trees grow substantially.
  • [nit] Lazy imports are correctly scoped
    Deferred imports avoid unrelated CLI startup cost and are amortized by the finalize phase.

This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.

Reconcile concurrent OpenAPM requirement additions as v0.1.17 and v0.1.18.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel
danielmeppiel merged commit cd78c86 into main Jul 17, 2026
18 checks passed
@danielmeppiel
danielmeppiel deleted the supersede/pr-2115 branch July 17, 2026 20:59
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.

[BUG] Gemini instructions are not compiled into GEMINI.md when installing a local package

2 participants