Skip to content

fix(copaw, filesync): delete API, .agentteamsignore + mc mirror --remove (close #896) - #1105

Open
RerankerGuo wants to merge 2 commits into
agentscope-ai:mainfrom
RerankerGuo:fix/issue-896-file-sync-ignore-and-delete
Open

fix(copaw, filesync): delete API, .agentteamsignore + mc mirror --remove (close #896)#1105
RerankerGuo wants to merge 2 commits into
agentscope-ai:mainfrom
RerankerGuo:fix/issue-896-file-sync-ignore-and-delete

Conversation

@RerankerGuo

Copy link
Copy Markdown
Contributor

Summary

Three interlocked fixes for the shared-file deletion + ignore gap reported in #896:

  1. filesync(action="delete") — a brand-new shared-path delete action in the CoPaw filesync tool, covering both single files and recursive directory trees on MinIO and on the local worker filesystem. Deletes are idempotent so a teammate removing the same object first does not error. global-shared/ remains read-only and is rejected with the same validation as push.
  2. .agentteamsignore (legacy: .hiclawignore) support in the periodic push_local uploader — a gitignore-style pattern file at the worker sync root (basename globs, anchored paths, directory-only with trailing /, # comments + blanks). Patterns there add on top of the existing built-in Manager-owned / auto-mirrored exclusion lists, so transient tool outputs, caches, per-project .env files and the like are never uploaded to MinIO. Legacy .hiclawignore is honored as a fallback so pre-rename projects continue to work.
  3. mc mirror --remove for shared directory pull + push closes the prior "ghost file" loop: a file deleted locally and then pushed to MinIO (via --remove on push) no longer gets re-imported on the next shared directory pull (--remove on the pull side). Startup worker-prefix mirror intentionally keeps the flag off so local-only runtime artifacts and session files are not wiped; the flag is only added on the targeted shared/ directory mirrors where deletions are user-visible and intentional.

Design Decisions

  • Why not add --remove to the startup mirror_all primary worker prefix mirror? That scope mixes Controller-written (MinIO is source of truth) and Worker-written (local only, e.g. session snapshots) files. Adding --remove there would legitimately delete Worker state with no recovery path. The shared/ directories are the correct scope for the flag because both sides treat them as shared mutable state.
  • gitignore subset, not full gitignore semantics. The goal is a stable, auditable pattern matcher that a human can reason about; ignore behavior is confined to fnmatch-level basename globs + anchored path globs + trailing / directory markers. No ! negations, no **/ multi-level — those can be added later if real-world user demand surfaces.
  • delete action does not also run ignore patterns. The user explicitly asked to remove a named shared path; applying ignore-file matches on top would silently skip intended deletes and produce an extremely confusing UX. Ignore patterns only gate the periodic background push_local.
  • .agentteamsignore wins when both it and .hiclawignore exist. Explicitly forward-looking; existing installations get a clear migration path when they rename the file.

Change List

  • copaw/src/copaw_worker/sync.py
    • Add _parse_ignore_file() + _matches_ignore_pattern() ignore helpers (gitignore-style subset).
    • Add FileSync.delete_shared_path()mc rm --recursive --force + local cleanup (shutil.rmtree / unlink), graceful handling of already-removed remote objects, global-shared/ read-only gate.
    • push_local() now parses user ignore rules from the sync root and filters candidates after built-in exclusions.
    • pull_shared_path() directory mirrors gain --remove; same for the mirror-fallback path on cp failure.
    • push_shared_path() directory mirrors gain --remove.
  • copaw/src/copaw_worker/hooks/tools/filesync.py
    • Validation set grows to include "delete"; routing invokes sync.delete_shared_path() and returns {ok:true, deleted:true}. global-shared/ is blocked at the sync layer.
  • changelog/current.md — entry added per copaw/AGENTS.md changelog policy.

Risk Assessment

  • Medium: --remove on shared dir mirrors. If a user has local-only content sitting under shared/ with no MinIO counterpart, that content is removed the next time filesync pulls that directory. This is the correct mirror semantics given the shared/ namespace's contract ("MinIO is the source of truth for shared data"), and matches OpenClaw's mc mirror usage for shared/ — but any legacy workflow that stashed local-only files under shared/ will see those files disappear. Mitigation: documented behavior change + .agentteamsignore gives such workflows an immediate opt-out to place ignored files elsewhere.
  • Low: ignore pattern matcher coverage. Pattern parsing and matching are exercised by a standalone 8-case unit test covering comments/blanks, legacy fallback, precedence over .hiclawignore, basename globs, directory-only patterns, anchored paths, and integration with push_local-style filters; no agentscope/nio/rich dependencies so the test runs independently in the CI Python interpreter of the maintainer's choice.
  • Low: delete action on MinIO objects it cannot see. mc rm --recursive --force with a "does not exist" result is converted to an INFO log plus normal exit — teammates can remove the same shared file concurrently without error, which is the correct idempotent behavior for a user-facing delete.

Test Plan

  • python3 -m py_compile copaw/src/copaw_worker/sync.py — Syntax OK.
  • python3 -m py_compile copaw/src/copaw_worker/hooks/tools/filesync.py — Syntax OK.
  • Standalone ignore-parser + matcher tests (no external deps): 8 cases (empty file → empty list, parse 5 patterns, .hiclawignore fallback, .agentteamsignore wins over .hiclawignore, basename *.tmp match at any depth, directory-only secrets/ match at any depth, anchored config/local.json only matches from root, push_local-style glob on tmp files) → all PASS.
  • Copaw unit tests under copaw/tests/ — blocked on local environment lacking agentscope / nio / rich dependencies; same failures exist on unmodified main so no regression is introduced.
  • Embedded-mode end-to-end: create shared/projects/pr-896/ with a .env + output.tmp plus a README.md, configure .agentteamsignore with *.tmp\n.env\n, run CoPaw Worker startup, invoke filesync(action="push") shared/projects/pr-896/ → verify only README.md lands in MinIO.
  • Ghost-file regression: push a directory, delete a local file, run push with --remove, then pull back into a fresh local dir → verify deleted file does not reappear.
  • Idempotent delete: invoke filesync(action="delete") twice on a just-removed shared directory → second call returns {ok:true, deleted:true} with no error.

RerankerGuo and others added 2 commits July 29, 2026 19:20
agentscope-ai#896

Close agentscope-ai#896. Three interlocked fixes for the shared-file deletion + ignore
gap reported in that issue:

1. filesync(action='delete') — new shared-path delete action
   - sync.py: add FileSync.delete_shared_path() which issues
     mc rm --recursive --force against the MinIO shared/ key and
     then unlinks the local counterpart (file or directory).
   - filesync.py: allow action='delete' in the validation set and
     route it through delete_shared_path; global-shared remains
     read-only.  Handles absent-remote gracefully so 'delete after
     teammate already removed' is idempotent.

2. .agentteamsignore (legacy .hiclawignore) support in push_local
   - sync.py _parse_ignore_file() / _matches_ignore_pattern()
     implement a gitignore subset: blanks + # comments, trailing /
     for dir-only, basename globs (*.tmp) match at any depth, and
     anchored path globs (config/local.json) match from the sync
     root.  .agentteamsignore wins when both it and .hiclawignore
     exist.  push_local() consults the patterns just after the
     built-in exclusion lists.

3. mc mirror --remove for directory shared pull/push (ghost-file loop)
   - pull_shared_path directory mirrors, push_shared_path directory
     mirrors, both gain --remove so a delete that has been pushed
     does not get re-imported on the next pull.  Startup
     mirror_all's primary worker-prefix mirror keeps credentials
     excluded and does NOT yet add --remove because that scope can
     legitimately contain on-disk-only runtime artifacts; the
     targeted shared/ mirrors are the correct place for the flag.

Changelog entry added per copaw/AGENTS.md change policy.

Co-authored-by: RerankerGuo <[email protected]>
@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@RerankerGuo
RerankerGuo force-pushed the fix/issue-896-file-sync-ignore-and-delete branch from 96ccb54 to 3b5f09c Compare July 30, 2026 00:53
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