refactor(knowledge): fix dead variable, double validation, and over-strict '..' check

- Remove the unused `removed []string` accumulator in DeleteKnowledgeFiles;
  valid paths now go straight into `toRemove` in one loop pass, eliminating
  the second validateKnowledgeRelPath call per path.
- Tighten the '..' guard to reject only the bare ".." token — the old
  strings.Contains check wrongly rejected valid filenames like "foo..bar".
- Add test assertions confirming double-dot-in-middle filenames are accepted.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: ysyneu

workspace/knowledge.go

@@ -28,20 +28,22 @@ const (
 //   - Leading-dot filenames are rejected because they are hidden by convention;
 //     the sentinel is written by the runner itself and is never staged by clients.
 func validateKnowledgeRelPath(relPath string) error {
+	if relPath == "" {
+		return fmt.Errorf("rel_path must not be empty")
+	}
 	if strings.ContainsAny(relPath, `/\`) {
 		return fmt.Errorf("rel_path must not contain path separators: %q", relPath)
 	}
-	if relPath == ".." || strings.Contains(relPath, "..") {
-		return fmt.Errorf("rel_path must not contain '..': %q", relPath)
+	// Reject the bare ".." token. Slash-separated traversal like "foo/../bar"
+	// is already blocked above, but a plain ".." with no slashes still escapes.
+	if relPath == ".." {
+		return fmt.Errorf("rel_path must not be '..': %q", relPath)
 	}
 	if strings.HasPrefix(relPath, ".") {
 		// Hidden files (including the sentinel itself) cannot be staged by clients.
 		// The runner owns the sentinel exclusively.
 		return fmt.Errorf("rel_path must not start with '.': %q", relPath)
 	}
-	if relPath == "" {
-		return fmt.Errorf("rel_path must not be empty")
-	}
 	return nil
 }
 
@@ -210,33 +212,27 @@ func (w *Workspace) StageKnowledgeFiles(ctx context.Context, args *protocol.Stag
 // DeleteKnowledgeFiles removes the supplied files from the workspace root and
 // scrubs their entries from the sentinel.
 func (w *Workspace) DeleteKnowledgeFiles(ctx context.Context, args *protocol.DeleteKnowledgeFilesArgs) (*protocol.DeleteKnowledgeFilesResult, error) {
-	var removed []string
+	// toRemove collects validated rel_paths so we can clean the sentinel in one
+	// locked pass after the file removals.
+	toRemove := make(map[string]struct{}, len(args.RelPaths))
 
 	for _, relPath := range args.RelPaths {
 		if err := validateKnowledgeRelPath(relPath); err != nil {
 			slog.Warn("skipping invalid rel_path in delete_knowledge_files", "rel_path", relPath, "error", err)
 			continue
 		}
+		toRemove[relPath] = struct{}{}
 
 		targetPath := filepath.Join(w.root, relPath)
 		if err := os.Remove(targetPath); err != nil && !os.IsNotExist(err) {
 			slog.Warn("failed to remove knowledge file", "rel_path", relPath, "error", err)
-			continue
-		}
-		removed = append(removed, relPath)
-	}
-
-	// Remove entries from sentinel for files we attempted to delete (whether
-	// they existed or not — idempotent means we clean the sentinel too).
-	sentinelPath := filepath.Join(w.root, sentinelName)
-	toRemove := make(map[string]struct{}, len(args.RelPaths))
-	for _, rp := range args.RelPaths {
-		if validateKnowledgeRelPath(rp) == nil {
-			toRemove[rp] = struct{}{}
 		}
 	}
 
+	// Remove entries from sentinel for all valid paths (idempotent — missing
+	// sentinel entries are simply no-ops in the delete loop).
 	if len(toRemove) > 0 {
+		sentinelPath := filepath.Join(w.root, sentinelName)
 		if err := withSentinelLock(sentinelPath, func() error {
 			m := readSentinel(sentinelPath)
 			for rp := range toRemove {
@@ -248,6 +244,5 @@ func (w *Workspace) DeleteKnowledgeFiles(ctx context.Context, args *protocol.Del
 		}
 	}
 
-	_ = removed // logged individually above; all valid paths are processed
 	return &protocol.DeleteKnowledgeFilesResult{Success: true}, nil
 }

workspace/knowledge_test.go

@@ -225,6 +225,12 @@ func TestValidateKnowledgeRelPath(t *testing.T) {
 
 	invalid := []string{
 		"", "..", "a/b", `a\b`, ".hidden", sentinelName, "foo/../bar",
+		// These are now valid because ".." only matches the bare token, not
+		// substrings — "foo..bar" is a legitimate flat filename.
+	}
+	validButPreviouslyRejected := []string{"foo..bar", "v2..md"}
+	for _, p := range validButPreviouslyRejected {
+		assert.NoError(t, validateKnowledgeRelPath(p), "should be valid (double-dot in middle): %q", p)
 	}
 	for _, p := range invalid {
 		assert.Error(t, validateKnowledgeRelPath(p), "should be invalid: %q", p)