Skip to content

Commit d2733bf

Browse files
danielmeppielDaniel MeppielCopilot
authored
fix(install,test): exclude cowork from --target all + stabilize defer-start timer test (#1191)
* fix(install,test): exclude cowork from --target all + stabilize defer-start timer test active_targets() for explicit_target='all' (project scope) was returning KNOWN_TARGETS minus EXPLICIT_ONLY_TARGETS, which still includes the experimental copilot-cowork target. Cowork is user-scope-only, so the unconditional project-scope gate in phases/targets.py raised SystemExit and aborted any 'apm install --target all' invocation. This made gh-aw shared workflows that pin 'target: all' unusable (run 25511043293). Excluding EXPERIMENTAL_TARGETS from the 'all' expansion matches the documented contract on EXPERIMENTAL_TARGETS in core/target_detection.py ('NOT included in parse_target_arg("all") expansion -- explicit opt-in only'). Updated three test_targets.py expectations and flipped test_cowork_absent_from_all_when_flag_off, which previously asserted the buggy behaviour with a misleading 'this is documented' comment. Added a parallel test for the flag-on case. Separately, test_install_over_defer_threshold_starts_live_once was flaky on macOS x86_64 (run 25496089562) because it waited only 100ms beyond _DEFER_SHOW_S before asserting the mocked _defer_start fired. threading.Timer scheduling under parallel pytest-xdist load can slip past that window. Joining tui._timer with a 2s timeout before the with block exits makes the assertion deterministic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(changelog): backfill PR number #1191 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Daniel Meppiel <copilot-rework@github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 8c21a50 commit d2733bf

5 files changed

Lines changed: 67 additions & 24 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1414
### Fixed
1515

1616
- `shared/apm.md` no longer wraps the `target` input in a `|| 'all'` fallback. The defensive expression broke gh-aw's bare-expression substitution regex, causing consumer-supplied `target:` values to be silently dropped; the `import-schema` default already covers the omitted-input case. (#1185)
17+
- `apm install --target all` no longer enumerates the experimental `copilot-cowork` target, which was crashing project-scope installs with a "requires --global" error and made `gh aw` workflows that pin `target: all` unusable. (#1191)
18+
- Stabilized `test_install_over_defer_threshold_starts_live_once` on slow CI runners by joining the deferred-start timer thread instead of relying on a 100ms grace window. (#1191)
1719

1820
## [0.12.4] - 2026-05-07
1921

src/apm_cli/integration/targets.py

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -775,14 +775,24 @@ def active_targets(
775775
for t in raw:
776776
canonical = "copilot" if t in ("copilot", "vscode", "agents") else t
777777
if canonical == "all":
778-
# Return all targets regardless of flag gating.
779778
# Exclude explicit-only targets (agent-skills) -- they must
780779
# be requested individually.
781-
# The project-scope gate in phases/targets.py and
782-
# for_scope() handle user-observable blocking.
783-
from apm_cli.core.target_detection import EXPLICIT_ONLY_TARGETS
780+
# Exclude experimental targets (copilot-cowork) -- they must
781+
# be opted into explicitly via `--target copilot-cowork`,
782+
# matching the documented contract on EXPERIMENTAL_TARGETS in
783+
# core/target_detection.py. Including cowork in `all` for
784+
# project scope hits the unconditional project-scope gate in
785+
# phases/targets.py and aborts the entire install (#1185 b).
786+
from apm_cli.core.target_detection import (
787+
EXPERIMENTAL_TARGETS,
788+
EXPLICIT_ONLY_TARGETS,
789+
)
784790

785-
return [p for p in KNOWN_TARGETS.values() if p.name not in EXPLICIT_ONLY_TARGETS]
791+
return [
792+
p
793+
for p in KNOWN_TARGETS.values()
794+
if p.name not in EXPLICIT_ONLY_TARGETS and p.name not in EXPERIMENTAL_TARGETS
795+
]
786796
profile = KNOWN_TARGETS.get(canonical)
787797
if profile and _flag_gated(profile) and profile.name not in seen:
788798
seen.add(profile.name)

tests/unit/integration/test_copilot_cowork_target.py

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -160,17 +160,29 @@ def test_cowork_absent_when_flag_off_explicit_cowork(
160160
assert results == []
161161

162162
def test_cowork_absent_from_all_when_flag_off(self, tmp_path: Path, inject_config: Any) -> None:
163+
"""`--target all` (project scope) must NOT include cowork.
164+
165+
cowork is in EXPERIMENTAL_TARGETS; per the documented contract in
166+
core/target_detection.py it is opt-in only -- explicit
167+
``--target copilot-cowork`` -- and never resolved via ``all``.
168+
Including it in the project-scope ``all`` set hits the
169+
project-scope gate in phases/targets.py and aborts the install.
170+
"""
163171
inject_config({"experimental": {"copilot_cowork": False}})
164172
results = active_targets(tmp_path, explicit_target="all")
165173
names = [t.name for t in results]
166-
# "all" returns all targets regardless of flag gating
167-
# but explicit_target="copilot-cowork" with flag off returns []
168-
# The "all" path returns list(KNOWN_TARGETS.values()) which
169-
# includes cowork. This is documented: "all" bypasses flag gate.
170-
# So cowork IS in the "all" set even when flag is off.
171-
# This matches the implementation comment:
172-
# "Return all targets regardless of flag gating."
173-
assert "copilot-cowork" in names
174+
assert "copilot-cowork" not in names
175+
176+
def test_cowork_absent_from_all_when_flag_on(self, tmp_path: Path, inject_config: Any) -> None:
177+
"""`--target all` (project scope) excludes cowork even when the
178+
experimental flag is enabled. cowork is user-scope only and the
179+
project-scope gate would error otherwise; ``all`` honors the
180+
documented EXPERIMENTAL_TARGETS exclusion regardless of flag.
181+
"""
182+
inject_config({"experimental": {"copilot_cowork": True}})
183+
results = active_targets(tmp_path, explicit_target="all")
184+
names = [t.name for t in results]
185+
assert "copilot-cowork" not in names
174186

175187
def test_cowork_absent_when_flag_on_resolver_returns_none(
176188
self, tmp_path: Path, inject_config: Any

tests/unit/integration/test_targets.py

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,17 @@ def test_explicit_claude(self):
7676
assert [t.name for t in targets] == ["claude"]
7777

7878
def test_explicit_all_returns_every_known_target(self):
79-
from apm_cli.core.target_detection import EXPLICIT_ONLY_TARGETS
79+
from apm_cli.core.target_detection import (
80+
EXPERIMENTAL_TARGETS,
81+
EXPLICIT_ONLY_TARGETS,
82+
)
8083

8184
targets = active_targets(self.root, explicit_target="all")
82-
assert len(targets) == len(KNOWN_TARGETS) - len(EXPLICIT_ONLY_TARGETS)
85+
expected = len(KNOWN_TARGETS) - len(EXPLICIT_ONLY_TARGETS) - len(EXPERIMENTAL_TARGETS)
86+
assert len(targets) == expected
87+
names = [t.name for t in targets]
88+
assert "copilot-cowork" not in names
89+
assert "agent-skills" not in names
8390

8491
def test_explicit_vscode_alias(self):
8592
targets = active_targets(self.root, explicit_target="vscode")
@@ -193,17 +200,25 @@ def test_explicit_list_deduplicates_aliases(self):
193200
assert [t.name for t in targets] == ["copilot"]
194201

195202
def test_explicit_list_with_all_returns_every_known_target(self):
196-
from apm_cli.core.target_detection import EXPLICIT_ONLY_TARGETS
203+
from apm_cli.core.target_detection import (
204+
EXPERIMENTAL_TARGETS,
205+
EXPLICIT_ONLY_TARGETS,
206+
)
197207

198208
targets = active_targets(self.root, explicit_target=["all"])
199-
assert len(targets) == len(KNOWN_TARGETS) - len(EXPLICIT_ONLY_TARGETS)
209+
expected = len(KNOWN_TARGETS) - len(EXPLICIT_ONLY_TARGETS) - len(EXPERIMENTAL_TARGETS)
210+
assert len(targets) == expected
200211

201212
def test_explicit_list_all_mixed_returns_every_known_target(self):
202213
"""'all' anywhere in the list wins."""
203-
from apm_cli.core.target_detection import EXPLICIT_ONLY_TARGETS
214+
from apm_cli.core.target_detection import (
215+
EXPERIMENTAL_TARGETS,
216+
EXPLICIT_ONLY_TARGETS,
217+
)
204218

205219
targets = active_targets(self.root, explicit_target=["claude", "all"])
206-
assert len(targets) == len(KNOWN_TARGETS) - len(EXPLICIT_ONLY_TARGETS)
220+
expected = len(KNOWN_TARGETS) - len(EXPLICIT_ONLY_TARGETS) - len(EXPERIMENTAL_TARGETS)
221+
assert len(targets) == expected
207222

208223
def test_explicit_list_all_unknown_returns_empty(self):
209224
"""When the parser is bypassed and all tokens are unknown, the

tests/unit/test_install_tui.py

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -132,12 +132,16 @@ def test_install_over_defer_threshold_starts_live_once(
132132
with patch.object(InstallTui, "_defer_start", autospec=True) as mock_defer:
133133
with tui:
134134
# Sleep slightly longer than the defer window so the
135-
# timer fires before __exit__ cancels it.
135+
# timer fires before __exit__ cancels it, then join the
136+
# Timer thread to deterministically wait for the
137+
# callback to complete. Without the join this test is
138+
# flaky on busy CI runners where threading.Timer
139+
# scheduling can slip past a fixed grace window.
136140
time.sleep(_DEFER_SHOW_S + 0.10)
137-
# Either the timer fired (preferred) or it was cancelled.
138-
# We assert at most one call -- never multiple.
139-
assert mock_defer.call_count <= 1
140-
# In the typical case the timer fires; assert it did.
141+
if tui._timer is not None:
142+
tui._timer.join(timeout=2.0)
143+
# The timer must have fired exactly once (deterministic
144+
# via the join above).
141145
assert mock_defer.call_count == 1
142146

143147

0 commit comments

Comments
 (0)