Skip to content

fix(install): do not treat an empty skill directory as an unmanaged collision - #3449

Open
Mohammed Alkindi (MohammedAlkindi) wants to merge 1 commit into
microsoft:mainfrom
MohammedAlkindi:fix/empty-skill-dir-collision
Open

Mohammed Alkindi (MohammedAlkindi) wants to merge 1 commit into
microsoft:mainfrom
MohammedAlkindi:fix/empty-skill-dir-collision

Conversation

@MohammedAlkindi

Copy link
Copy Markdown
Contributor

Description

apm install treats an empty pre-existing skill directory as an unmanaged collision because target_path.exists() is true, skipping the skill while reporting success. check_collision and target_existed now verify whether a directory contains files, deploying into empty directory trees.

Issue and approved scope

Issue: Fixes #3441

Human scope-approval comment: #3441

Does this PR complete the issue, or what remains? Completes the issue.

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Maintenance / refactor

Testing

  • Tested locally
  • All existing tests pass
  • Added tests for new functionality (if applicable)

Spec conformance (OpenAPM v0.1)

  • Spec edit: docs/src/content/docs/specs/openapm-v0.1.md updated
  • Manifest edit: docs/src/content/docs/specs/manifests/openapm-v0.1.requirements.yml updated.
  • Test edit: a @pytest.mark.req("req-XXX") test under tests/spec_conformance/ added or extended.
  • CONFORMANCE.{md,json} regenerated via uv run --extra dev python -m tests.spec_conformance.gen_statement and committed.
  • N/A -- this PR does not change OpenAPM-observable behaviour.

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.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

This branch has not been deployed

No deployments
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] An empty pre-existing skill directory makes apm install silently skip the skill (exit 0, nothing deployed)

2 participants