Skip to content

feat(advise): migrate-or-packageize advisories for DIY capability patterns - #63

Merged
basicScandal merged 4 commits into
starloghq:mainfrom
kavin0x:feat/advise-migrate-or-packageize
Jul 27, 2026
Merged

feat(advise): migrate-or-packageize advisories for DIY capability patterns#63
basicScandal merged 4 commits into
starloghq:mainfrom
kavin0x:feat/advise-migrate-or-packageize

Conversation

@kavin0x

@kavin0x kavin0x commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds starlog_advise (MCP + CLI) to track DIY capability patterns and recommend MIGRATE to safe corpus libraries (e.g. Clerk/Auth0/Supabase) when they pass a facts safety gate, or PACKAGEIZE only when no safe alternative exists (WATCH below recurrence threshold).
  • Introduces .starlog/patterns.json scanning/store (starlog patterns scan|list) plus starlog advise packageize scaffolding for private corpus/facts.
  • Extends the auth corpus with Supabase Auth + L2 facts for migration targets, bundled playbooks, and doctor checks for the pattern store / starlog_advise handshake.

kavin0x and others added 4 commits July 19, 2026 16:36
…geization recommendations

Added the `starlog_advise` tool to assist users in identifying DIY capability code and recommending whether to migrate to safe libraries or packageize when no alternatives exist. This includes enhancements to the CLI for advising actions, pattern tracking, and integration with the existing corpus of facts. Updated documentation and added playbooks for common migration scenarios, including support for Clerk, Auth0, and Supabase.

- Introduced `starlog_advise` for migration/packageize decisions.
- Enhanced CLI commands for advising and pattern management.
- Updated CHANGELOG for version 0.9.0 with new features and improvements.
…rn scan

Enhanced the `checkPrivateOverlays` function to include a warning when `patterns.json` is missing, nudging users to run a pattern scan. This change improves user guidance for managing DIY patterns and ensures better visibility into potential issues with pattern tracking.
Three fixes to the migrate-or-packageize advisor, all verified against the
existing tests:

- runAdvise passed a project `context` to runSearch, which triggers search()'s
  per-candidate LLM enrichment (vs_custom/context_fit/tradeoffs). Advise never
  reads those fields, so every call paid a multi-second OpenRouter round-trip
  (and token cost) for discarded output — and it made the two integration
  tests exceed vitest's 5s default whenever a rank key was in the environment.
  Ranking is independent of context, so dropping it is behaviour-preserving for
  the advice itself and removes the wasted call.

- The migrate/packageize branches re-called upsertPattern purely to set the
  pattern status, but upsertPattern always increments occurrences — so every
  actioned advise counted the same observation twice, inflating the recurrence
  threshold. Switched to updatePatternStatus, which sets status without
  incrementing.

- Fixed a no-op `category.replace(/-/g, '-')` in the packageize suggested name.

Tests: isolate the global pattern store via a per-test $HOME so recurrence
counts can't leak in from the developer's real ~/.starlog store (the source of
the flaky watch/threshold assertion), and add regression guards for both the
no-context and single-increment behaviours. Full suite green (606 passed).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…able"

The packed-tarball smoke test grepped doctor's post-init handshake output for
the literal "starlog_search available". This PR changed doctor's success detail
to "starlog_search + starlog_advise available" (both tools present), which does
not contain that substring — so the gate failed on a *healthy* install on both
Node 20 and 22. Update the grep to the new success wording.

Verified: `npm pack` + isolated smoke run → 19 passed, 0 failed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@basicScandal
basicScandal merged commit 2c7659a into starloghq:main Jul 27, 2026
4 checks passed
@basicScandal

Copy link
Copy Markdown
Member

Merged — thanks @kavin0x, this is a genuinely well-structured feature. The layering (detect → safety gate → migrate/packageize/watch), the playbook fallbacks, the corpus + L2 facts additions, and the doctor wiring were all solid, and it came with real tests. 🙏

I folded in three maintainer tweaks before merge so it went in green:

  1. Dropped the project context passed to runSearch. A context triggers search()'s per-candidate LLM enrichment (vs_custom/context_fit/tradeoffs), but runAdvise never reads those fields — so every advise call was paying a multi-second OpenRouter round-trip (and tokens) for output it discards, and it pushed the two integration tests past vitest's 5s default whenever a rank key was in the env. Ranking is independent of context, so advice is unchanged.

  2. Fixed occurrence double-counting. The migrate/packageize branches re-called upsertPattern just to set status, but upsertPattern always increments occurrences — so each actioned advise counted the same observation twice and inflated the recurrence threshold. Switched to updatePatternStatus (status-only, no increment).

  3. Updated the packed-tarball smoke test. doctor.ts's success wording became starlog_search + starlog_advise available, which the main smoke test's grep "starlog_search available" no longer matched — so it failed on a healthy install (Node 20 + 22). Pointed the grep at the new wording.

Also isolated the global pattern store behind a per-test $HOME so recurrence counts can't leak in from a developer's real ~/.starlog, and added regression guards for #1 and #2.

Full suite green (606 passing) and packed smoke 19/0 on Node 20 + 22. Nice work — looking forward to more.

@basicScandal

Copy link
Copy Markdown
Member

@kavin0x a bit of direct feedback now that it's in — you clearly put real thought into this, so this is meant as "carry it forward," not nitpicking.

What was genuinely strong

  • The core shape is right: detect → facts safety gate → migrate / packageize / watch is exactly the decision tree this should be, and the recurrence threshold before advising action is a nice touch (avoids nagging on a one-off).
  • The safety gate degrading from L2 facts → manifest quality signals (with an explicit "vet with starlog_facts before adopting" note) is thoughtful — it fails safe instead of over-trusting.
  • The playbook fallback chain (specific → category-fallback → hardcoded default) is a clean pattern, and you shipped real tests + corpus + L2 facts, not just code. That's the bar.
  • Your inline comments explained the non-obvious choices (e.g. why ADVISE_MIN_RELEVANCE drops to 50) — that's the kind of comment that actually earns its keep.

Two things worth internalizing (they caused the red build)

  1. Test hermeticity. The two failing tests reached live network/LLM because they didn't mock runSearch and relied on the ambient ~/.starlog store + whatever keys were in the shell. Your third test already had the right pattern (mock runSearch). Rule of thumb: a unit test must pass identically with keys set and unset, and must never read/write the developer's real home dir. Running the suite both ways locally would have surfaced this instantly.
  2. Know what an API triggers before reusing it. Passing context to runSearch was a reasonable instinct — but that flag exists to drive an LLM enrichment pass whose output advise never consumes, so it was pure latency/cost. Similarly, updatePatternStatus already existed for status-only writes; reaching for upsertPattern is what caused the double-count. Worth a quick scan of the module's exports before adding a call.

Minor code notes (left as-is, for next time)

  • assessPackageSafety in engine/advise.ts: the UNSAFE_MAINTENANCE branch is fully subsumed by the !SAFE_MAINTENANCE check right below it — both return the same verdict, so the first is dead. Harmless, just tidy.
  • scanProject's known-library suppression is per-file: a repo using Clerk in one file and hand-rolled JWT in another will still flag DIY. Fine as a heuristic, but worth a comment noting the limitation.
  • @acme/<category> as the packageize scope is a fine scaffold placeholder; someday deriving it from the org (or making it configurable) would be a nice follow-up.

Process: CI didn't auto-run because it's a fork PR (needs maintainer approval), so you couldn't see it was red — that's on our side, not yours. npm run typecheck && npm test && npm run build locally per CONTRIBUTING will catch the unit/type issues; the packed smoke (npm pack + scripts/smoke-test.sh) catches the tarball ones.

Really nice first contribution. If you're up for more, per CONTRIBUTING the highest-leverage thing is new capability manifests — and you've now got the safety-gate context to add strong ones. Thanks again. 🙌

basicScandal pushed a commit that referenced this pull request Jul 27, 2026
Drop the (unreleased) marker now that the migrate-or-packageize advisor (#63)
is on main and about to be tagged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@kavin0x
kavin0x deleted the feat/advise-migrate-or-packageize branch July 28, 2026 05:24
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