Skip to content

#338: Filter non-material upstream commits from update notifications#343

Open
bguidolim wants to merge 1 commit intomainfrom
bruno/338-update-check-noise-filter
Open

#338: Filter non-material upstream commits from update notifications#343
bguidolim wants to merge 1 commit intomainfrom
bruno/338-update-check-noise-filter

Conversation

@bguidolim
Copy link
Copy Markdown
Collaborator

Summary

Upstream pack repos accumulate README, LICENSE, CI, and .github/ commits that never affect what mcs sync installs. Every one of these fires the SessionStart hook's "pack update available" banner, training users to ignore it. This PR filters those commits out so the notification only fires on commits that actually change the install surface — closes the first half of #338.

Changes

  • After git ls-remote detects a new SHA, the update check now runs a shallow fetch + name-only diff in the local pack clone and classifies the changed-path list. If every path is deny-listed infrastructure (README, LICENSE, CHANGELOG, CONTRIBUTING, .gitignore, .editorconfig, package/lock files, Makefile, Dockerfile, .github/, .gitlab/, .vscode/, node_modules/, __pycache__, .build/), the notification is suppressed and the local pack-registry baseline advances to the new remote SHA so the same commits don't re-trigger on future cooldown windows.
  • techpack.yaml changes are always material, regardless of what else is in the diff — a manifest edit can swap hook scripts, MCP server commands, or the install surface entirely, so silent suppression would be a supply-chain risk.
  • Filter failures (offline, fetch error, diff error, missing clone) surface the notification unfiltered — the filter can only suppress, never manufacture silence.
  • Phase 1 of the Update checker flags every commit; filter out noise files (README, CI, LICENSE) to reduce false positives #338 noise filter. An author-supplied ignore: field on techpack.yaml and a validate-time remediation hint ship in follow-up PRs.

Test plan

  • swift test passes locally
  • swiftformat --lint . and swiftlint pass without violations
  • Affected commands verified with a real pack (e.g. mcs sync, mcs doctor)

Manual end-to-end:

  1. mcs pack add <a test pack>; note the commitSHA in ~/.mcs/registry.yaml.
  2. Push a README-only commit upstream; delete ~/.mcs/update-check.json to bust the cache; run mcs doctor → expect no "pack update available" notification, and expect commitSHA in ~/.mcs/registry.yaml to advance to the new upstream SHA.
  3. Push a commit that changes a file the pack actually installs (e.g. a hook script); bust the cache again; run mcs doctor → expect the notification.
  4. Push a commit touching techpack.yaml only; bust the cache; run mcs doctor → expect the notification.
  5. Disconnect from the network, repeat step 3 → expect the notification to surface regardless (never-hide).
Checklist for engine changes
  • Docs updated if behavior changed (CLAUDE.md, docs/, techpack.yaml schema in ExternalPackManifest.swift)

- Shallow git fetch + name-only diff after ls-remote detects a new SHA; when every changed path is deny-listed infrastructure (README, LICENSE, CHANGELOG, .github/, node_modules/, .build/, Makefile), the notification is suppressed and the registry baseline advances so the same commits don't re-trigger on future cooldown windows.
- techpack.yaml changes always surface unfiltered — manifest edits can swap hook scripts and MCP commands, so suppressing them would be a supply-chain risk.
- Fetch or diff failure surfaces the notification unfiltered (never-hide invariant); pipeline covered by pure classifier + mock-shell orchestrator + end-to-end PackRegistryFile write-back tests.
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.

1 participant