Skip to content

feat(experimental): copilot-app target deploys scheduled prompts to App DB#1405

Open
danielmeppiel wants to merge 10 commits into
mainfrom
danielmeppiel/feat-workflow-primitive-copilot-app
Open

feat(experimental): copilot-app target deploys scheduled prompts to App DB#1405
danielmeppiel wants to merge 10 commits into
mainfrom
danielmeppiel/feat-workflow-primitive-copilot-app

Conversation

@danielmeppiel
Copy link
Copy Markdown
Collaborator

@danielmeppiel danielmeppiel commented May 19, 2026

feat(experimental): copilot-app target deploys scheduled prompts to App DB

TL;DR

Adds an experimental copilot-app target so apm install deploys prompts (with optional workflow frontmatter: flat top-level interval / schedule_hour / schedule_day keys) directly into the GitHub Copilot desktop App's ~/.copilot/data.db workflows table. Gated behind experimental.copilot_app; user-scope only. No new CLI surface — install / update / uninstall / list cover the lifecycle. Workflows always install disabled so the user explicitly opts in from the App.

Important

This is a dark-shipped experimental target. Default-off; no impact on existing users. Lifecycle parity is the contract: every supported CLI verb deletes / preserves the DB row correctly, verified by a five-scenario E2E suite.

Problem (WHY)

  • APM has zero presence on the GitHub Copilot desktop App. The App's "Workflows" surface is a first-class scheduling primitive, but today users must hand-craft rows in ~/.copilot/data.db to use it — there is no package-manager path.
  • The inner loop (Copilot CLI prompts) and outer loop (gh-aw CI workflows) already have APM targets. The middle of the loop — scheduled prompts on the desktop App — is the only gap, and the one most users notice first because the App is the daily driver.
  • [!] Reverse-engineering the App's schema and stuffing rows in via sqlite3 is exactly the kind of brittle, undocumented surface APM exists to abstract. Without a native target, every adopter pays that tax.

Why these matter: APM's three-promise narrative is consume / produce / govern. Without the App target, the "consume" promise stops at the CLI — the surface where prompts are most valuable. The fix is a single new target, not a new primitive type; the existing prompts primitive gains optional workflow frontmatter (flat top-level interval / schedule_hour / schedule_day keys) and routes to the App DB when the target resolves.

Approach (WHAT)

# Decision Why
1 New copilot-app target (separate from copilot CLI) Same ~/.copilot/ root, different surface (DB vs files). Keeps the CLI target's file-based contract clean.
2 NO new primitive type — extend prompts with optional flat workflow keys (interval, schedule_hour, schedule_day) "Less is more." Schedule is metadata on a prompt, not a new noun. Preserves the principle Favor small, chainable primitives over monolithic frameworks.
3 NO new CLI subcommands apm install / update / uninstall / list cover the lifecycle. Zero surface area to deprecate.
4 enabled = 0 always on install (source value ignored) Security: never auto-arm a scheduled task. User opts in from the App where the schedule is visible.
5 Gated behind experimental.copilot_app flag Default-off dark ship; iterate without breaking-change pressure.
6 Lockfile URI scheme copilot-app-db://workflows/<namespaced-id> Precedent: cowork://skills/.... Encodes that the entry is a DB row, not a file path.
7 PRAGMA user_version guard (currently 13) Refuse to write if the App's schema is newer than tested.
8 mode: autopilot policy-blocked for third-party packages until v3 (signing) Safe default; user-authored prompts can use any mode.
9 Update preserves user-side state (enabled, next_run_at, last_run_at, schedule overrides) INSERT … ON CONFLICT DO UPDATE that touches only APM-owned columns.

Implementation (HOW)

  • src/apm_cli/integration/copilot_app_db.py (NEW, 529 LOC) — WAL-safe SQLite client. ID namespacing (apm--<owner>--<pkg>--<prompt>), URI helpers (to_lockfile_uri, COPILOT_APP_LOCKFILE_PREFIX), version guard, custom CopilotAppDBError exception, retry-on-locked. Pure module; no Click or integrator coupling.
  • src/apm_cli/integration/targets.py — Registers copilot-app TargetProfile with requires_flag="copilot_app" and a user-root resolver that returns ~/.copilot/ only when data.db exists.
  • src/apm_cli/integration/prompt_integrator.py — When target.name == "copilot-app", delegates to copilot_app_db.deploy() per prompt instead of writing files. Synthesises <root>/workflows/<id> as the addressing token for the lockfile pipeline.
  • src/apm_cli/install/services.py_deployed_path_entry now checks dynamic-root targets FIRST (by target_path.relative_to(_t.resolved_deploy_root)) before falling back to project-root encoding. This was the critical fix for --global installs where project root is the user home and the synthetic copilot-app root sits inside it.
  • src/apm_cli/install/phases/targets.py — Gates explicit --target copilot-app with three actionable errors: flag-off hint, project-scope rejection (user-scope only), DB-missing.
  • src/apm_cli/commands/uninstall/engine.py — Adds a copilot-app URI scan over sync_managed so user-scope DB-backed targets are cleaned even when the local apm.yml does not enumerate them. Also adds _is_dynamic skip to bypass the dir-exists guard for DB-backed targets.
  • src/apm_cli/integration/base_integrator.py — Extends partition_managed_files dynamic-root branch with the prompts_copilot-app bucket so prompts are correctly routed during sync.
  • src/apm_cli/core/experimental.py — Registers copilot_app flag.
  • Tests (1130+ new LOC) — Five-file unit suite: DB module (335 LOC), error UX (221), schedule frontmatter (104), target registration (118), end-to-end install/update/uninstall (352).
  • Docs — New page docs/src/content/docs/integrations/copilot-app.md; apm-usage skill updated per docs.instructions.md rule 4.

Diagrams

End-to-end deploy flow when the user runs apm install <pkg> --target copilot-app --global against a prompt that carries flat workflow keys (interval, schedule_hour, schedule_day):

sequenceDiagram
    autonumber
    participant U as User
    participant CLI as apm CLI
    participant Resolver as targets.py
    participant Prompt as PromptIntegrator
    participant DB as copilot_app_db
    participant App as ~/.copilot/data.db

    U->>CLI: apm install pkg --target copilot-app --global
    CLI->>Resolver: resolve(copilot-app, user_scope=True)
    Resolver->>Resolver: check experimental.copilot_app
    Resolver->>App: stat data.db (exists?)
    App-->>Resolver: ok, user_version=13
    Resolver-->>CLI: TargetProfile(resolved_deploy_root=~/.copilot/)
    CLI->>Prompt: integrate_prompts_for_target(copilot-app)
    Prompt->>DB: deploy(prompt + schedule, namespaced_id)
    DB->>App: INSERT ... ON CONFLICT DO UPDATE (enabled=0)
    App-->>DB: row written
    DB-->>Prompt: synthetic target_path = ~/.copilot/workflows/apm--owner--pkg--name
    Prompt-->>CLI: target_paths
    CLI->>CLI: _deployed_path_entry emits copilot-app-db URI
    CLI-->>U: 1 prompt installed (disabled, enable in the App)
Loading

The lockfile URI scheme is the single source of truth for cleanup — uninstall scans for the prefix and deletes the matching rows without needing the package's apm.yml to be present:

flowchart LR
    Lock[apm.lock.yaml<br/>deployed_files]
    Scan{Scan for<br/>copilot-app-db://}
    DB[(data.db<br/>workflows)]

    Lock --> Scan
    Scan -->|"copilot-app-db://workflows/apm--..."| Resolve[Synthesise user-scope<br/>copilot-app TargetProfile]
    Resolve --> Delete[sync_for_target<br/>deletes rows]
    Delete --> DB

    Scan -->|"any/other/path"| Skip[Skip - handled by<br/>per-target loop]
Loading

Trade-offs

  • Coupling to App schema version (user_version = 13). We refuse to write on unknown / newer versions and surface an actionable upgrade hint. Alternative considered: schema-agnostic write. Rejected — silently writing into an unknown schema is the exact bug class users would blame APM for.
  • No DB-level auth. SQLite has no authentication primitive; "auth" is filesystem-permission only (App DB is mode 0600 owned by the user). Documented transparently; we do NOT promise auth we don't have.
  • enabled = 0 is non-configurable. Source-file enabled: true is ignored. Trade-off: one extra click for the user, but no third-party package can auto-arm a scheduled task on install. Reversal is one PR if it becomes friction.
  • Local-path installs need the dynamic-root check first. The pre-fix code used relative_to(project_root) to decide whether to apply the URI scheme; for --global installs that succeeded spuriously because the user home contains ~/.copilot/. Fixed by checking dynamic-root targets up front; the legacy out-of-tree branch is preserved for cowork:// semantics.

Benefits

  1. Zero new CLI surfaceinstall / update / uninstall / list cover the lifecycle. Nothing to deprecate later.
  2. One namespaced ID convention (apm--<owner>--<pkg>--<prompt>) makes APM-managed rows trivially distinguishable from user-created ones; uninstall touches only APM rows.
  3. Lifecycle parity verified — 5-scenario E2E suite covers install, re-install (update), uninstall, missing-DB error UX, and the local-path roundtrip that surfaced the URI-scheme bug.
  4. Dark-ship safe — default-off via experimental.copilot_app; existing installs unchanged.
  5. Extension path clear — same pattern (target + dynamic-root + URI scheme) will plug gh-aw (outer loop) in v2 without refactoring.

Validation

Lint (canonical contract from .apm/instructions/linting.instructions.md):

$ uv run --extra dev ruff check src/ tests/ && uv run --extra dev ruff format --check src/ tests/
All checks passed!
793 files already formatted

Full unit suite (8795 passed, single pre-existing Windows-only skip):

$ uv run --extra dev pytest tests/unit/ -q --tb=line
8795 passed, 1 skipped, 1 deselected, 1 warning, 33 subtests passed in 35.13s

Scenario Evidence

# User-promise scenario Test path APM principle
1 apm install writes one DB row per scheduled prompt, namespaced and disabled tests/unit/install/test_install_target_copilot_app_e2e.py::TestCopilotAppDeployUninstall::test_install_then_uninstall_roundtrip Consume — pkg manager owns the deploy contract
2 apm install <local-path> --global deploys with the copilot-app-db:// URI scheme, and apm uninstall deletes the DB row TestCopilotAppDeployUninstall::test_install_local_pkg_then_uninstall_deletes_db_row Consume — lifecycle parity, no orphaned state
3 --target copilot-app without the flag emits an actionable enable hint TestCopilotAppParserE2E::test_flag_off_emits_enable_hint Govern — experimental features are opt-in and discoverable
4 --target copilot-app without --global rejects project scope TestCopilotAppParserE2E::test_project_scope_requires_global Govern — surface contracts must match the underlying resource
5 DB missing → actionable error pointing at App install TestCopilotAppParserE2E::test_flag_on_db_missing_errors Consume — fail loudly, fail helpfully
6 Version-guard refuses on unknown user_version tests/unit/integration/test_copilot_app_db.py (version-guard cases) Govern — never write into an unverified schema
7 Schedule frontmatter validates intervals / hours / days / modes tests/unit/integration/test_copilot_app_schedule.py Produce — package authors get loud errors at compile, not at App runtime
Five copilot-app E2E results (verbatim)
$ uv run --extra dev pytest tests/unit/install/test_install_target_copilot_app_e2e.py -v
tests/unit/install/test_install_target_copilot_app_e2e.py::TestCopilotAppParserE2E::test_flag_off_emits_enable_hint PASSED [ 20%]
tests/unit/install/test_install_target_copilot_app_e2e.py::TestCopilotAppParserE2E::test_flag_on_db_missing_errors PASSED [ 40%]
tests/unit/install/test_install_target_copilot_app_e2e.py::TestCopilotAppParserE2E::test_project_scope_requires_global PASSED [ 60%]
tests/unit/install/test_install_target_copilot_app_e2e.py::TestCopilotAppDeployUninstall::test_install_then_uninstall_roundtrip PASSED [ 80%]
tests/unit/install/test_install_target_copilot_app_e2e.py::TestCopilotAppDeployUninstall::test_install_local_pkg_then_uninstall_deletes_db_row PASSED [100%]
============================== 5 passed in 1.28s ===============================

How to test

  1. apm config set experimental.copilot_app true
  2. Create a tiny package pkg/.apm/prompts/daily-digest.prompt.md with flat workflow keys at the top level (interval: daily, schedule_hour: 9).
  3. apm install ./pkg --target copilot-app --global — expect [+] 1 prompt installed; verify sqlite3 ~/.copilot/data.db "SELECT name, enabled FROM workflows WHERE name LIKE 'apm--%';" shows the row with enabled = 0.
  4. Open the Copilot App, find the workflow under "Workflows", flip the toggle on — enabled flips to 1 in the DB.
  5. apm uninstall ./pkg --global — expect Cleaned up 1 integrated prompts; the row is gone and any user-created rows are untouched.

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

Daniel Meppiel and others added 6 commits May 19, 2026 21:38
…pp DB

Dark-shipped under the new `copilot_app` experimental flag (off by default).

When enabled, `apm install --target copilot-app --global` writes prompts
that carry a `schedule:` frontmatter block as rows in the GitHub Copilot
desktop App's SQLite store at `~/.copilot/data.db`.  No new CLI surface;
`install` / `update` / `uninstall` / `list` all flow through unchanged.

Hard contracts:
- `enabled = 0` on every insert -- user opts in from the App.
- Namespaced ids (`apm--<owner>--<pkg>--<prompt>`) so uninstall never
  touches user-authored rows.
- `PRAGMA user_version` guard (13 currently); refuse to write on unknown.
- WAL-safe SQLite with retry on `database is locked`.
- Update path preserves user state (`enabled`, `last_run_at`, overrides).
- Lockfile URIs use `copilot-app-db://workflows/<id>` (cowork precedent).

Tests: 53 new (DB module, schedule parser, target gating, install E2E).
Full unit suite: 8787 passed (one pre-existing macOS shlex failure
unrelated to this change).

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

Wave 4 + Wave 6a of the copilot-app dark-ship:

- docs/src/content/docs/integrations/copilot-app.md mirrors the
  copilot-cowork page: enable flag, lifecycle, DB resolution, 'auth'
  model, schema guard, concurrency, lockfile URI scheme, out-of-scope.
- apm-usage skill: commands.md notes copilot-app under experimental;
  package-authoring.md documents the optional schedule: frontmatter
  block.
- tests/unit/integration/test_copilot_app_error_ux.py (5 tests)
  exercises CopilotAppDbMissingError, CopilotAppDbSchemaError,
  CopilotAppDbLockedError mid-deploy: each surfaces as an actionable
  per-prompt diagnostic; one failing prompt does not block the next;
  resolver returning None mid-run is defensive (no crash).
- tests/unit/install/test_services.py adds a round-trip test for
  copilot-app-db:// URI generation through _deployed_path_entry.

Full unit suite: 8794 passed (1 pre-existing unrelated macOS skip).
Lint contract green.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When 'apm install <local-pkg> --target copilot-app --global' was invoked,
the lockfile stored 'workflows/apm--...' without the 'copilot-app-db://'
scheme prefix. As a result, the subsequent uninstall could not detect the
copilot-app entry and the DB row was orphaned in the Copilot App.

Root cause: _deployed_path_entry tried 'target_path.relative_to(project_root)'
first. For --global installs, project_root is the user home and the
synthetic copilot-app root (~/.copilot/workflows) sits inside it, so the
relative_to() succeeded and skipped the dynamic-root URI branch entirely.

Fix: detect dynamic-root target match (cowork, copilot-app) before
attempting the project_root-relative encoding. The cowork PathTraversalError
behavior is preserved for the legacy out-of-tree case.

Adds 'test_install_local_pkg_then_uninstall_deletes_db_row' end-to-end
regression covering the install -> lockfile URI -> uninstall -> DB row
deletion roundtrip. Also extends partition_managed_files dynamic-root
branch with the 'prompts_copilot-app' bucket and adds a copilot-app scan
in uninstall engine so user-scope DB-backed targets are cleaned even when
the local apm.yml does not enumerate them.

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

Address apm-review-panel CEO synthesis for PR #1405:

Security (supply-chain-security-expert blocking):
- Remove 'autopilot' from _VALID_MODES (copilot_app_db.py) and
  _VALID_SCHEDULE_MODES (prompt_integrator.py). Earlier docstring
  claimed third-party autopilot was policy-blocked but no code
  enforced it -- this lands the actual enforcement at the writer.
- deploy_workflow UPDATE branch now compares prompt body, mode,
  interval, schedule, model, and reasoning_effort against the
  existing row; when any execution-affecting field changes the
  user's prior opt-in is revoked (enabled = 0, next_run_at = NULL).
  Display-only changes (e.g. just the name) still preserve enabled,
  last_run_at, next_run_at. Closes the silent-malicious-update
  vector the panel flagged.

Test coverage (test-coverage-expert):
- Split the prior 'preserves enabled across updates' test into
  two scenarios that match the new semantics and add a third
  test covering schedule changes and a regression test that
  pins mode='autopilot' as rejected.

Docs (doc-writer blocking):
- Register copilot-app in the Starlight sidebar.
- Add copilot-app row to experimental flag table and update the
  targets-matrix experimental note + auto-detection callout.
- Strip false 'apm list' lifecycle row; replace the 'autopilot
  policy-blocked' paragraph with the secure-by-default rationale;
  expand the lifecycle table so the content-change reset is
  documented; fix two 'copilot_app flag' -> 'copilot-app flag'
  kebab-case drifts.

CHANGELOG (devx-ux nit):
- Replace 'apm config set experimental.copilot_app true' with
  the canonical 'apm experimental enable copilot-app'.

Tests: 62/62 copilot-app suite green; 1970/1970 integration+install
suite green; lint and format silent.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- copilot_app_db.deploy_workflow INSERT now hardcodes enabled=0 in the
  SQL (was: row.enabled passthrough). Defence in depth: a future caller
  cannot bootstrap an auto-running APM-deployed row even if the row
  dataclass carries enabled=1. The user opt-in path stays the same:
  enable from the App UI after install.
- New test: test_insert_forces_enabled_zero_even_if_caller_passes_one.
- Docs (copilot-app.md): lifecycle table row 3 now lists all 7
  execution-affecting fields (prompt, schedule, mode, model, reasoning
  effort), matching deploy_workflow comparison semantics.
- Docs (copilot-app.md): error wording for locked-DB paraphrased
  instead of quoting a string the code never emits.
- Docs (package-authoring.md): YAML example drops the autopilot
  comment; rationale aligned with the integrations/copilot-app.md
  framing (intentionally not accepted via this target).

Closes iter-2 panel feedback. No blocking findings from any of 8
panelists; this iteration converges the residual recommended items.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel
Copy link
Copy Markdown
Collaborator Author

APM Review Panel: ship_now

Iteration 3 closes all four recommended residuals from iter-2; blocking surface is empty and ship_now is justified.

The panel's trajectory across three iterations is the cleanest convergence we've had on an experimental target. Iter-1 surfaced three legitimate in-PR blocking clusters (autopilot enforcement at the writer boundary, enabled=0 reset on update, and the docs surface gap). Those landed in iter-2 commit 4cfca02c, and the iter-2 panel -- with full coverage from supply-chain, test-coverage, python-architecture, doc-writer, cli-logging, devx-ux, and growth, plus auth-expert correctly self-deactivating -- returned zero required findings. The four recommended residuals (force enabled=0 at the DB writer per defence-in-depth, lifecycle-table row 3 listing all seven execution-affecting fields, paraphrasing the locked-DB error string the code doesn't actually emit, and removing the stale autopilot example from package-authoring.md) all landed in iter-3 commit fc40650d with new test coverage on the writer invariant.

The load-bearing evidence is unambiguous on this commit: supply-chain verified 7-field comparison completeness with atomic single-UPDATE-inside-BEGIN-IMMEDIATE semantics under a RESERVED lock; test-coverage certified 30/30 copilot-app DB tests pass in 0.65s with assertions pinning the exact deploy_workflow control flow (including the new test_insert_forces_enabled_zero_even_if_caller_passes_one); iter-3 targeted sweep is 63/63 in 1.27s on top of the iter-2 full 1970/1970 integration baseline; lint is silent. Auth-expert active:false is the correct call -- this PR touches no token, no remote host, no credential surface. The remaining items in the panelist returns (devx scan-quality polish, growth narrative ramps, cli-log nit #2 install-summary surface, py-arch's DeploymentBackend strategy refactor) are recommended/nit-tier and were either explicitly deferred in iter-1 as scope-creep or are GA-graduation work that belongs in the follow-up tracker once copilot-app exits the experimental gate.

No dissent to arbitrate. No new blocker emerged. The experimental gate (apm experimental enable copilot-app) plus user-scope-only plus enabled=0 reset gives us the safety envelope to ship now and iterate based on real telemetry from early adopters, which is exactly the use case the experimental flag was built for.

Dissent. None. Auth/sec/test-coverage were unanimous-clean in iter-2 and remain so on iter-3 HEAD. Recommended/nit items from python-architect, doc-writer, cli-logging-expert, devx-ux-expert, and oss-growth-hacker were either landed in iter-3 (4 items) or already classified as deferrable polish/strategy work by the originating panelists themselves.

Aligned with: secure-by-default (iter-3 hardens the safety envelope at three independent layers: experimental gate, writer-side INSERT forcing enabled=0 in SQL regardless of caller input pinned by a dedicated test, and update-path reset across all seven execution-affecting fields); devx (deferral discipline held across all three iterations -- DeploymentBackend strategy refactor, synthetic install-summary path, schema version escape hatch, and validation mirror dedup were all correctly kept out of this PR); pragmatic-trust (63/63 targeted + 1970/1970 integration baseline + new INSERT-invariant test means the writer contract is mechanically pinned, not just asserted in prose).

Growth signal. Growth-hacker's discoverability cross-link and the 60s demo recording are the two highest-leverage moves for the GA narrative -- both belong in the follow-up tracker tagged for copilot-app graduation, not in this PR. The current experimental framing is correct: ship the capability gated, let early adopters validate the schedule semantics, then narrative-amplify at GA.

Panel summary

Persona B R N Takeaway
Python Architect 0 1 1 Iter-3 hardening landed; DeploymentBackend strategy + validation mirror dedup deferred.
CLI Logging Expert 0 0 1 Doc-truthfulness fix landed; install-summary surface nit deferred to broader UX work.
DevX UX Expert 0 0 3 Scan-quality polish nits; no blocking ergonomics regression.
Supply Chain Security Expert 0 0 0 Clean. Atomic single-UPDATE under RESERVED lock; 7-field comparison verified.
OSS Growth Hacker 0 0 3 Narrative ramps deferred to GA-graduation; experimental framing is correct.
Doc Writer 0 0 0 Both recommended items landed iter-3 (lifecycle row + autopilot example).
Test Coverage Expert 0 0 1 Load-bearing PASSED evidence: 63/63 targeted on HEAD fc40650d.
Auth Expert -- -- -- Inactive: no token, remote host, or credential surface touched.

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

Top 4 follow-ups

  1. [OSS Growth Hacker] Cross-link copilot-app from prompt-frontmatter reference page -- highest-leverage discoverability surface; anyone reading schedule: frontmatter docs is the exact target audience. Deferrable to copilot-app GA graduation when narrative ramp-up matters most.
  2. [Python Architect] Extract DeploymentBackend strategy when a third non-file target is proposed -- two string-dispatch sites is the threshold-approaching signal, not the threshold itself. Premature abstraction now would force the shape of a third target we haven't seen; defer until the third backend's contract is concrete.
  3. [CLI Logging Expert] Add one-line diagnostics.info on silent enabled=0 reset during update -- today the safety reset is invisible at install-time, which can surprise a user who expected their scheduled prompt to stay enabled after a content edit. One info-line closes the surprise gap without the broader install-summary work.
  4. [OSS Growth Hacker] Record 60-second scheduled-prompts demo for GA launch beat -- the "scheduled prompts in your desktop Copilot" framing is the strongest narrative angle this target has. Worth a dedicated launch artifact when copilot-app graduates from experimental.

Recommendation

Iter-3 closes the loop on every legitimate recommended residual from iter-2 (4/4 landed). The blocking surface is empty: supply-chain is clean with atomic writer semantics under RESERVED lock, test-coverage is load-bearing positive with 63/63 targeted + 1970/1970 integration on the baseline + a new test pinning the INSERT enabled=0 invariant, auth surface is untouched, and lint is silent. The capability is gated behind apm experimental enable copilot-app and user-scope only, which is the correct safety envelope for a target that writes into a Copilot desktop App SQLite store. All remaining items in the panelist returns are recommended/nit polish or strategy work -- DeploymentBackend strategy extraction, install-summary surface, GA-time narrative beats -- and belong in the follow-up tracker, not in this PR. Ship it, log the four follow-ups, and let early experimental adopters generate the telemetry that informs the GA-graduation work.

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

@danielmeppiel danielmeppiel marked this pull request as ready for review May 19, 2026 20:58
Copilot AI review requested due to automatic review settings May 19, 2026 20:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds an experimental copilot-app target that deploys scheduled prompts into the GitHub Copilot desktop App’s SQLite DB (~/.copilot/data.db) while keeping lifecycle parity via lockfile URIs.

Changes:

  • Introduces a new SQLite integration module (copilot_app_db) + a copilot-app-db://... lockfile URI scheme for DB-backed workflow rows.
  • Extends prompt integration + cleanup flows to deploy/sync Copilot App workflows (including schedule frontmatter parsing and uninstall scanning).
  • Adds extensive unit/E2E coverage and documentation for the experimental feature flag + target behavior.
Show a summary per file
File Description
tests/unit/integration/test_data_driven_dispatch.py Updates bucket expectations to include the new prompts_copilot-app bucket.
tests/unit/integration/test_copilot_app_target.py Adds gating/flag/target-resolution tests for copilot-app.
tests/unit/integration/test_copilot_app_schedule.py Adds unit tests for schedule: frontmatter parsing + owner derivation.
tests/unit/integration/test_copilot_app_error_ux.py Validates per-prompt error surfacing when DB deploy fails.
tests/unit/integration/test_copilot_app_db.py Adds unit coverage for DB schema guard, deploy/update semantics, and cleanup helpers.
tests/unit/install/test_services.py Exercises lockfile encoding for synthetic copilot-app paths.
tests/unit/install/test_install_target_copilot_app_e2e.py Adds E2E install/uninstall scenarios for the experimental target.
tests/unit/core/test_target_detection.py Locks EXPERIMENTAL_TARGETS membership to include copilot-app.
tests/unit/core/test_scope.py Updates known-target assertions to include copilot-app.
src/apm_cli/integration/targets.py Registers copilot-app target with user-scope resolver + experimental gating.
src/apm_cli/integration/prompt_integrator.py Adds DB-backed deploy/sync branches + schedule parsing helpers.
src/apm_cli/integration/copilot_app_db.py New SQLite client module for workflow row deploy/delete/list with WAL lock retry + schema guard.
src/apm_cli/integration/base_integrator.py Extends managed-file partitioning to route copilot-app URIs to the right bucket.
src/apm_cli/install/services.py Changes lockfile path encoding to prefer dynamic-root targets (copilot-app/cowork) first.
src/apm_cli/install/phases/targets.py Adds explicit --target copilot-app gating + actionable errors.
src/apm_cli/core/target_detection.py Adds copilot-app to EXPERIMENTAL_TARGETS (exclude from --target all).
src/apm_cli/core/experimental.py Registers new copilot_app experimental flag.
src/apm_cli/commands/uninstall/engine.py Ensures DB-backed prompts are removed by scanning lockfile URIs during uninstall.
packages/apm-guide/.apm/skills/apm-usage/package-authoring.md Documents schedule: frontmatter for Copilot App (experimental).
packages/apm-guide/.apm/skills/apm-usage/commands.md Documents enabling/installing copilot-app and the env override.
docs/src/content/docs/reference/targets-matrix.md Documents experimental target detection behavior (copilot-app explicit-only).
docs/src/content/docs/reference/experimental.md Adds copilot-app to the experimental flags list.
docs/src/content/docs/integrations/copilot-app.md New integration doc page for Copilot App workflows.
docs/astro.config.mjs Adds Copilot App integration page to the docs sidebar.
CHANGELOG.md Announces the experimental copilot-app target and its behavior.

Copilot's findings

  • Files reviewed: 25/25 changed files
  • Comments generated: 4

Comment on lines 74 to +91
try:
return target_path.relative_to(project_root).as_posix()
except ValueError:
# Path is outside the project tree -- must be a dynamic-root
# target. Find the matching target and translate.
# Path is outside the project tree and no dynamic-root target
# contained it. Fall through to the legacy cowork translation
# which security-validates against deploy_root and raises
# PathTraversalError when out of bounds.
if targets:
for _t in targets:
if _t.resolved_deploy_root is not None:
from apm_cli.integration.copilot_cowork_paths import to_lockfile_path
if _t.resolved_deploy_root is None:
continue
if _t.name == "copilot-app":
from apm_cli.integration.copilot_app_db import to_lockfile_uri

return to_lockfile_uri(target_path.name)
from apm_cli.integration.copilot_cowork_paths import to_lockfile_path

return to_lockfile_path(target_path, _t.resolved_deploy_root)
return to_lockfile_path(target_path, _t.resolved_deploy_root)
Comment on lines +164 to +172
Returns ``None`` when no DB file exists, signalling target
unavailability to the resolver. Callers should treat this as
"Copilot App not installed" and skip the target (auto-detect) or
raise an actionable error (explicit ``--target copilot-app``).
"""
env_override = os.environ.get("APM_COPILOT_APP_DB")
if env_override:
candidate = Path(env_override).expanduser()
return candidate if candidate.is_file() else None
Comment on lines +423 to +427
_VALID_SCHEDULE_INTERVALS: frozenset[str] = frozenset({"manual", "hourly", "daily", "weekly"})
_VALID_SCHEDULE_MODES: frozenset[str] = frozenset({"interactive", "plan"})
"""Mirror of ``copilot_app_db._VALID_MODES``. ``autopilot`` is
deliberately omitted -- see that module's docstring for the
secure-by-default rationale."""
Comment on lines +197 to +199
def _slugify(token: str) -> str:
"""Reduce *token* to safe ASCII-alphanumeric + hyphen/underscore."""
return _SLUG_RE.sub("-", token).strip("-").lower() or "unknown"
Daniel Meppiel and others added 3 commits May 19, 2026 23:18
A team-shared scheduled prompt declared in a project's apm.yml now
deploys to the developer's ~/.copilot/data.db on 'apm install', without
requiring '--global' user-scope install. The previous gate forced every
contributor to repeat the install at user scope to receive workflows
the team had already declared in the manifest.

Architectural change:
- Add TargetProfile.scope_invariant_resolver (default False).
- copilot-app sets scope_invariant_resolver=True because its deploy
  root (~/.copilot/data.db) is a user-machine resource that exists
  regardless of install intent.
- TargetProfile.for_scope(user_scope=False) now runs user_root_resolver
  for scope-invariant targets, populating resolved_deploy_root so the
  lockfile enrichment can map the synthetic 'workflows/<id>' path to
  the copilot-app-db://workflows/<id> URI.
- Cowork remains scope-sensitive (project-scope cowork still rejected).

Security envelope: the experimental copilot_app flag remains the
single opt-in gate. Removing the --global gate folds two consent
layers (flag + user-scope) into one (flag), which matches v1's stated
'apm install just works' UX promise. The DB row is still INSERTed with
enabled=0, the namespaced 'apm--<owner>--<pkg>--<prompt>' ID is
preserved, and the lockfile URI keeps uninstall surgical.

Tests:
- 8801 unit tests pass (full sweep).
- 64 copilot-app tests pass (was 63).
- New test_install_project_scope_then_uninstall_deletes_db_row
  exercises the full roundtrip via project apm.yml + chdir; rewrites
  the prior test_project_scope_requires_global which asserted the
  inverse.
- Manual verification in /tmp: install -> DB row appears with
  enabled=0 -> uninstall -> DB row gone.

Docs:
- integrations/copilot-app.md install incantation updated.
- apm-usage skill commands.md + package-authoring.md mention both
  project and user scope.
- CHANGELOG entry rewritten.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Two recommended findings from devx-ux-expert re-panel (Opus 4.7,
agent_id devx-on-gate-lift):

1. Install output is silent about the 'enable in Copilot App' step.
   Added one-line trailing hint after the 'N prompts integrated ->
   copilot-app/workflows/' line, only when copilot-app actually wrote
   rows in this run:

       [+] /pkg (local)
         |-- 1 prompts integrated -> copilot-app/workflows/
         |-- workflows arrive disabled; enable from the Copilot App's
             Workflows tab

   This closes the first-contributor failure mode that the gate-lift
   surfaces (someone runs plain 'apm install' on a project that
   declares copilot-app in targets, sees the integrated line, doesn't
   realise the row landed enabled=0 and needs a Copilot App toggle to
   fire).

2. targets-matrix.md docs row understated project-scope ride-along
   for the three never-auto-detected targets. Reworded to call out
   that a project apm.yml 'targets:' field lets contributors pick
   them up via plain 'apm install'.

Plus the test-coverage nit: pinned verbatim install output shape in
the new project-scope roundtrip test (asserts 'prompts integrated'
AND 'enable from the Copilot App' appear).

Verification:
- 64 copilot-app tests pass
- Full unit sweep 8800 pass (1 pre-existing flake on
  test_runtime_windows.py unrelated to gate-lift -- fails on
  fc40650 too because local 'codex' binary is installed)
- Lint+format silent
- Manual e2e:
    [+] /pkg (local)
      |-- 1 prompts integrated -> copilot-app/workflows/
      |-- workflows arrive disabled; enable from the Copilot App's Workflows tab

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Option B refinement: distinguish workflow-shape prompts from plain
.prompt.md unambiguously. Only {interval, schedule_hour, schedule_day}
mark a prompt as a Copilot App workflow row; `mode` and
`reasoning_effort` are valid OPTIONAL fields on a workflow but cannot
flip the shape because plain VSCode prompts use `mode: agent|ask|edit`
legitimately.

Without this narrow, any plain prompt that set `mode:` would silently
land as a (broken) workflow when the user passed --target copilot-app,
or a workflow row could be lossy when a writer set only `mode:`.

Live e2e verified:
- Single-target copilot: workflow-shape SKIPPED, plain ships to
  .github/prompts/ correctly.
- Single-target copilot-app: workflow row in ~/.copilot/data.db with
  enabled=0; plain prompt warns then skips.
- Multi-target copilot,copilot-app (comma-separated): both dispatch
  paths fire; no leak between them.
- Update preserves user-side enabled=1 across re-install.
- Lockfile records copilot-app-db:// URIs cleanly; apm audit clean.

Warning text narrowed to actually-mandatory keys so the hint is
truthful and reproducible.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel
Copy link
Copy Markdown
Collaborator Author

APM Review Panel: ship_with_followups

First APM surface lighting up the GitHub Copilot desktop App via .prompt.md workflows behind apm experimental enable copilot-app, ships lint-clean with 70/70 tests and improvement-tier follow-ups only.

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

All eight panelist threads converged: this PR has no correctness or trust blockers and is a strong candidate to land as the experimental, opt-in third primitive surface it claims to be. The strategic framing from oss-growth is the load-bearing one to internalize before merge -- APM now spans Copilot CLI (inner loop), Copilot App (middle / daily-driver), and gh-aw CI (outer loop). That is a positioning shift worth treating as a real release beat at graduation, not just a CHANGELOG line.

Two recommended findings deserve more weight than the rest of the recommended tier. First, test-coverage-expert produced the only evidence-bearing finding in the panel: a missing regression test for the workflow-shape skip on --target copilot (PromptIntegrator path), with outcome: missing on a surface tagged both devx and secure-by-default. Per the panel's evidence-weighting rule, a missing automated guardrail on a secure-by-default surface ranks at or near the top of follow-up priority. The sibling CommandIntegrator path is covered; the PromptIntegrator path is not, and silent drift here would let App-only schedule metadata bleed into .github/prompts/. Second, supply-chain-security-expert flagged that _SLUG_RE does not collapse repeated -, leaving a theoretical cross-package ID collision via author-controlled segments. Auto-arm + enabled=0 + schema guard hold, but the -- separator is the only thing keeping per-package uninstall scoped to its own rows. Both fixes are tiny.

No dissent across panelists. Python-architect's Strategy refactor on TargetProfile is real architectural debt to schedule before the third dynamic-root target lands, not a precondition for this one; the unreachable _deployed_path_entry fallback they flagged is latent and worth deleting in the same follow-up. DevX UX's --target help-text and multiple=True footgun are honest daily-driver friction and the cheapest UX wins on the surface. CLI logging's severity nit on the flag-off branch and doc-writer's sidebar order collision are clean post-merge cleanups. The PR body's schedule: block wording vs the implementation's flat top-level keys is the one item worth fixing in-place before merge to avoid seeding wrong mental models in the merge commit log.

Aligned with: secure-by-default -- Auto-arm enabled=0, autopilot-mode block, schema-version guard, and workflow-shape skip all hold; the slugify -- collision and the missing PromptIntegrator workflow-shape regression test are the two gaps to close so the guardrails stay automatic rather than reviewer-enforced. | portable-by-manifest -- Lockfile entries round-trip through copilot-app-db://workflows/<id> URIs, so uninstall remains manifest-driven rather than filesystem-scan-driven; preserves the promise even on a non-file target. | pragmatic-as-npm -- Third primitive surface lands without breaking the install/uninstall mental model; the --target help text gap and silent last-wins on repeated -t flags are the residual friction worth smoothing before this graduates. | OSS-community-driven -- Graduation story compounds only if github/awesome-copilot ships a canonical scheduled .prompt.md so the 60-second demo command is real; track as a sibling-repo follow-up, not a blocker here.

Growth signal. First APM surface that lands on the GitHub Copilot desktop App closes the inner-loop / outer-loop story with a daily-driver middle loop. Worth logging the strategic beat in WIP/growth-strategy.md now and pre-staging the enabled=0 toggle moment in announcement copy so the unavoidable consent step at graduation reads as a feature, not friction.

Panel summary

Persona B R N Takeaway
Python Architect 0 2 2 Clean DB I/O boundary and well-narrowed shape predicate; name-based dispatch for dynamic-root targets is now creeping across 4+ sites and wants a small Strategy on TargetProfile before a third dynamic target lands.
CLI Logging 0 0 1 Diagnostics are ASCII-clean, actionable, STATUS_SYMBOLS-consistent; one nit on severity level for flag-off path.
DevX UX 0 4 1 Solid mental model and hard-fail diagnostic; small discoverability + footgun gaps in --target help and one body/code shape drift to reconcile.
Supply Chain Security 0 1 1 Auto-arm, autopilot block, URI scheme, schema guard, workflow-shape skip all hold; one ID-collision vector worth a cheap slugify tightening.
OSS Growth 0 3 3 Huge conversion potential -- first APM surface lighting up the daily-driver Copilot App; ship and log strategy follow-ups.
Doc Writer 0 2 3 copilot-app.md accurate to the 3-key shape and lifecycle; minor wiring gaps (sidebar collision, missing cross-refs) and one WAL phrasing nit.
Test Coverage 0 1 0 Workflow-shape skip on the Copilot target (.github/prompts/) lacks a direct regression test; other 7 promises well-defended.

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

Top 5 follow-ups

  1. [Test Coverage] Add a regression test asserting a workflow-shape .prompt.md is NOT written under .github/prompts/<pkg>/ when shipped via --target copilot, mirroring the existing CommandIntegrator probe (~15 LOC in tests/unit/integration/test_copilot_app_error_ux.py). -- Only evidence-bearing finding in the panel (outcome: missing on a surface tagged devx + secure-by-default). The CommandIntegrator skip is covered; the PromptIntegrator skip is not.
  2. [Supply Chain Security] Collapse repeated - in _slugify (copilot_app_db.py:198) so -- cannot appear inside any owner/pkg/prompt segment, and pin it with a test (e.g. weekly--report -> weekly-report). -- The -- separator is the sole guarantee that one package's uninstall only deletes its own rows. The cross-package delete invariant should not depend on reviewer vigilance.
  3. [Python Architect] Introduce a small LockfileCodec Protocol on TargetProfile (uri_prefix + encode/decode/is_uri) and migrate cowork + copilot-app off target.name == ... string-equality dispatch; delete the unreachable _deployed_path_entry fallback at services.py:81-91 in the same change. -- Name-equality dispatch now lives at 4+ sites; the next dynamic-root target becomes a 5-site patch instead of a one-line registration. Schedule before the third dynamic target lands.
  4. [DevX UX] Mention copilot-app in the --target help text (mirroring the copilot-cowork clause at install.py:930), and either reject duplicate -t flags with an actionable error or call out the comma-vs-repeat trap explicitly in help. Also reconcile the PR body's schedule: block phrasing with the actual flat top-level keys before merge. -- Discoverability gap right where every user looks first, plus a real footgun for users coming from gh/docker reflex of -t a -t b.
  5. [OSS Growth] Open a sibling-repo follow-up to land a canonical scheduled .prompt.md (e.g. daily-digest) in github/awesome-copilot so apm install github/awesome-copilot/prompts/daily-digest --target copilot-app --global becomes a real demo command; log the strategic beat in WIP/growth-strategy.md. -- Without a public package the graduation screencast has to use a hand-rolled local example.

Architecture

classDiagram
    direction LR
    class TargetProfile {
      +name str
      +primitives dict
      +resolved_deploy_root Path
    }
    class PromptIntegrator {
      +integrate_prompts_for_target(target, pkg)
      +sync_for_target(target, pkg)
      -_integrate_prompts_for_copilot_app(target, pkg)
      -_sync_copilot_app(managed)
    }
    class BaseIntegrator {
      +partition_managed_files(files, targets)
      +partition_bucket_key(prim, target)
    }
    class CommandIntegrator {
      +integrate_commands_for_target(...)
    }
    class CopilotAppDb {
      +resolve_copilot_app_db_path() Path
      +deploy_workflow(db, row) str
      +delete_workflows(db, ids) int
      +list_managed_workflow_ids(db) list
      +to_lockfile_uri(id) str
      +from_lockfile_uri(uri) str
      +namespaced_id(o, p, n) str
    }
    class WorkflowRow {
      +id str
      +name str
      +prompt str
      +interval str
      +schedule_hour int
      +schedule_day int
      +enabled int
      +mode str
    }
    class DeployedPathEntry {
      +_deployed_path_entry(path, root, targets) str
    }
    PromptIntegrator --|> BaseIntegrator
    CommandIntegrator --|> BaseIntegrator
    PromptIntegrator ..> TargetProfile : reads name branch
    PromptIntegrator ..> CopilotAppDb : delegates I/O
    PromptIntegrator ..> WorkflowRow : constructs
    CopilotAppDb ..> WorkflowRow : validates+writes
    BaseIntegrator ..> TargetProfile : reads name branch
    DeployedPathEntry ..> TargetProfile : reads name branch
    DeployedPathEntry ..> CopilotAppDb : URI encode
    CommandIntegrator ..> PromptIntegrator : imports _is_workflow_shape
    class PromptIntegrator:::touched
    class CommandIntegrator:::touched
    class BaseIntegrator:::touched
    class CopilotAppDb:::touched
    class WorkflowRow:::touched
    class DeployedPathEntry:::touched
    classDef touched fill:#fff3b0,stroke:#d47600
Loading
flowchart TD
    A[apm install --target copilot-app] --> B[resolve_targets]
    B --> C{copilot-app flag on?}
    C -->|no| C1[[error: enable flag]]
    C -->|yes| D[integrate_package_primitives]
    D --> E[PromptIntegrator.integrate_prompts_for_target]
    E -->|target.name == copilot-app branch| F[_integrate_prompts_for_copilot_app]
    F --> G[resolve_copilot_app_db_path ~/.copilot/data.db]
    G -->|missing| G1[return empty IntegrationResult]
    G -->|present| H[loop find_prompt_files]
    H --> I{_is_workflow_shape?}
    I -->|no| I1[warn no workflow frontmatter, skip]
    I -->|yes| J[_parse_workflow_frontmatter]
    J --> K[namespaced_id apm--owner--pkg--prompt]
    K --> L[WorkflowRow enabled=0 hardcoded]
    L --> M[deploy_workflow db_path, row]
    M --> N[sqlite3.connect WAL]
    N --> O[PRAGMA user_version guard 13]
    O -->|outside range| O1[[CopilotAppDbSchemaError]]
    O -->|in range| P[BEGIN IMMEDIATE retry 5s backoff]
    P -->|exceeded| P1[[CopilotAppDbLockedError]]
    P -->|acquired| Q{existing row?}
    Q -->|no| R[INSERT enabled=0]
    Q -->|yes execution_changed| S{prompt/mode/schedule differ?}
    S -->|yes| T[UPDATE reset enabled=0 clear next_run_at]
    S -->|no| U[UPDATE name+updated_at only]
    R --> V[COMMIT]
    T --> V
    U --> V
    V --> W[to_lockfile_uri copilot-app-db://workflows/id]
    W --> X[services._deployed_path_entry encode]
    X --> Z[lockfile deployed_files += copilot-app-db URI]
    CMD[CommandIntegrator per target] -.->|imports _is_workflow_shape| I2[skip workflow-shape prompts]
    UN[apm uninstall] --> UN1[engine direct-scan for copilot-app-db://]
    UN1 --> UN3[sync_for_target -> _sync_copilot_app -> delete_workflows]
Loading

Recommendation

Land it. The PR is experimental, opt-in behind apm experimental enable copilot-app, lint-clean, fully tested at 70/70, and every finding is improvement-oriented rather than correctness-blocking. Track the missing PromptIntegrator workflow-shape regression test and the _slugify repeated-dash tightening as the two highest-signal follow-ups before this graduates out of experimental; the architecture Strategy refactor and the awesome-copilot demo package are the medium-term plays that turn this from a quiet third primitive into the daily-driver headline beat.


Full per-persona findings

Python Architect

  • [recommended] Dynamic-root target dispatch via target.name == 'copilot-app' creeps across 4+ sites; introduce a small Strategy on TargetProfile before the third dynamic target lands. at src/apm_cli/integration/base_integrator.py:418
    Name-equality branching on target.name == 'copilot-app' (and parallel cowork special-cases) now lives in: prompt_integrator.integrate_prompts_for_target (L86), prompt_integrator.sync_for_target (L117), base_integrator.partition_managed_files (L412-427), install/services._deployed_path_entry (L67, L85), and a direct-scan fallback in commands/uninstall/engine.py (L592-615). That is the classic 'extract when shared at 3+ sites' threshold. Each branch couples to the SAME two facts (this target's lockfile URI prefix + its custom encode/decode + its bucket-key partition) wired through string equality instead of through the TargetProfile value object. The third dynamic-root target becomes a 5-site patch instead of a one-place registration.
    Suggested: Attach an optional lockfile_codec: LockfileCodec | None to TargetProfile, a small frozen dataclass / Protocol carrying uri_prefix, encode(local_id), decode(uri), is_uri(s). Cowork and copilot-app each register one (~20 LOC each).
  • [recommended] _deployed_path_entry fallback (lines 81-91) is unreachable in practice and picks the first dynamic-root target arbitrarily if reached -- latent wrong-encoding bug. at src/apm_cli/install/services.py:81
    The primary loop (L59-73) exhaustively iterates every dynamic-root target and returns on first containment match. The fallback at L81-91 does NOT re-check target_path.relative_to(_t.resolved_deploy_root); it returns the encoded URI of the FIRST dynamic-root target in iteration order. With both cowork and copilot-app active that would encode a cowork file path as a copilot-app URI. Today unreachable, but any future refactor that drops the containment check activates the bug.
    Suggested: Delete L81-91 entirely; the primary loop already handles every legitimate dynamic-root path, and falling through to RuntimeError gives a faster, louder bug report than a silently-misencoded URI.
  • [nit] _VALID_SCHEDULE_MODES in prompt_integrator.py shadows _VALID_MODES in copilot_app_db.py with no test pinning their equality. at src/apm_cli/integration/prompt_integrator.py:449
    Two copies of the same set with a docstring acknowledging the mirror is a known smell. The autopilot-exclusion invariant is security-load-bearing; a future contributor who adds 'autopilot' to one but not the other passes silently.
    Suggested: Re-export the canonical set: from .copilot_app_db import _VALID_MODES as _VALID_SCHEDULE_MODES, or add a one-line equality assertion test.
  • [nit] Bucket key 'prompts_copilot-app' contains a hyphen; reads like a typo at the call site and is not normalized. at src/apm_cli/integration/base_integrator.py:423
    Every other bucket key in _BUCKET_ALIASES is single-word or underscore-joined (agents_github, rules_cursor). The dynamically-generated prompts_copilot-app is the first hyphenated key and the only one mixing underscore-join with hyphen-from-target-name. Reading _buckets.get('prompts_copilot-app') at a call site looks like a slip.
    Suggested: Normalize target-name hyphens to underscores inside partition_bucket_key, or add an explicit alias entry so the bucket key is prompts_copilot_app.

CLI Logging

  • [nit] Flag-off copilot-app hint uses info symbol when user-requested target is silently dropped at src/apm_cli/install/phases/targets.py:193
    User explicitly passes --target copilot-app, flag is OFF, target gets dropped from _targets with only a blue [i] hint. Per the traffic-light rule, an explicitly-requested-but-not-honored target reads closer to yellow [!] warning ('should know') than blue info ('FYI'). The DB-missing branch a few lines down correctly uses symbol="cross" + SystemExit; the flag-off branch could use ctx.logger.warning(...) so severity matches user impact (their --target was silently no-op'd). Wording itself is good and actionable.
    Suggested: Switch ctx.logger.progress(..., symbol="info") to ctx.logger.warning(...) (or progress(..., symbol="warn")) on the flag-off branch only; the success/info path for unrequested targets stays info.

DevX UX

  • [recommended] --target help text omits copilot-app while listing copilot-cowork at src/apm_cli/commands/install.py:930
    src/apm_cli/commands/install.py:930 explicitly mentions copilot-cowork is accepted behind its experimental flag, but does not mention copilot-app at all. Mirror the existing precedent: add one clause noting copilot-app is accepted when apm experimental enable copilot-app is on. Discoverability gap for the new target right where every user looks first (apm install --help).
    Suggested: Append to the --target help text: 'copilot-app is accepted when apm experimental enable copilot-app is on.'
  • [recommended] --target is not click multiple=True; -t a -t b silently honors only the last at src/apm_cli/commands/install.py:924
    install.py:924-930 declares --target without multiple=True, while the help text only says 'Comma-separated for multiple'. Users coming from gh/docker reflexively try --target copilot --target copilot-app and get a silent half-install (last flag wins, no warning). Real footgun.
    Suggested: Either reject duplicate --target invocations with an actionable 'use commas, not repeated flags' error, or add one sentence to the help text explicitly calling out the trap.
  • [recommended] PR body says 'schedule: block' but code uses flat top-level keys
    The Approach table row Integrate copilot runtime #2 in the PR body says the prompts primitive gains an 'optional schedule: block'. The actual shape predicate at prompt_integrator.py:464 looks for flat top-level keys interval/schedule_hour/schedule_day, and docs author them flat. A reader copy-pasting from the merge-commit message will write a nested schedule: block that gets silently treated as a plain prompt and then hard-error at copilot-app.
    Suggested: Edit the PR body to drop the nested schedule: framing and describe the 3 flat top-level keys explicitly.
  • [recommended] Local-package uninstall leaves orphan DB rows when source .prompt.md is deleted before apm uninstall at docs/src/content/docs/integrations/copilot-app.md
    Lifecycle table in copilot-app.md does not cover the source-deletion case. If a user deletes a workflow .prompt.md from a local package and re-syncs, the lockfile entry drops but the DB row survives (pre-existing sync semantics). For workflows this matters more than for files because the row can still fire on schedule once re-enabled.
    Suggested: One-line addition to the Lifecycle table: 'Removing the source .prompt.md and re-syncing drops the lockfile entry but does not delete the DB row -- run apm uninstall <pkg> to remove the row.'
  • [nit] Shape-predicate subtlety deserves an inline doc callout for authors at docs/src/content/docs/integrations/copilot-app.md
    The asymmetry (mode/model/reasoning_effort are NOT shape markers; only interval/schedule_hour/schedule_day are) is the kind of rule a package author will hit once and remember forever -- bad enough to warrant a 2-line :::note in the Authoring section. The hard-fail at install time is solid, but catching this at doc-read time is cheaper.
    Suggested: Add a :::note callout under Authoring: 'Only interval, schedule_hour, schedule_day mark a .prompt.md as a workflow. Setting mode: or reasoning_effort: alone keeps it a plain VSCode prompt.'

Supply Chain Security

  • [recommended] Namespace separator -- not enforced as inviolable across id segments; collision/silent-delete possible at src/apm_cli/integration/copilot_app_db.py:198
    _SLUG_RE = [^a-zA-Z0-9_-]+ preserves - (and therefore --) in inputs, then namespaced_id joins segments with --. The docstring claims -- is invalid inside owner/pkg/prompt, but: (a) prompt names come from file stems (e.g. weekly--report.prompt.md is a legal filename); (b) _derive_package_owner falls back to author-controlled author field or source parsing; (c) apm.yml package name is author-controlled. A malicious package crafting (owner='acme--reports', pkg='weekly', prompt='draft') collides with victim (owner='acme', pkg='reports', prompt='weekly--draft') -> same primary key. Auto-arm defense holds (UPDATE forces enabled=0), but pkg B's uninstall would silently delete pkg A's workflow row, violating the 'only my package's rows' invariant.
    Suggested: Collapse repeated - in _slugify so -- truly can't appear in any segment: slug = re.sub('-+', '-', slug) after the existing substitution. Add a test asserting weekly--report -> weekly-report (single dash) at the segment boundary.
  • [nit] list_managed_workflow_ids skips _check_user_version at src/apm_cli/integration/copilot_app_db.py:570
    Unlike deploy_workflow and delete_workflows, the read-only listing path does not call _check_user_version. Read-only against a newer schema is safe today (SELECT uses only id), but consistency is worth tightening so a future caller can't accidentally widen the SELECT without re-discovering the guard.
    Suggested: Add self._check_user_version(conn) after the connection opens in list_managed_workflow_ids to make the guard uniform across read and write paths.

OSS Growth

  • [recommended] Log this strategic moment in WIP/growth-strategy.md
    First APM surface that touches the GitHub Copilot desktop App (the daily-driver harness). Until now APM lived in the inner loop (CLI prompts) and outer loop (gh-aw CI); this PR closes the middle. Story arc 'apm install -> workflows just appear in your daily app' reframes APM from dev-tool plumbing to 'something my Copilot App got new powers from'.
    Suggested: Append a dated entry to WIP/growth-strategy.md capturing: (a) Copilot App as the first daily-driver surface APM lights up; (b) launch beats once experimental.copilot_app graduates; (c) awesome-copilot bridge as the compounding play. Maintainer-local/gitignored -- won't ship with the PR.
  • [recommended] Ship a canonical scheduled-prompt in github/awesome-copilot to enable the killer demo
    awesome-copilot already supports workflow primitives, and APM resolves github/awesome-copilot/skills/<name> cleanly. The 60-second demo arc 'browse awesome-copilot -> click install -> workflow appears in App' is one PR away in awesome-copilot, but this PR doesn't seed it. Without a real public package, the launch screencast has to use a hand-rolled local example.
    Suggested: Follow-up PR (not blocking): land a tiny canonical scheduled-prompt (e.g. daily-digest) in github/awesome-copilot with interval: + schedule_hour: frontmatter, so apm install github/awesome-copilot/prompts/daily-digest --target copilot-app --global becomes a real demo command.
  • [recommended] Pre-stage the enabled = 0 demo beat in CLI output + announcement copy
    Security default of enabled = 0 is correct (PR body trade-off Will there be MCP coverage? #3), but the 60-second screencast now has an unavoidable beat where the user must switch to the App and toggle on. That's the 'wait, did it work?' moment.
    Suggested: (a) Make the CLI success line loud about the toggle step ('[+] 1 workflow installed (disabled) -- open the Copilot App and flip the toggle to arm it'); (b) frame the toggle as a feature in announcement copy ('one-click consent for anything that runs on a schedule').
  • [nit] apm experimental list is flat; copilot-app should stand out post-launch
    Cold-start path 'I heard APM has something for the Copilot App, how do I find it?' currently ends at apm experimental list where copilot-cowork and copilot-app are indistinguishable. Once launch traffic lands here, the daily-driver flag should stand out.
    Suggested: Follow-up issue: recommended: true field, a one-line tagline per flag, or list ordered with the daily-driver flag first.
  • [nit] README/quickstart will need a hero bullet at graduation
    Correct for an experimental dark-ship not to promise flag-gated work in README. But at graduation, scheduled workflows become the headline use case. Capturing it now costs nothing.
    Suggested: Open a tracking issue: update README hero ('Scheduled workflows -- install daily prompts straight into the GitHub Copilot App') and add a quickstart side-route at graduation.
  • [nit] CHANGELOG is faithful; announcement-grade hook is missing for graduation
    Current Unreleased entry reads as an internal change note. For graduation, the hook should compress to one repostable line.
    Suggested: Seed for the launch draft: 'apm install now writes directly to the GitHub Copilot App -- your daily workflows are now a pip install away.'

Auth -- inactive

PR does not touch authentication, token management, credential resolution, host classification, or HTTP/git auth header surfaces; copilot-app target's filesystem-only 'auth' (data.db mode 0600) is documented transparently as such.

Doc Writer

  • [recommended] Sidebar order collision in integrations: copilot-app.md uses order:5, same as github-rulesets.md at docs/src/content/docs/integrations/copilot-app.md:4
    Starlight orders ties alphabetically within the same numeric slot, producing unstable nav placement. The PR introduced the collision by picking 5 for the new page.
    Suggested: Change order: 5 to order: 6 in copilot-app.md frontmatter.
  • [recommended] copilot-app.md does not cross-link to canonical Experimental or Targets matrix pages at docs/src/content/docs/integrations/copilot-app.md
    PROSE Orchestrated Composition: authoritative definitions live once and every other page links them. copilot-app.md re-states experimental enable/disable/list verbatim and never links to ../../reference/experimental/, and never points readers at ../../reference/targets-matrix/ for where this target fits among the rest.
    Suggested: In Enable and check, replace the standalone command block with one line linking to ../../reference/experimental/ for the full subcommand surface, and add a one-liner near the top linking to ../../reference/targets-matrix/.
  • [nit] Concurrency section says APM 'opens the DB in WAL mode' but APM does not set journal_mode at docs/src/content/docs/integrations/copilot-app.md
    The Copilot App owns the DB and runs it in WAL; APM's _connect only opens with isolation_level=None and timeout=0, then drives BEGIN IMMEDIATE + bounded retry. Current wording reads as if APM is the WAL author.
    Suggested: Rephrase: 'The Copilot App keeps the DB in WAL mode while running. APM uses BEGIN IMMEDIATE with bounded retry to coexist with the App's writer connection...'
  • [nit] Schema compat says 'current tested version is 13' without naming the accepted range at docs/src/content/docs/integrations/copilot-app.md
    Implementation enforces a closed interval [_MIN_SUPPORTED_USER_VERSION=13, _MAX_SUPPORTED_USER_VERSION=13]. Surfacing the range matches the diagnostic wording the user will actually see.
    Suggested: 'APM accepts user_version in the closed range [13, 13] today. Newer schemas are refused with an actionable error until APM is updated and re-tested.'
  • [nit] Lifecycle table 'execution-affecting field changed' cell is dense; split rationale below the table at docs/src/content/docs/integrations/copilot-app.md
    PROSE Succinct: the cell spans multiple clauses (field list + behaviour + rationale) which is hard to scan in a 4-column table.
    Suggested: Keep the row terse ('UPDATE row; reset enabled=0; clear next_run_at') and move the consent-surface rationale to a one-liner under the table.

Test Coverage

  • [recommended] No regression test for workflow-shape skip on --target copilot (.github/prompts/) at src/apm_cli/integration/prompt_integrator.py:370
    PromptIntegrator._integrate_prompts_for_copilot skips workflow-shape prompts at prompt_integrator.py:370 so a scheduled .prompt.md does not leak into .github/prompts/ when the user runs --target copilot,copilot-app. The sibling skip in CommandIntegrator IS covered by test_workflow_shape_skipped_by_slash_command_integrator, but that test's loop is for target_name in ('claude', 'cursor', 'gemini') -- it does NOT exercise the copilot target, which routes through PromptIntegrator (integrator='github_prompt' in targets.py), not CommandIntegrator. Probed via grep -rn on tests/ for _integrate_prompts_for_copilot, github/prompts + workflow.shape, and integrate_prompts_for_target.*copilot; no test asserts that a workflow-shape .prompt.md is NOT written to .github/prompts/<pkg>/. Silent-drift risk on user-promise 8: removing the skip would let App-only metadata bleed into a slash-command surface, and the suite would stay green.
    Suggested: Add a sibling test in tests/unit/integration/test_copilot_app_error_ux.py::TestDispatchByShape: build a package with SCHEDULED_PROMPT + PLAIN_PROMPT, call PromptIntegrator().integrate_prompts_for_target(KNOWN_TARGETS['copilot'], pkg, project_root=tmp_path), and assert no file matching *scheduled* appears under project_root / '.github' / 'prompts'. Mirrors the existing CommandIntegrator probe (~15 LOC).
    Proof (missing): tests/unit/integration/test_copilot_app_error_ux.py::TestDispatchByShape::test_workflow_shape_skipped_by_copilot_prompt_integrator -- proves: A scheduled (.prompt.md with interval/schedule_hour/schedule_day) prompt does not leak into .github/prompts/ when shipped via --target copilot, so users running --target copilot,copilot-app see workflow metadata only in the App and slash-command users see only plain prompts. [devx,secure-by-default]
    assert not any('scheduled' in p.name for p in (project_root / '.github' / 'prompts').glob('*.prompt.md'))

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

- install.py --target help: mention copilot-app + warn that repeated
  flag (-t a -t b) silently honors only the last value; use commas
  (devx-ux #1, #2)
- copilot-app.md: bump sidebar order 5 -> 6 (collision with
  github-rulesets.md), cross-link to reference/experimental/ and
  reference/targets-matrix/, rephrase WAL ownership to reflect that
  the App owns WAL and APM coexists via BEGIN IMMEDIATE + bounded
  retry, surface accepted schema range [13, 13], split lifecycle
  table cell with rationale below the table, add :::note callout
  clarifying the shape predicate, document source-deletion orphan
  case (doc-writer #1-5, devx-ux #4, #5)
- tests: add test_workflow_shape_skipped_by_copilot_prompt_integrator
  regression test asserting workflow-shape .prompt.md does NOT leak
  into .github/prompts/ when --target includes copilot
  (test-coverage #1)

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