Skip to content

Commit 9eede9d

Browse files
committed
refactor: address remaining panel review items
- Fix stale _CONFIG_KEY_DISPLAY_NAMES entry (ssh -> prefer_ssh) - Lowercase bool output in apm config get (True/False -> true/false) - Suppress false-default transport rows in apm config / apm config get - Success message: "set to true/false" instead of "enabled/disabled" - FALLBACK_HINT: mention apm config set as persistent alternative config.py - Add _unset_config_key(key) private helper; delegate all four unset_* functions through it (DRY, Python Architect recommended) commands/config.py - Emit logger.warning() when allow-protocol-fallback is persisted in a CI environment ($CI set) advising APM_ALLOW_PROTOCOL_FALLBACK=1 for invocation-scoped use instead (Supply Chain Security recommended) install/validation.py - Rename allow_fallback_env -> allow_fallback; _env suffix was misleading after the value started resolving from config too (Auth Expert nit) deps/github_downloader.py - Update __init__ docstring for protocol_pref and allow_fallback to accurately describe env > config > default resolution instead of implying env-only lookups (Auth Expert recommended) CHANGELOG.md - Merge second ### Added block bullets into first; restore ### Fixed section; one subsection per type per release per Keep a Changelog (Doc Writer recommended) docs/getting-started/authentication.md - Add forward-pointer to "apm config set prefer-ssh false" after the APM_GIT_PROTOCOL=https escape-hatch block; highest-intent conversion point for the new config surface (OSS Growth Hacker recommended)
1 parent 1ead6cc commit 9eede9d

10 files changed

Lines changed: 136 additions & 72 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1010
### Added
1111

