Concurrency-auditor HIGH: the cycle walk + UPDATE used to run as
separate statements. Two concurrent moves (A→B and B→A) could each
pass the walk against the pre-move tree and both write, leaving an
A↔B cycle. Whole sequence now runs inside one db.transaction().
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three follow-ups from Task 6 code review:
1. applyEntityArchivedSuffix short-circuits when the folder is already
archived — prevents archivedAt drift on backfill replay.
2. applyEntityRestoredSuffix short-circuits when the folder was never
archived — matches the docstring's "no-op" claim.
3. Inline comment on archiveClient's fire-and-forget hook documents
why Task 6 uses void (archive UI doesn't depend on folder sync)
while Task 5 uses await (rename should be visible to the next
read).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
applyEntityArchivedSuffix stamps " (archived)" + archived_at on the
entity subfolder so the UI mutes it and auto-deposit halts. Restore
is the inverse. demoteSystemFolderOnEntityDelete flips
system_managed=false, appends " (deleted)", and clears the entity FK
so the partial unique index releases the slot — orphaned files
retain their entity FK snapshots and surface in the rep's clean-up
view.
All three helpers are best-effort from the entity-side hooks; folder
errors are logged at warn level but do not fail the entity-update
operation. UPDATE WHERE clauses include port_id (defense-in-depth).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two follow-ups from code review:
1. The UPDATE in the retry loop now scopes by both id and port_id so
it matches every other mutation in document-folders.service.ts and
honours the CLAUDE.md defense-in-depth pattern.
2. The three entity-rename hooks now log at warn level (not error) —
a missed folder rename is best-effort cosmetic drift, not a paging
incident. Matches the existing convention used elsewhere in the
codebase for non-fatal background work.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per-entity subfolder names mirror the entity's current display string.
Wired into updateClient / updateCompany / updateYacht; runs only when
the name field changes. Best-effort (logged + swallowed) so a folder-
sync error never fails an entity update. Preserves the (archived)
suffix when present; skips entirely when the folder has been demoted
to (deleted) — the rep owns the name at that point.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
assertNotSystemManaged centralises the guard so the three mutation
paths surface identical ConflictError shapes. System roots and per-
entity subfolders are immutable through the rep-facing API; the only
way for system_managed to flip back to false is the entity-hard-
delete demotion path (next task).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Idempotent per-entity subfolder creation under the matching system
root. Fast-path SELECT short-circuits the common case. Inserts race
safely via uniq_document_folders_entity (partial unique on
port_id+entity_type+entity_id) — the loser re-SELECTs the winner's
row. Sibling-name collisions between two entities with the same
display name append (2), (3), … to the new folder; existing folders
never rename. Exports EntityType for use by downstream tasks.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds inline comments explaining (a) why no-target onConflictDoNothing
is safe for root inserts (the only unique index that can fire on a
root row is uniq_document_folders_sibling_name; the partial entity
index excludes entity_id=NULL rows) and (b) why createPort doesn't
wrap the root bootstrap in a transaction (ensureSystemRoots is re-
runnable; the backfill script heals orphaned ports). Surfaces the
assumption that Task 3 (ensureEntityFolder) must not blindly copy
this pattern — it inserts with entity_id NOT NULL and needs an
explicit conflict target.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds idempotent root-folder bootstrap (Clients/Companies/Yachts)
called on every port-init. ON CONFLICT DO NOTHING on the sibling-name
unique index prevents racing inserts; the re-SELECT returns the stable
row set in SYSTEM_ROOT_NAMES order. Same helper is invoked by the
backfill script in a later task.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- renameFolder/moveFolder UPDATE and deleteFolderSoftRescue DELETE now
carry an explicit port_id predicate so the write is bounded to the
same tenancy the pre-fetch verified, defending against future
refactors that drop or reorder the ownership check.
- FolderRow's collapsed-children chevron is `invisible` for layout
purposes, but it was still in the tab order with a misleading
Expand/Collapse aria-label. Add aria-hidden + tabIndex=-1 when no
children so keyboard users skip it.
Surfaced by post-implementation review (subagent code-review pass).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
listDocuments accepts folderId (string | null | undefined) and
includeDescendants. folderId=null returns only docs at root;
includeDescendants=true expands the subtree via collectDescendantIds
(in-memory walk over the cached tree -- folder trees are small).
PATCH /api/v1/documents/[id]/folder moves a single document under
documents.manage_folders, with audit-log metadata { type: 'folder_move' }.
Bumping updatedAt is correct for per-doc moves because reps deliberately
acted on that document -- different semantics from the bulk soft-rescue
in Task 4.
createDocument accepts an optional folderId for the upcoming UI's
"create in current folder" affordance.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code-review followups on e9251a3:
- Move createAuditLog OUT of the deleteFolderSoftRescue transaction
callback so a rolled-back transaction can't leave a phantom audit
row. Pattern matches clients.service.ts, expense-dedup.service.ts.
- Add portId filter to the moveFolder ancestor-walk findFirst —
defense-in-depth so corrupted parentId pointing at another port
short-circuits the walk instead of silently traversing it.
- Drop updatedAt bump on rescued documents — folder rescue is an
administrative storage op, not a content change; bumping made
every rescued doc appear "recently modified" in list views.
- Add userId param + audit-log emission on renameFolder and
moveFolder for parity with createFolder + deleteFolderSoftRescue.
Tests updated to pass TEST_USER_ID as the new 4th arg.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
renameFolder + moveFolder enforce sibling-name uniqueness via the
shared isSiblingNameConflict helper and reject cross-port leakage at
the service boundary. moveFolder walks the destination's ancestor
chain to refuse cycles before the write.
deleteFolderSoftRescue re-parents every child folder and document up
to the deleted folder's parent (or to root) inside a transaction,
then drops the folder row. Children never disappear silently — a
wrong click moves work up the tree, never deletes it. Audit-logged
with rescuedTo metadata.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code-review followups on 4b31f01:
- beforeEach now scopes the documentFolders cleanup to the test port
via .where(eq(documentFolders.portId, portId)) so parallel suites
don't wipe each other's fixtures.
- Cross-port parent guard message changed from "Parent folder not
found in this port" (read like a 404) to "Invalid parent folder"
to match the ValidationError type that already maps to 400.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
In-memory tree build (single SELECT + JS nesting); the folder tree is
small enough that a recursive CTE buys nothing. Sibling-name conflict
maps the Postgres unique-index 23505 to a typed ConflictError so the
UI can render a clean toast. Cross-port parentId rejected at the
service boundary. Also adds document_folders to the global teardown
CTE so test ports can be cleaned up without FK violations.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
