Skip to content

fix(cli): align ensureCheckout with PM-unified store layout#82

Merged
amondnet merged 2 commits into
mainfrom
amondnet/github-agent-skills-documentation-query
Apr 14, 2026
Merged

fix(cli): align ensureCheckout with PM-unified store layout#82
amondnet merged 2 commits into
mainfrom
amondnet/github-agent-skills-documentation-query

Conversation

@amondnet

@amondnet amondnet commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • ensureCheckout returned the legacy github/checkouts/<owner>__<repo>/<ref>/ path while GithubSource.fetch writes to the PM-unified github/github.com/<owner>/<repo>/<tag>/ layout.
  • The mismatch made ask docs github:<owner>/<repo> and ask src … exit 0 with no output after a successful fetch — callers walked a non-existent directory and findDocLikePaths returned [].
  • Fixed by switching ensureCheckout to githubStorePath(askHome, 'github.com', owner, repo, ref) so the lazy and eager pipelines agree on the exact same path.

Changes

  • packages/cli/src/commands/ensure-checkout.ts — use githubStorePath instead of legacy checkouts/ path
  • packages/cli/test/commands/ensure-checkout.integration.test.ts — new integration test that mocks the real fetch by writing to the PM-unified layout and asserts ensureCheckout + runDocs both land on the same existing path
  • packages/cli/test/commands/ensure-checkout.test.ts, cache-sharing.test.ts, src.test.ts — updated to use the PM-unified layout in fixtures and structural assertions

Test Plan

  • bun run --cwd packages/cli test test/commands/ → 57/57 pass
  • Manual: ask docs github:agentskills/agentskills now prints the checkout root and /docs subdir instead of emitting nothing

Summary by cubic

Aligns the CLI checkout path with the PM‑unified GitHub store layout and returns the fetcher’s actual store path when a fallback ref wins, so ask docs/ask src read the real directory and no longer emit empty output. ensureCheckout now resolves to ~/.ask/github/github.com/<owner>/<repo>/<ref>/ or the fetcher-reported storePath.

  • Bug Fixes
    • Switched to githubStorePath(askHome, 'github.com', owner, repo, ref) and prefer FetchResult.storePath when present to fix both naming and ref-candidate divergence.
    • Added integration tests pinning ensureCheckoutGithubSource.fetchrunDocs path agreement (including fallback-ref winner); updated unit tests and fixtures to the PM‑unified layout.

Written for commit 9f25e8f. Summary will update on new commits.

`ensureCheckout` returned `<askHome>/github/checkouts/<owner>__<repo>/<ref>/`
while `GithubSource.fetch` writes to `<askHome>/github/<host>/<owner>/<repo>/<tag>/`.
The path mismatch caused `ask docs github:<owner>/<repo>` and `ask src …` to
exit 0 with no output after a successful fetch — callers walked a directory
that never exists.

Switches `ensureCheckout` to `githubStorePath(askHome, 'github.com', owner,
repo, ref)` so the lazy resolution path matches the eager fetcher exactly.

Adds `ensure-checkout.integration.test.ts` which mocks the real fetch by
writing to the PM-unified layout and asserts that both `ensureCheckout` and
`runDocs` land on the same path — pinning the contract so the two pipelines
cannot silently diverge again.

Updates existing unit tests in `ensure-checkout.test.ts`, `cache-sharing.test.ts`,
and `src.test.ts` to use the PM-unified layout in their pre-populated fixtures
and structural assertions.
@dosubot dosubot Bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Apr 14, 2026
@codecov

codecov Bot commented Apr 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@dosubot dosubot Bot added type:bug Something isn't working type:test Testing improvements or additions labels Apr 14, 2026
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Apr 14, 2026

Copy link
Copy Markdown

Deploying ask-registry with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9f25e8f
Status: ✅  Deploy successful!
Preview URL: https://6acdc6fc.ask-registry.pages.dev
Branch Preview URL: https://amondnet-github-agent-skills.ask-registry.pages.dev

View logs

@cubic-dev-ai cubic-dev-ai 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.

No issues found across 5 files