1212
- `apm config set prefer-ssh true` / `apm config set allow-protocol-fallback true` persist transport preferences to `~/.apm/config.json` so SSH-only and corporate GHES users no longer need to re-pass `--ssh` / `--allow-protocol-fallback` (or export env vars in shell profiles) on every `apm install`. Resolution order: CLI flag > `APM_GIT_PROTOCOL` / `APM_ALLOW_PROTOCOL_FALLBACK` env var > `apm config` > built-in default. `apm config unset prefer-ssh` and `apm config unset allow-protocol-fallback` remove the persisted value. (#1243)
13+
- `apm pack --marketplace=FORMATS` filters which marketplace formats are built in a single run; accepts comma-separated names and sentinels `all`/`none`. (#1317)
14+
- `apm pack --marketplace-path FORMAT=PATH` overrides the output path for a specific marketplace format at invocation time. Env var overrides (`APM_MARKETPLACE_<FORMAT>_PATH`) are planned for v0.15. (#1317)
15+
- `apm pack --json` emits a stable JSON contract to stdout (`{ok, dry_run, warnings, errors, marketplace: {outputs: [{format, path, ...}]}}`); all logs move to stderr so downstream tooling can `jq` the output safely. (#1317)
16+
- `marketplace.outputs` in `apm.yml` now accepts a map form keyed by format name (`outputs: {claude: {}, codex: {path: ...}}`), replacing the deprecated list form; the list form still parses with a one-cycle deprecation warning. (#1317)
17+
- `apm marketplace init` now scaffolds the explicit map-form `outputs: {claude: {}}` so the default state is observable in the manifest. (#1317)
1318

1419
### Fixed
1520

@@ -18,14 +23,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1823
- Non-skill integrators (agent, instruction, prompt, command, hook script-copy) silently adopt byte-identical pre-existing files so a degraded `deployed_files=[]` lockfile no longer permanently blocks installs gated by `required-packages-deployed`. (#1313)
1924
- `apm audit` drift check now returns skip-with-info (`passed=True`) when the install cache is cold, instead of failing the audit; bare `apm audit` surfaces the skip reason on stderr so CI pipelines that have not yet run `apm install` are not incorrectly red-marked. (#1289)
2025

21-
### Added
22-
23-
- `apm pack --marketplace=FORMATS` filters which marketplace formats are built in a single run; accepts comma-separated names and sentinels `all`/`none`. (#1317)
24-
- `apm pack --marketplace-path FORMAT=PATH` overrides the output path for a specific marketplace format at invocation time. Env var overrides (`APM_MARKETPLACE_<FORMAT>_PATH`) are planned for v0.15. (#1317)
25-
- `apm pack --json` emits a stable JSON contract to stdout (`{ok, dry_run, warnings, errors, marketplace: {outputs: [{format, path, ...}]}}`); all logs move to stderr so downstream tooling can `jq` the output safely. (#1317)
26-
- `marketplace.outputs` in `apm.yml` now accepts a map form keyed by format name (`outputs: {claude: {}, codex: {path: ...}}`), replacing the deprecated list form; the list form still parses with a one-cycle deprecation warning. (#1317)
27-
- `apm marketplace init` now scaffolds the explicit map-form `outputs: {claude: {}}` so the default state is observable in the manifest. (#1317)
28-
2926
### Changed
3027

3128
- `--marketplace-output PATH` is now hidden from `--help` and emits a stderr deprecation warning; it auto-translates to `--marketplace-path claude=PATH`. Removal tracked in #1318. (#1317)

docs/src/content/docs/getting-started/authentication.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,16 @@ apm install --https
449449
export APM_GIT_PROTOCOL=https
450450
```
451451

452+
To persist the HTTPS preference across all future installs without re-exporting the variable:
453+
454+
```bash
455+
apm config set prefer-ssh false # explicit: never prefer SSH for shorthand deps
456+
# or, if you want APM to always try HTTPS for shorthand deps:
457+
# export APM_GIT_PROTOCOL=https # process-scoped; add to shell profile for persistence
458+
```
459+
460+
See [apm config](../../reference/cli/config/) for the full transport-preference config surface.
461+
452462
## Choosing transport (SSH vs HTTPS)
453463

454464
Authentication and transport are independent decisions:

docs/src/content/docs/reference/cli/config.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ Write `KEY` to `~/.apm/config.json`. Validates the value before writing:
4545

4646
### `apm config unset KEY`
4747

48-
Remove `KEY` from `~/.apm/config.json`. No-op if the key is not set. Only `temp-dir` and `copilot-cowork-skills-dir` are unsettable; boolean keys are reset by `set`-ing them to their default.
48+
Remove `KEY` from `~/.apm/config.json`. No-op if the key is not set. All settable keys are unsettable: `temp-dir`, `copilot-cowork-skills-dir`, `prefer-ssh`, and `allow-protocol-fallback`. After unsetting a key the effective value falls back to the environment variable, then the built-in default.
4949

5050
## Configuration keys
5151

docs/src/content/docs/reference/environment-variables.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ Controls how APM clones packages from Git hosts. These settings can also be pers
3737

3838
| Variable | Purpose | Default | Notes |
3939
|---|---|---|---|
40-
| `APM_GIT_PROTOCOL` | Preferred clone protocol for shorthand (`owner/repo`) dependencies. Accepted values: `ssh`, `https`. | unset | Equivalent to `--ssh` / `--https` flag. Resolution: CLI flag → env var → `ssh` key in `~/.apm/config.json` → git `insteadOf` rules → HTTPS. |
40+
| `APM_GIT_PROTOCOL` | Preferred clone protocol for shorthand (`owner/repo`) dependencies. Accepted values: `ssh`, `https`. | unset | Equivalent to `--ssh` / `--https` flag. Resolution: CLI flag → env var → `prefer-ssh` key in `~/.apm/config.json` → git `insteadOf` rules → HTTPS. |
4141
| `APM_ALLOW_PROTOCOL_FALLBACK` | Set to `1` (or `true`/`yes`/`on`) to enable the legacy cross-protocol fallback chain. When enabled, a failed clone is retried with the opposite protocol. | unset | Equivalent to `--allow-protocol-fallback`. Resolution: CLI flag → env var → `allow-protocol-fallback` key in `~/.apm/config.json``false`. |
4242

4343
## Registry (MCP and proxy)

src/apm_cli/commands/config.py

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
"temp_dir": "temp-dir",
2222
"copilot_cowork_skills_dir": "copilot-cowork-skills-dir",
2323
"allow_protocol_fallback": "allow-protocol-fallback",
24-
"ssh": "ssh",
24+
"prefer_ssh": "prefer-ssh",
2525
}
2626

2727

@@ -134,8 +134,14 @@ def config(ctx):
134134
if _temp_dir_val:
135135
config_table.add_row("", "Temp Directory", _temp_dir_val)
136136

137-
config_table.add_row("", "Allow Protocol Fallback", str(_get_apf()))
138-
config_table.add_row("", "Prefer SSH Transport", str(_get_prefer_ssh_cfg()))
137+
# Only surface transport keys when they have been enabled -- the
138+
# false-default rows add noise for users who never configured them.
139+
_apf = _get_apf()
140+
_prefer_ssh = _get_prefer_ssh_cfg()
141+
if _apf:
142+
config_table.add_row("", "Allow Protocol Fallback", "true")
143+
if _prefer_ssh:
144+
config_table.add_row("", "Prefer SSH Transport", "true")
139145

140146
from ..core.experimental import is_enabled as _is_enabled
141147

@@ -178,8 +184,8 @@ def config(ctx):
178184
if _temp_dir_fb:
179185
click.echo(f" Temp Directory: {_temp_dir_fb}")
180186

181-
click.echo(f" allow-protocol-fallback: {_get_apf_fb()}")
182-
click.echo(f" prefer-ssh: {_get_prefer_ssh_fb()}")
187+
click.echo(f" allow-protocol-fallback: {str(_get_apf_fb()).lower()}")
188+
click.echo(f" prefer-ssh: {str(_get_prefer_ssh_fb()).lower()}")
183189

184190
from ..core.experimental import is_enabled as _is_enabled_fb
185191

@@ -252,10 +258,20 @@ def set(key, value): # noqa: F811
252258

253259
setter, label = config_entry
254260
setter(enabled)
255-
if enabled:
256-
logger.success(f"{label} enabled")
257-
else:
258-
logger.success(f"{label} disabled")
261+
logger.success(f"{label} set to {'true' if enabled else 'false'}")
262+
263+
# Warn when persisting allow-protocol-fallback=true in a CI environment where
264+
# $HOME is often shared across jobs -- the persisted value will affect all
265+
# subsequent apm install runs on that host. The env var is safer for CI.
266+
import os as _os
267+
268+
if key == "allow-protocol-fallback" and enabled and _os.environ.get("CI"):
269+
logger.warning(
270+
"allow-protocol-fallback is now persisted to ~/.apm/config.json. "
271+
"In CI environments with a shared $HOME this will affect all subsequent "
272+
"apm install runs on this host. "
273+
"Prefer APM_ALLOW_PROTOCOL_FALLBACK=1 as an invocation-scoped alternative."
274+
)
259275

260276

261277
@config.command(help="Get a configuration value")
@@ -299,21 +315,30 @@ def get(key):
299315
)
300316
sys.exit(1)
301317
value = getter()
302-
click.echo(f"{key}: {value}")
318+
# Render booleans as lowercase true/false (npm convention).
319+
if isinstance(value, bool):
320+
click.echo(f"{key}: {str(value).lower()}")
321+
else:
322+
click.echo(f"{key}: {value}")
303323
else:
304324
# Show all user-settable keys with their effective values (including
305325
# defaults). Iterating raw config keys would hide settings that
306326
# have not been written yet (e.g. auto_integrate on a fresh install).
307327
from ..config import get_allow_protocol_fallback, get_prefer_ssh
308328

309329
logger.progress("APM Configuration:")
310-
click.echo(f" auto-integrate: {get_auto_integrate()}")
330+
click.echo(f" auto-integrate: {str(get_auto_integrate()).lower()}")
311331
temp_dir = get_temp_dir()
312332
click.echo(
313333
f" temp-dir: {temp_dir if temp_dir is not None else 'Not set (using system default)'}"
314334
)
315-
click.echo(f" allow-protocol-fallback: {get_allow_protocol_fallback()}")
316-
click.echo(f" prefer-ssh: {get_prefer_ssh()}")
335+
# Only show transport keys when non-default to reduce noise.
336+
_apf_val = get_allow_protocol_fallback()
337+
_ssh_val = get_prefer_ssh()
338+
if _apf_val:
339+
click.echo(" allow-protocol-fallback: true")
340+
if _ssh_val:
341+
click.echo(" prefer-ssh: true")
317342

318343
from ..core.experimental import is_enabled as _is_enabled_get
319344

src/apm_cli/config.py

Lines changed: 31 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""Configuration management for APM."""
22

3+
import contextlib
34
import json
45
import os
56
from typing import Optional # noqa: F401
@@ -18,13 +19,21 @@
1819

1920

2021
def ensure_config_exists():
21-
"""Ensure the configuration directory and file exist."""
22+
"""Ensure the configuration directory and file exist.
23+
24+
The directory is created with mode ``0o700`` (owner-only) and the
25+
initial config file with mode ``0o600`` to prevent other users on a
26+
shared system from reading persisted tokens or transport preferences.
27+
Both restrictions are silently ignored on Windows.
28+
"""
2229
if not os.path.exists(CONFIG_DIR):
23-
os.makedirs(CONFIG_DIR)
30+
os.makedirs(CONFIG_DIR, mode=0o700, exist_ok=True)
2431

2532
if not os.path.exists(CONFIG_FILE):
2633
with open(CONFIG_FILE, "w", encoding="utf-8") as f:
2734
json.dump({"default_client": "vscode"}, f)
35+
with contextlib.suppress(NotImplementedError):
36+
os.chmod(CONFIG_FILE, 0o600)
2837

2938

3039
def get_config():
@@ -133,20 +142,32 @@ def set_temp_dir(path: str) -> None:
133142
update_config({"temp_dir": resolved})
134143

135144

136-
def unset_temp_dir() -> None:
137-
"""Remove the ``temp_dir`` key from the config file.
145+
def _unset_config_key(key: str) -> None:
146+
"""Remove *key* from the config file atomically.
138147
139-
No-op if the key is not present.
148+
No-op when *key* is not present. Invalidates the in-process cache
149+
before and after the write so subsequent reads see the updated state.
150+
151+
Args:
152+
key: The JSON key to remove from ``~/.apm/config.json``.
140153
"""
141154
_invalidate_config_cache()
142155
config = get_config()
143-
if "temp_dir" in config:
144-
del config["temp_dir"]
156+
if key in config:
157+
del config[key]
145158
with open(CONFIG_FILE, "w", encoding="utf-8") as f:
146159
json.dump(config, f, indent=2)
147160
_invalidate_config_cache()
148161

149162

163+
def unset_temp_dir() -> None:
164+
"""Remove the ``temp_dir`` key from the config file.
165+
166+
No-op if the key is not present.
167+
"""
168+
_unset_config_key("temp_dir")
169+
170+
150171
# ---------------------------------------------------------------------------
151172
# Protocol transport preferences (issue #1243)
152173
# ---------------------------------------------------------------------------
@@ -196,13 +217,7 @@ def unset_allow_protocol_fallback() -> None:
196217
``APM_ALLOW_PROTOCOL_FALLBACK`` env var and then the built-in
197218
default (``False``).
198219
"""
199-
_invalidate_config_cache()
200-
config = get_config()
201-
if "allow_protocol_fallback" in config:
202-
del config["allow_protocol_fallback"]
203-
with open(CONFIG_FILE, "w", encoding="utf-8") as f:
204-
json.dump(config, f, indent=2)
205-
_invalidate_config_cache()
220+
_unset_config_key("allow_protocol_fallback")
206221

207222

208223
def unset_prefer_ssh() -> None:
@@ -212,13 +227,7 @@ def unset_prefer_ssh() -> None:
212227
:func:`get_apm_protocol_pref` will fall through to the
213228
``APM_GIT_PROTOCOL`` env var and then the built-in default (``None``).
214229
"""
215-
_invalidate_config_cache()
216-
config = get_config()
217-
if "prefer_ssh" in config:
218-
del config["prefer_ssh"]
219-
with open(CONFIG_FILE, "w", encoding="utf-8") as f:
220-
json.dump(config, f, indent=2)
221-
_invalidate_config_cache()
230+
_unset_config_key("prefer_ssh")
222231

223232

224233
def _parse_allow_protocol_fallback_env(raw: str | None) -> bool | None:
@@ -325,13 +334,7 @@ def unset_copilot_cowork_skills_dir() -> None:
325334
326335
No-op if the key is not present.
327336
"""
328-
_invalidate_config_cache()
329-
config = get_config()
330-
if "copilot_cowork_skills_dir" in config:
331-
del config["copilot_cowork_skills_dir"]
332-
with open(CONFIG_FILE, "w", encoding="utf-8") as f:
333-
json.dump(config, f, indent=2)
334-
_invalidate_config_cache()
337+
_unset_config_key("copilot_cowork_skills_dir")
335338

336339

337340
def get_apm_temp_dir() -> str | None:

src/apm_cli/deps/github_downloader.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,10 +170,14 @@ def __init__(
170170
transport_selector: TransportSelector for protocol decisions.
171171
Defaults to a new selector with GitConfigInsteadOfResolver.
172172
protocol_pref: User-stated transport preference for shorthand
173-
deps. When None, reads APM_GIT_PROTOCOL env.
173+
deps. When None, resolved from ``APM_GIT_PROTOCOL`` env var,
174+
then ``prefer-ssh`` in ``~/.apm/config.json``, then ``None``
175+
(let git insteadOf rules decide).
174176
allow_fallback: When True, permits cross-protocol fallback
175-
(legacy behavior). When None, reads
176-
APM_ALLOW_PROTOCOL_FALLBACK env.
177+
(legacy behavior). When None, resolved from
178+
``APM_ALLOW_PROTOCOL_FALLBACK`` env var, then
179+
``allow-protocol-fallback`` in ``~/.apm/config.json``,
180+
then ``False``.
177181
"""
178182
self.auth_resolver = auth_resolver or AuthResolver()
179183
self.token_manager = self.auth_resolver._token_manager # Backward compat

src/apm_cli/deps/transport_selection.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@
3030
# Documented escape-hatch hint surfaced on strict-mode failures.
3131
FALLBACK_HINT = (
3232
"To allow cross-protocol fallback (not recommended), pass "
33-
"--allow-protocol-fallback or set APM_ALLOW_PROTOCOL_FALLBACK=1."
33+
"--allow-protocol-fallback, set APM_ALLOW_PROTOCOL_FALLBACK=1, "
34+
"or run: apm config set allow-protocol-fallback true"
3435
)
3536

3637

src/apm_cli/install/validation.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ def _warn(msg: str) -> None:
314314
# the clone path honors) restores the legacy permissive chain.
315315
from apm_cli.config import get_apm_allow_protocol_fallback
316316

317-
allow_fallback_env = get_apm_allow_protocol_fallback()
317+
allow_fallback = get_apm_allow_protocol_fallback()
318318

319319
# For generic hosts (not GitHub, not ADO), relax the env so native
320320
# credential helpers (macOS Keychain, credential-store,
@@ -349,11 +349,9 @@ def _warn(msg: str) -> None:
349349
dep_ref.repo_url, use_ssh=True, dep_ref=dep_ref
350350
)
351351
if explicit_scheme in ("http", "https"):
352-
urls_to_try = (
353-
[package_url] if not allow_fallback_env else [package_url, ssh_url]
354-
)
352+
urls_to_try = [package_url] if not allow_fallback else [package_url, ssh_url]
355353
elif explicit_scheme == "ssh":
356-
urls_to_try = [ssh_url] if not allow_fallback_env else [ssh_url, package_url]
354+
urls_to_try = [ssh_url] if not allow_fallback else [ssh_url, package_url]
357355
else:
358356
# Shorthand has no user-stated transport; keep the legacy
359357
# SSH-first chain so existing flows (e.g. SSH-key users on

0 commit comments

Comments
 (0)