Release v1.0.0 - First stable release

[nitrobass24]

Sync develop → master for v1.0.0. See CHANGELOG.md for the full entry.

Summary

First stable release, covering 154 commits / ~46 PRs since v0.18.1. This is a milestone bump from the 0.18.x line (the [Unreleased] section had been accumulating toward 0.19.0; promoted to 1.0.0). Headlines:

• Optional FTPS transfer protocol (FTP instead of SFTP #485/feat: optional FTPS transfer protocol #564) — sftp (default) or ftps, hybrid design (discovery stays on SSH), dedicated Transfer Protocol settings section.
• Backend reliability & persistence hardening — config-handler atomicity/rollback, worker fault isolation + recovery, failed-move surfacing (MOVE_FAILED) + retry, auto-queue thread-safety, bounded teardown, 504 action timeout, lftp parser robustness.
• Frontend performance — coalesced SSE rebuilds, virtualized logs, ViewFileService split.
• Angular 21 → 22 (Upgrade Angular 21 → 22 (unblocks angular-eslint 22 + the Angular dep group) #556) incl. TypeScript 6.0.
• Security baseline — security-critical path test coverage, API key kept out of browser storage, documented hardened Docker runtime baseline.

Release process

• ✅ CHANGELOG.md: [Unreleased] → ## [1.0.0] - 2026-06-10
• ✅ src/angular/package.json: 0.19.0 → 1.0.0 (About page)
• ✅ Branch release/v1.0.0 cut off latest origin/develop (60eb99b, through feat(settings): rename FTPS section to "Transfer Protocol" #569)
• ⬜ After merge: tag v1.0.0 on the merge commit → CI builds/publishes the multi-arch image and creates the GitHub release

Changelog completeness & accuracy

The [Unreleased] section was audited PR-by-PR against the real diffs of every change since v0.18.1. Corrections made in the release commit:

Added (were missing):

• #567 — cross-device move raced lftp's *.lftp temp-file rename (crashed MoveProcess); now defers + recovers.
• #530 — integrations handlers wrap persistence in try/except (controlled HTTP 500).
• #527 — version-check banner bounded with a 10s timeout.
• Dependency line extended to cite #560, #565 and list shell-quote.

Corrected (were stale/inaccurate):

• FTPS (FTP instead of SFTP #485): dropped the "FTPS-tuned connection profile" claim — #568 removed that override; FTPS now uses the same persisted Connections settings as SFTP.
• fix(ssh): use full timeout for password prompt, disable GSSAPI auth #562: removed the GSSAPIAuthentication=no clause — reverted by #566 (Alpine openssh rejects it). See note below.
• Config: '%' in any value crashes persistence; rollback gap on write #507: re-attributed the %/ConfigParser(interpolation=None) fix to #530; #507 was the rollback-broadening.
• Wrap auto-queue and path-pairs persistence in try/except (#469.1, #469.4) #497: clarified auto-queue now rolls back on persist failure (#518), path-pairs retains the in-memory mutation.
• Docker: ship a hardened runtime baseline (no-new-privileges, cap_drop) #528 (Security): reworded as a documented/opt-in baseline (docker-compose.dev.yml + docs), not something the published image enforces.

Editorial note — GSSAPI: the GSSAPIAuthentication=no flag was both added (#562) and removed (#566) after v0.18.1, so it never shipped in a release and nets to zero for users. It's intentionally omitted from the changelog rather than documented as add-then-revert.

Test plan

• CI (ci.yml) is green on the develop commit this branch is cut from (feat(settings): rename FTPS section to "Transfer Protocol" #569 merge). This PR re-runs Build & Test (Docker build + container startup + web UI check).
• No source code changes in this PR — only CHANGELOG.md and package.json version.
• Verified only those two files are staged; 4 untracked local scratch design docs were deliberately excluded.

Heads-up (pre-existing on develop, not introduced here)

• A CodeQL javascript-typescript run failed on develop with a transient GitHub API auth error during its init step ("Requires authentication … init Action failed") — infrastructure, not a code finding (js-ts CodeQL passed on fix(lftp): stop overriding Connections settings under FTPS #568). It should re-run green; it does not gate the release.
• Editor may flag OVERRIDE_NOTE in settings-page.component.ts as unused (TS6133) — false positive; it's intentionally retained and read by the spec via a statics cast.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added optional FTPS transfer protocol with dedicated settings and certificate verification control
  • Added download-start notifications
  • Automatic extract/validate worker recovery on failure

• Bug Fixes

  • Improved move operation error handling and recovery
  • Enhanced error reporting for API actions
  • Fixed SSE streaming timeout handling
  • Improved parser robustness

• Security

  • Hardened Docker container runtime with restricted capabilities
  • Removed client-side persistence for web API keys

• Documentation

  • Added FTPS configuration and troubleshooting guidance
  • Updated Docker security hardening recommendations

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b37ef3fe-0065-4e31-bdaf-046e3974200c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

This PR adds FTPS support, move-failure persistence and retry handling, and broader UI/service refactors. It also tightens CI, documentation, web-handler responses, and several parser/error-handling paths across Python and Angular code.

Changes

Infrastructure and release updates

Layer / File(s)	Summary
CI, docs, and packaging /.github/workflows/ci.yml, .gitignore, CHANGELOG.md, CLAUDE.md, doc/DeveloperReadme.md, docker-compose.dev.yml, src/angular/angular.json, src/angular/package.json, website/docs/*, website/package.json	CI jobs gain timeouts and pinned actions, docs describe new release/live-test/FTPS behavior, and package metadata plus Docker runtime settings are updated.

Angular config and settings

Layer / File(s)	Summary
Angular config and settings src/angular/src/app/app.config.ts, src/angular/src/app/models/config.ts, src/angular/src/app/pages/settings/*, src/angular/src/app/services/settings/config.service.*, src/angular/src/app/services/utils/api-key.interceptor.*, src/angular/src/app/services/utils/version-check.service.*	FTPS and download-start config fields are added, settings contexts gain typed disable rules, config persistence/redaction behavior changes, and related tests cover the new option and interceptor behavior.

Files, logs, and notifications

Layer / File(s)

Summary

Files, logs, and notifications src/angular/src/app/pages/files/*, src/angular/src/app/pages/logs/*, src/angular/src/app/models/model-file.*, src/angular/src/app/models/view-file.ts, src/angular/src/app/services/files/*, src/angular/src/app/services/logs/log.service.*, src/angular/src/app/services/server/server-status.service.*, src/angular/src/app/services/base/stream-dispatch.service.*, src/angular/src/app/services/autoqueue/autoqueue.service.*

File actions now emit callbacks, logs use virtual scrolling and traceback toggles, view-file selection/command/state logic is extracted, and services/tests handle batching, backoff, malformed SSE payloads, and notification banners.

Python controller, lftp, and persistence

Layer / File(s)	Summary
Python controller, lftp, and persistence src/python/common/config.py, src/python/controller/*, src/python/lftp/*, src/python/model/file.py, src/python/seedsync.py, src/python/ssh/sshcp.py, src/python/tests/*	Backend config and transfer code now support FTPS, controller state tracks move-failed files and retries them through worker supervisors, parsers and SSH/Lftp handling are hardened, and tests cover the new behavior and edge cases.

Web handlers and serialization

Layer / File(s)	Summary
Web handlers and serialization src/python/web/handler/*, src/python/web/serialize/serialize_model.py	Config, auto-queue, integrations, controller, and path-pairs handlers serialize writes with locks or helper responses, return controlled timeout/persistence failures, and serialize the new move-failed state.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

• nitrobass24/seedsync#495: Both PRs update src/python/web/handler/config.py’s ConfigHandler /server/config/set flow to roll back in-memory config on Config.to_file persistence OSError and only trigger the LFTP hot-reload callback after a successful disk write.
• nitrobass24/seedsync#557: Yes—both PRs modify the same frontend code paths, including logs-page.component.*, ConfigService.set/AutoQueueService.add-remove, and ViewFileCommandService/ViewFileService command dispatch extraction.
• nitrobass24/seedsync#226: Both PRs add and extend integration coverage for the /server/pathpairs web handler, with this PR adding persistence-failure cases in src/python/tests/integration/test_web/test_handler/test_path_pairs.py.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch release/v1.0.0

[coderabbitai]

Actionable comments posted: 12

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ci.yml:
- Line 228: The workflow currently uses the floating tag actions/checkout@v6;
replace both occurrences of actions/checkout@v6 with the corresponding v6 commit
SHA to pin the action for supply-chain security. Locate the two usages of
"actions/checkout@v6" and update each to the exact v6 commit SHA (the canonical
SHA for the v6 release) so the workflow references the immutable commit instead
of the tag.
- Around line 227-228: The actions/checkout step currently lacks the
persist-credentials setting; update the Checkout step (uses:
actions/checkout@v6) to include persist-credentials: false so the GITHUB_TOKEN
is not left in git config for subsequent steps—add the persist-credentials:
false key under the Checkout step configuration in the workflow.

In `@src/angular/src/app/models/config.ts`:
- Around line 47-49: The ftp_ssl_verify_certificate default is insecurely set to
false; update the default for the ftp_ssl_verify_certificate property to true
wherever the Config model is initialized (the ftp_ssl_verify_certificate field
in the config model and any defaultConfig or initializer that sets it — e.g.,
the object that currently assigns ftp_ssl_verify_certificate: false at
initialization) so FTPS connections verify certificates by default, and
run/update any related unit tests or serializers that assume the old default.

In `@src/angular/src/app/pages/logs/logs-page.component.scss`:
- Around line 109-166: Add a blank line before the first CSS declaration inside
the .log-viewport p.record block so Stylelint is satisfied (insert an empty line
above the --log-row-height: 22px; declaration); this is purely formatting—do not
change the virtualization logic or values tied to LogsPageComponent.ROW_HEIGHT
or the traceback popover rules—and re-run stylelint to confirm the warning is
gone.

In `@src/angular/src/app/pages/logs/logs-page.component.spec.ts`:
- Around line 165-190: The test relies on a fragile type-assertion to access the
private changeDetector field and spy on detectChanges; instead, verify batching
via public observable/state behavior: remove the (component as unknown as {
changeDetector: ChangeDetectorRef }).changeDetector spy and assert on
component.records emissions or subscribe to the records source (or the logs$
test subject) to count the number of change batches after advancing timers by
LogsPageComponent.BATCH_WINDOW_MS; if you must keep internal timing visibility,
add a test-only hook or public callback (e.g., a protected/test-only method
invoked by flushPendingRecords) that you can spy on, but do not access private
changeDetector directly.

In `@src/angular/src/app/services/files/view-file-command.service.ts`:
- Line 113: The log message in createAction currently hardcodes "File to queue
not found" which is misleading because createAction handles multiple actions;
update the logger.error call inside createAction (in view-file-command.service)
to use a generic message that includes the action and the key (e.g., "File not
found for action <action>: <key>") so all actions (queue, stop, extract, delete,
validate) produce an accurate log entry.

In `@src/python/controller/controller_persist.py`:
- Line 84: Serializing move_failed_file_names alongside other persist sets
creates a torn-snapshot race because Seedsync.persist() calls
ControllerPersist.to_str() from the main thread while the controller mutates
those sets concurrently; fix by taking one atomic snapshot under a single lock
and routing all mutations through that same lock. Modify
ControllerPersist.to_str() to acquire the controller's persist lock (use the
existing lock field, e.g., self._persist_lock or self._lock) and copy all
persist sets (including move_failed_file_names and the downloaded/extracted
sets) while holding it, and update all mutation points that touch
move_failed_file_names, downloaded, or extracted to also acquire that same lock
before modifying. Ensure Seedsync.persist() continues to call
ControllerPersist.to_str() but now receives a consistent snapshot.

In `@src/python/seedsync.py`:
- Line 408: The code currently sets insecure default by assigning
config.lftp.ftp_ssl_verify_certificate = False; change this to enable
verification by default (set to True) and make the option explicitly opt-out via
configuration or documented flag so users must deliberately disable it. Locate
the assignment to config.lftp.ftp_ssl_verify_certificate in seedsync.py (the
configuration initialization code) and flip the default to True, preserving any
existing paths that read this value so disabling remains possible if explicitly
set by the user.

In `@src/python/tests/unittests/test_controller/test_model_builder.py`:
- Around line 983-1036: The tests and helper __build_deep_wide_tree inside
TestModelBuilder are adding large, unrelated coverage to test_model_builder.py;
split them into a new focused test module (e.g.,
test_model_builder_deep_tree.py) that contains the helper __build_deep_wide_tree
and the two tests test_build_state_deep_wide_tree_downloaded and
test_build_state_deep_wide_tree_incomplete_leaf (and any adjacent cases
mentioned around lines 1906-1975) so the original file stays under the
size/concern threshold. Move any setup/fixture usage of self.model_builder or
TestModelBuilder-specific utilities into the new module (or import them from the
existing test support) and update imports accordingly so the tests still call
model_builder.set_remote_files, set_local_files, build_model, and use
ModelFile.State comparisons unchanged.

In `@src/python/tests/unittests/test_system/test_scanner.py`:
- Around line 655-675: The test test_symlink_to_outside_directory_is_traversed
assumes SystemScanner follows directory symlinks and recurses outside the scan
root; change the scanner behavior (SystemScanner.scan and any DirEntry handling)
to either skip directory symlinks entirely or resolve symlinks and reject
targets outside the configured scan root, then update this test (and the similar
case around 678-687) to assert the new safe behavior: that "link_to_outside" is
present as a symlink entry but not traversed (is_dir False or children empty) or
that it is omitted if out-of-root, and ensure TestSystemScanner.temp_dir-related
assertions reflect the updated contract.

In `@website/docs/configuration.md`:
- Around line 37-42: The fenced ASCII-art connection diagram block is missing a
language identifier; add the identifier text right after the opening fenced code
block (the block containing "File discovery / scan  ── always ──▶  SSH" and
"Bulk file transfer     ── toggle ──▶  SFTP ...") so the block becomes a text
code fence (keep the existing closing fence) to silence the markdownlint
warning.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 12b9ca34-c361-42c4-8404-61f42dcb79c8

📥 Commits

Reviewing files that changed from the base of the PR and between 6fc8342 and 1d800aa.

⛔ Files ignored due to path filters (3)

• src/angular/package-lock.json is excluded by !**/package-lock.json, !**/package-lock.json
• src/python/uv.lock is excluded by !**/*.lock
• website/package-lock.json is excluded by !**/package-lock.json, !**/package-lock.json

📒 Files selected for processing (130)

• .github/workflows/ci.yml
• .gitignore
• CHANGELOG.md
• CLAUDE.md
• MODERNIZATION_PLAN.md
• doc/DeveloperReadme.md
• docker-compose.dev.yml
• src/angular/angular.json
• src/angular/package.json
• src/angular/src/app/app.config.ts
• src/angular/src/app/models/config.ts
• src/angular/src/app/models/model-file.spec.ts
• src/angular/src/app/models/model-file.ts
• src/angular/src/app/models/view-file.ts
• src/angular/src/app/pages/files/bulk-action-bar.component.spec.ts
• src/angular/src/app/pages/files/bulk-action-bar.component.ts
• src/angular/src/app/pages/files/file-list.component.spec.ts
• src/angular/src/app/pages/files/file-list.component.ts
• src/angular/src/app/pages/files/file.component.html
• src/angular/src/app/pages/files/file.component.scss
• src/angular/src/app/pages/files/file.component.spec.ts
• src/angular/src/app/pages/files/file.component.ts
• src/angular/src/app/pages/logs/logs-page.component.html
• src/angular/src/app/pages/logs/logs-page.component.scss
• src/angular/src/app/pages/logs/logs-page.component.spec.ts
• src/angular/src/app/pages/logs/logs-page.component.ts
• src/angular/src/app/pages/settings/options-list.spec.ts
• src/angular/src/app/pages/settings/options-list.ts
• src/angular/src/app/pages/settings/path-pairs.component.spec.ts
• src/angular/src/app/pages/settings/path-pairs.component.ts
• src/angular/src/app/pages/settings/settings-page.component.html
• src/angular/src/app/pages/settings/settings-page.component.spec.ts
• src/angular/src/app/pages/settings/settings-page.component.ts
• src/angular/src/app/pages/settings/settings-page.disable-rules.spec.ts
• src/angular/src/app/services/autoqueue/autoqueue.service.spec.ts
• src/angular/src/app/services/autoqueue/autoqueue.service.ts
• src/angular/src/app/services/base/stream-dispatch.service.spec.ts
• src/angular/src/app/services/base/stream-dispatch.service.ts
• src/angular/src/app/services/files/model-file.service.spec.ts
• src/angular/src/app/services/files/model-file.service.ts
• src/angular/src/app/services/files/view-file-capabilities.spec.ts
• src/angular/src/app/services/files/view-file-capabilities.ts
• src/angular/src/app/services/files/view-file-command.service.spec.ts
• src/angular/src/app/services/files/view-file-command.service.ts
• src/angular/src/app/services/files/view-file-selection.service.spec.ts
• src/angular/src/app/services/files/view-file-selection.service.ts
• src/angular/src/app/services/files/view-file.service.spec.ts
• src/angular/src/app/services/files/view-file.service.ts
• src/angular/src/app/services/logs/log.service.spec.ts
• src/angular/src/app/services/logs/log.service.ts
• src/angular/src/app/services/server/server-status.service.spec.ts
• src/angular/src/app/services/server/server-status.service.ts
• src/angular/src/app/services/settings/config.service.spec.ts
• src/angular/src/app/services/settings/config.service.ts
• src/angular/src/app/services/utils/api-key.interceptor.spec.ts
• src/angular/src/app/services/utils/api-key.interceptor.ts
• src/angular/src/app/services/utils/version-check.service.spec.ts
• src/angular/src/app/services/utils/version-check.service.ts
• src/docker/build/docker-image/run_as_user
• src/docker/build/docker-image/scp
• src/docker/build/docker-image/setup_default_config.sh
• src/docker/build/docker-image/ssh
• src/python/common/config.py
• src/python/controller/auto_queue.py
• src/python/controller/command_pipeline.py
• src/python/controller/commands.py
• src/python/controller/controller.py
• src/python/controller/controller_persist.py
• src/python/controller/model_builder.py
• src/python/controller/model_updater.py
• src/python/controller/move/__init__.py
• src/python/controller/move/move_process.py
• src/python/controller/notification_formatters.py
• src/python/controller/notifier.py
• src/python/controller/pair_context.py
• src/python/controller/persist_sync.py
• src/python/controller/validate/validate_process.py
• src/python/controller/worker_supervisor.py
• src/python/lftp/job_status_parser.py
• src/python/lftp/lftp.py
• src/python/model/file.py
• src/python/seedsync.py
• src/python/ssh/sshcp.py
• src/python/tests/integration/test_controller/test_controller.py
• src/python/tests/integration/test_lftp/test_lftp.py
• src/python/tests/integration/test_web/test_handler/test_auto_queue.py
• src/python/tests/integration/test_web/test_handler/test_config.py
• src/python/tests/integration/test_web/test_handler/test_controller.py
• src/python/tests/integration/test_web/test_handler/test_integrations.py
• src/python/tests/integration/test_web/test_handler/test_path_pairs.py
• src/python/tests/unittests/test_common/test_config.py
• src/python/tests/unittests/test_controller/test_auto_queue.py
• src/python/tests/unittests/test_controller/test_command_pipeline.py
• src/python/tests/unittests/test_controller/test_commands.py
• src/python/tests/unittests/test_controller/test_controller_exit.py
• src/python/tests/unittests/test_controller/test_controller_lftp_wiring.py
• src/python/tests/unittests/test_controller/test_controller_persist.py
• src/python/tests/unittests/test_controller/test_extract/test_extract.py
• src/python/tests/unittests/test_controller/test_model_builder.py
• src/python/tests/unittests/test_controller/test_model_updater.py
• src/python/tests/unittests/test_controller/test_move_process.py
• src/python/tests/unittests/test_controller/test_notification_formatters.py
• src/python/tests/unittests/test_controller/test_notifier.py
• src/python/tests/unittests/test_controller/test_pair_context.py
• src/python/tests/unittests/test_controller/test_persist_sync.py
• src/python/tests/unittests/test_controller/test_validate/test_validate_process.py
• src/python/tests/unittests/test_controller/test_worker_supervisor.py
• src/python/tests/unittests/test_lftp/test_job_status_parser.py
• src/python/tests/unittests/test_lftp/test_lftp.py
• src/python/tests/unittests/test_lftp/test_lftp_queue_command.py
• src/python/tests/unittests/test_lftp/test_lftp_spawn.py
• src/python/tests/unittests/test_model/test_file.py
• src/python/tests/unittests/test_seedsync.py
• src/python/tests/unittests/test_ssh/test_sshcp.py
• src/python/tests/unittests/test_ssh/test_sshcp_run_command.py
• src/python/tests/unittests/test_ssh/test_sshcp_shell.py
• src/python/tests/unittests/test_system/test_file.py
• src/python/tests/unittests/test_system/test_scanner.py
• src/python/tests/unittests/test_web/test_serialize/test_serialize_model.py
• src/python/tests/utils.py
• src/python/web/handler/auto_queue.py
• src/python/web/handler/config.py
• src/python/web/handler/controller.py
• src/python/web/handler/integrations.py
• src/python/web/handler/path_pairs.py
• src/python/web/serialize/serialize_model.py
• website/docs/configuration.md
• website/docs/faq.md
• website/docs/installation.md
• website/package.json

💤 Files with no reviewable changes (5)

• src/docker/build/docker-image/run_as_user
• src/docker/build/docker-image/scp
• MODERNIZATION_PLAN.md
• src/docker/build/docker-image/setup_default_config.sh
• src/docker/build/docker-image/ssh

👮 Files not reviewed due to content moderation or server errors (8)

• src/python/lftp/job_status_parser.py
• src/python/lftp/lftp.py
• src/python/tests/integration/test_web/test_handler/test_auto_queue.py
• src/python/tests/unittests/test_controller/test_pair_context.py
• src/python/tests/unittests/test_lftp/test_job_status_parser.py
• src/python/web/handler/auto_queue.py
• src/python/web/handler/integrations.py
• src/python/web/handler/path_pairs.py

[coderabbitai]

🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff

Type assertion to access private field is fragile.

Lines 168-169 use a type assertion to access the private changeDetector field for spying. This couples the test to implementation details and will break if the field is renamed or removed.

Consider testing the observable behavior (e.g., counting emissions on records) rather than spying on internal change detection calls, or exposing a test-only hook if batching validation is critical.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/angular/src/app/pages/logs/logs-page.component.spec.ts` around lines 165
- 190, The test relies on a fragile type-assertion to access the private
changeDetector field and spy on detectChanges; instead, verify batching via
public observable/state behavior: remove the (component as unknown as {
changeDetector: ChangeDetectorRef }).changeDetector spy and assert on
component.records emissions or subscribe to the records source (or the logs$
test subject) to count the number of change batches after advancing timers by
LogsPageComponent.BATCH_WINDOW_MS; if you must keep internal timing visibility,
add a test-only hook or public callback (e.g., a protected/test-only method
invoked by flushPendingRecords) that you can spy on, but do not access private
changeDetector directly.

[nitrobass24]

Intentional tradeoff (documented in the test) — leaving as-is.

This is the only assertion that proves the #539 batching invariant: a burst of 10 SSE records must trigger exactly one detectChanges() per BATCH_WINDOW_MS. flushPendingRecords() calls this.changeDetector.detectChanges() directly and the component exposes no CD-pass counter or observable, so asserting this via public state alone is impossible without adding production instrumentation purely for the test — strictly more invasive than the cast. The public half (records.length === 10) is already asserted alongside it. The as unknown as { ... } pattern is a codebase-wide convention (10+ specs), the comment on lines 166-167 documents the rationale, ng lint is clean, and all 567 unit tests pass.

[coderabbitai]

@nitrobass24, understood — the rationale is solid. Testing an exact detectChanges() call-count invariant requires reaching into the concrete CDR instance, and since the component has no public CD-pass counter or observable, the cast is the least-invasive option. The documented comment, the public-state assertion alongside it, and the codebase-wide precedent all make this a reasonable tradeoff.

---

✏️ Learnings added

Learnt from: nitrobass24
Repo: nitrobass24/seedsync PR: 570
File: src/angular/src/app/pages/logs/logs-page.component.spec.ts:165-190
Timestamp: 2026-06-10T19:58:20.180Z
Learning: In `src/angular/src/app/pages/logs/logs-page.component.spec.ts` (and other Angular spec files in this codebase), the `(component as unknown as { somePrivateField: T }).somePrivateField` pattern is an accepted convention for accessing private fields in tests when there is no public API or observable that surfaces the same invariant (e.g., spying on `changeDetector.detectChanges()` to assert batching call-counts). Do not flag this cast as a fragile/bad practice in this project — it is intentional, documented in the test comment, and lint-clean.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

---

🧠 Learnings used

Learnt from: nitrobass24
Repo: nitrobass24/seedsync PR: 363
File: src/angular/src/app/pages/stats/stats-page.component.spec.ts:87-91
Timestamp: 2026-04-15T05:13:23.760Z
Learning: In this codebase’s Angular spec files, it’s acceptable to use array indexing on the results of a class-based `querySelectorAll` (e.g., `el.querySelectorAll('.some-class')[0]`). Treat the “positional selectors” restriction as targeting CSS pseudo-selectors like `:nth-of-type` / `:nth-child`, not array indexing after `querySelectorAll` that first narrows by a stable CSS class. Therefore, do not flag `querySelectorAll('.some-class')[n]` as a positional selector violation. Also, don’t introduce `data-testid` attributes as a testing strategy in these specs because this project does not follow a `data-testid` convention.

---

Learnt from: nitrobass24
Repo: nitrobass24/seedsync PR: 363
File: src/angular/src/app/pages/stats/stats-page.component.ts:219-224
Timestamp: 2026-04-15T05:16:53.864Z
Learning: In this repository’s Angular code (under `src/angular/`), do not flag the byte-unit threshold literals `1024` (KB), `1048576` (MB), and `1073741824` (GB) as “magic numbers” when used inline for byte-size conversions (e.g., in components/pipes like `FileSizePipe`). These specific values are intentionally used as universally recognized boundaries in the codebase; only flag magic-number usage when the literal does not match these established byte-unit thresholds.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Preserve the dead worker's queued results before closing it.

recreate_if_dead() swaps _worker to the fresh process and _reap_dead() then closes the replaced worker's queues immediately. Any pop_completed / pop_failed items or queued exception payloads emitted just before the crash become unreachable at that point, so finished extract/validate work can be lost and the crash cause disappears. Drain or buffer the dead worker's queues through the supervisor before close_queues() runs.

Also applies to: 163-172

[nitrobass24]

Verified — leaving as-is for this release. The practical impact is a narrow, self-healing edge case, and the crash-cause concern is already handled:

1. Crash cause is not lost. propagate_exception() consumes and reports the worker fault at ERROR before recreate_if_dead() runs — see controller.process() order: pipeline.propagate_exceptions() (which calls propagate_exception() → report-dead → recreate_if_dead()) happens before updater.update(). So the exception is surfaced before close_queues() ever runs.
2. Result queues are drained every cycle by ModelUpdater.update(); only completions produced in the window between the last drain and the crash are at risk.
3. Such a lost completion is self-healing — the next scan cycle rebuilds model state from disk and re-queues the work, which is exactly the Recreate dead extract/validate worker processes (follow-up to #511) #535 recovery contract (in-flight work is redone, not guaranteed-preserved).

Draining/buffering the dead worker's queues through the supervisor before close_queues() is the "Heavy lift" the comment itself notes — a behavior change that doesn't belong on a release-sync branch. I can open a follow-up issue on develop if you'd like to also preserve last-moment results.

[coderabbitai]

@nitrobass24, that's a thorough and well-reasoned justification — the three-point argument holds up:

• Crash cause preserved: propagate_exceptions() → propagate_exception() runs before recreate_if_dead(), so the fault is surfaced at ERROR before any queue closing.
• Narrow at-risk window: only completions emitted between the last update() drain and the crash are reachable, which is a very small window in practice.
• Self-healing: the #535 recovery contract explicitly accepts that in-flight work is redone rather than guaranteed preserved, so a lost last-moment completion is a known, acceptable trade-off.

Deferring a behavior change on a release-sync branch is the right call. If you'd like, I can open a follow-up issue on develop to track the potential improvement (drain dead worker queues through the supervisor before close_queues() in recreate_if_dead()).

---

🧠 Learnings used

Learnt from: nitrobass24
Repo: nitrobass24/seedsync PR: 146
File: src/python/tests/unittests/test_controller/test_exclude_patterns.py:11-11
Timestamp: 2026-03-05T20:23:51.664Z
Learning: In this repository Ruff is not configured (no ruff.toml, .ruff.toml, or [tool.ruff] in pyproject.toml). Do not flag Ruff-specific rules (e.g., FBT003, ANN) for any Python files. Do not enforce Ruff rules unless explicitly configured. Also, the project uses positional boolean arguments for SystemFile constructor calls throughout; preserve this pattern unless there is a specific refactor. Apply this guidance to all Python files in the repo.

---

Learnt from: nitrobass24
Repo: nitrobass24/seedsync PR: 406
File: src/python/web/handler/logs.py:64-64
Timestamp: 2026-04-23T00:18:17.447Z
Learning: In this repository’s Python code, avoid extracting single-use code blocks into helper functions/methods that will only be used once. If you need to address a one-off complexity violation (e.g., cyclomatic complexity like "# noqa: C901"), prefer using an explicit noqa suppression over creating a one-time helper that doesn’t provide reuse or long-term maintainability benefits.

[nitrobass24]

Open a follow up issue on develop for future consideration

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

gh issue create \
  --repo nitrobass24/seedsync \
  --title "WorkerSupervisor: drain dead worker queues before close_queues() in recreate_if_dead()" \
  --assignee nitrobass24 \
  --body "## Summary

When \`WorkerSupervisor.recreate_if_dead()\` replaces a dead worker, it calls \`_reap_dead()\` which immediately closes the dead worker's queues. Any \`pop_completed\` / \`pop_failed\` items or queued exception payloads emitted by the worker just before it crashed become unreachable at that point, so finished extract/validate work can be silently dropped.

## Context

Deferred from PR `#570` (release v1.0.0) — see [review comment](https://github.com/nitrobass24/seedsync/pull/570#discussion_r3390591086).

The impact is narrow and self-healing for the current release:

1. **Crash cause is not lost** — \`propagate_exceptions()\` calls \`propagate_exception()\` and reports the fault at ERROR *before* \`recreate_if_dead()\` runs, so the exception is surfaced before any queue closing.
2. **Narrow at-risk window** — only completions emitted between the last \`ModelUpdater.update()\` drain cycle and the crash are at risk.
3. **Self-healing** — the `#535` recovery contract accepts that in-flight work is redone rather than guaranteed preserved; a lost last-moment completion triggers a rescan and re-queue on the next cycle.

## Proposed improvement

Before calling \`_reap_dead(dead)\` / \`close_queues()\` in \`recreate_if_dead()\`, drain the dead worker's result queues (e.g. \`pop_completed\`, \`pop_failed\`, \`pop_latest_statuses\`) and buffer/surface the results so no finished work is silently dropped.

**File:** \`src/python/controller/worker_supervisor.py\`, \`recreate_if_dead()\` method (lines ~120–148).

## Requested by

`@nitrobass24` (follow-up from PR `#570` review)
"

Length of output: 1880

---

---

@nitrobass24, done! Follow-up issue opened for tracking on develop: #571 — WorkerSupervisor: drain dead worker queues before close_queues() in recreate_if_dead()

It captures the full context (the three-point deferral rationale, the affected file/method, and the proposed improvement) so nothing is lost for a future cycle. 🐇

---

🧠 Learnings used

Learnt from: nitrobass24
Repo: nitrobass24/seedsync PR: 146
File: src/python/tests/unittests/test_controller/test_exclude_patterns.py:11-11
Timestamp: 2026-03-05T20:23:51.664Z
Learning: In this repository Ruff is not configured (no ruff.toml, .ruff.toml, or [tool.ruff] in pyproject.toml). Do not flag Ruff-specific rules (e.g., FBT003, ANN) for any Python files. Do not enforce Ruff rules unless explicitly configured. Also, the project uses positional boolean arguments for SystemFile constructor calls throughout; preserve this pattern unless there is a specific refactor. Apply this guidance to all Python files in the repo.

---

Learnt from: nitrobass24
Repo: nitrobass24/seedsync PR: 406
File: src/python/web/handler/logs.py:64-64
Timestamp: 2026-04-23T00:18:17.447Z
Learning: In this repository’s Python code, avoid extracting single-use code blocks into helper functions/methods that will only be used once. If you need to address a one-off complexity violation (e.g., cyclomatic complexity like "# noqa: C901"), prefer using an explicit noqa suppression over creating a one-time helper that doesn’t provide reuse or long-term maintainability benefits.

---

Learnt from: nitrobass24
Repo: nitrobass24/seedsync PR: 570
File: src/python/seedsync.py:408-408
Timestamp: 2026-06-10T19:58:18.251Z
Learning: In this repo’s Python code, treat `ftp_ssl_verify_certificate = False` (used for FTPS certificate verification) as an explicitly documented and intentional product default, not as a security vulnerability. This default exists to support common self-signed seedbox certificates and is paired with `ftp:ssl-force` so the data channel remains TLS-encrypted; only server certificate verification is opt-in. When reviewing changes, do not flag this as a security issue if it remains the documented default and the related rationale/tests (e.g., assertions in `test_seedsync.py` / `test_config.py`) and the changelog context are preserved. If the behavior is changed (e.g., enabling verification by default or removing the documented rationale), then it should be reviewed/flagged accordingly.

---

Learnt from: nitrobass24
Repo: nitrobass24/seedsync PR: 570
File: src/angular/src/app/models/config.ts:47-49
Timestamp: 2026-06-10T19:58:15.285Z
Learning: In the seedsync repo, the setting `ftp_ssl_verify_certificate` is intentionally defaulted to `false` (TLS encryption is used, but certificate authentication is opt-in to avoid first-connect failures with self-signed/mismatched certs). During future code reviews, do NOT flag `ftp_ssl_verify_certificate`’s default of `false` as an insecure configuration as long as the implementation documents/remediates this trade-off (e.g., via the referenced changelog) and/or emits a runtime `WARNING` when verification is disabled. Only flag if verification is disabled without the corresponding warning/documentation/handling.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't pin directory-symlink traversal as supported scanner behavior.

These tests make SystemScanner recurse through directory symlinks, including ones that escape path_to_scan. That turns a writable entry inside the managed tree into reachability to arbitrary host paths, and the cycle case shows the same behavior can also take scanning down with raw OS errors. The safer contract is to skip directory symlinks entirely or require their resolved path to stay under the scan root, then update these assertions to match.

Also applies to: 678-687

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/python/tests/unittests/test_system/test_scanner.py` around lines 655 -
675, The test test_symlink_to_outside_directory_is_traversed assumes
SystemScanner follows directory symlinks and recurses outside the scan root;
change the scanner behavior (SystemScanner.scan and any DirEntry handling) to
either skip directory symlinks entirely or resolve symlinks and reject targets
outside the configured scan root, then update this test (and the similar case
around 678-687) to assert the new safe behavior: that "link_to_outside" is
present as a symlink entry but not traversed (is_dir False or children empty) or
that it is omitted if out-of-root, and ensure TestSystemScanner.temp_dir-related
assertions reflect the updated contract.

[nitrobass24]

This reads a behavior-pinning regression test as a behavior to harden — leaving the test (and scanner) as-is here.

The test docstring states it "Pins current behavior ... guards against a silent change in traversal behavior." #530 documented the existing scanner behavior; it didn't endorse hardening it. Changing SystemScanner to skip or reject out-of-root directory symlinks is a real behavior change that could break legitimate symlinked seedbox layouts, and it needs its own design + PR — not a test edit folded into a release sync. Note that out-of-root realpath guards are deliberately applied where they're a genuine risk (delete_process.py, extract.py), but intentionally not in the read-only scanner. Happy to open a separate issue on develop if you want to revisit scanner symlink policy.

[coderabbitai]

@nitrobass24, understood — I misread the intent. The test docstring is clear that it's pinning existing traversal behavior, and the reasoning is sound: the read-only scanner intentionally follows symlinks to support legitimate symlinked seedbox layouts, while the actual risk surfaces (delete, extract) carry their own realpath guards. Leaving the scanner and the test as-is is the right call here.

Would you like me to open a GitHub issue on develop to track a future design discussion on scanner symlink policy? That way the trade-off is documented for later consideration without blocking this release.

---

✏️ Learnings added

Learnt from: nitrobass24
Repo: nitrobass24/seedsync PR: 570
File: src/python/tests/unittests/test_system/test_scanner.py:655-675
Timestamp: 2026-06-10T19:58:26.854Z
Learning: In the seedsync repository (`src/python/system/`), `SystemScanner` intentionally follows directory symlinks (including those pointing outside the scan root) to support legitimate symlinked seedbox layouts. This is a deliberate design decision: out-of-root `realpath` guards are applied only on write/destructive paths (`delete_process.py`, `extract.py`), not on the read-only scanner. The test `test_symlink_to_outside_directory_is_traversed` in `src/python/tests/unittests/test_system/test_scanner.py` is a behavior-pinning regression test, not a security hardening target. Do not flag scanner symlink traversal as a security issue or suggest restricting it without a dedicated design discussion.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

---

🧠 Learnings used

Learnt from: nitrobass24
Repo: nitrobass24/seedsync PR: 146
File: src/python/tests/unittests/test_controller/test_exclude_patterns.py:11-11
Timestamp: 2026-03-05T20:23:51.664Z
Learning: In this repository Ruff is not configured (no ruff.toml, .ruff.toml, or [tool.ruff] in pyproject.toml). Do not flag Ruff-specific rules (e.g., FBT003, ANN) for any Python files. Do not enforce Ruff rules unless explicitly configured. Also, the project uses positional boolean arguments for SystemFile constructor calls throughout; preserve this pattern unless there is a specific refactor. Apply this guidance to all Python files in the repo.

---

Learnt from: nitrobass24
Repo: nitrobass24/seedsync PR: 232
File: src/python/tests/unittests/test_controller/test_extract/test_extract_process.py:93-93
Timestamp: 2026-03-11T00:57:47.504Z
Learning: In tests that use multiprocessing.Queue, do not flag time.sleep() after put() as unnecessary. multiprocessing.Queue.put() is asynchronous (it enqueues into an internal buffer and a background feeder thread transfers to the pipe), so a get() immediately after put() may not find the item. If a sleep is used deliberately to flush the feeder thread, that is valid. This guidance applies to test files using multiprocessing.Queue, such as src/python/tests/unittests/test_controller/test_extract/test_extract_process.py and similar tests.

---

Learnt from: nitrobass24
Repo: nitrobass24/seedsync PR: 232
File: src/python/tests/unittests/test_controller/test_extract/test_extract_process.py:146-151
Timestamp: 2026-03-11T00:58:15.968Z
Learning: In tests using multiprocessing.Queue in Python, be aware that Queue.put() is not fully synchronous end-to-end. It places items into a process-local buffer, and a background feeder thread asynchronously transfers them to the underlying pipe. Therefore, include a short time.sleep() after put() calls (before get()) in tests that use multiprocessing.Queue, even if producer and consumer run in the same process. Do not flag these sleeps as unnecessary; they are required to give the feeder thread time to flush the buffer to the pipe. Apply this guidance to test files in similar contexts within the repository (e.g., test_extract_process.py) and other analogous tests using multiprocessing.Queue.

---

Learnt from: nitrobass24
Repo: nitrobass24/seedsync PR: 406
File: src/python/web/handler/logs.py:64-64
Timestamp: 2026-04-23T00:18:17.447Z
Learning: In this repository’s Python code, avoid extracting single-use code blocks into helper functions/methods that will only be used once. If you need to address a one-off complexity violation (e.g., cyclomatic complexity like "# noqa: C901"), prefer using an explicit noqa suppression over creating a one-time helper that doesn’t provide reuse or long-term maintainability benefits.

[nitrobass24]

Open a follow up issue on develop for future consideration

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

gh issue create \
  --repo nitrobass24/seedsync \
  --title "Evaluate scanner symlink policy: skip or bound out-of-root directory symlinks" \
  --body "## Background

During the review of PR `#570` (release v1.0.0), a behavior-pinning test \`test_symlink_to_outside_directory_is_traversed\` in \`src/python/tests/unittests/test_system/test_scanner.py\` was flagged and then clarified by `@nitrobass24`.

\`SystemScanner\` currently follows directory symlinks by default (\`DirEntry.is_dir()\` uses \`follow_symlinks=True\`), including symlinks that point outside the configured scan root. This is **intentional**: it supports legitimate symlinked seedbox layouts and the scanner is read-only, so the risk profile differs from write/destructive paths.

Out-of-root \`realpath\` guards are already applied on destructive surfaces (\`delete_process.py\`, \`extract.py\`), but deliberately not in the read-only scanner.

## Discussion points

- Should \`SystemScanner\` be given an option to restrict traversal to targets whose resolved path stays within the scan root?
- Should out-of-root symlinks be skipped silently, skipped with a log warning, or remain fully traversed (current behavior)?
- Are there seedbox layouts in the wild that legitimately symlink to directories outside the configured local path?
- The cyclic-symlink case (\`test_cyclic_symlink_terminates_with_error\`) shows the OS raises \`ELOOP\`; document whether that is sufficient protection or whether an explicit guard is wanted.

## References

- PR `#570` review comment: https://github.com/nitrobass24/seedsync/pull/570#discussion_r3390591118
- Test file: \`src/python/tests/unittests/test_system/test_scanner.py\` (lines 654–687)

/cc `@nitrobass24`"

Length of output: 8023

---

---

@nitrobass24, the follow-up issue has been opened: #572 — Evaluate scanner symlink policy: skip or bound out-of-root directory symlinks. It captures the background, the current intentional behavior, and the open discussion points for whenever you want to revisit scanner symlink policy on develop.

---

🧬 Code Graph Analysis Results

Relevant snippets

src/python/scan_fs.py

• Lines 44-57 (SystemFile property accessors):

def name(self) -> str:
        return self.__name

def size(self) -> int:
        return self.__size

def is_dir(self) -> bool:
        return self.__is_dir

def children(self) -> "List[SystemFile]":
        return self.__children

• Lines 92-103 (exclude prefixes/suffixes and directory scan entry point):

def add_exclude_prefix(self, prefix: str):
        self.exclude_prefixes.append(prefix)

def add_exclude_suffix(self, suffix: str):
        self.exclude_suffixes.append(suffix)

def scan(self) -> "List[SystemFile]":
        if not os.path.exists(self.path_to_scan):
            raise SystemScannerError("Path does not exist: {}".format(self.path_to_scan))
        if not os.path.isdir(self.path_to_scan):
            raise SystemScannerError("Path is not a directory: {}".format(self.path_to_scan))
        return self.__create_children(self.path_to_scan)

• Lines 157-183 (parse lftp status content into real file size):

def _lftp_status_file_size(status: str) -> int:
        size_pattern_m = re.compile(r"^size=(\d+)$")
        pos_pattern_m = re.compile(r"^\d+\.pos=(\d+)$")
        limit_pattern_m = re.compile(r"^\d+\.limit=(\d+)$")
        lines = [s.strip() for s in status.splitlines()]
        lines = list(filter(None, lines))
        if not lines:
            return 0
        empty_size = 0
        result = size_pattern_m.search(lines[0])
        if not result:
            return 0
        total_size = int(result.group(1))
        lines.pop(0)
        while lines:
            if len(lines) < 2:
                return 0
            result_pos = pos_pattern_m.search(lines[0])
            result_limit = limit_pattern_m.search(lines[1])
            if not result_pos or not result_limit:
                return 0
            pos = int(result_pos.group(1))
            limit = int(result_limit.group(1))
            empty_size += limit - pos
            lines.pop(0)
            lines.pop(0)
        return total_size - empty_size

src/python/system/file.py

• Lines 54-55 (timestamp_modified getter used by tests):

def timestamp_modified(self) -> datetime | None:
        return self.__timestamp_modified

src/python/model/file.py

• Lines 88-101 (basic model file getters used throughout code/tests):

def name(self) -> str:
        return self.__name

def is_dir(self) -> bool:
        return self.__is_dir

src/python/system/scanner.py

• Lines 52-58 (scan() with error handling):

def scan(self) -> list[SystemFile]:
        """
        Scan the path to generate list of system files
        :return:
        """
        if not os.path.exists(self.path_to_scan):
            raise SystemScannerError(f"Path does not exist: {self.path_to_scan}")
        if not os.path.isdir(self.path_to_scan):
            raise SystemScannerError(f"Path is not a directory: {self.path_to_scan}")
        return self.__create_children(self.path_to_scan)

• Lines 88-108 (scan_single() for a single file/dir, including optional lftp temp-file behavior):

def scan_single(self, name: str) -> SystemFile:
        """
        Scan a single file/dir
        :param name:
        :return:
        """
        path = os.path.join(self.path_to_scan, name)
        temp_path = (path + self.__lftp_temp_file_suffix) if self.__lftp_temp_file_suffix else None

        if os.path.exists(path):
            # We're good to go
            pass
        elif temp_path and os.path.isfile(temp_path):
            # There's a temp file, use that
            path = temp_path
        else:
            raise SystemScannerError(f"Path does not exist: {path}")

        return self.__create_system_file(
            PseudoDirEntry(name=name, path=path, is_dir=os.path.isdir(path), stat=os.stat(path))
        )

• Lines 68-75 (set_lftp_temp_suffix() used by tests):

def set_lftp_temp_suffix(self, suffix: str):
        """
        Set the suffix used by LFTP temp files
        Scanner will ignore the suffix and show these files with their
        original name
        :return:
        """
        self.__lftp_temp_file_suffix = suffix

• Lines 52-58 / 77-86 (lftp status size parsing—implementation mirrors tests’ expectations):

def _lftp_status_file_size(status: str) -> int:
        """
        Returns the real file size as indicated by an lftp status content
        :param status:
        :return:
        """
        size_pattern_m = re.compile(r"^size=(\d+)$")
        pos_pattern_m = re.compile(r"^\d+\.pos=(\d+)$")
        limit_pattern_m = re.compile(r"^\d+\.limit=(\d+)$")
        lines = [s.strip() for s in status.splitlines()]
        lines = list(filter(None, lines))  # remove blank lines
        if not lines:
            return 0

        empty_size = 0
        # First line should be a size
        result = size_pattern_m.search(lines[0])
        if not result:
            return 0
        total_size = int(result.group(1))
        lines.pop(0)
        while lines:
            # There should be pairs of lines
            if len(lines) < 2:
                return 0
            result_pos = pos_pattern_m.search(lines[0])
            result_limit = limit_pattern_m.search(lines[1])
            if not result_pos or not result_limit:
                return 0
            pos = int(result_pos.group(1))
            limit = int(result_limit.group(1))
            empty_size += limit - pos
            lines.pop(0)
            lines.pop(0)

        return total_size - empty_size

• Lines 60-66 and Lines 52-58 (exclude suffix/prefix APIs and scan method are referenced directly by tests):

def add_exclude_prefix(self, prefix: str):
        """
        Exclude files that begin with the given prefix
        :param prefix:
        :return:
        """
        self.exclude_prefixes.append(prefix)

def add_exclude_suffix(self, suffix: str):
        """
        Exclude files that end with the given suffix
        :param suffix:
        :return:
        """
        self.exclude_suffixes.append(suffix)

---

🧠 Learnings used

Learnt from: nitrobass24
Repo: nitrobass24/seedsync PR: 146
File: src/python/tests/unittests/test_controller/test_exclude_patterns.py:11-11
Timestamp: 2026-03-05T20:23:51.664Z
Learning: In this repository Ruff is not configured (no ruff.toml, .ruff.toml, or [tool.ruff] in pyproject.toml). Do not flag Ruff-specific rules (e.g., FBT003, ANN) for any Python files. Do not enforce Ruff rules unless explicitly configured. Also, the project uses positional boolean arguments for SystemFile constructor calls throughout; preserve this pattern unless there is a specific refactor. Apply this guidance to all Python files in the repo.

---

Learnt from: nitrobass24
Repo: nitrobass24/seedsync PR: 232
File: src/python/tests/unittests/test_controller/test_extract/test_extract_process.py:93-93
Timestamp: 2026-03-11T00:57:47.504Z
Learning: In tests that use multiprocessing.Queue, do not flag time.sleep() after put() as unnecessary. multiprocessing.Queue.put() is asynchronous (it enqueues into an internal buffer and a background feeder thread transfers to the pipe), so a get() immediately after put() may not find the item. If a sleep is used deliberately to flush the feeder thread, that is valid. This guidance applies to test files using multiprocessing.Queue, such as src/python/tests/unittests/test_controller/test_extract/test_extract_process.py and similar tests.

---

Learnt from: nitrobass24
Repo: nitrobass24/seedsync PR: 232
File: src/python/tests/unittests/test_controller/test_extract/test_extract_process.py:146-151
Timestamp: 2026-03-11T00:58:15.968Z
Learning: In tests using multiprocessing.Queue in Python, be aware that Queue.put() is not fully synchronous end-to-end. It places items into a process-local buffer, and a background feeder thread asynchronously transfers them to the underlying pipe. Therefore, include a short time.sleep() after put() calls (before get()) in tests that use multiprocessing.Queue, even if producer and consumer run in the same process. Do not flag these sleeps as unnecessary; they are required to give the feeder thread time to flush the buffer to the pipe. Apply this guidance to test files in similar contexts within the repository (e.g., test_extract_process.py) and other analogous tests using multiprocessing.Queue.

---

Learnt from: nitrobass24
Repo: nitrobass24/seedsync PR: 406
File: src/python/web/handler/logs.py:64-64
Timestamp: 2026-04-23T00:18:17.447Z
Learning: In this repository’s Python code, avoid extracting single-use code blocks into helper functions/methods that will only be used once. If you need to address a one-off complexity violation (e.g., cyclomatic complexity like "# noqa: C901"), prefer using an explicit noqa suppression over creating a one-time helper that doesn’t provide reuse or long-term maintainability benefits.

---

Learnt from: nitrobass24
Repo: nitrobass24/seedsync PR: 570
File: src/python/seedsync.py:408-408
Timestamp: 2026-06-10T19:58:18.251Z
Learning: In this repo’s Python code, treat `ftp_ssl_verify_certificate = False` (used for FTPS certificate verification) as an explicitly documented and intentional product default, not as a security vulnerability. This default exists to support common self-signed seedbox certificates and is paired with `ftp:ssl-force` so the data channel remains TLS-encrypted; only server certificate verification is opt-in. When reviewing changes, do not flag this as a security issue if it remains the documented default and the related rationale/tests (e.g., assertions in `test_seedsync.py` / `test_config.py`) and the changelog context are preserved. If the behavior is changed (e.g., enabling verification by default or removing the documented rationale), then it should be reviewed/flagged accordingly.

---

Learnt from: nitrobass24
Repo: nitrobass24/seedsync PR: 570
File: src/angular/src/app/models/config.ts:47-49
Timestamp: 2026-06-10T19:58:15.285Z
Learning: In the seedsync repo, the setting `ftp_ssl_verify_certificate` is intentionally defaulted to `false` (TLS encryption is used, but certificate authentication is opt-in to avoid first-connect failures with self-signed/mismatched certs). During future code reviews, do NOT flag `ftp_ssl_verify_certificate`’s default of `false` as an insecure configuration as long as the implementation documents/remediates this trade-off (e.g., via the referenced changelog) and/or emits a runtime `WARNING` when verification is disabled. Only flag if verification is disabled without the corresponding warning/documentation/handling.

---

Learnt from: nitrobass24
Repo: nitrobass24/seedsync PR: 570
File: src/python/tests/unittests/test_controller/test_model_builder.py:983-1036
Timestamp: 2026-06-10T19:58:20.783Z
Learning: In nitrobass24/seedsync, treat test-file length (e.g., the ~500-line target mentioned in CLAUDE.md) as a soft guideline rather than a hard violation: do not flag long test files solely for size when there is no CI/mechanical enforcement for file length (and note that ruff C901 is excluded for test files via pyproject.toml per-file-ignores). Also, on release-sync branches (develop→master), avoid recommending refactors that would cause extra churn (e.g., splitting test modules) unless they are directly related to the release; defer larger structural refactors to a dedicated refactor on develop.

[nitrobass24]

Review-findings triage (verified against current code)

All 11 inline findings were verified against the actual code on this branch. 10 were skipped (not valid here) and 1 was fixed (686a1e3).

Fixed

• view-file-command.service.ts — misleading log (F6): the shared createAction helper logged "File to queue not found" for all six commands (queue/stop/extract/delete-local/delete-remote/validate). Changed to action-agnostic "File not found", matching the already action-agnostic user-facing errorMessage and the sibling "Action failed for file" log. Developer-facing log only; no behavior change. ng lint clean, 567/567 unit tests pass.

Skipped — invalid (contradicts intentional, documented/tested design)

• FTPS cert default false→true (config.ts + seedsync.py): off-by-default is the documented design (CHANGELOG FTP instead of SFTP #485 + per-connection WARNING log in lftp.py) for self-signed seedbox certs. The data channel is always TLS-encrypted (lftp.py:203-204); only cert authentication is opt-in. config.ts only mirrors the backend, and unit tests pin False (test_seedsync.py:137, test_config.py:1064). Flipping it breaks FTPS for the target users and fails CI.
• controller_persist.py torn-snapshot race: to_str() snapshots via list(set), atomic under the GIL — the claimed "Set changed size during iteration" RuntimeError cannot occur. Deliberately contrasted with AutoQueuePersist (AutoQueue / AutoQueuePersist thread-safety: cross-thread set & list races #509, list-backed). A 2000-iteration concurrent-mutation test (test_controller_persist.py:244, Tests: cover security-critical paths (extraction zip-slip, ssh quoting, persist race, config special chars, scanner symlinks, webhook scheme, integrations persist-failure) #530) pins this contract.
• test_scanner.py symlink traversal: the test explicitly pins current behavior ("guards against a silent change in traversal"); it documents, not hardens. Changing scanner traversal is a behavior change needing its own design/PR.
• Pin actions/checkout to SHA: contradicts the documented Repo & CI hygiene: .gitignore, action pinning, job timeouts, dead scripts #527 policy — third-party actions are SHA-pinned, official actions/* left on major tags. Pinning only checkout is self-inconsistent; nothing enforces it.

Skipped — linter not in this repo

• SCSS blank line: no stylelint is configured or run anywhere.
• Markdown code-fence language: no markdownlint gate (only advisory, non-blocking CodeRabbit); renders fine in Docusaurus; bare fences are a repo-wide style.

Skipped — real but not minimal / wrong place for a release sync

• CI persist-credentials: false: default is true, but no step pushes via the persisted git credential (docker-login + gh-release use the token explicitly). Pure defense-in-depth; belongs on a dedicated CI-hardening branch off develop.
• logs spec private CD spy: an intentional, code-commented tradeoff — the only way to assert the Logs page: CDK virtual scroll + batched change detection (follow-up to #522) #539 "one change-detection pass per batch window" invariant (no public counter exists). 567 tests pass.
• Split test_model_builder.py: code-health nicety, not a bug; size target is a soft guideline and test files are excluded from the complexity gate. Splitting is the opposite of minimal.

🤖 Generated with Claude Code