Skip to content

ci: validate batch preset skill toggles - #1

Draft
vipinchan wants to merge 19 commits into
mainfrom
perf/batch-preset-skill-toggle
Draft

ci: validate batch preset skill toggles#1
vipinchan wants to merge 19 commits into
mainfrom
perf/batch-preset-skill-toggle

Conversation

@vipinchan

Copy link
Copy Markdown
Owner

Temporary draft PR used to run the fork's full CI matrix for the branch submitted upstream in xingkongliang#336. This PR will be closed after CI validation.

xingkongliang and others added 19 commits July 5, 2026 18:22
…rge incident)

A device that had only ever pushed hit a permanent 'object merge aborted
(zero changes)' the first time it actually merged. Three compounding
causes, found on a real library:

- A committed atomic-write leftover (x.json.tmp.<uuid>) under the
  metadata namespace: the strict validator rejects any non-json file
  there. Temp/OS junk inside the managed metadata subdirs is now dropped
  from the merged tree at plan stage, and every commit path deletes such
  leftovers from the working tree first (a push-only machine never runs
  the reconcile cleanup).
- Unclaimed skill dirs from old app versions in the shared history:
  rule 4 now grandfathers dirs that were already unclaimed in either
  merge input — the invariant defends against merges introducing
  orphans, not against pre-existing ones syncing through.
- The failure card only showed the outermost error context: git-classified
  command errors now carry the full anyhow chain, so the actual
  validation rule appears in the UI.
"Back Up Now" with "local changes: 0 · remote updates: 1" read as a
push that might overwrite the remote — the opposite of what it does.
The pending state now names its three situations: remote-only updates
get their own title, description and a "Sync Now" button, and both
mixed-state descriptions say explicitly that syncing merges per skill
and neither side overwrites the other.
…dex review)

Two findings from a codex review of the incident fixes, both accepted:

- The old-client single-parent tip validation tolerated unclaimed dirs
  but not committed .tmp. metadata junk, so a legacy tip carrying one
  still hard-blocked before the plan-stage drop could run. Input-tip
  validation (validate_input_tip) now skips metadata-namespace junk;
  the merged tree stays strict — the planner guarantees junk-free
  output there, so junk would mean an engine bug.
