Skip to content

fix(cli): prevent stale embedded skills manifest drift#115

Merged
pentaxis93 merged 2 commits intomainfrom
issue-112/fix-cli-manifest-freshness-check
Mar 10, 2026
Merged

fix(cli): prevent stale embedded skills manifest drift#115
pentaxis93 merged 2 commits intomainfrom
issue-112/fix-cli-manifest-freshness-check

Conversation

@pentaxis93
Copy link
Owner

@pentaxis93 pentaxis93 commented Mar 10, 2026

Summary

  • make groundwork init/groundwork update attempt a runtime fetch of skills/skills.toml from GitHub
  • fall back to the embedded manifest with explicit warnings when fetch/parse/validation fails
  • add groundwork doctor manifest freshness reporting for embedded-vs-repo drift

Changes

  • add manifest-loading helpers for runtime fetch, embedded fallback, and parse/validation flow
  • add doctor freshness check with SHA-256 comparison and skipped-check info output
  • add unit tests for runtime success, fallback on failure, and freshness states (matching/stale/skipped)
  • update changelog with the new CLI freshness behavior

Issue(s)

Closes #112

Test plan

  • cargo fmt --all
  • cargo test -p groundwork-cli

Prefer runtime manifest fetch during init/update with embedded fallback warnings, and add doctor freshness drift detection for embedded vs repo manifest. Includes unit tests for fetch/fallback/freshness states.\n\nRefs #112
@github-actions
Copy link

github-actions bot commented Mar 10, 2026

I did not find any blocking correctness or reliability regressions in the diff against origin/main. The new runtime-manifest fetch path includes explicit fallback behavior and tests for the primary failure modes.

Raw outputs: codex-review.md, codex-review.jsonl

Comment on lines +838 to +845
"gh",
"api",
"-H",
"Accept: application/vnd.github.raw",
"/repos/pentaxis93/groundwork/contents/skills/skills.toml?ref=main",
])
.context("failed to fetch current skills manifest from GitHub")
}

Choose a reason for hiding this comment

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

[P2] Preserve fetched manifest bytes for freshness hashing

fetch_remote_manifest_text uses run_command_capture, which trims stdout before returning it. That strips trailing newlines from skills.toml, so check_embedded_manifest_freshness_with hashes different bytes than SHIPPED_SKILLS_TOML even when the remote file is identical (the current manifest ends with a newline). In that case groundwork doctor will incorrectly report the embedded manifest as stale.

Use raw command capture for remote manifest fetch so trailing newlines are preserved, preventing false stale warnings in doctor. Add regression test for raw capture newline preservation.\n\nRefs #112
@pentaxis93
Copy link
Owner Author

Addressed the freshness-hash false-positive concern in aa09aa4.

Changes:

  • switched remote manifest fetch to raw stdout capture (no .trim()), preserving trailing newline bytes for hashing
  • kept existing trimmed capture helper for other call sites
  • added regression test raw_command_capture_preserves_trailing_newline

Verification run:

  • cargo fmt --all
  • cargo test -p groundwork-cli

@pentaxis93 pentaxis93 merged commit fb8f753 into main Mar 10, 2026
1 check passed
@pentaxis93 pentaxis93 deleted the issue-112/fix-cli-manifest-freshness-check branch March 10, 2026 09:15
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.

fix(cli): embedded skills manifest goes stale with no runtime freshness check

1 participant