Skip to content

feat(copilot-app): hybrid WS-IPC + SQLite project scoping for workflows#1431

Merged
danielmeppiel merged 11 commits into
mainfrom
danielmeppiel/feat-copilot-app-ws-project-scope
May 21, 2026
Merged

feat(copilot-app): hybrid WS-IPC + SQLite project scoping for workflows#1431
danielmeppiel merged 11 commits into
mainfrom
danielmeppiel/feat-copilot-app-ws-project-scope

Conversation

@danielmeppiel

@danielmeppiel danielmeppiel commented May 21, 2026

Copy link
Copy Markdown
Collaborator

feat(copilot-app): scope workflows to a real project via WS project-registration + SQLite

TL;DR

apm install --target copilot-app previously wrote every workflow with project_id = NULL, so the App ran them with CWD ~/.copilot and the Workflows tab could white-screen on a workflow attached to an unknown project. This PR resolves the current repo to a real Copilot App project and stamps project_id on every workflow row. When the App is running, project registration goes through the App's authenticated loopback WebSocket (~/.copilot/run/ws.{port,token}) so the project flows through the App's own owner/repo discovery and is immediately known to the webview; when the App is closed, it falls through to a direct-SQLite BEGIN IMMEDIATE resolver. Workflow rows are always written via SQLite (namespaced ids keep lockfile entries stable across both branches). The previous CWD-pivot and white-screen failure modes go away.

Note

Tracks upstream github/github-app#5483 (request that ProjectsChanged become a global broadcast). Until that lands, the first install in a new repo emits a one-time "restart the Copilot App once" hint.

Problem (WHY)

  • Workflows installed with project_id = NULL. App "Run now" CWDs to ~/.copilot (containing mcp-config.json, hooks/, session-store.db, agents/), a clear least-privilege violation and a malicious-prompt pivot. Verified live; full repro in the originating session's livedb-findings.md.
  • Workflows tab white-screened on first install per repo when the row's project_id pointed at a project the webview projectStore had never loaded. Empirically reproduced; root cause traced to the upstream asymmetry WorkflowsChanged (send_global) vs ProjectCreated/Updated/Deleted (per-connection ctx.tx).
  • --global + workflow-shape silently produced unattached workflows. The flag was accepted but yielded a workflow with no project binding and no warning, so users never learned why "Run now" misbehaved.

Why these matter: APM's contract is Secure by default and DevX (pragmatic as npm). A workflow that runs from ~/.copilot violates the first; a white-screen on first install violates the second. The asymmetry that drives the white-screen is documented at src-tauri/src/handlers/workflows.rs:40 (event_bus.send_global(WorkflowsChanged)) vs src-tauri/src/handlers/workspace.rs:216 (ctx.tx.send(ProjectCreated{...})) — captured in the reverse-engineering report.

Approach (WHAT)

# Fix
1 Resolve current repo → existing Copilot App project (HIT) or auto-register it (MISS).
2 Stamp project_id on every workflows row written by APM; suffix display name with (<repo-basename>) for disambiguation.
3 When the App is running (~/.copilot/run/ws.{port,token} + TCP probe), register the project over WS so the App runs full discovery and the project is immediately known to the webview. When the App is closed, register via direct SQLite. Workflow rows are written via SQLite in both branches.
4 --global + workflow-shape now warns (does not fail) with explicit CWD-pivot security callout; install proceeds with NULL project_id since user explicitly opted into user-scope.

Implementation (HOW)

  • src/apm_cli/integration/copilot_app_project.py (new, 401 lines) — pure derive_repo_context() + derive_project_recipe() (no I/O, snapshot-testable) and a SQLite-fallback resolve_or_register_project() that reuses copilot_app_db._connect / _check_user_version / _begin_immediate_with_retry and handles UNIQUE-collision race recovery on main_repo_path.
  • src/apm_cli/integration/copilot_app_ws.py (new, ~440 lines) — sync WS client built on websockets.sync.client.connect. Narrow public surface: ws_available() (file-presence + 100 ms TCP probe) and WsClient context-manager with create_project_from_path only. Origin tauri://localhost, token in URI query, permissive parser tolerates unknown fields, drain-interleaves push notifications. Custom exceptions: WsError, WsAppNotRunning, WsAuthError, WsProtocolError. Defends two credential surfaces: refuses to read ~/.copilot/run/ws.token if its mode is group- or other-readable (matches the App's documented 0o600 posture), and scrubs ?token=... out of any library exception text before wrapping it into WsError so credential material cannot land in diagnostics.
  • src/apm_cli/integration/copilot_app_db.pyWorkflowRow.project_id: str | None = None (optional, last field, non-breaking). INSERT + both UPDATE branches (the bare-UPDATE branch self-heals pre-PR NULL rows on next install).
  • src/apm_cli/integration/prompt_integrator.py_integrate_prompts_for_copilot_app rewritten as a single-loop hybrid dispatcher. Resolves the project ONCE (try WS, fall back to SQLite on WsError; silently swallow WsAppNotRunning and WsAuthError so stale tokens after a restart never surface), then writes every workflow row via SQLite using the resolved project_id. One restart-once hint fires when the resolution branch reports a fresh was_created. Display-name suffix happens here (the only site with RepoContext in scope).
  • pyproject.toml — pinned websockets>=12,<17 to lock the current resolution and prevent a silent major-version upload.
  • src/apm_cli/install/phases/targets.py + src/apm_cli/install/services.py — thread user_scope and project_root into the integrator call site; rewrite the previous gate-comment that mis-described the semantics.
  • Four *_integrator.py files (agent/command/hook/instruction) — receive a scope= kwarg for signature parity. No behavior change in those integrators.
  • tests/unit/integration/test_copilot_app_project.py (new, 14 tests, 96 % cov) — derive_*, recipe projection, HIT, MISS, race-collision recovery, missing-DB + schema-too-new errors.
  • tests/unit/integration/test_copilot_app_ws.py (new, narrowed to project-only surface + token-hardening) — real websockets.sync.server in a background thread; covers handshake (origin, token, refused), create_project_from_path round-trip + project_updated reply + permissive-parser, drain interleaving with push messages, recv timeout, plus TestTokenScrub (3 tests) and TestTokenFileMode (3 tests) for the two new credential defences.
  • tests/integration/test_copilot_app_ws_fallback.py (new, 3 tests) — regression trap for the WS-error → SQLite fallback path: WsProtocolError mid-deploy still produces a project_id-stamped workflow row; WsAppNotRunning and WsAuthError are silently swallowed so stale tokens never warn.
  • tests/unit/install/test_install_target_copilot_app_e2e.py — adapted to expect non-NULL project_id; new tests for project-scope round-trip, --global warn-and-proceed, restart-once hint.

Diagrams

Legend: hot-path dispatch inside _integrate_prompts_for_copilot_app. Green boxes are new branches; the yellow box highlights the WS-error catch that hands off project resolution to SQLite. Workflow rows are written via SQLite in every branch.

flowchart TD
    Start([apm install --target copilot-app])
    Derive[derive_repo_context project_root]
    GlobalQ{user_scope == global<br/>AND any workflow-shape?}
    Warn[/Warn: CWD pivot risk<br/>install with project_id=NULL/]:::new
    WsQ{ws_available?}
    WsTry[WsClient: create_project_from_path]:::new
    SqliteResolve[resolve_or_register_project_sqlite]:::new
    SqliteDeploy[deploy_workflow per row<br/>with resolved project_id]
    Hint[/Emit restart-once hint when was_created/]:::new
    Done([IntegrationResult])

    Start --> Derive --> GlobalQ
    GlobalQ -- yes --> Warn --> SqliteDeploy
    GlobalQ -- no --> WsQ
    WsQ -- yes --> WsTry
    WsTry -- "WsError caught" --> SqliteResolve
    WsQ -- no --> SqliteResolve
    WsTry -- success --> SqliteDeploy
    SqliteResolve --> SqliteDeploy --> Hint --> Done

    classDef new fill:#dff5d6,stroke:#3a7d2b,color:#1a3d10
Loading

Legend: project-registration over WS — auth handshake, project create, then APM closes the WS connection and writes every workflow row via SQLite. The WS surface is intentionally narrow: project registration only, so the App's own owner/repo discovery runs and the project is known to the webview immediately.

sequenceDiagram
    autonumber
    participant APM as apm install
    participant Files as ~/.copilot/run
    participant WS as App WS server
    participant DB as ~/.copilot/data.db

    APM->>Files: read ws.port and ws.token (mode 0o600 check)
    APM->>WS: TCP probe 100ms
    APM->>WS: GET Upgrade Origin=tauri/localhost token=...
    WS-->>APM: 101 Switching Protocols
    APM->>WS: create_project_from_path path=...
    WS-->>APM: project_created project=...
    APM->>DB: BEGIN IMMEDIATE
    loop per workflow row
        APM->>DB: INSERT/UPDATE workflows SET project_id=...
    end
    APM->>DB: COMMIT
Loading