- The legacy-dirt integration test committed its tmp file through the
  app path, which now cleans temp files first — the 'junk already in
  history' topology was silently uncovered. The test commits via raw
  git (as the incident device's history actually looks), asserts the
  junk is really in the remote history, and was mutation-verified:
  disabling the plan-stage drop makes it fail. The old-client test now
  also carries committed junk to pin the tip-validation tolerance.
… watcher self-write mute (xingkongliang#248) (xingkongliang#285)

* fix(startup): stop the ~8s stranded-target backfill from blocking launch (xingkongliang#248)

`backfill_stranded_agent_targets` ran a full multi-agent filesystem scan +
per-skill hashing synchronously inside Tauri `setup()`, freezing the window.
Measured ~7.9s on every launch: its `has_candidate` fast-path never went
false because un-repairable stranded skills (diverged locals we intentionally
skip) keep a `source_ref` with no target row forever, so the expensive scan
re-ran indefinitely.

- Move the backfill into a background `spawn_blocking` after the window shows,
  mirroring the existing credential-migration pattern. `setup()` body: ~8000ms
  -> ~15-25ms.
- Gate the scan on a signature of the stranded candidate set. An unchanged set
  skips the scan entirely; a newly-stranded skill re-arms it. Recorded
  post-repair so it converges in one launch.

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

* test(git_backup): persist git identity in pre-protocol restore test

The test built its repo with transient `-c user.email/user.name` flags
that only apply to its own git closure. The production commit path
(commit_all_unlocked via plain run_git) then found no identity on CI
runners lacking a global git config, panicking with "Author identity
unknown". Persist the identity into repo-local config after init so the
production path has an author, matching the convention in the other
commit_all_unlocked tests.

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Render a hand-drawn (star-history.com style) star-growth SVG from the
owner-authenticated gh stargazer timeline and commit it as a static
asset. GitHub restricted the stargazer-history API to repo owners in
2026-06, so the live star-history.com embed no longer works for this
repo; generating locally with the owner token is now the only reliable
path. Auto-refreshed during release:prepare (best-effort, never blocks).

Handwritten look uses the bundled Patrick Hand font (OFL), embedded into
the SVG so it renders on GitHub without external fonts.
- Backfill gate: fold available-adapter set, DB content hash, and local/
  central mtime fingerprints into the stranded-candidate signature so the
  scan re-arms when repairability changes (tool newly installed/enabled,
  diverged local restored, center edited) — not only when the set changes.
- Backfill race: re-hash both sides of a skill immediately before the
  sync write; the scan snapshot can be seconds old and the backfill now
  runs concurrently with the user, so a fresh local edit must never be
  clobbered by the central copy.
- Watcher mute: defer (not drop) relevant events that arrive inside the
  1.2s self-write mute window and flush once it closes, so an external
  edit during the window can no longer leave the UI stale.
- Path-scoped watcher mute: record the dirs the app actually writes and
  swallow only THEIR echoes; foreign paths seen inside the window defer.
  Blanket-deferring everything re-introduced one redundant refresh per
  self-write batch — the exact waste xingkongliang#248 removed.
- Backfill gate fingerprint: replace max-mtime+entry-count (churned by
  .DS_Store-style junk, blind to same-mtime restores) with the same
  content hash the repair itself compares, plus an existence marker.
- Backfill pre-write check: explicit existence checks and fail-closed
  hash comparison (hash_directory treats missing dirs as empty and its
  errors were compared as equal None).
- Tests: three-state mute verdict table, path-scoped classification,
  diverged-then-restored re-arm scenario (replaces the mtime-bump test,
  which the content fingerprint correctly no longer trips on).
Wrap the README star chart in a link to the tool that generates it
(github.com/xingkongliang/star-history-svg) and correct the in-image
watermark from "star-history.com" (which did not produce this chart)
to "star-history-svg".
…ling back to container (xingkongliang#280)

Fixes xingkongliang#278. find_skill_dir() now bail!s when a specific skill_id is requested but matches nothing, instead of falling through to the skills/ container or root — which had installed the entire repo as one skill. Fallback preserved only for skill_id == None (enumeration flows). Verified: 287 Rust tests pass; codex read-only review confirmed no caller regressions.
Follow-up to xingkongliang#280 (codex review): lock in the legitimate case where a
requested skill_id matches the root SKILL.md's frontmatter `name`, so the
xingkongliang#278 bail! never regresses single-skill repos whose id resolves via the
recursive name match rather than a directory basename.
Per codex review of 60564f7: the positive-only assertion could pass for
the wrong reason — a root SKILL.md also satisfies the skill_id==None root
fallback, so the test wouldn't fail if the old xingkongliang#278 fall-through were
reintroduced. Add a sibling assertion that a non-matching id on the same
root-skill repo errors, proving the root is returned via name_match, not
fallback.
…ingkongliang#241)

Replaces the two native <select> dropdowns for auto-update (check interval, auto-apply) with the segmented-control button pattern already used elsewhere on the Settings page (theme, sync mode, tray, etc.). Wrapper and active/inactive classes match the existing controls verbatim; adds aria-pressed. Values and change handlers unchanged — behavior identical. Verified: npm run build (tsc -b + vite build) passes.
…ly files (xingkongliang#252)

On Windows, relocating the central library to another drive could leave
read-only Git pack files at the destination after an incomplete move. The
next launch tried to fs::copy over them, hit "access denied (os error 5)",
and the error bubbled to the .expect in run() — which executes before the
window, logger, and panic hook exist. The process died silently: no UI, no
log, no panic file. The app simply wouldn't open.

Root cause is overwriting an existing read-only file. Fix:

- migrate_repo_if_needed only ever moves into an empty/absent target. A
  non-empty target (real data or failed-attempt debris, indistinguishable)
  is never blind-merged — the app falls back to the intact source instead,
  so it can't overwrite newer data with older, and a fresh target makes the
  copy structurally incapable of overwriting a read-only pack.
- The function is now infallible (returns MigrationOutcome, never Err), so a
  migration failure can no longer panic through run()'s pre-window .expect.
  On failure it runs the session against the source via a runtime base-dir
  override and shows an amber `migration_incomplete` banner (en/zh/zh-TW).
- Detailed errors are deferred via record_startup_error and flushed in
  setup() once the logger exists (the pre-logger log::error! was a no-op).
- Migration is skipped when a CLI base override (--skills-root/--path) is
  active, so the shared pending marker can't hijack an explicit root.

Hardened after a grok review pass:
- Compare source/target canonically (fs::canonicalize), so a cosmetic path
  difference (case-insensitive volume, 8.3, symlink) isn't taken for a real
  move and looped forever on `migration_incomplete`.
- Reworded the fallback banner so it never tells the user to empty the path
  shown under "Central Repo Path" — under the override that chip shows the
  live library (get_central_repo_path returns base_dir()), so the old wording
  risked pointing a user at their own data.

+6 Rust tests. Full suite 393 passing; npm run build clean.
… paths

- backup: refresh presets after clone/reclone/sync/github-connect/restore and
  first-run restore so the sidebar preset list isn't empty until an app
  restart — metadata reindex already restored them to the DB (xingkongliang#302)
- workspace: overview cards count each agent's real on-disk skills (not just the
  managed library) so an agent whose skills were installed outside Skills
  Manager no longer shows 0, matching the per-agent detail badge (xingkongliang#287)
- icons: invert monochrome-black agent icons (codex, roo_code) under the dark
  theme so they aren't invisible on dark backgrounds; augment excluded because
  it already self-adapts via prefers-color-scheme (xingkongliang#279, xingkongliang#304)
- windows: embed an app manifest declaring longPathAware so installing skills
  with deeply nested paths no longer fails with 'path too long' (xingkongliang#298, xingkongliang#299)

codex(gpt-5.5)-reviewed; follow-ups applied: conflict-resolution and background
auto-backup completion also refresh presets/skills; overview counts depend on
the managedSkills ref and rebuild fresh so failed scans fall back to managed.
- skill_store: 新增 batch_add_skills_to_scenario / batch_remove_skills_from_scenario,
  单事务内完成批量 INSERT/DELETE
- presets: 新增 batch_toggle_preset_skills Tauri 命令,metadata 写 1 次 + tray 刷 1 次
- lib.rs: 注册 batch_toggle_preset_skills
- tauri.ts: 新增 batchTogglePresetSkills API
- MySkills.tsx: handleBatchTogglePreset 从 N 次独立 IPC 改为 1 次批量调用
- N 选 30 时: 30 次 IPC + metadata 写入 + tray 重建 → 1 次
@vipinchan vipinchan closed this Jul 22, 2026
@vipinchan vipinchan reopened this Jul 22, 2026
@vipinchan vipinchan closed this Jul 22, 2026
@vipinchan vipinchan reopened this Jul 22, 2026
@vipinchan vipinchan closed this Jul 22, 2026
@vipinchan vipinchan reopened this Jul 22, 2026
@vipinchan
vipinchan force-pushed the perf/batch-preset-skill-toggle branch from 1a719cd to 4a24f89 Compare July 26, 2026 16:58
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.

5 participants