[BUG] _fallback_port_warned set has no lock; concurrent _clone_with_fallback calls can race on dedup

[edenfunf]

Follow-up from PR #789 review panel -- auth-expert "Follow-up B: `_fallback_port_warned` thread-safety," deliberately deferred per the verdict ("CLI is single-threaded today; revisit if/when we parallelize installs").

Describe the bug

`GitHubPackageDownloader._fallback_port_warned` (`src/apm_cli/deps/github_downloader.py:215`) is a plain `set` with no surrounding lock:

```python

src/apm_cli/deps/github_downloader.py:211-215

Dedup set for the issue #786 cross-protocol port warning: one install

run calls _clone_with_fallback multiple times per dep (ref-resolution

clone, then the actual dep clone). We want the warning exactly once

per (host, repo, port) identity across all those calls.

self._fallback_port_warned: set = set()
```

The add-if-absent sequence at `:776-777` is a check-then-act:

```python
if warn_key not in self._fallback_port_warned:
self._fallback_port_warned.add(warn_key)
...emit warning...
```

Two threads entering this region concurrently for the same `warn_key` can both pass the `not in` check before either `.add()` lands, resulting in a duplicate warning. Worst case is a duplicate `[!]` line; there is no correctness or security fallout.

Today's call graph is single-threaded, so this is latent, not currently observable. The reason to track it now: if/when `apm install` gains parallel downloads (the natural next optimisation -- a dozen deps at O(n) clone wall-time is the single biggest `apm install` cost), the same `GitHubPackageDownloader` instance becomes shared mutable state, and this dedup set is one of the items that has to be hardened together with the rest.

To Reproduce

Not reproducible on current main (single-threaded install loop). Would reproduce the moment we introduce concurrent `_clone_with_fallback` callers against the same downloader instance. Concrete trigger:

1. Parallelise the per-dep loop in `install/phases/download.py` (current site of serial `_clone_with_fallback` calls).
2. Install a manifest with multiple custom-port deps on the same host.
3. Observe duplicate `[!]` warnings for the first dep under `--allow-protocol-fallback`.

Expected behavior

Dedup add-if-absent should be atomic. Two shapes are reasonable:

• Option A (preferred, smallest surface): wrap the check-and-add in a `threading.Lock`, matching the pattern `AuthResolver` already uses for its `_cache` at `core/auth.py:105`.

  ```python
  self._fallback_port_warned_lock = threading.Lock()
  self._fallback_port_warned: set = set()
  ...
  with self._fallback_port_warned_lock:
  if warn_key in self._fallback_port_warned:
  return # already warned -- skip
  self._fallback_port_warned.add(warn_key)
  ...emit warning outside the lock...
  ```

• Option B: replace the set with a thread-safe dedup primitive (`weakref.WeakSet` is not the right fit; a `queue.SimpleQueue` flip or a class-level `threading.local` is overkill). Mentioning for completeness; Option A is correct.

Add the fix alongside the parallelism PR so the invariant lands and gets exercised in the same change.

Environment

• Not currently reproducible; single-threaded CLI.
• Will surface once `apm install` ships concurrent downloads.

Additional context

• `AuthResolver` already uses the single-`threading.Lock` pattern for its cache -- following the same idiom keeps the module-local concurrency story consistent.
• Lockfile writes and `_credential_cache` in `GitHubTokenManager` will need similar hardening when parallelism lands; this issue can become part of a broader "parallel-install thread-safety audit" umbrella.
• Scope: two-line change (lock init + `with` around the check-then-act) plus one concurrency test.

Refs

• PR #789 (cross-protocol fallback warning)
• PR #789 review verdict, Follow-up B
• Sibling: #800 (dedup key case-sensitivity -- same dedup set, adjacent correctness concern)

[edenfunf]

@danielmeppiel -- quick scoping question before I queue this up.

Your merge verdict on #789 said "revisit when/if we parallelize installs," so the default path is to defer, and I want to respect that. One thing I wanted to float, just in case: the actual fix is small -- a threading.Lock mirroring the one already on AuthResolver._cache, wrapped around the check-then-act at github_downloader.py:776-777. Two source lines + one concurrency test.

Two ways to sequence it:

1. Defer (your original call) -- bundle the hardening with the parallelism PR when it lands, together with _credential_cache and lockfile writes, so the whole shared-state audit lands in one place.
2. Land now as defensive hygiene -- add the lock + test; zero behavior change on the current single-threaded path, one less item the parallelism PR has to remember to audit later.

I can see the YAGNI argument for (1) -- adding synchronization for a non-observable race is the textbook case for deferring. I tentatively leaned (2) because the change is so small, but that might just be premature-optimization brain on my end, and you have much better context on how the parallelism work is likely to shape up.

Which way would you like me to take it?