fix: bust giget cache on skill update; heading-bounded docs inject

[rgdevme]

Summary

Two small fixes the user hit while exercising agnos:

• agnos skill update <name> now actually re-fetches upstream. runUpdate already passed noCache: true and fetchGit wiped the agnos-side <cacheDir>/repos/<key> dir, but giget keeps its own tarball at ~/.cache/giget/<provider>/<owner>-<repo>/<ref>.tar.gz and short-circuits inside download() whenever the upstream ETag matches the cached sidecar. With proxies / CDNs / unchanged HEAD this reused the stale tarball. Now when noCache: true is set, fetchGit also removes that tarball and its .json sidecar before invoking downloadTemplate. New gigetTarballPath() helper mirrors giget's cacheDirectory() (XDG_CACHE_HOME → ~/.cache/giget) so the path stays in sync with giget v1.x.

• Docs injection is now bounded by markdown structure, not custom end markers. The >__Documentation index end__ / >__Documentation rules end__ lines are gone. RULES_BLOCK / INDEX_BLOCK collapse to plain string constants RULES_HEADING / INDEX_HEADING. replaceBetweenMarkers is replaced by replaceUnderHeading(text, heading, payload), which finds the ## Documentation Index (or ## Documentation Rules) line and overwrites everything until the next ## / # heading or EOF, keeping a single blank line before the next heading for idempotency. renderIndexBody now emits ### Section instead of ## Section, so the injected layout matches the spec:

  ## Documentation Index
  <!-- agnos-docs:autogenerated — do not edit by hand; run `agnos docs generate` -->
  
  ### Overview
  - ...
  

Files

• packages/core/src/resolver.ts — gigetTarballPath() + wipe-on-noCache.
• packages/core/test/resolver-nocache.test.ts — new test, mocks downloadTemplate.
• packages/domain-docs/src/schema.ts — RULES_HEADING / INDEX_HEADING.
• packages/domain-docs/src/inject/markers.ts — replaceUnderHeading().
• packages/domain-docs/src/cli/inject.ts — wired to new API.
• packages/domain-docs/src/cli/generate.ts — ### index sections.
• packages/domain-docs/test/{markers,inject,generate}.test.ts — updated.

Test plan

• pnpm -F @luxia/core typecheck — clean
• pnpm -F @luxia/core test — 119/119
• pnpm -F @luxia/domain-docs typecheck — clean
• pnpm -F @luxia/domain-docs test — 42/42
• pnpm -F @luxia/domain-docs build — clean
• Manual: agnos skill update <name> against a repo with a fresh upstream commit → confirm ~/.cache/giget/.../<ref>.tar.gz is removed and the new content extracts into .agnos/skills/<name>/.
• Manual: agnos docs generate && agnos docs inject → confirm AGENTS.md has ## Documentation Index / ## Documentation Rules headings with ### subsections under the index, no >__Documentation…end__ lines, and rerunning is a no-op.

[qodo-code-review]

Review Summary by Qodo

Fix giget cache busting and heading-bounded docs injection

🐞 Bug fix ✨ Enhancement

Walkthroughs

Description

• Clear giget's cached tarballs when noCache: true to prevent stale content
• Replace custom end-marker lines with markdown heading boundaries for docs injection
• Promote generated index sections from ## to ### for proper nesting
• Add gigetTarballPath() helper to mirror giget's cache directory logic

Diagram

flowchart LR
  A["skill update<br/>with noCache"] -->|calls fetchGit| B["fetchGit removes<br/>giget tarball"]
  B -->|clears cache| C["downloadTemplate<br/>fetches fresh"]
  C -->|extracts| D["Updated skill<br/>content"]
  
  E["docs generate"] -->|creates payload| F["replaceUnderHeading<br/>finds ## heading"]
  F -->|replaces until<br/>next ## or EOF| G["Bounded injection<br/>no end markers"]
  G -->|promotes to ###| H["Proper markdown<br/>nesting"]

Loading

File Changes

1. packages/core/src/resolver.ts 🐞 Bug fix +19/-0

Add giget cache busting on noCache flag

packages/core/src/resolver.ts

---

2. packages/core/test/resolver-nocache.test.ts 🧪 Tests +76/-0

New tests for giget tarball cache wiping

packages/core/test/resolver-nocache.test.ts

---

3. packages/domain-docs/src/schema.ts ✨ Enhancement +2/-9

Replace marker blocks with heading constants

packages/domain-docs/src/schema.ts

---

View more (6)

4. packages/domain-docs/src/inject/markers.ts ✨ Enhancement +23/-19

Implement heading-bounded content replacement

packages/domain-docs/src/inject/markers.ts

---

5. packages/domain-docs/src/cli/inject.ts ✨ Enhancement +4/-4

Wire new heading-based injection API

packages/domain-docs/src/cli/inject.ts

---

6. packages/domain-docs/src/cli/generate.ts ✨ Enhancement +1/-1

Promote index sections to level-3 headings

packages/domain-docs/src/cli/generate.ts

---

7. packages/domain-docs/test/markers.test.ts 🧪 Tests +53/-33

Update tests for heading-bounded replacement

packages/domain-docs/test/markers.test.ts

---

8. packages/domain-docs/test/inject.test.ts 🧪 Tests +3/-1

Verify new injection behavior without markers

packages/domain-docs/test/inject.test.ts

---

9. packages/domain-docs/test/generate.test.ts 🧪 Tests +2/-2

Assert generated sections use level-3 headings