Auto-approved: Aligns checkout paths with the unified store layout to fix a bug causing silent failures. Low-risk logic change with comprehensive integration and unit test coverage.

Architecture diagram
sequenceDiagram
    participant User
    participant CLI as CLI (docs/src command)
    participant EC as ensureCheckout
    participant GS as GithubSource
    participant FS as Local Store (~/.ask/github/)
    participant GH as GitHub API

    User->>CLI: run "ask docs github:owner/repo"
    CLI->>EC: ensureCheckout(spec)

    Note over EC,FS: PATH RESOLUTION
    EC->>EC: CHANGED: Compute checkoutDir using githubStorePath<br/>(github.com/owner/repo/ref)
    
    EC->>FS: Check if directory exists
    
    alt Cache Hit (Path Exists)
        FS-->>EC: Directory found
    else Cache Miss (Path Missing)
        EC->>GS: fetch(repo, ref)
        GS->>GH: Download repository source
        GH-->>GS: Source Data
        GS->>FS: CHANGED: Write to PM-unified layout<br/>(github.com/owner/repo/ref)
        FS-->>GS: Success
        GS-->>EC: Return storePath
    end

    EC-->>CLI: Return checkoutDir (Absolute Path)

    Note over CLI,FS: CONTENT DISCOVERY
    CLI->>CLI: findDocLikePaths(checkoutDir)
    CLI->>FS: Read directory content at checkoutDir
    
    alt Path Alignment Success (NEW)
        FS-->>CLI: File list (README, /docs, etc.)
        CLI->>User: Print documentation paths
    else Path Mismatch (Legacy Bug)
        Note over CLI,FS: Legacy path (checkouts/owner__repo) != Unified path
        FS-->>CLI: Error / Directory Not Found
        CLI->>User: Exit 0 (Silent - No Output)
    end
Loading

Close the second silent-failure vector on the ref-candidate axis.
`GithubSource.fetch` writes the store dir under the WINNING candidate —
which may differ from the primary ref when a `fallbackRef` (monorepo
tags like `ai@6.0.158`) wins, or when `cloneAtTag` rescues a non-`v`
ref via `v<ref>`. Previously `ensureCheckout` discarded the fetch
result and returned a `checkoutDir` computed solely from the primary
`ref`, reproducing the same empty-output bug on a different axis.

Now threads `FetchResult.storePath` through and uses it as the
authoritative path, falling back to the pre-computed path only when
the fetcher does not populate it (keeps cache-hit short-circuit and
legacy test mocks working).

Adds an integration test simulating a `fallbackRefs`-driven resolver
and asserts `ensureCheckout` surfaces the fallback's on-disk path.
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Apr 14, 2026

@cubic-dev-ai cubic-dev-ai 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.

0 issues found across 4 files (changes from recent commits).

Requires human review: Aligning checkout paths with the PM-unified layout involves architectural context and impacts how CLI commands locate files, which warrants human verification.

@amondnet amondnet self-assigned this Apr 14, 2026
@amondnet
amondnet merged commit 4ee9db2 into main Apr 14, 2026
5 checks passed
@github-actions github-actions Bot mentioned this pull request Apr 14, 2026
amondnet added a commit that referenced this pull request Apr 14, 2026
Strengthen the existing `.claude/agent-memory/` Gotcha to explicitly
require that any agent-authored memory files be committed alongside
the code change that produced them. Prevents accidental loss of
cross-session agent learnings when PRs are squashed or when authors
stage selectively.

Also flushes the two silent-failure-hunter memory files that the
agent updated after PR #82 merged — keeps `main` in sync with the
current agent-memory state.
amondnet added a commit that referenced this pull request Apr 14, 2026
)

Strengthen the existing `.claude/agent-memory/` Gotcha to explicitly
require that any agent-authored memory files be committed alongside
the code change that produced them. Prevents accidental loss of
cross-session agent learnings when PRs are squashed or when authors
stage selectively.

Also flushes the two silent-failure-hunter memory files that the
agent updated after PR #82 merged — keeps `main` in sync with the
current agent-memory state.
@github-actions github-actions Bot mentioned this pull request Apr 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files. type:bug Something isn't working type:test Testing improvements or additions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant