Skip to content

feat(skill-doctor): add Amp harness support - #83

Open
thesammykins wants to merge 1 commit into
warpdotdev:mainfrom
thesammykins:feat/skill-doctor-amp-harness
Open

feat(skill-doctor): add Amp harness support#83
thesammykins wants to merge 1 commit into
warpdotdev:mainfrom
thesammykins:feat/skill-doctor-amp-harness

Conversation

@thesammykins

@thesammykins thesammykins commented Aug 29, 2026

Copy link
Copy Markdown

skill-doctor currently rejects Amp at its startup gate, even though Amp exposes the conversation and skill data that the collector needs. This change adds Amp as a supported harness without changing scoring, aggregation, sampling, suggestions, or report generation.

Review guide

The diff is limited to the supported-harness reference and the collector. Start with the harness contract, then review the Amp adapter in collect_sessions.py. The main review consideration is the normalization from Amp thread JSON into the existing shared session shape.

What changed

  • Adds amp source selection for --harness amp, Amp-aware auto, and all.
  • Discovers repository-scoped or all accessible threads, applies the existing time window, and handles direct child threads through --include-subagents.
  • Extracts visible user and assistant text, tool calls and results, timestamps, edit/error statistics, repository context, and activated skills.
  • Discovers Amp-compatible global skill paths and personal/workspace skills from amp skill list --json, while excluding built-ins.
  • Normalizes Unix, macOS, Windows drive, and UNC file: paths.

The collector uses authenticated structured amp threads JSON instead of filesystem scraping. Amp threads are available through the same CLI contract in orbs and local runners across operating systems; no stable platform-specific transcript path is required.

Validation

  • The existing collector and renderer suites pass: python3 -m unittest -v test_collect_sessions.py test_render_report.py (16 tests).
  • All skill-doctor Python scripts compile with python3 -m py_compile.
  • A live orb smoke exported and parsed one repository-matched Amp thread with harness: amp; sampling was disabled and no transcript file was emitted.
  • An authenticated macOS runner smoke passed CLI discovery, compilation, help-surface, and zero-sample collection checks.
  • The commit has a verified Amp-managed ED25519 SSH signature for 26049947+thesammykins@users.noreply.github.com.

Transient synthetic checks covered source selection, repository/all-conversation discovery, malformed data, child-thread handling, timestamps, content extraction, Windows paths, and global skill discovery. They are intentionally not included in this diff, so this PR does not add upstream Amp regression tests.

Privacy and limitations

No real conversation data, transcripts, reports, logs, or generated artifacts are included. Collection requires an authenticated compatible Amp CLI. Child classification uses parent:<thread ID> queries because Amp thread summaries do not expose parent metadata.
Closes #84

@thesammykins

Copy link
Copy Markdown
Author

Review breadcrumbs

  • Scope: Two files only: the supported-harness reference and collect_sessions.py.
  • Source decision: The adapter reads authenticated structured amp threads JSON. Filesystem scraping would depend on executor and operating-system-specific storage that is not part of Amp's thread contract.
  • Behavior boundary: Amp data is normalized into the existing collector shape. Scoring, aggregation, sampling, suggestions, and report rendering are untouched.
  • Evidence: The existing 16 tests pass; live zero-sample smokes passed in an orb and on macOS; the single commit has a verified Amp-managed ED25519 SSH signature for Sammy's GitHub noreply identity.
  • Review consideration: Synthetic Amp checks were transient and are not in the PR. There is no committed Amp regression suite, so please focus review on the adapter's schema assumptions and source-selection semantics.

@warp-for-oss

warp-for-oss Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

@thesammykins

Every PR must be linked to a same-repo issue before Oz can review it.

Next step: open or find a same-repo issue describing this change, then link it to this PR by adding Closes #123 to the PR description (or using the "Development" sidebar on GitHub). A maintainer will mark the issue ready-to-implement when it is ready. Once it is marked, comment /warp-agent-review to re-trigger review.

Powered by Oz

@warp-for-oss warp-for-oss Bot 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.

@thesammykins

Every PR must be linked to a same-repo issue before Oz can review it.

Next step: open or find a same-repo issue describing this change, then link it to this PR by adding Closes #123 to the PR description (or using the "Development" sidebar on GitHub). A maintainer will mark the issue ready-to-implement when it is ready. Once it is marked, comment /warp-agent-review to re-trigger review.

Powered by Oz

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.

Add Amp support to skill-doctor

1 participant