packages/domain-docs/test/generate.test.ts

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0)

1. gigetTarballPath missing index export ✓ Resolved 📘 Rule violation ⌂ Architecture

Description

gigetTarballPath is newly exported from resolver.ts but is not re-exported from
packages/core/src/index.ts, forcing consumers to deep-import internal modules. This violates the
requirement to expose the public API exclusively via src/index.ts re-exports.

Code

packages/core/src/resolver.ts[R101-105]

+export function gigetTarballPath(source: GitSource, ref: string | undefined): string {
+  const name = `${source.owner}-${source.repo}`.replace(/[^\da-z-]/gi, "-");
+  const version = ref ?? "main";
+  return path.join(gigetCacheDir(), source.provider, name, `${version}.tar.gz`);
+}

Evidence

Rule 704862 requires public symbols to be exposed via src/index.ts. The PR adds a new export
gigetTarballPath in resolver.ts, but index.ts only re-exports createRepoFetcher from
./resolver.js and does not re-export gigetTarballPath.

Rule 704862: Expose public API via src/index.ts re-exports
packages/core/src/resolver.ts[101-105]
packages/core/src/index.ts[66-70]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new public symbol (`gigetTarballPath`) is exported from `packages/core/src/resolver.ts` but is not re-exported from `packages/core/src/index.ts`, which breaks the package's public-API surfacing convention.

## Issue Context
The compliance rule requires that public API symbols be exposed via `src/index.ts` re-exports, avoiding deep imports into `src/*` modules.

## Fix Focus Areas
- packages/core/src/index.ts[66-70]
- packages/core/src/resolver.ts[101-105]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. resolver-nocache uses deep imports ✓ Resolved 📘 Rule violation ▣ Testability

Description

The new test imports production code from ../src/resolver.js and ../src/source.js rather than
from the package’s public entry point, coupling tests to internal module paths. This violates the
requirement to test behavior via the public API surface only.

Code

packages/core/test/resolver-nocache.test.ts[R16-17]

+import { createRepoFetcher, gigetTarballPath } from "../src/resolver.js";
+import { parseSource } from "../src/source.js";

Evidence

Rule 704854 requires tests to use only the public API surface. The new test imports
createRepoFetcher, gigetTarballPath, and parseSource via deep relative ../src/* paths
instead of importing them from the package entry point (src/index.ts).

Rule 704854: Test behavior via public API only, not internal helpers
packages/core/test/resolver-nocache.test.ts[16-17]
packages/core/src/index.ts[66-70]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test file deep-imports internal modules (`../src/resolver.js`, `../src/source.js`) instead of importing from the package public surface.

## Issue Context
Per the compliance requirement, tests should exercise behavior through the public API surface to avoid tight coupling to internal file layout.

## Fix Focus Areas
- packages/core/test/resolver-nocache.test.ts[16-17]
- packages/core/src/index.ts[66-70]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Ref path traversal delete ✓ Resolved 🐞 Bug ⛨ Security

Description

When noCache: true, fetchGit() deletes gigetTarballPath(source, ref) but gigetTarballPath()
incorporates ref directly into path.join(), so a ref containing .. or path separators can
resolve outside the giget cache directory and delete unintended files. This is reachable via the
public RepoFetcher.fetch(..., { ref?: string, noCache?: boolean }) API where ref is an arbitrary
string.

Code

packages/core/src/resolver.ts[R101-105]

+export function gigetTarballPath(source: GitSource, ref: string | undefined): string {
+  const name = `${source.owner}-${source.repo}`.replace(/[^\da-z-]/gi, "-");
+  const version = ref ?? "main";
+  return path.join(gigetCacheDir(), source.provider, name, `${version}.tar.gz`);
+}

Evidence

The PR adds deletion of a path derived from ref (a public, arbitrary string) without validation.
Because path.join() normalizes .. segments, an attacker-controlled ref can change the target
of fs.rm() to a location outside the intended cache directory.

packages/core/src/resolver.ts[59-63]
packages/core/src/resolver.ts[101-105]
packages/core/src/types/public.ts[96-100]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`gigetTarballPath()` uses the caller-provided `ref` as part of a filesystem path. With `noCache: true`, this path is passed to `fs.rm()`. A crafted `ref` containing `..` and/or path separators can cause `path.join()`/normalization to escape the intended cache directory, enabling unintended file deletions.

### Issue Context
- `RepoFetcher.fetch()` exposes `ref?: string` publicly, so `ref` is not guaranteed to be a commit SHA.
- `fetchGit()` calls `fs.rm()` on the computed path when `noCache` is enabled.

### Fix Focus Areas
- packages/core/src/resolver.ts[59-63]
- packages/core/src/resolver.ts[95-105]

### Suggested fix
1. Sanitize and/or validate `ref` before using it in a path:
  - Reject any `ref` where `ref.includes('..')` or `ref` contains `/` or `\\`.
  - Or replace path separators with `-` and then ensure the final resolved path stays within `gigetCacheDir()`.
2. Add a containment check using `path.resolve` + `path.relative`:
  - Compute `base = gigetCacheDir()` and `resolved = path.resolve(base, provider, name, `${safeVersion}.tar.gz`)`.
  - If `path.relative(base, resolved).startsWith('..')` (or is absolute), throw or skip deletion.
3. Add a regression test that passes a malicious `ref` with `noCache: true` and asserts that:
  - The function rejects the ref (preferred), or
  - No deletion occurs outside the cache directory.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---