Trade-offs

  • WS-IPC narrowed to project registration only. Considered (and removed in-review) routing workflow rows through a WS create_workflow envelope; rejected because the lockfile contract depends on stable APM-namespaced ids, and the WS surface returns App-side ids that would either need a schema migration or a parallel id map. Project rows do not appear in the lockfile, so the WS surface there is pure win — APM gets the App's authenticated owner/repo discovery for free.
  • Hybrid resolver over WS-only. apm install is a CLI promise — it must work when the App is not running. The SQLite fallback uses the existing retry-aware writer, so both branches converge on the same idempotent state.
  • Warn instead of fail on --global + workflow-shape. Tokens migrated out of data.db (App migration 017), the supply-chain risk is degraded, and --global is the user's explicit opt-in. We surface the CWD-pivot risk in the warning text.
  • Self-heal pre-PR NULL rows via the existing bare-UPDATE branch. Self-heal-on-next-install; rejected a one-shot apm copilot-app migrate-workflows subcommand as over-engineered for a pure data evolution.
  • Stale-token suppression (WsAuthError swallowed). A ws.token left over from a previous App session is a transient condition, not a user error. We swallow it silently and let the SQLite branch resolve the project; the user-visible signal is the standard restart-once hint when a fresh project gets registered.
  • Restart-once hint on was_created=True. Cannot be avoided locally — the App's webview projectStore is hydrated once per WS connect. The right fix is upstream (#5483); the hint disappears as soon as that lands.

Benefits

  1. Workflows attach to the right repo. Run now CWDs to projects.main_repo_path instead of ~/.copilot, eliminating the malicious-prompt CWD pivot for APM-installed workflows.
  2. No more white-screen on first install when the App is running — project registration goes through the App's own validated discovery path, so the webview projectStore knows the project as soon as the row exists.
  3. Token material never leaks into diagnostics. WS handshake URLs are scrubbed before wrapping; ws.token is refused if its mode is group- or other-readable.
  4. 40+ new test cases covering the new dispatcher branches plus regression-traps for race collisions, protocol drift, the WS→SQLite fallback path, and the new credential defences.
  5. Strictly additive at module boundaries. WorkflowRow.project_id defaults to None, so every existing caller compiles and every existing test fixture continues to construct rows the same way.

Validation

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

All checks passed!

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

1040 files already formatted

uv run --extra dev pytest tests/unit/integration/test_copilot_app_project.py tests/unit/integration/test_copilot_app_ws.py tests/unit/install/test_install_target_copilot_app_e2e.py tests/integration/test_copilot_app_ws_fallback.py -q:

......................................................                  [100%]
47 passed in 6.88s

Full unit-integration + install suites (tests/unit/integration/ tests/unit/install/): 2766 passed, no regressions from the WS narrowing or the credential-defence additions.

Manual end-to-end probe against the live Copilot App

A reproducible WS-IPC PoC client matches the production WsClient shape. It was run live against ~/.copilot/data.db + ~/.copilot/run/ws.{port,token}:

  • Handshake succeeds with Origin: tauri://localhost + URI-token.
  • create_project_from_path returns project_created; the project is immediately known to the webview (no white-screen on first install).
  • Once the App's ProjectsChanged broadcast lands upstream, the "restart once" hint can be dropped.

Scenario Evidence

# Scenario (user promise) Principle(s) Test(s) proving it Type
1 apm install --target copilot-app inside a repo registers that repo as a Copilot App project and attaches every workflow to it. DevX, Secure by default tests/unit/integration/test_copilot_app_project.py::TestResolverMiss::test_insert_creates_row ; tests/unit/install/test_install_target_copilot_app_e2e.py::TestCopilotAppDeployUninstall::test_install_project_scope_then_uninstall_deletes_db_row unit + e2e
2 Repeated apm install in the same repo reuses the existing project row (no duplicates). DevX, Governed by policy tests/unit/integration/test_copilot_app_project.py::TestResolverHit::test_returns_existing_id_without_insert ; TestResolverHit::test_external_row_is_reused unit
3 Race between two concurrent apm install invocations resolves to one project row. Secure by default, Governed by policy tests/unit/integration/test_copilot_app_project.py::TestRaceCollisionRecovery::test_race_collision_resolves_to_winning_id unit
4 When the App is running, project registration goes via authenticated WS-IPC and survives token/origin gates. Secure by default tests/unit/integration/test_copilot_app_ws.py::TestHandshake::test_bad_token_raises_auth_error ; TestHandshake::test_origin_header_is_tauri_localhost unit
5 When the App is closed, apm install still works via direct SQLite. DevX, Multi-harness tests/unit/integration/test_copilot_app_ws.py::TestWsAvailable::test_no_files_returns_false ; TestWsAvailable::test_stale_port_no_listener_returns_false ; tests/unit/install/test_install_target_copilot_app_e2e.py::TestCopilotAppDeployUninstall::test_install_then_uninstall_roundtrip unit + e2e
6 --global + workflow-shape no longer silently corrupts UX — user sees the CWD-pivot warning. Secure by default, DevX tests/unit/install/test_install_target_copilot_app_e2e.py::TestCopilotAppDeployUninstall::test_global_install_with_workflow_emits_warning e2e
7 First install per repo emits a "Restart the Copilot App once" hint so the sidebar can hydrate. DevX tests/unit/install/test_install_target_copilot_app_e2e.py::TestCopilotAppParserE2E::test_project_scope_now_supported e2e
8 Protocol drift (extra fields, malformed reply, server error) does not corrupt installer state. Secure by default, Governed by policy tests/unit/integration/test_copilot_app_ws.py::TestCreateProject::test_extra_fields_are_tolerated ; TestCreateProject::test_malformed_reply_raises_protocol_error ; TestCreateProject::test_server_error_raises_protocol_error unit
9 WS failure mid-install does not corrupt workflow rows: SQLite fallback still stamps project_id, and stale-token / app-down conditions stay silent. Secure by default, DevX tests/integration/test_copilot_app_ws_fallback.py::test_ws_protocol_error_falls_back_to_sqlite_with_project_id ; test_ws_app_not_running_is_silent ; test_ws_auth_error_is_silent integration
10 Credential material (token in handshake URL, group-readable token file) cannot leak. Secure by default tests/unit/integration/test_copilot_app_ws.py::TestTokenScrub ; TestTokenFileMode unit

How to test

  • Open the Copilot App. Then in any local git repo with a workflow-shape prompt in its apm.yml, run apm install --target copilot-app. Open the Workflows tab — the new workflow should appear with display name suffixed (<repo>). Run now should CWD to your repo, not ~/.copilot.
  • On first install per repo, observe the post-install hint: "Restart the Copilot App once to see <repo> in the sidebar." After restart, the project appears.
  • Quit the Copilot App. Run apm install --target copilot-app again. Install succeeds via SQLite fallback. Relaunch the App — workflows and project hydrate cleanly.
  • Re-run apm install --target copilot-app --global against a package with a workflow-shape prompt. Observe the warning text calling out the CWD-pivot risk; install proceeds.
  • Run uv run --extra dev pytest tests/unit/integration/test_copilot_app_project.py tests/unit/integration/test_copilot_app_ws.py tests/unit/install/test_install_target_copilot_app_e2e.py tests/integration/test_copilot_app_ws_fallback.py -q — 47 tests pass.

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

danielmeppiel and others added 7 commits May 21, 2026 16:42
Required by the hybrid project-scoping path (PR A). When the App is
running, APM prefers the localhost WS-IPC surface so workflow rows
fire the live WorkflowsChanged broadcast instead of relying on the
webview re-reading SQLite.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
New module copilot_app_project.py: frozen dataclasses for RepoContext /
ProjectRecipe / ResolvedProject, pure derivers (derive_repo_context,
derive_project_recipe), and resolve_or_register_project_sqlite that
SELECTs by main_repo_path or INSERTs a fresh row inside BEGIN
IMMEDIATE. Race-collision recovery: on sqlite3.IntegrityError, re-
SELECT inside the same transaction to return the winning id.

Backed by 14 unit tests covering the no-repo / HTTPS / SSH / non-
github / subdir-walk-up derive paths and HIT / MISS / external-row /
race-recovery / missing-db / schema-too-new resolver branches.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
New module copilot_app_ws.py: WsClient context manager using
websockets.sync.client (no asyncio in install loop), ws_available()
liveness probe bounded to 100ms, and create_project_from_path /
create_workflow / update_workflow RPCs. Custom exceptions
(WsAppNotRunning / WsAuthError / WsProtocolError) let the caller
distinguish silent fallback from warn-and-fallback.

Reads ~/.copilot/run/ws.{port,token}, presents Origin: tauri://
localhost (the App rejects any other Origin), and walks past
interleaved server push frames to find the response to our request.
Permissive JSON parser tolerates unknown / added top-level fields so
upstream schema additions don't break us.

Backed by 19 unit tests against a real websockets.sync.server harness
covering handshake / auth / origin / round-trip / drain / timeout.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds project_id: str | None to WorkflowRow and threads it through both
INSERT and UPDATE paths in deploy_workflow. The execution-changed
UPDATE includes project_id alongside the other execution fields; the
name-only UPDATE also writes project_id so pre-PR-A rows with NULL
project_id self-heal on the next install without forcing a content
change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
prompt_integrator.integrate_prompts_for_target now accepts scope= and
forwards user-scope detection to _integrate_prompts_for_copilot_app,
which:

1. Parses all candidate workflow-shape prompts up front so the path
   decision (WS / SQLite / global-warn) is made once per package.
2. Emits a warn-and-proceed diagnostic when --global is combined with
   workflow-shape prompts, explaining that those workflows will run
   with CWD=~/.copilot and pointing at the App's Workflows tab to
   attach them to a project manually.
3. Derives a RepoContext from project_root; when the App is running
   AND we have a real repo, tries the WS-IPC path first
   (create_project_from_path + per-workflow create_workflow). On any
   WsError, warns and falls back to SQLite.
4. SQLite fallback: resolve_or_register_project_sqlite, stamp
   project_id on every WorkflowRow, suffix display name with
   ' (<repo>)' so workflows from different repos are distinguishable
   in the App UI.
5. When the project row was freshly created (was_created=True), emits
   a one-time restart-the-App info diagnostic referencing the upstream
   live-refresh gap (github/github-app#5483).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
services.py now passes scope= to integrate_*_for_target so the
prompt integrator can distinguish project-scope from user-scope at
the copilot-app branch. The four sibling integrators (agent, command,
hook, instruction) accept scope= as a keyword-only no-op default to
keep the call site uniform without changing their behavior.

Also refreshes the copilot-app explainer in phases/targets.py to
document the PR A auto-register + restart-once UX.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Extends the seed schema fixture with a projects table so the
  resolver can run end-to-end.
- Asserts on the project-scope roundtrip that the workflow row carries
  a non-NULL project_id pointing at a projects row with main_repo_path
  equal to the consumer directory, and that the display name carries
  the '(<repo>)' suffix.
- git init's the consumer directory so derive_repo_context returns a
  real RepoContext.
- Asserts the restart-the-App hint is emitted on the first install
  into a new repo.
- New test_global_install_with_workflow_emits_warning: --global plus
  workflow-shape prompts warn-and-proceeds (exit 0, row inserted,
  warning mentions --global + 'attach') instead of the prior hard-fail.

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

Copy link
Copy Markdown
Collaborator Author

APM Review Panel: ship_with_followups

Hybrid WS+SQLite project scoping is a real adoption beat for the Copilot App inner loop, but the PR's headline claim of live WS-driven workflow refresh is unrealized in production -- worth shipping with a tight set of follow-ups, not blocking.

cc @danielmeppiel -- a fresh advisory pass is ready for your review.

The panel converges cleanly: this is the strongest inner-loop adoption beat APM has shipped for the Copilot App (one apm install, project auto-registers, workflows scoped to the right repo), and the transport design (loopback + Origin + token query) is correct for the upstream-constrained handshake. Architecture, auth, and supply-chain personas independently agree the bones are sound. The single most consequential finding -- raised by python-architect and corroborated by doc-writer -- is that _deploy_via_ws only registers the project; workflow rows still flow through SQLite deploy_workflow, so the PR body's "free workflows_changed broadcasts refresh the UI live" promise does not fire for workflow writes, and WsClient.create_workflow / update_workflow are dead production code (~120 lines). This is a false marketing claim plus carrying cost, not a correctness regression -- recommended severity, but it must drive the framing. Author's namespaced-ID rationale at prompt_integrator.py:430-435 is reasonable; the honest fix is either path (a) wire workflow writes through WS and adapt the lockfile, or (b) narrow the WS surface to project-only registration and delete the dead methods. Either lands as a fast-follow; in this PR, correct the PR body and CHANGELOG so the public story matches the code.

The other signals stack predictably. Auth + supply-chain agree on two cheap hardenings (scrub ?token= from exception text before wrapping into WsError; verify ws.token mode/owner to match the 0o600 docstring claim) -- strong cross-signal, recommended for this PR. Test-coverage's one real gap (no integration test for WS-available + mid-deploy WsError -> SQLite fallback) is exactly the branch the PR diagram highlights and should land with the architectural fast-follow. UX nits are mostly polish, with the stale-token warning recurring on every install as the one user-impact item worth a quick fix. The websockets>=12 open upper bound (currently resolving to 16.0) is a one-line <17 cap. Doc drift is real but bounded -- the CHANGELOG entry is the lowest-friction one to land in this PR.

Dissent. No substantive panelist dissent. Growth-hacker initially flagged a per-workflow restart-hint spam risk and retracted after verification -- hint is one-shot in both branches. Auth and supply-chain independently raised the same two token-hygiene items, which strengthens rather than splits the signal.

Aligned with: Portable_by_manifest -- Lockfile stability preserved via namespaced workflow IDs (prompt_integrator.py:430-435); manifest-driven install flow unchanged.; Secure_by_default -- Transport hardening is correct (loopback + Origin + token); two recommended defense-in-depth gaps remain (token leak via exception text, missing file-mode check on ws.token).; Governed_by_policy -- --global CWD-pivot warning is the right governance nudge; wording can be tightened but the guardrail is present.; Pragmatic_as_npm -- One apm install and a workflow appears live in the Copilot App scoped to the right repo -- the inner loop now matches the npm-class promise.; Oss_community_driven -- Worth surfacing as a launch beat once the PR body and CHANGELOG are corrected to match the code that actually ships.

Growth signal. Per oss-growth-hacker: this is launch-beat material for the next release narrative -- 'install a workflow, see it appear live in the Copilot App, scoped to your repo'. Amplify only after the PR body / CHANGELOG / integration doc are corrected so the public story matches shipped behavior; amplifying the current 'live workflows_changed broadcast' framing would create a credibility debt the next contributor pays.

Panel summary

Persona B R N Takeaway
Python Architect 0 3 3 Hybrid dispatch is sound, but WsClient.create_workflow/update_workflow are dead code -- WS path only registers the project; workflow writes still go via SQLite. The 'live workflows_changed broadcast' promise in the PR body is therefore unrealized in production.
Cli Logging Expert 0 0 4 User-facing output goes through DiagnosticCollector.warn/info (no rich* bypass), is ASCII-only, leads with outcome, and the --global warning includes remediations + docs link. Four polish nits, no blocking issues.
Devx Ux Expert 0 2 2 Substantive UX gaps: WS fallback warn leaks internal jargon (WS/IPC), stale-token case spams every install, restart-hint wording diverges across paths, --global warn is verbose. None blocking install correctness.
Supply Chain Security Expert 0 3 0 WS transport design (loopback + Origin + token) is correct. Two recommended hardenings: verify ws.token file mode/owner before reading, and scrub token from exception text before it lands in diagnostics. One real supply-chain gap: websockets is pinned >=12 with no upper bound; currently resolves to 16.0.
Oss Growth Hacker 0 1 1 This is the strongest inner-loop adoption beat APM has shipped: one apm install and a workflow appears live in the Copilot App, scoped to the right repo. Surface as a launch beat in the release narrative. One claim ('restart hint may fire per-workflow') was a false alarm -- verified hint is one-shot in both branches.
Auth Expert 0 3 1 Transport design is correct (?token= query is the only option the App accepts, per upstream websocket.rs:104-110). AuthResolver / GITHUB_APM_PAT / token_manager isolation is clean. Two real hardenings: scrub token from exception messages, optionally verify ws.token file mode. One brittle classification gotcha.
Doc Writer 0 6 2 PR ships substantial user-facing behavior changes (project_id auto-registration, --global CWD-pivot warning, restart-once hint) with zero accompanying doc updates. Integration page, CHANGELOG Unreleased, and apm-usage skill all drift. No actively false claim, so no blocking finding. Six recommended doc updates fit in one follow-up pass.
Test Coverage Expert 0 1 1 All 8 scenario-evidence rows verified present and exercising their claims (race-collision is real, not a stub; e2e regression trap asserts non-NULL project_id + FK). One real gap: the WsError -> SQLite mid-deploy fallback inside _integrate_prompts_for_copilot_app has zero integration-tier coverage.

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

Top 5 follow-ups

  1. [Python Architect] Resolve the WS-vs-SQLite workflow-write mismatch: either wire workflow writes through WS (and adapt the lockfile to server-assigned IDs) or narrow the WS surface to project-only registration and delete WsClient.create_workflow / update_workflow. In this PR, at minimum correct the PR body and CHANGELOG so the 'live workflows_changed broadcast' claim matches shipped behavior. -- The PR's headline benefit is unrealized in production and ~120 lines of dead code carry maintenance cost. Most consequential finding; drives the ship framing.
  2. [Auth Expert] Scrub the ?token=... query string from exception text before wrapping into WsError subtypes, so token material cannot land in diagnostics or user-visible error output. -- Cross-signaled by auth and supply-chain. Cheap fix, defense-in-depth on the one handshake surface upstream forces us to use.
  3. [Supply Chain Security Expert] Verify ws.token file mode/owner before reading, matching the module docstring's 0o600 claim; refuse to read if world/group-readable. -- Cross-signaled by auth and supply-chain. Closes the gap between documented and enforced posture on the credential file.
  4. [Test Coverage Expert] Add an integration test for the WS-available + mid-deploy WsError -> SQLite fallback path inside _integrate_prompts_for_copilot_app. -- Exactly the branch the PR diagram highlights; without it, the hybrid dispatch's main differentiator is a silent-drift risk.
  5. [Supply Chain Security Expert] Pin websockets with an upper bound (>=12,<17) to lock the current resolution and avoid surprise major-version uptake. -- One-line additive change; prevents a future websockets 17.x from silently landing across user installs.

Architecture

classDiagram
    direction LR
    class PromptIntegrator {
      +integrate_prompts_for_target(target, pkg, root, scope)
      -_integrate_prompts_for_copilot_app(...)
      -_deploy_via_ws(...)
    }
    class RepoContext { +repo_root
      +repo_name
      +github_owner
      +default_branch }
    class ResolvedProject { +project_id
      +was_created }
    class WsClient {
      +create_project_from_path(path) ProjectCreated
      +create_workflow(...) WorkflowCreated
      +update_workflow(...) WorkflowCreated
    }
    class ProjectCreated { +project_id
      +was_created }
    class WorkflowCreated { +workflow_id }
    class WsError
    class WsAppNotRunning
    class WsAuthError
    class WsProtocolError
    class copilot_app_db {
      +deploy_workflow(db_path, row)
      +WorkflowRow
    }
    WsError <|-- WsAppNotRunning
    WsError <|-- WsAuthError
    WsError <|-- WsProtocolError
    PromptIntegrator ..> WsClient : try first when ws_available
    PromptIntegrator ..> copilot_app_db : deploy_workflow BOTH branches
    PromptIntegrator ..> RepoContext : derive_repo_context
    WsClient ..> ProjectCreated
    WsClient ..> WorkflowCreated
    WsClient ..> WsError
    class WsClient:::touched
    class PromptIntegrator:::touched
    class RepoContext:::touched
    class ResolvedProject:::touched
    class ProjectCreated:::touched
    class WorkflowCreated:::touched
    class WsError:::touched
    classDef touched fill:#fff3b0,stroke:#d47600
Loading
flowchart TD
    Start([apm install --target copilot-app])
    Parse[parse: _is_workflow_shape + frontmatter]
    GlobalQ{user_scope == global<br/>AND workflow-shape?}
    Warn[/warn: CWD-pivot risk; proceed/]:::new
    Derive[derive_repo_context]
    HasRepo{repo_ctx?}
    WsAvail{ws_available?<br/>file probe + 100ms TCP}:::new
    WsConn[WsClient handshake<br/>Origin=tauri/localhost + token]:::new
    WsProj[client.create_project_from_path]:::new
    WsLoop[per workflow:<br/>deploy_workflow SQLite<br/>NOT client.create_workflow]:::warn
    WsCatch{WsError?}
    SqlResolve[resolve_or_register_project_sqlite<br/>BEGIN IMMEDIATE + race recovery]:::new
    Hint[/info: restart App once #5483/]:::new
    Deploy[per row: deploy_workflow w/ project_id]
    Done([IntegrationResult])
    Start --> Parse --> GlobalQ
    GlobalQ -- yes --> Warn --> Derive
    GlobalQ -- no --> Derive
    Derive --> HasRepo
    HasRepo -- yes --> WsAvail
    HasRepo -- no --> Deploy
    WsAvail -- yes --> WsConn --> WsProj --> WsLoop --> Hint --> Done
    WsAvail -- no --> SqlResolve
    WsConn -- WsError --> WsCatch
    WsProj -- WsError --> WsCatch
    WsCatch -- caught --> SqlResolve
    SqlResolve --> Hint --> Deploy --> Done
    classDef new fill:#dff5d6,stroke:#3a7d2b,color:#1a3d10
    classDef warn fill:#ffe1a8,stroke:#a85a00,color:#3d1f00
Loading
sequenceDiagram
    autonumber
    participant CLI as apm install
    participant FS as ~/.copilot/run
    participant WS as App WS server
    participant DB as ~/.copilot/data.db
    participant Bus as App event_bus
    participant UI as Workflows tab
    CLI->>FS: read ws.port + ws.token
    CLI->>WS: GET Upgrade Origin=tauri/localhost ?token=...
    WS-->>CLI: 101 Switching Protocols
    CLI->>WS: create_project_from_path
    WS-->>CLI: project_created / project_updated
    Note over CLI,WS: create_workflow NEVER sent --<br/>workflow rows go to SQLite directly
    loop per workflow-shape prompt
      CLI->>DB: deploy_workflow INSERT/UPDATE with project_id
    end
    Note over DB,Bus: SQLite write does NOT fire send_global WorkflowsChanged
    CLI->>UI: no live refresh from broadcast
    CLI-->>CLI: IntegrationResult
Loading

Recommendation

Ship after correcting the PR body and landing a CHANGELOG entry so the public story matches what the code actually does (project-only WS registration; workflow rows via SQLite). Open a tracking issue for the architectural fast-follow (path A: wire workflow writes through WS, or path B: delete WsClient.create_workflow/update_workflow and narrow the WS surface) plus the two token-hygiene hardenings and the missing fallback integration test. The inner-loop adoption win is real and worth shipping now; do not amplify it externally until the framing is corrected.


Full per-persona findings

Python Architect

  • [recommended] _deploy_via_ws does not call WsClient.create_workflow; workflow rows still go through SQLite deploy_workflow at src/apm_cli/integration/prompt_integrator.py:414
    Verified at prompt_integrator.py:436-454: per workflow, _deploy_via_ws calls deploy_workflow(db_path, row) directly. WsClient.create_workflow and WsClient.update_workflow are never called from production code; only unit tests exercise them. Because SQLite writes do NOT fire send_global(WorkflowsChanged), the 'free workflows_changed broadcasts refresh the UI live' claim in the PR body does not trigger for APM-installed workflows. Architecturally this is ~120 lines of dead WS surface plus a false promise. The author's comment at lines 430-435 explains the choice (namespaced IDs for lockfile stability), but the WS branch as shipped provides no live-refresh benefit over SQLite. Pick one: (a) route workflow writes through client.create_workflow and adapt the lockfile to server-issued IDs, or (b) delete create_workflow/update_workflow and rename _deploy_via_ws to _deploy_via_ws_project_only.
  • [recommended] Handshake error classification relies on substring-matching exception messages at src/apm_cli/integration/copilot_app_ws.py:300
    _connect catches bare Exception and str-matches '401'/'403'/'unauthor'/'refused' to choose WsAuthError vs WsAppNotRunning vs WsProtocolError. websockets exposes typed exceptions (InvalidStatus with response.status_code, plus stdlib ConnectionRefusedError). A library upgrade that changes wording silently downgrades 401 into WsProtocolError. Prefer typed catch + isinstance branching.
  • [recommended] copilot_app_project imports leading-underscore helpers from sibling module copilot_app_db at src/apm_cli/integration/copilot_app_project.py:47
    _connect, _check_user_version, _begin_immediate_with_retry are imported by the new resolver. Sibling-private coupling means any refactor of copilot_app_db breaks the project resolver. Promote them to a public copilot_app_sqlite_io module in a follow-up.
  • [nit] Dead try/except in _deploy_via_ws (try: ... except Exception: raise) at src/apm_cli/integration/prompt_integrator.py:415
    Lines 415-418 are a no-op wrapper around client.create_project_from_path. Drop it.
  • [nit] contextlib.suppress tuples include both specific exceptions and bare Exception at src/apm_cli/integration/copilot_app_project.py:192
    _safe_repo and derive_repo_context use except (InvalidGitRepositoryError, Exception) and contextlib.suppress(AttributeError, ValueError, Exception). Exception subsumes the others; collapse or be explicit.
  • [nit] WsClient.update_workflow has no production caller (speculative API) at src/apm_cli/integration/copilot_app_ws.py:464
    Only unit test exercises update_workflow. Defer the method until a real second caller appears.

Cli Logging Expert

  • [nit] 'direct database write' is jargon in the WS-fallback warning at src/apm_cli/integration/prompt_integrator.py
    Users have no mental model for 'direct database write' vs 'WS path'. Prefer outcome-first: 'Copilot App live IPC unavailable ({exc}); installing workflows offline -- restart the App to see them in the Workflows tab.'
  • [nit] Two restart-hint wordings diverge across WS vs SQLite paths at src/apm_cli/integration/prompt_integrator.py
    WS branch says 'If the Workflows tab does not refresh, restart the App once'; SQLite branch says 'Restart the Copilot App once so the new project appears in the UI'. Extract a single _RESTART_HINT_TEMPLATE constant.
  • [nit] aka.ms/apm/copilot-app-global shortlink must resolve before merge
    The --global warning ends with 'See https://aka.ms/apm/copilot-app-global for details.' Dead shortlinks in warning text are a recurring papercut. Verify the redirect is provisioned or point at the docs page directly.
  • [nit] SQLite-register failure warning offers no remediation
    'Could not register project for Copilot App: {exc}. Workflows will be installed without a project binding.' is outcome-first but no next-step. Add a soft pointer like 'Re-run with --verbose for the full database error.'

Devx Ux Expert

  • [recommended] WS fallback warning leaks internal jargon (WS, IPC, handshake) to end users at src/apm_cli/integration/prompt_integrator.py
    Message reads 'Copilot App live IPC unavailable ({exc}); falling back to direct database write.' where {exc} interpolates 'WS auth rejected: 401' or 'WS recv timed out after 5.0s'. New users have no model for WS/IPC/handshake/recv. Map exceptions to user-vocabulary at the integrator boundary; keep raw {exc} behind --verbose.
  • [recommended] WS-auth fallback warning repeats on every apm install when token is stale at src/apm_cli/integration/prompt_integrator.py
    Restart-once hint is gated on was_created (good). But the WsAuthError fallback warn has no gate: stale ws.token after ungraceful App quit means every apm install in every repo emits the cryptic 'WS auth rejected' warning until restart. Swap for actionable: 'Copilot App local install token is stale -- restart the App to refresh it.'
  • [nit] Two divergent wordings for restart-once hint (WS branch vs SQLite branch) at src/apm_cli/integration/prompt_integrator.py
    Same user-promise, two phrasings depending on dispatch path. Unify via single helper. Also consider dropping the github/github-app#5483 link from user-facing string -- belongs in code comment.
  • [nit] --global warning is 4 sentences; could be one + a link at src/apm_cli/integration/prompt_integrator.py
    Reads as a paragraph, not a CLI warning. npm/pip warnings are typically one sentence + URL. Tighten to: 'Installed with --global: workflows will run from ~/.copilot, not your repo. Re-run without --global from inside a repo to bind them, or attach in the App's Workflows tab. '

Supply Chain Security Expert

  • [recommended] Verify ws.token file mode/ownership before reading (defense-in-depth) at src/apm_cli/integration/copilot_app_ws.py
    _read_creds reads ~/.copilot/run/ws.token without checking it is a regular file (not symlink), mode 0o600, owned by current uid. Module docstring claims '(mode 0o600)' but doesn't enforce. Stat the file, refuse if mode & 0o077 != 0 or owner != geteuid(), surface as WsAuthError so install falls through to SQLite silently. One os.stat call on the hot path.
  • [recommended] Scrub token from any exception propagated to diagnostics at src/apm_cli/integration/copilot_app_ws.py
    _connect builds 'ws://127.0.0.1:{port}/?token={token}' and on failure raises WsError subtypes wrapping str(exc) which then lands in diagnostics.warn. If websockets ever embeds the URI in InvalidStatus/OSError repr, the token appears in user-visible warnings and log capture. Mitigations: (a) scrub 'token=...' from str(exc) before wrapping, or (b) wait for the App to accept Authorization header (currently only ?token= is supported, per upstream websocket.rs:104-110,510).
  • [recommended] Add an upper bound on websockets to constrain blast radius at pyproject.toml
    pyproject.toml pins websockets>=12 with no upper bound; uv.lock currently resolves to 16.0 -- already a major jump from the floor. websockets is now a hard runtime dep and the sync API has had breaking refactors between majors. Recommend websockets>=12,<17 to prevent a future major release from silently flowing into apm install.

Oss Growth Hacker

  • [recommended] Surface this as a launch beat -- README hook + release-note story angle
    Strategically THE inner-loop killer-UX moment: apm install --target copilot-app and the workflow appears live in the desktop App, scoped to the repo, no restart when the App is running. Cleanest 'package-manager-for-AI' demo APM has shipped. Recommended (non-blocking) growth actions for release-time: 10-second screen capture, one-line README addition under 'works with' (live install into desktop App), reuse the 'outer loop gh-aw + inner loop Copilot App, same apm install' framing in release narrative.
  • [nit] Restart-hint URL (github/github-app#5483) belongs behind --verbose, not in the first-run path at src/apm_cli/integration/prompt_integrator.py
    Including the issue URL inline reads as 'something is broken' to first-run users. Optional polish: keep URL behind -v and shorten user-facing string to 'Restart the Copilot App once so the new project appears in the sidebar.'

Auth Expert

  • [recommended] Scrub token from exception messages before re-raising / logging at src/apm_cli/integration/copilot_app_ws.py
    _connect builds 'ws://127.0.0.1:{port}/?token={token}' then on failure raises WsAuthError(f'WS auth rejected: {exc}') etc. These propagate into diagnostics.warn. The websockets/tungstenite stack can embed the request URI in exception messages (InvalidURI, redirect/handshake errors), which would surface the per-launch token in user-visible install output and any captured logs. Add a helper that runs str(exc) through re.sub(r'token=[^&\s]+', 'token=', ...) before wrapping into WsError subtypes.
  • [recommended] Optionally verify ws.token file mode on POSIX (defense-in-depth) at src/apm_cli/integration/copilot_app_ws.py
    _read_creds trusts the App wrote ws.token with 0o600. A future App regression that drops the mode bit would silently let APM use a world-readable token. os.stat(token_path).st_mode & 0o077 == 0 check on POSIX with WsAuthError on failure costs nothing.
  • [recommended] Brittle HTTP-status classification by substring match at src/apm_cli/integration/copilot_app_ws.py
    _connect classifies auth failures with 'if 401 in msg or 403 in msg or unauthor in msg.lower()'. websockets>=13 exposes exc.response.status_code on InvalidStatus; matching on that is robust against upstream changing wording.
  • [nit] Doc nit: token is opaque, not necessarily base64
    Module docstring says 'Token is supplied via the ?token= query string'. The App generates it via cryptographic randomness (websocket.rs:81-83) and the wire format is opaque. Drop the 'base64' qualifier.

Doc Writer

  • [recommended] copilot-app integration page does not mention project_id stamping or repo auto-registration at docs/src/content/docs/integrations/copilot-app.md
    PR's headline behavior (resolve-or-register project, stamp project_id, suffix display name with ()) is invisible in docs/src/content/docs/integrations/copilot-app.md. Lifecycle table still describes only workflows INSERT/UPDATE. Readers upgrading from 0.14.1 won't understand the display-name change or the new sidebar row. Add a 'Project scoping' subsection.
  • [recommended] Page states App 'reads new rows on next launch (or refresh)' -- stale once WS-IPC is preferred at docs/src/content/docs/integrations/copilot-app.md
    WS-IPC path SHOULD trigger workflows_changed broadcast and live-refresh the Workflows tab without restart. NOTE: python-architect finding shows current PR does NOT actually call create_workflow over WS, so this claim is currently FALSE in production. Either fix the PR or update docs honestly: 'When the App is running, the project is registered live; workflow rows still require an App refresh.'
  • [recommended] --global behavior change (warn + CWD-pivot callout) is undocumented at docs/src/content/docs/integrations/copilot-app.md
    PR adds explicit warning on --global + workflow-shape. The doc page mentions --global neutrally and gives no guidance on CWD-pivot risk. Add a :::caution callout. Also confirm where aka.ms/apm/copilot-app-global should redirect; the docs page is the natural target.
  • [recommended] Restart-once hint is a new visible diagnostic with no doc mention at docs/src/content/docs/integrations/copilot-app.md
    Users searching docs for 'restart' find nothing. Add a one-line 'Known limitation' note under Lifecycle: APM emits a one-time restart hint on first install per repo; subsequent installs are silent. Tracked upstream at github/github-app#5483.
  • [recommended] CHANGELOG.md Unreleased has no entry for PR feat(copilot-app): hybrid WS-IPC + SQLite project scoping for workflows #1431 at CHANGELOG.md
    Unreleased only has a 'Changed: coverage' line. PR is a material Fixed + Changed item (closes CWD-pivot security gap, fixes first-install white-screen, changes --global semantics). Following the pattern of fix(uninstall): accept Windows absolute paths as local packages (re-tag v0.14.1) #1413, two entries are warranted (Fixed for project_id stamping, Changed for WS-IPC path + --global warn).
  • [recommended] packages/apm-guide/.apm/skills/apm-usage/commands.md copilot-app paragraph is stale at packages/apm-guide/.apm/skills/apm-usage/commands.md
    Long inline description does not mention project auto-registration, --global warning, or live-refresh behavior. This file is consumed by the apm-guide skill, so agents teaching users will give outdated answers. Append two clauses describing the new behavior.
  • [nit] Two restart-hint strings diverge across WS and SQLite paths at src/apm_cli/integration/prompt_integrator.py
    Same root cause, two phrasings. Extract _RESTART_HINT constant.
  • [nit] README.md killer-UX angle deferred until copilot-app graduates from experimental at README.md
    Out of scope to write speculatively because README needs maintainer approval AND integration is still gated behind apm experimental enable copilot-app. Flagging as future opportunity, not request.

Test Coverage Expert

  • [recommended] WS->SQLite mid-deploy fallback in _integrate_prompts_for_copilot_app has no test at tests/unit/integration/test_copilot_app_ws.py
    The PR body diagram explicitly highlights this branch (yellow box) as the chosen design. prompt_integrator.py:282-305 wraps _deploy_via_ws in try/except WsError and emits diagnostics.warn before falling through to the SQLite resolver. test_copilot_app_ws.py exercises WsClient in isolation, never via prompt_integrator. test_install_target_copilot_app_e2e.py exercises the SQLite-only branch (ws_available()==False). The cross-module contract -- WS available + WsClient raises WsError mid-deploy + integrator falls through to SQLite + emits named diagnostic -- has zero assertions. A regression that re-raised WsError (turning a transient socket close into a hard install failure) would pass CI today. Add one integration test that starts the real _Server fixture configured to raise WsProtocolError mid-create_workflow, seeds the DB, invokes _integrate_prompts_for_copilot_app, and asserts exit_code==0 + project_id IS NOT NULL + warning captured.
    Proof (missing): tests/unit/integration/test_copilot_app_ws.py::TestHybridDispatch::test_ws_error_mid_deploy_falls_back_to_sqlite -- proves: WS mid-deploy failure does not turn into install failure; row still lands project-scoped via SQLite fallback.
  • [nit] Scenario Evidence audit -- all 8 rows verified passing
    Audited each row: file paths exist, test names match, test bodies actually exercise the named scenario. Race-collision test is NOT a stub -- monkeypatches _connect to capture the resolver's connection, then monkeypatches derive_project_recipe to insert a colliding row ON THE SAME CONNECTION mid-transaction, producing real sqlite3.IntegrityError that drives recovery SELECT. All 40 tests pass in 8.55s.
    Proof (passed): tests/unit/install/test_install_target_copilot_app_e2e.py + tests/unit/integration/test_copilot_app_project.py + tests/unit/integration/test_copilot_app_ws.py

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

Path B narrow WS surface (project-registration only), token hardening,
fallback regression test, and doc/PR-body honesty pass.

Source:
- copilot_app_ws.py: delete create_workflow/update_workflow/
  WorkflowCreated/_extract_workflow_id; add _scrub_token regex that
  redacts ?token=... in wrapped exception text; add _token_file_mode_ok
  check refusing ws.token if group/other-readable (matches App's 0o600
  posture); raise WsError from None so library exceptions with the
  unscrubbed URL are not reachable via __context__.
- prompt_integrator.py: rewrite _integrate_prompts_for_copilot_app as
  a single-loop dispatcher. Resolve project_id once (try WS, fall back
  to SQLite on WsError; silently swallow WsAppNotRunning and
  WsAuthError so stale tokens after a restart never warn). Write every
  workflow row via SQLite with the resolved project_id. One restart-
  once hint when was_created. Drop the WS/IPC jargon from user-facing
  wording. Delete _deploy_via_ws.
- pyproject.toml + uv.lock: pin websockets>=12,<17.

Tests:
- tests/unit/integration/test_copilot_app_ws.py: drop TestCreateWorkflow
  + TestUpdateWorkflow; port TestDrainAndInterleavedPush and
  TestRecvTimeout onto create_project_from_path; chmod 0o600 in the
  _write_creds fixture so the new mode check passes; add TestTokenScrub
  (3) and TestTokenFileMode (3).
- tests/integration/test_copilot_app_ws_fallback.py (new): regression
  trap for WS-error -> SQLite fallback. WsProtocolError mid-deploy
  still stamps project_id on the workflow row; WsAppNotRunning and
  WsAuthError stay silent.

Docs:
- CHANGELOG.md: corrected Unreleased entries (Added/Security).
- docs/.../integrations/copilot-app.md: Project scoping + One-time
  restart hint + --global warn-and-proceed sections.
- packages/apm-guide/.apm/skills/apm-usage/commands.md: project-scope
  + restart-hint + --global warn callouts in the experimental section.

PR body: rewritten to drop misleading mode-allowlist bullet and the
'live workflows_changed broadcast' framing; sequence diagram now shows
WS for project registration and SQLite for workflow rows; scenario
table extended with WS-fallback and token-hardening evidence.

Validation: ruff check + format --check silent; 2766 unit
integration+install tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel danielmeppiel marked this pull request as ready for review May 21, 2026 15:28
Copilot AI review requested due to automatic review settings May 21, 2026 15:28

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 fixes copilot-app workflow scoping by resolving/registering a real Copilot App projects row for the current repo (prefer WS-IPC when the App is running; fall back to direct SQLite when it is not) and stamping project_id onto every deployed workflow row, preventing workflows from running with CWD ~/.copilot by default and avoiding the Workflows-tab “unknown project” failure mode.

Changes:

  • Add a hybrid project resolver: WS-IPC project registration when available, otherwise a SQLite BEGIN IMMEDIATE resolver/registrar.
  • Extend Copilot App workflow DB writes to include project_id (including self-healing older NULL rows) and thread install scope through integrator call sites.
  • Add focused unit + integration coverage plus docs/skill-guide/changelog updates; pin websockets with an upper bound.
Show a summary per file
File Description
src/apm_cli/integration/copilot_app_ws.py New sync WebSocket client (project-registration only) with token/file-mode hardening and token scrubbing.
src/apm_cli/integration/copilot_app_project.py New pure repo/project derivation + SQLite fallback resolver with race-collision recovery.
src/apm_cli/integration/copilot_app_db.py Add WorkflowRow.project_id and write it on INSERT/UPDATE (including self-heal path).
src/apm_cli/integration/prompt_integrator.py Hybrid dispatcher: resolve project once (WS then SQLite), suffix workflow names, always write workflow rows via SQLite, handle --global warning.
src/apm_cli/install/services.py Thread scope through integrator calls.
src/apm_cli/install/phases/targets.py Update target-phase commentary to reflect project scoping + restart hint behavior.
src/apm_cli/integration/agent_integrator.py Accept scope= for signature parity (no behavior change).
src/apm_cli/integration/command_integrator.py Accept scope= for signature parity (no behavior change).
src/apm_cli/integration/hook_integrator.py Accept scope= for signature parity (no behavior change).
src/apm_cli/integration/instruction_integrator.py Accept scope= for signature parity (no behavior change).
pyproject.toml Add dependency constraint websockets>=12,<17.
uv.lock Lock websockets dependency.
tests/unit/integration/test_copilot_app_ws.py New unit tests for WS availability, handshake, protocol handling, token scrubbing, and token file-mode checks.
tests/unit/integration/test_copilot_app_project.py New unit tests for repo derivation and SQLite resolver (HIT/MISS/race/errors).
tests/integration/test_copilot_app_ws_fallback.py Integration regression tests for WS->SQLite fallback and silent-fallback cases.
tests/unit/install/test_install_target_copilot_app_e2e.py Update/add E2E assertions for non-NULL project_id, restart hint, and --global warn-and-proceed.
docs/src/content/docs/integrations/copilot-app.md Document project scoping, resolver order, restart hint, and --global behavior.
packages/apm-guide/.apm/skills/apm-usage/commands.md Update copilot-app usage guidance to match new scoping behavior and warnings.
CHANGELOG.md Add Unreleased entries for the new scoping behavior, security hardening, and dependency pin.

Copilot's findings

  • Files reviewed: 18/19 changed files
  • Comments generated: 3

# ------------------------------------------------------------------
# Resolve project_id ONCE -- WS (preferred) then SQLite fallback.
# ------------------------------------------------------------------
repo_ctx = derive_repo_context(project_root)
Comment on lines +449 to +455
``was_created`` is best-effort: the server's
``project_created`` reply carries a ``project`` payload but
not (currently) a "was new" flag, so we treat any successful
reply as "created or already existed -- caller should ALWAYS
emit the restart hint until upstream issue github/github-app#5483
lands the live broadcast". The hint is suppressed once the
webview learns about the row (i.e. after the user restarts).
Comment on lines +201 to +205
is found. Symlinks in the chain are not followed (defense against
being lured into an attacker-controlled root).
"""
start = start.resolve()
for candidate in (start, *start.parents):
danielmeppiel and others added 3 commits May 21, 2026 21:35
…iew nits

The copilot-app target deploys *.prompt.md as Copilot App workflow rows
(SQLite + optional WS-IPC) instead of files. That path shared NOTHING
with the file-based prompt deploy except the source artefact, but it
was squatting inside PromptIntegrator (~300 lines of unrelated logic
plus its own module-level frontmatter helpers). Extract into a sibling
integrator so the file matches how agent_integrator / command_integrator
/ hook_integrator / instruction_integrator are organised.

Refactor:

* New src/apm_cli/integration/copilot_app_workflow_integrator.py with
  CopilotAppWorkflowIntegrator(BaseIntegrator). Inherits BaseIntegrator
  for find_files_by_glob only -- the file-based collision /
  link-resolution machinery is irrelevant on the workflow surface
  (deploy_workflow is an UPSERT keyed on a namespaced id; sync deletes
  by id from the lockfile).
* PromptIntegrator.integrate_prompts_for_target and .sync_for_target
  keep one trivially small branch per method that delegates to the
  workflow integrator -- grep copilot-app in prompt_integrator.py and
  one call site appears for each direction.
* Module-level frontmatter helpers (_is_workflow_shape, Schedule,
  _parse_workflow_frontmatter, _parse_schedule, _derive_package_owner)
  move with the path that uses them. Re-exported from prompt_integrator
  for back-compat with tests / external callers.
* copilot_app_db / copilot_app_project / copilot_app_ws stay put --
  they are already at the right level. Only prompt_integrator was
  oversized.
* Test fixtures unchanged; the integration test that patches
  copilot_app_ws.WsClient.create_project_from_path still works because
  the import path did not move.

Folded Copilot reviewer nits (PR #1431):

* B1: when derive_repo_context(project_root) returns None AND workflow
  prompts are present AND scope is not user, emit a parallel warn to
  the existing --global warn. project_id=NULL workflows have the same
  CWD=~/.copilot pivot risk; the user deserves the same heads-up plus
  the fix recipe (run inside a git repo or attach from the App UI).
* B2: WsClient.create_project_from_path docstring rewritten to match
  the implementation -- was_created is inferred from the reply type
  (project_created -> True, project_updated -> False) via
  _extract_project_fields, not 'always True'. Restart hint only fires
  on the project_created branch.
* B3: _find_repo_root docstring no longer claims symlinks-in-the-chain
  are blocked. The cheap fix: drop the false claim. The 'right' fix
  (walk + reject symlinked parents) would break macOS /tmp ->
  /private/tmp and other legitimate symlinked-ancestor setups, and the
  threat model (user's own CWD, not adversary input) does not justify
  the cost. The .git-marker is_symlink() check stays as
  defence-in-depth.

Behavior change: one new warn diagnostic when copilot-app workflows
install without a detected git repo. Three pre-existing unit tests in
test_copilot_app_error_ux.py updated to filter the new warn out of
their per-prompt deploy-error counts (the no-repo warn fires once for
the whole install; deploy-error warns are per-prompt).

Test gate: 54 targeted + 2766 broad -- all pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Extract _open_write_txn() helper in copilot_app_db so both
  deploy_workflow and copilot_app_project.resolve_or_register_project
  share the missing-DB / version-guard / BEGIN-IMMEDIATE prelude.
  Closes the pylint R0801 duplicate-code finding without changing
  behavior or error wording.
- Drop the now-unused _connect / _check_user_version /
  _begin_immediate_with_retry imports from copilot_app_project.
- Update test_race_collision_resolves_to_winning_id to patch
  copilot_app_db._connect (the real entry point) rather than the
  project module re-export, which is gone.
- Add websockets entry to scripts/notice-metadata.yaml and regenerate
  NOTICE so the drift check passes for the new runtime dependency.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel danielmeppiel merged commit aded37e into main May 21, 2026
11 checks passed
@danielmeppiel danielmeppiel deleted the danielmeppiel/feat-copilot-app-ws-project-scope branch May 21, 2026 20:23
danielmeppiel added a commit that referenced this pull request May 27, 2026
…5 drift sweep) (#1511)

* docs: backfill apm-usage and consolidate registry guides (v0.14->v0.15 drift sweep)

Holistic docs-sync retrospective on the v0.14.0->v0.15.0 release window
flagged 23 of 39 user-impact PRs as docs-debt: 7 Rule 4 violations
(apm-usage/ skipped) plus 16 silent-drift PRs. This PR closes the
highest-priority gaps (P0/P1 from the retrospective) in one sweep.

Backfills (apm-usage/ training corpus):
- dependencies.md: registry-sourced APM dep object form (#1471)
- authentication.md: APM_REGISTRY_TOKEN_{NAME} precedence (#1471)
- governance.md: registry_source + allow_non_registry policy (#1471)
- package-authoring.md: apm publish workflow (#1471) and project-scope
  hook command path semantics (#1396)
- commands.md: apm publish entry (#1471), apm config transport keys
  (#1308), apm compile live-reload + --clean --watch warning (#1403),
  Claude Code instruction dedup (#1146), MCP env-var placeholder
  resolution (#1277), AppLocker/WDAC staged-install diagnostic (#1390)

Structural fix (per docs-impact-architect verdict):
- Merge guides/private-registries.md INTO guides/registries.md with
  progressive disclosure (public -> private -> per-dep routing ->
  enterprise link). Adds Starlight redirect for the old slug, patches
  5 cross-references across consumer/, reference/cli/.

Editorial fixes (per editorial-owner sweep):
- integrations/copilot-app.md (#1431): lead with user value before
  WS-IPC/SQLite mechanics; add 'restart the Copilot App once'
  troubleshooting hint
- producer/compile.md: dedup the Claude Code instruction dedup
  explanation (was stated twice)
- enterprise/security.md: reframe defensive memo voice ('do not call
  this X') to user voice ('here is what we provide / here is what we
  don't')

Method: docs-sync skill end-to-end. 5-panelist fan-out plus CDO
synthesis. Every CLI claim in the apm-usage adds was verified against
the live 'apm <verb> --help' surface (S7 tool bridge).

Out of scope (tracked as P1 follow-up): backfilling docs for the 16
silent-drift PRs grouped by subsystem (MCP, install, compile, auth).

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

* docs: full-corpus regrounding audit (55 pages, 14 surgical fixes)

Wave-batched grounding audit across 55 high-risk pages (CLI ref x27,
schemas/specs x10, consumer ramp x12, onboarding x6). Each page's
factual claims (flags, env vars, exit codes, schema fields, file
paths, code links) was extracted and verified against current
src/apm_cli/ and 'apm <verb> --help' output via S7 tool-bridge.

Fixes applied (14 files):

CLI reference:
- pack.md: add --check-versions, --check-clean flags + exit codes 3, 4
- targets.md: expand copilot detection signals (5, not 1)
- experimental.md: add copilot-app, marketplace-authoring, registries
- install.md: dedup duplicate '## Exit codes' + '## Notes' sections

Schemas / specs:
- lockfile-spec.md: expand package_type enum to full 6-value list
- manifest-schema.md: document plural 'targets:' alias (#1335)
- environment-variables.md: add APM_BROAD_FETCH_DEPTH, APM_COPILOT_APP_DB
- package-types.md: add 5th layout (hook_package, hooks/*.json only)

Consumer ramp:
- install-mcp-servers.md: fix stale code citation + 'Or' -> 'And'
- private-and-org-packages.md: drop nonexistent BITBUCKET_APM_PAT

Onboarding (6 broken navigation links, 4 files):
- quickstart.mdx, getting-started/installation.md,
  getting-started/first-package.md, getting-started/migration.md:
  repoint self-loops and dead routes to actual page paths

Process: dispatched as 6 parallel grounding-verifier agents (general-
purpose) across disjoint page scopes; each agent had edit authority
on its scope and applied surgical fixes inline. Reusable pattern via
the docs-corpus-audit sibling skill design (PANEL + WAVE EXECUTION
+ S7 verifier fan-out, see files/docs-corpus-audit-design.md).

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

* docs: wave 3 corpus audit + IA-reshuffle dead-link cleanup (53 pages)

Second sweep of the regrounding audit. Covers the 57 pages deferred in
wave 2: producer/ (15), enterprise/ (15), concepts/ (6), integrations/
(7), troubleshooting/ (7), contributing/ (3), reference tail (3), 404.

Process: 6 parallel grounding-verifier agents on disjoint scopes; each
agent extracts factual claims, S7-verifies against current source
('apm <verb> --help' + grep src/apm_cli/), and applies surgical edits
inline. Same pattern as wave 2 (PANEL + WAVE EXECUTION + S7 verifier
fan-out). Orchestrator post-pass swept three cross-corpus broken-link
patterns the per-scope agents could not fix alone.

High-signal factual fixes:

enterprise/governance-guide.md:
- --output-file -> --output (real flag is --output / -o)
- 7+17 check count -> 8+17 (8 baseline checks, not 7)

enterprise/apm-policy.md:
- '16 of 22 checks' -> '17 of 25 checks' (phantom counts)
- conflated --no-policy (install-only) with APM_POLICY_DISABLE (env)

enterprise/apm-policy-getting-started.md:
- dropped 'apm compile' from list of commands that run policy
  (compile enforces zero policy per governance-overview.md L57)

enterprise/policy-reference.md:
- compilation.target.allow: added copilot, gemini, vscode, windsurf,
  agent-skills (only 5 of 9 runtimes were listed)

enterprise/registry-proxy.md:
- 'apm marketplace add --branch main' -> '--ref main' (no --branch flag)

enterprise/security-and-supply-chain.md:
- 3 stale source line-number citations corrected

producer/author-primitives/index.md:
- legacy '.hook.md' extension -> '.json' (hook_integrator scans JSON)
- removed nonexistent '.apm/commands/' subdirectory from layout example

concepts/lifecycle.md:
- 4 reference-page links all pointed at install/ (copy-paste)

Cross-corpus IA-reshuffle dead-link cleanup (orchestrator pass):
- introduction/* -> concepts/* (4 links across 2 files)
- guides/ci-policy-setup/ -> enterprise/enforce-in-ci/ (8 links, 4 files)
- guides/pack-distribute/ -> producer/pack-a-bundle/ (5 links, 4 files)
- guides/dependencies/ -> consumer/manage-dependencies/ (1 link)
- guides/agent-workflows/ -> contextual canonical (3 links, 3 files)
- guides/install-and-use/mcp-servers/ -> consumer/install-mcp-servers/ (3)
- guides/compilation/ -> producer/compile/ (1)
- guides/prompts/ -> producer/author-primitives/prompts/ (2)
- guides/drift-detection/ -> enterprise/drift-detection/ (1)

enterprise/security.md side-fix:
- 'apm unpack scheduled for removal in v0.14' -> drop version target
  (APM is 0.15.0 and unpack still ships marked DEPRECATED in --help).
  Upstream remediation (refresh deprecation timeline in source or
  remove the shim) tracked outside this PR.

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

* docs: close deferred items from corpus regrounding audit

Closes the three items deferred from the v0.14->v0.15 docs-sync
retrospective and the full-corpus regrounding waves (commits
4f00c2b, 242bb9e, b80da69):

1. apm unpack source-side deprecation timeline
   - src/apm_cli/commands/pack.py: 'will be removed in v0.14'
     -> 'will be removed in a future release'. Current version
     is 0.15.0; the v0.14 target had already passed. Docs were
     softened in wave 3; this mirrors the choice in source.
   - CHANGELOG.md: [Unreleased] Fixed entry.

2. Bucket-C silent-drift backfills (20 PRs, parallel triage)
   - 3 grounding-verifier subagents reviewed 20 of the 21
     bucket-C PRs (#1477 excluded as test-flake fix, no doc
     surface). Verdicts: 17 ALREADY_COVERED or NO_DOC_SURFACE
     (verified honestly against wave 2-3 backfills, not
     manufactured), 3 BACKFILLED:
     - #1385 SSH dep user-from-URL: added supported-form row in
       docs/src/content/docs/consumer/manage-dependencies.md
       and bullet in apm-usage/dependencies.md.
     - #1434 Copilot App schema range [13,15] + warn-not-fail:
       rewrote the 'Schema compatibility' paragraph in
       docs/src/content/docs/integrations/copilot-app.md
       (was factually wrong, claimed [13,13] hard-fail).
     - #1440 Copilot file-based detection signals: added the
       four .github/{instructions,agents,prompts,hooks}/
       directories to the canonical-signals list in
       troubleshooting/compile-zero-output-warning.md and to
       the apm-usage commands.md + package-authoring.md
       auto-detect rules.

3. docs-corpus-audit skill extracted
   - .apm/skills/docs-corpus-audit/SKILL.md: first-class skill
     module emitted from the genesis design artifact used to
     drive waves 2 and 3. Pattern: PANEL + WAVE EXECUTION + S7
     verification. Wave-batched (scales as O(waves), not
     O(claims)), disjoint page ownership (no merge conflicts),
     orchestrator post-pass for cross-corpus drift patterns
     invisible to per-scope agents.
   - references/design-handoff.md: full design artifact preserved
     for future maintainers.
   - Sibling to docs-sync (per-PR), not a replacement.

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

* docs: fix dead links + address Copilot review findings

Two classes of fix on PR #1511:

1. Deploy Docs CI -- starlight-links-validator failure (2 dead links)
   - getting-started/first-package.md:18 and quickstart.mdx:40 used
     absolute /apm/getting-started/installation/ paths introduced in
     wave 2 (242bb9e). Converted to relative paths matching the
     surrounding link convention.
   - Verified with local 'npm run build' under docs/: 'All internal
     links are valid.'

2. Copilot PR review -- 7 inline factual accuracy comments, all
   verified against source and addressed:
   - apm-usage/package-authoring.md: hook path rewrite is performed
     by 'apm install' (hook integrator pass), not 'apm compile'.
   - apm-usage/dependencies.md + docs/guides/registries.md: registry
     resolver requires semver per apm_cli/deps/registry/semver.py
     (is_semver_range gate). Removed examples implying opaque labels
     (#stable, #v2.0.0, 'latest') route through a registry; updated
     selector tables to flag non-semver refs as rejected for registry
     sources.
   - apm-usage/dependencies.md + docs/guides/registries.md:
     lockfile_version: '2' promotion triggers on registry deps OR
     git-source semver resolution fields (constraint / resolved_tag /
     resolved_at per lockfile.py:_needs_v2, issue #1488), not just
     registry deps.
   - apm-usage/authentication.md: 'token:' in apm-policy.yml is not
     parse-rejected, only surfaces as an 'Unknown top-level policy
     key' warning per policy/parser.py. Still discouraged (leaks to
     repo), but the rejection mechanism is different from apm.yml.

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

* skill(docs-corpus-audit): refactor under genesis discipline + self-test

Round-trip assessment found the original SKILL.md draft violated
genesis SoC in 7 ways:

1. Invented inline 'grounding-verifier' persona instead of composing
   shared agent personas (python-architect for S7, doc-writer for
   edits). R3 EXTRACT in reverse.
2. Subagent prompt template inlined in SKILL body (~40 lines that
   belong in assets/).
3. IA-reshuffle grep patterns hard-coded in body as bash heredoc --
   the patterns rot per release and belong in scripts/ with --help
   and a versioned update cadence.
4. PHANTOM DEPENDENCY on docs-sync's substrate (.apm/docs-index.yml,
   personas, panelist-return-schema, the apm-usage Rule-4 corpus)
   never declared via tool-call probes -- A9 SUPERVISED EXECUTION
   violation per genesis Step 7b.
5. Missing A8 ALIGNMENT LOOP: wave agents edited inline and nothing
   re-verified the edits grounded.
6. DISPATCH COLLISION risk vs docs-sync: identical 'drift between
   docs and code' triggers; dispatcher LLM could misroute.
7. BUNDLE LEAKAGE: references/design-handoff.md was session-history
   (maintainer-scope), not runtime-loaded. Per genesis 3.5 it must
   NOT ship with the user-facing bundle.

Refactor:
- SKILL.md (218 lines, well under 500-line cap): adds explicit
  Sibling Contract table with docs-sync; declares roster as
  composition of existing personas via relative links;
  PROBE / RISK-TRIAGE / WAVE / POST-PASS / ALIGNMENT-LOOP /
  COMMIT / PR phases; sharpened trigger description naming
  whole-corpus scope.
- assets/subagent-prompt-template.md: extracted the per-scope
  prompt that composes python-architect + doc-writer.
- assets/panelist-return-schema.json: explicit JSON schema for
  agent returns; orchestrator validates and rejects malformed.
- scripts/scan-cross-corpus-drift.sh: deterministic cross-corpus
  drift sweep with 4 pattern groups (ia-links, stale-deprecation,
  absolute-base, ascii-leak). Non-interactive, --help-documented,
  stdout/stderr split per genesis script conventions.
- evals/{trigger,content}-evals.json + README.md: ship gate
  exercising 10+10 trigger queries (docs-sync boundary is the
  load-bearing distinction) and 3 seeded-drift scenarios with
  control baselines.
- Deleted references/design-handoff.md (bundle leak; design
  artifact stays in session state only).

Self-test (proves the refactor works end-to-end):
- Ran scan-cross-corpus-drift.sh against the live corpus; it
  immediately surfaced two genuine misses that wave 3 missed:
  - src/apm_cli/commands/pack.py:606: click help= string still
    said 'removed in v0.14' (the logger.warning at line 633 was
    fixed last commit; this is a sibling string the wave 3 agent
    didn't see because each agent only owned ~9 pages).
  - docs/src/content/docs/reference/cli/unpack.md:9: caution
    banner still said 'scheduled for removal in v0.14'.
- Both softened to 'in a future release' (consistent with the
  rest of the wave 3 choice).
- Lint clean; docs build clean ('All internal links are valid').

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

* skill(docs-grounding-verifier): claim-level grounding harness + 7 drift fixes

New sibling skill to docs-corpus-audit. Genesis-designed PIPELINE-of-PANELS
(RAGAS-faithfulness adapted from RAG to docs/code):
- Stage 1: per-page LLM claim extraction
- Stage 2: deterministic grep-based evidence retrieval (S7, no LLM)
- Stage 3: adversarial LLM grounding judge (A7, 4-verdict calibrated)

Empirical proof bundle (.apm/skills/docs-grounding-verifier/evals/runs/proof/):
- 5 high-stakes pages -> 75 atomic claims extracted
- Tally: 63 GROUNDED / 6 PARTIAL / 4 CONTRADICTED / 2 UNSUPPORTED (84%)
- Trigger eval: 20/20 dispatch classification correct
  (precision=1.0, recall=1.0, specificity=1.0, pass_gate=true)

High-confidence drift fixes applied:
- apm-policy.md: MCP transport defaults (was 'block sse/streamable-http
  by default' -> actually allow=None means all permitted; sample policy
  now correctly framed as restriction example)
- apm-policy.md: inheritance levels (was '5 levels including team policy'
  -> canonical chain is 3 semantic levels; 5 is MAX_CHAIN_DEPTH for
  intermediate extends: jumps)
- Plus 5 editorial fixes from prior pass (examples, registries x2,
  security, copilot-app)

Lower-confidence findings (judge retrieval gaps, vague reasoning) left
for follow-up rather than risk introducing new drift via speculative
edits.

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

---------

Co-authored-by: danielmeppiel <danielmeppiel@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

2 participants