feat: add Folder Autopilot module slice - #51
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (10)
📝 WalkthroughWalkthroughFolder Autopilot adds versioned contracts, deterministic engine planning, guarded Desktop execution, tenant-scoped API persistence, Web and Android workflows, and cross-runtime validation. ChangesFolder Autopilot
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (20)
apps/desktop/src/features/folder-autopilot/file-observation.ts-189-198 (1)
189-198: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftWhole-file buffering conflicts with the 10 GiB size bound.
readBytesmust return the complete file as oneUint8Array, andfingerprintByteshashes it in a singleupdatecall.MAX_FILE_BYTESon Line 3 accepts files up to 10 GiB, sovalidateStatadmits sizes that this read path cannot hold. A singleBuffer/Uint8Arrayon 64-bit Node is capped near 4 GiB, and the desktop process exhausts memory well before that.Select one of the following:
- Hash through a stream and pass incremental chunks to
createHash().update(), then compare the total byte count againstfirst.sizeBytes.- Lower
MAX_FILE_BYTESto a value the buffered read can support.🤖 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 `@apps/desktop/src/features/folder-autopilot/file-observation.ts` around lines 189 - 198, Resolve the mismatch between the 10 GiB MAX_FILE_BYTES limit and the whole-file buffering in the observation flow. Prefer updating fingerprintBytes and its callers to hash streamed chunks incrementally with createHash().update(), track the total bytes read, and compare that count with first.sizeBytes; otherwise lower MAX_FILE_BYTES to a safely supported buffered-read limit.apps/desktop/src/features/folder-autopilot/local-journal.ts-200-206 (1)
200-206: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
recoverJournalnever derives the terminalCOMMITTEDstate, and no test covers the fully recovered case.recordJournalStepderives the terminal state on Lines 162-164, butrecoverJournalreturnsCOMMITTINGregardless of the checkpoints it applies. After a crash in which every step committed on disk, recovery promotes every step toCOMMITTEDwhile the journal staysCOMMITTING.recordJournalStepthen rejects each step withDUPLICATE_STEP,failJournalonly moves toCOMPENSATING, andbuildUndoPlanrejects withUNDO_NOT_AVAILABLE. The execution has no terminal outcome and undo is permanently unavailable.
apps/desktop/src/features/folder-autopilot/local-journal.ts#L200-L206: derivenextStatefromnextStepsexactly asrecordJournalStepdoes, and pass it tocloneJournal.apps/desktop/test/folder-autopilot-journal.test.ts#L89-L104: add a case that checkpoints bothrename-1andmove-1asCOMMITTEDand asserts the recovered state isCOMMITTED.🤖 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 `@apps/desktop/src/features/folder-autopilot/local-journal.ts` around lines 200 - 206, Update recoverJournal to derive nextState from nextSteps using the same terminal-state logic as recordJournalStep, then pass that state to cloneJournal so fully committed recovery returns COMMITTED. In apps/desktop/test/folder-autopilot-journal.test.ts lines 89-104, add coverage with both rename-1 and move-1 checkpoints marked COMMITTED and assert the recovered journal state is COMMITTED.apps/desktop/src/features/folder-autopilot/local-journal.ts-219-231 (1)
219-231: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftUndo of a
COPYstep emits aRENAMEthat cannot succeed.
buildUndoPlanmaps every step toaction: 'RENAME'withsourcePath: step.destinationPathanddestinationPath: step.sourcePath. That inverse is correct forRENAMEandMOVE. It is wrong forCOPY, because the original file atstep.sourcePathstill exists.
executeLocalPlanthen finds the destination present and rejects withDESTINATION_COLLISION. No data is lost, but undo is unavailable for any journal that contains an undoableCOPYstep, and the reported reason does not explain why. TheUndoOperationtype only permits'RENAME', so a copy cannot be undone by deletion either.Select one of the following:
- Reject
undoable: trueforaction: 'COPY'invalidateStep.- Add a delete action to
UndoOperationand emit it forCOPYsteps.🤖 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 `@apps/desktop/src/features/folder-autopilot/local-journal.ts` around lines 219 - 231, Update the undo flow so COPY steps cannot produce an invalid RENAME inverse: either have validateStep reject COPY actions marked undoable, or extend UndoOperation with a delete action and have buildUndoPlan emit deletion for COPY steps. Preserve the existing RENAME inverse for RENAME and MOVE steps, and ensure the chosen behavior reports the appropriate rejection or removes only the copied destination.apps/desktop/src/features/folder-autopilot/local-actions.ts-133-145 (1)
133-145: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winBoth runtime validators accept a missing identifier, because
RegExp.prototype.testcoerces its argument. Each validator appliesSAFE_ID.test(...)to a value it never type-checks.undefinedbecomes the string"undefined", which matches^[A-Za-z0-9][A-Za-z0-9_-]{0,127}$, so an operation or step with no identifier passes. A number such as5also passes. The shared fix is an explicittypeofcheck before each regex test.
apps/desktop/src/features/folder-autopilot/local-actions.ts#L133-L145: addtypeof operation.operationId !== 'string'andtypeof operation.sourceFingerprint !== 'string'to the guard, so a plan reachingexecuteLocalPlanasunknowncannot produce a receipt withoperationId: undefined.apps/desktop/src/features/folder-autopilot/local-journal.ts#L84-L99: addtypeof step.operationId !== 'string'tovalidateStep, socreateJournalbuilds its duplicate check on Line 124 from validated identifiers andrecordJournalStepcan address every step.🤖 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 `@apps/desktop/src/features/folder-autopilot/local-actions.ts` around lines 133 - 145, Update validateOperation in apps/desktop/src/features/folder-autopilot/local-actions.ts at lines 133-145 to require operation.operationId and operation.sourceFingerprint to be strings before applying SAFE_ID and SHA256; update validateStep in apps/desktop/src/features/folder-autopilot/local-journal.ts at lines 84-99 to require step.operationId to be a string before regex validation.apps/desktop/src/features/folder-autopilot/local-actions.ts-262-275 (1)
262-275: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftA mid-plan failure discards the receipts for the writes that already landed.
The loop applies operations in sequence. If
copyExclusiveorrenameExclusivefails at operation n, Line 266 throwsLOCAL_IO_FAILEDand every receipt collected for operations 1..n-1 is lost. Those writes are already on disk.The caller cannot then determine which operations completed.
local-journal.tsneeds exactly that information to driverecordJournalStepandcompensateJournal, so compensation cannot be reconstructed after a partial failure.Report the applied receipts together with the failure. Two options:
- Attach the receipts to the thrown error.
- Accept a per-operation callback and invoke it after each successful write.
♻️ Proposed direction: carry the applied receipts on the error
+export class LocalActionFailure extends LocalActionError { + readonly appliedReceipts: readonly LocalActionReceipt[]; + + constructor(code: LocalActionCode, appliedReceipts: readonly LocalActionReceipt[]) { + super(code); + this.name = 'LocalActionFailure'; + this.appliedReceipts = Object.freeze([...appliedReceipts]); + } +}try { if (operation.action === 'COPY') await fileSystem.copyExclusive(source, destination); else await fileSystem.renameExclusive!(source, destination); } catch { - return reject('LOCAL_IO_FAILED'); + throw new LocalActionFailure('LOCAL_IO_FAILED', receipts); }🤖 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 `@apps/desktop/src/features/folder-autopilot/local-actions.ts` around lines 262 - 275, Update the operation loop containing copyExclusive and renameExclusive so a mid-plan I/O failure preserves the receipts already added for successful operations. Attach the accumulated receipts to the LOCAL_IO_FAILED error (or equivalent failure result) while retaining the existing failure classification, allowing local-journal.ts to recover them for recordJournalStep and compensateJournal.apps/desktop/src/features/folder-autopilot/file-observation.ts-26-27 (1)
26-27: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftUse a representation that preserves nanosecond timestamps.
If
modifiedAtNsstores epoch nanoseconds,Number.isSafeIntegerrejects current timestamps because they exceedNumber.MAX_SAFE_INTEGER. ConvertingmtimeNstonumberalso loses nanosecond precision. Use a JSON-safe decimal string, or rename the field and use milliseconds. Add a test with a current-scale timestamp.🤖 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 `@apps/desktop/src/features/folder-autopilot/file-observation.ts` around lines 26 - 27, Update the file-observation model and its mtimeNs conversion to preserve epoch nanoseconds using a JSON-safe decimal string, including renaming modifiedAtNs if needed for the chosen representation. Remove Number.isSafeInteger validation and numeric coercion that lose precision, update consumers accordingly, and add coverage using a current-scale timestamp.services/engine/src/databreeze_engine/processors/folder_autopilot_plan.py-29-38 (1)
29-38: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep generated destination names within the contract limit.
destinationNamecan contain 255 characters._unique_namethen appends" (1)"and can return a longer name.The evaluator emits this value as a valid operation because
PlanOperation.destinationNamehas no corresponding length constraint. The plan can reportREADYfor a destination that violates the input contract.Reserve space for the suffix and extension before constructing each candidate. Apply the same destination-name validator to generated operations.
🤖 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 `@services/engine/src/databreeze_engine/processors/folder_autopilot_plan.py` around lines 29 - 38, Update _unique_name to reserve space for the numeric suffix and extension so every generated candidate stays within the destination-name contract limit, including when the original name is already at the limit. Apply the existing destination-name validator when creating generated PlanOperation values, ensuring invalid generated names cannot produce a READY plan.services/engine/src/databreeze_engine/folder_autopilot_contracts.py-54-76 (1)
54-76: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winBind
stableExecutionKeyto the observation fields.The model accepts any 64-character digest as
stableExecutionKey. A raw execution request can therefore supply a key that does not matchobservationId,displayName,sizeBytes,modifiedAtNs, andcontentSha256.
_plan_hashtrusts this key. Different observations can consequently receive the same plan identity.Move the canonical key derivation to a shared contract helper. Recompute and compare the key during
FileObservationvalidation, or remove the field from caller input.🤖 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 `@services/engine/src/databreeze_engine/folder_autopilot_contracts.py` around lines 54 - 76, Add a shared contract helper that derives the canonical stable execution key from FileObservation’s observationId, displayName, sizeBytes, modifiedAtNs, and contentSha256 fields, then update FileObservation validation to recompute and compare stableExecutionKey against that helper result. Keep the existing digest-format validation, but reject caller-supplied keys that do not match the observation fields and ensure _plan_hash continues using the validated canonical key.services/engine/src/databreeze_engine/processors/folder_autopilot_plan.py-66-70 (1)
66-70: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse Windows-equivalent destination keys for collision checks.
The
occupiedset compares destination names with exact string equality. On Windows, an occupiedInvoice.csvcan therefore fail to collide with a requestedinvoice.csv.The evaluator then reports
READY, although both names select the same destination. Use the same canonical destination-key function as the Desktop safety layer for existing, requested, generated, and newly occupied names.Add a test for names that differ only by case.
Also applies to: 97-109
🤖 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 `@services/engine/src/databreeze_engine/processors/folder_autopilot_plan.py` around lines 66 - 70, Update the collision-key handling in the folder autopilot plan logic around the occupied, requested, generated, and newly occupied destination sets to use the canonical destination-key function shared with the Desktop safety layer, rather than exact destination-name strings. Ensure existing and requested names differing only by case collide on Windows and cannot produce a READY result, and add coverage for that case.services/api/src/features/fa/adapter/prisma-folder-autopilot-repository.adapter.ts-213-235 (1)
213-235: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftAll three save methods read the existing row and then insert it in a separate statement. Under concurrency both callers observe no existing row, both call
create, and the primary key rejects the second insert. No duplicate row persists, so the data stays correct, but the caller receives a raw Prisma unique-constraint error instead of the typedFA_IMMUTABLE_*code. Wrap eachcreatein a try/catch that re-reads the row, compares it, and throws the matching typed code.
services/api/src/features/fa/adapter/prisma-folder-autopilot-repository.adapter.ts#L213-L235: catch the insert failure insaveProfile, re-read by(profileId, version), and throwFA_IMMUTABLE_PROFILEwhen the stored row differs.services/api/src/features/fa/adapter/prisma-folder-autopilot-repository.adapter.ts#L266-L284: apply the same handling insaveBindingand throwFA_IMMUTABLE_BINDING.services/api/src/features/fa/adapter/prisma-folder-autopilot-repository.adapter.ts#L313-L340: apply the same handling insaveAssignmentand throwFA_IMMUTABLE_ASSIGNMENT. This method also races on therecipe_assignments_scope_idempotency_keyindex, so the catch must distinguish the primary-key violation from the idempotency-key violation.🤖 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 `@services/api/src/features/fa/adapter/prisma-folder-autopilot-repository.adapter.ts` around lines 213 - 235, Update services/api/src/features/fa/adapter/prisma-folder-autopilot-repository.adapter.ts lines 213-235 in saveProfile to catch create failures, re-read by profileId and version, compare the stored row, and throw FA_IMMUTABLE_PROFILE when it differs. Apply the same create-failure handling at lines 266-284 in saveBinding with FA_IMMUTABLE_BINDING, and at lines 313-340 in saveAssignment with FA_IMMUTABLE_ASSIGNMENT; there, distinguish primary-key conflicts from recipe_assignments_scope_idempotency_key conflicts and preserve the latter’s existing behavior.services/api/src/features/fa/fa.module.ts-43-47 (1)
43-47: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject missing FA persistence in production.
main.tscallscreateApiApplication()without FA persistence options, so production selectsInMemoryFolderAutopilotRepositoryAdapter. Throw whenNODE_ENVisproductionand neither persistence option is supplied.🤖 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 `@services/api/src/features/fa/fa.module.ts` around lines 43 - 47, Update the repository selection in the FA module’s application factory to throw when NODE_ENV is production and both folderAutopilotRepository and folderAutopilotDatabase are missing; retain the existing adapter selection for non-production environments or when either persistence option is supplied.services/api/prisma/migrations/20260804050000_fa_folder_autopilot/migration.sql-68-69 (1)
68-69: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftUse a non-null scope key for idempotency uniqueness. The current composite index allows duplicate
idempotency_keyvalues whenworkspace_idorproject_idisNULL. Prisma 7.9.1 cannot represent PostgreSQLNULLS NOT DISTINCTin@@unique, so adding that clause only to the migration would create drift. Replace the nullable composite key inservices/api/prisma/schema/fa.prismaand its migration with a non-null canonical scope key, or use scope-specific indexes with a matching Prisma-compatible design.🤖 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 `@services/api/prisma/migrations/20260804050000_fa_folder_autopilot/migration.sql` around lines 68 - 69, Replace the nullable composite idempotency uniqueness design in services/api/prisma/schema/fa.prisma:65 and services/api/prisma/migrations/20260804050000_fa_folder_autopilot/migration.sql:68-69 with a Prisma-compatible non-null canonical scope key, and make the unique constraint use that key with idempotency_key. Keep the schema and migration definitions aligned without relying on PostgreSQL NULLS NOT DISTINCT.services/api/src/features/fa/api/folder-autopilot-dashboard.ts-48-84 (1)
48-84: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winDo not build user-facing display names in the API projection.
Lines 51 and 74 generate English-only labels:
Folder profile ${profile.version}andAssignment ${assignment.assignmentId.slice(0, 8)}. The projection is rendered by the Web and Android surfaces, so these strings reach the user without translation. Vietnamese is the default product locale, and English must stay complete, so the API cannot hard-code one language.Emit the raw fields and let each client localize. For example, return
versionon the profile andassignmentIdon the assignment, and dropdisplayNamefrom the projection contract. If the field must stay, return a message key plus parameters instead of a formatted sentence.Note that
services/api/test/features/fa/folder-autopilot-dashboard.test.tsline 61 asserts the literal'Assignment 44444444', so update that assertion with the contract.As per coding guidelines: "Keep Vietnamese the default product locale and English complete."
🤖 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 `@services/api/src/features/fa/api/folder-autopilot-dashboard.ts` around lines 48 - 84, The API projections must not generate hard-coded English display names. Update the FolderAutopilotDashboardProfileV1 and FolderAutopilotDashboardAssignmentV1 contracts and their profileProjection and assignmentProjection functions to remove displayName and expose the raw profile version and assignmentId fields instead; update the corresponding dashboard test assertion to match the revised contract.Source: Coding guidelines
services/api/src/features/fa/api/folder-autopilot.controller.ts-56-57 (1)
56-57: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winA Folder Autopilot list failure is reported to the client as an empty dashboard. The service does not map repository errors for the list methods, and the dashboard route discards a non-accepted result. During a persistence outage the response is
accepted: truewith emptyprofilesandassignments, so the user cannot distinguish an empty workspace from a failed read.
services/api/src/features/fa/api/folder-autopilot.controller.ts#L56-L57: stop substituting[]for a rejected result. Return the servicecodeto the caller, or map it to a status code, so the Web surface can render an error state instead of an empty dashboard.services/api/src/features/fa/application/folder-autopilot.service.ts#L331-L347: wrap eachthis.repository.list*call intry/catchand returnrejected(mapPersistenceError(error)), matchingcreateProfileandupdateAssignmentState. Without this the promise rejects and the route never receives a code to forward.🤖 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 `@services/api/src/features/fa/api/folder-autopilot.controller.ts` around lines 56 - 57, Propagate Folder Autopilot list failures instead of converting them to empty results: in services/api/src/features/fa/api/folder-autopilot.controller.ts lines 56-57, forward the rejected service code or map it to an HTTP status; in services/api/src/features/fa/application/folder-autopilot.service.ts lines 331-347, wrap each repository list call in try/catch and return rejected(mapPersistenceError(error)), matching createProfile and updateAssignmentState.services/api/src/features/fa/api/folder-autopilot.controller.ts-64-73 (1)
64-73: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not return 201 for a rejected result.
createProfilereturns the service result unchanged. A rejection such asFA_IMMUTABLE_PROFILE,INVALID_IDENTIFIER, orFA_PERSISTENCE_UNAVAILABLEis therefore sent with HTTP 201 Created and a body of{ accepted: false, code }.createBindingat line 98 andcreateAssignmentat line 126 behave the same way, andupdateAssignmentandpauseAssignmentreturn rejections with 200.The same OpenAPI document maps rejected results to real status codes elsewhere.
IamMembershipControllerdeclares 400, 403, 404, 409, 410, and 503 withMembershipRejectedResponseDto. A client that follows status codes will treat a failed Folder Autopilot create as a success. Map eachFolderAutopilotServiceErrorV1to a status code, and declare those responses in the OpenAPI paths atservices/api/openapi/v1.jsonlines 10410-11368.🤖 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 `@services/api/src/features/fa/api/folder-autopilot.controller.ts` around lines 64 - 73, Update createProfile, createBinding, createAssignment, updateAssignment, and pauseAssignment to translate each FolderAutopilotServiceErrorV1 rejection into its corresponding HTTP status instead of returning rejected results as successful responses. Preserve accepted responses while using the established error-to-status mapping and rejection DTO. Declare the same error responses for all affected Folder Autopilot operations in the v1 OpenAPI document, including the relevant 400, 403, 404, 409, 410, and 503 statuses.apps/web/src/features/folder-autopilot/folder-autopilot-api.ts-510-513 (1)
510-513: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not fall back to a constant idempotency key.
If
crypto.randomUUIDis unavailable, every mutation in the session sendsIdempotency-Key: autopilot-client-generated. The server then treats a pause, an approval, and an undo as replays of the first request and can silently drop them. Fail closed instead, which matchescreateFolderAutopilotProfileat Line 557 andsha256Hexat Line 517.🐛 Proposed fix
function idempotencyKey(prefix: string): string { - const random = globalThis.crypto?.randomUUID?.(); - return `${prefix}-${random ?? 'client-generated'}`; + const random = globalThis.crypto?.randomUUID?.(); + if (random === undefined) throw new Error('AUTOPILOT_CRYPTO_UNAVAILABLE'); + return `${prefix}-${random}`; }🤖 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 `@apps/web/src/features/folder-autopilot/folder-autopilot-api.ts` around lines 510 - 513, Update idempotencyKey to fail closed when globalThis.crypto?.randomUUID?.() is unavailable: do not use the constant 'client-generated' fallback, and instead propagate the same failure behavior established by createFolderAutopilotProfile and sha256Hex.apps/web/src/features/folder-autopilot/folder-autopilot-page.tsx-76-79 (1)
76-79: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRefresh the dashboard after a profile is created.
submitshowsautopilot.profile.savedand callsonSaved().FolderAutopilotPagepassesonSaved={() => undefined}at Line 589, so thefolder-autopilotquery is never invalidated. The profile list above the form keeps showing stale data after a successful create, and the user must reload the page. Pass the query refetch into the prop that already exists for this purpose.🐛 Proposed fix at the call site (Line 589)
- <ProfileAuthoring onSaved={() => undefined} profiles={dashboard.profiles} /> + <ProfileAuthoring onSaved={() => void query.refetch()} profiles={dashboard.profiles} />🤖 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 `@apps/web/src/features/folder-autopilot/folder-autopilot-page.tsx` around lines 76 - 79, Update the FolderAutopilotPage call site to pass the existing folder-autopilot query refetch function through the onSaved prop instead of the current no-op callback, so the profile list refreshes after submit successfully creates a profile.apps/web/src/features/folder-autopilot/folder-autopilot-api.ts-293-293 (1)
293-293: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject an invalid
revisioninstead of clamping it.
Math.max(1, count(...))rewrites a server-sentrevisionof0to1. The clamped value is then sent back asexpectedRevisionbypauseFolderAutopilotAssignmentandrequestFolderAutopilotUndo, so the optimistic concurrency check compares against a revision the record never had. Every other field in this module fails closed. Do the same here. The same clamp exists at Line 408 inparseExecution.🐛 Proposed fix
+function revision(input: unknown): number { + const value = count(input); + if (value < 1) throw new Error(UUID_ERROR); + return value; +}- revision: Math.max(1, count(value['revision'])), + revision: revision(value['revision']),🤖 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 `@apps/web/src/features/folder-autopilot/folder-autopilot-api.ts` at line 293, Update the revision parsing in the surrounding parser and parseExecution to reject invalid or non-positive server revisions instead of clamping them with Math.max. Preserve valid revisions unchanged and fail closed consistently with the module’s other fields, so pauseFolderAutopilotAssignment and requestFolderAutopilotUndo never send a fabricated expectedRevision.apps/web/src/features/folder-autopilot/folder-autopilot-api.ts-552-572 (1)
552-572: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRemove unsupported profile fields from the form.
FolderAutopilotProfileInputcontainsdisplayName,confidenceThreshold,approvalRequired, anddataModeConstraint, but the canonical profile contract excludes them. The form reports success while discarding these values. Remove them from the profile form and input type. If the product needs them, submit them through their owning JRA or assignment contracts. Do not adddataModeConstraintto the profile contract.The API limits
undoWindowHoursto168, butparseProfilepermits8_760. Align the parser with the API limit.🤖 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 `@apps/web/src/features/folder-autopilot/folder-autopilot-api.ts` around lines 552 - 572, Remove displayName, confidenceThreshold, approvalRequired, and dataModeConstraint from the folder autopilot profile form and FolderAutopilotProfileInput, leaving those concerns to their owning JRA or assignment contracts; do not add dataModeConstraint to the profile payload. Update parseProfile to cap undoWindowHours at 168, matching createFolderAutopilotProfile’s boundedSeconds limit.apps/android/app/src/main/java/com/databreeze/android/folderautopilot/FolderAutopilotModels.kt-45-71 (1)
45-71: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftReject approval decisions after
expiresAt.A
PENDINGapproval remains actionable after its deadline. At2026-08-05T00:00:00Z, the bundled sample expires, but the app can still enqueue a decision and mark it locally as approved or rejected. Parse and validate the deadline, use an injected clock before queueing or transitioning state, disable expired controls, and add an expired-approval test.
apps/android/app/src/main/java/com/databreeze/android/folderautopilot/FolderAutopilotModels.kt#L45-L71: enforceexpiresAtin the approval transition.apps/android/app/src/main/java/com/databreeze/android/folderautopilot/FolderAutopilotOfflineActionQueue.kt#L29-L40: reject an expired approval before writing an offline intent.apps/android/app/src/main/java/com/databreeze/android/folderautopilot/FolderAutopilotScreen.kt#L93-L104: disable approval controls when the approval has expired.apps/android/app/src/main/java/com/databreeze/android/MainActivity.kt#L119-L145: do not apply an optimistic decision after an expired action.apps/android/app/src/main/java/com/databreeze/android/MainActivity.kt#L189-L197: replace the release-date-bound sample deadline with time-safe fixture data.🤖 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 `@apps/android/app/src/main/java/com/databreeze/android/folderautopilot/FolderAutopilotModels.kt` around lines 45 - 71, Reject expired pending approvals across the complete flow: in FolderAutopilotApprovalSummary.decide, parse and validate expiresAt and require the injected clock to be before the deadline; in FolderAutopilotOfflineActionQueue reject expired approvals before persisting offline intents; in FolderAutopilotScreen disable approval controls once expired; in MainActivity prevent optimistic transitions for expired actions; and replace the release-date-bound sample deadline with time-safe fixture data. Update or add coverage for an expired approval.
🟡 Minor comments (15)
apps/desktop/src/features/folder-autopilot/local-journal.ts-155-159 (1)
155-159: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAn unknown
operationIdreportsDUPLICATE_STEP.Line 156 rejects with
DUPLICATE_STEPwhenfindIndexreturns-1. An identifier that is absent from the journal is not a duplicate. The caller cannot distinguish "already recorded" from "not part of this journal", which makes a recovery bug hard to diagnose.compensateJournalrepeats the same mapping on Line 178.Use
INVALID_JOURNALfor the not-found case and keepDUPLICATE_STEPfor the wrong-state case.♻️ Proposed fix
const index = journal.steps.findIndex((step) => step.operationId === operationId); - if (index < 0) return reject('DUPLICATE_STEP'); + if (index < 0) return reject('INVALID_JOURNAL'); const step = journal.steps[index]; - if (step === undefined) return reject('DUPLICATE_STEP'); + if (step === undefined) return reject('INVALID_JOURNAL'); if (step.state !== 'PENDING') return reject('DUPLICATE_STEP');🤖 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 `@apps/desktop/src/features/folder-autopilot/local-journal.ts` around lines 155 - 159, Update the not-found handling in the journal step lookup used by the relevant function and compensateJournal: when findIndex returns -1, reject with INVALID_JOURNAL instead of DUPLICATE_STEP. Preserve DUPLICATE_STEP for steps found in the journal whose state is not PENDING.apps/desktop/src/features/folder-autopilot/file-observation.ts-121-134 (1)
121-134: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winStrengthen
isByteArraysofingerprintBytesstays fail-closed.
isByteArrayonly inspectsbyteLength. A plain object such as{ byteLength: 4 }satisfies the predicate and is asserted asUint8Array.createHash().update()then throws a rawTypeErrorfromnode:crypto. Line 211 callsfingerprintBytesoutside atryblock, so thatTypeErrorescapes instead of the stableINVALID_OBSERVATIONcode, and the error message is not guaranteed to be content-free.Check the value is an
ArrayBufferview.🛡️ Proposed fix
function isByteArray(value: unknown): value is Uint8Array { - return ( - typeof value === 'object' && - value !== null && - typeof (value as { readonly byteLength?: unknown }).byteLength === 'number' && - Number.isSafeInteger((value as { readonly byteLength: number }).byteLength) && - (value as { readonly byteLength: number }).byteLength >= 0 - ); + return ArrayBuffer.isView(value) && value instanceof Uint8Array; }🤖 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 `@apps/desktop/src/features/folder-autopilot/file-observation.ts` around lines 121 - 134, Strengthen isByteArray to require the value to be an ArrayBuffer view before validating byteLength, while preserving the existing non-null, numeric, safe-integer, and non-negative checks. This must reject plain objects such as { byteLength: 4 } so fingerprintBytes returns INVALID_OBSERVATION instead of allowing createHash().update() to throw.services/api/src/features/fa/api/folder-autopilot.dto.ts-192-195 (1)
192-195: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject an empty
decisionReason.
decisionReasonis required and bounded at 500 characters, but no minimum length is set.@IsString()accepts'', so a caller can approve or reject with no reason. The field records the approval justification, so an empty value defeats its purpose.🛡️ Proposed fix to require a non-empty reason
- `@ApiProperty`({ maxLength: 500 }) + `@ApiProperty`({ minLength: 1, maxLength: 500 }) `@IsString`() + `@MinLength`(1) `@MaxLength`(500) decisionReason!: string;Add
MinLengthto theclass-validatorimport at lines 2-18.🤖 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 `@services/api/src/features/fa/api/folder-autopilot.dto.ts` around lines 192 - 195, Update the DTO field decisionReason by importing and applying class-validator’s MinLength validator with a minimum of one character, while preserving its existing string and 500-character maximum validation.services/api/openapi/v1.json-10557-10562 (1)
10557-10562: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winBound the
versionquery parameter in the published contract.The
versionquery parameter declaresminimum: 1and no maximum.FolderAutopilotService.findProfilerejects any version above10_000withINVALID_VERSION, andCreateFolderAutopilotProfileDto.versiondeclaresmaximum: 10000at line 12949. The contract therefore advertises versions the API rejects.This file is generated, so fix the source decorator in
services/api/src/features/fa/api/folder-autopilot.controller.tsline 84:♻️ Proposed fix at the decorator source
- `@ApiQuery`({ name: 'version', required: false, type: 'integer', minimum: 1 }) + `@ApiQuery`({ name: 'version', required: false, type: 'integer', minimum: 1, maximum: 10_000 })One related note on the new schemas at lines 12945-13080: none declare
additionalProperties: false, but the validation pipe rejects unknown fields with 400, asservices/api/test/features/fa/folder-autopilot.controller.test.tslines 75-80 confirm. Declaring the closed shape would make the contract match runtime behavior.🤖 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 `@services/api/openapi/v1.json` around lines 10557 - 10562, Update the version query-parameter decorator in FolderAutopilotController to declare a maximum of 10000, matching FolderAutopilotService.findProfile and CreateFolderAutopilotProfileDto.version. Regenerate services/api/openapi/v1.json from the source decorator; do not modify the generated contract directly.services/api/src/features/fa/adapter/in-memory-folder-autopilot-repository.adapter.ts-189-238 (1)
189-238: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPublic reads bypass the transaction queue and can observe uncommitted state.
findProfile,listProfiles,findBinding,listBindings,findAssignment, andlistAssignmentscall the*Unlockedmethods directly.withTransactionserializes only against otherwithTransactioncalls. A transaction callback awaits between writes, so a concurrent read can observe records that a later rollback discards at lines 269-271.
FolderAutopilotService.listProfilesandlistAssignmentsrun concurrently throughPromise.allin the dashboard route, so interleaving with an in-flight create transaction is reachable.Route the public reads through
withTransactionso they queue behind pending work:♻️ Proposed fix to serialize reads
- public findProfile(context: IamTenantContextV1, profileId: StableIdentifierV1, version?: number) { - return this.findProfileUnlocked(context, profileId, version); - } + public findProfile(context: IamTenantContextV1, profileId: StableIdentifierV1, version?: number) { + return this.withTransaction(context, (transaction) => + transaction.findProfile(context, profileId, version), + ); + } - public listProfiles(context: IamTenantContextV1) { - return this.listProfilesUnlocked(context); - } + public listProfiles(context: IamTenantContextV1) { + return this.withTransaction(context, (transaction) => transaction.listProfiles(context)); + }Note one related hazard: the public methods that already wrap
withTransactiondeadlock if a caller invokes them from inside anotherwithTransactioncallback, because the inner call awaits a tail that only the outer call releases. The service usestransaction.*inside callbacks today, so the deadlock is not reachable. Add a comment onwithTransactionto record that the lock is not re-entrant.🤖 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 `@services/api/src/features/fa/adapter/in-memory-folder-autopilot-repository.adapter.ts` around lines 189 - 238, Route the public read methods findProfile, listProfiles, findBinding, listBindings, findAssignment, and listAssignments through withTransaction, invoking their corresponding *Unlocked methods inside the transaction callback so reads wait for pending writes. Add a concise comment on withTransaction documenting that its lock is not re-entrant and callers inside a transaction must use the transaction methods directly.services/api/src/features/fa/application/folder-autopilot.service.ts-331-347 (1)
331-347: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winMap persistence failures in the list methods.
createProfile,createBinding,createAssignment, andupdateAssignmentStateconvert repository errors into a rejected result. The three list methods do not. If the repository throws, the promise rejects and the caller receives an unmapped error instead ofFA_PERSISTENCE_UNAVAILABLE.FolderAutopilotController.dashboardalready handles a non-accepted result, so mapping keeps the dashboard route available during a persistence outage.♻️ Proposed fix for consistent error mapping
public async listProfiles( context: IamTenantContextV1, ): Promise<FolderAutopilotServiceResultV1<readonly FolderAutopilotProfileV1[]>> { - return Object.freeze({ accepted: true, value: await this.repository.listProfiles(context) }); + try { + return Object.freeze({ accepted: true, value: await this.repository.listProfiles(context) }); + } catch (error) { + return rejected<readonly FolderAutopilotProfileV1[]>(mapPersistenceError(error)); + } } public async listBindings( context: IamTenantContextV1, ): Promise<FolderAutopilotServiceResultV1<readonly AutopilotFolderBindingV1[]>> { - return Object.freeze({ accepted: true, value: await this.repository.listBindings(context) }); + try { + return Object.freeze({ accepted: true, value: await this.repository.listBindings(context) }); + } catch (error) { + return rejected<readonly AutopilotFolderBindingV1[]>(mapPersistenceError(error)); + } } public async listAssignments( context: IamTenantContextV1, ): Promise<FolderAutopilotServiceResultV1<readonly RecipeAssignmentV1[]>> { - return Object.freeze({ accepted: true, value: await this.repository.listAssignments(context) }); + try { + return Object.freeze({ + accepted: true, + value: await this.repository.listAssignments(context), + }); + } catch (error) { + return rejected<readonly RecipeAssignmentV1[]>(mapPersistenceError(error)); + } }The same gap exists in
findProfile,findBinding, andfindAssignmentat lines 295-329.🤖 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 `@services/api/src/features/fa/application/folder-autopilot.service.ts` around lines 331 - 347, Update listProfiles, listBindings, and listAssignments to catch repository failures and return the same rejected result mapped to FA_PERSISTENCE_UNAVAILABLE used by createProfile, createBinding, createAssignment, and updateAssignmentState. Apply the identical error mapping to findProfile, findBinding, and findAssignment, while preserving successful accepted results and existing controller handling.services/api/test/features/fa/folder-autopilot.service.test.ts-127-144 (1)
127-144: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAssert the assignment isolation that the test name claims.
The test is named
sibling tenant cannot read a profile or assignment, but the body only callsservice.findProfile. Assignment isolation is untested, so a regression infindAssignmentvisibility would pass.💚 Proposed fix to cover assignment isolation
const tenant = context(); await service.createProfile(tenant, profileInput); + await service.createBinding(tenant, bindingInput(ids.inputBindingId, 'INPUT')); + await service.createBinding(tenant, bindingInput(ids.outputBindingId, 'OUTPUT')); + await service.createAssignment(tenant, assignmentInput); const sibling = context( { scopeType: 'workspace', organizationId: ids.organizationId, workspaceId: 'ffffffff-ffff-4fff-8fff-ffffffffffff', }, 'fa-sibling', ); assert.deepEqual(await service.findProfile(sibling, ids.profileId), { accepted: false, code: 'FA_PROFILE_NOT_FOUND', }); + assert.deepEqual(await service.findAssignment(sibling, ids.recipeId), { + accepted: false, + code: 'FA_ASSIGNMENT_NOT_FOUND', + });🤖 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 `@services/api/test/features/fa/folder-autopilot.service.test.ts` around lines 127 - 144, Extend the test case around `service.findProfile` to also call `service.findAssignment` using the sibling context and the relevant assignment identifier, then assert it returns the same rejected `FA_ASSIGNMENT_NOT_FOUND` result. Ensure the test creates or references an assignment associated with the created profile so it verifies sibling-tenant assignment isolation.services/api/src/features/fa/api/folder-autopilot-dashboard.ts-29-29 (1)
29-29: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAlign the dashboard projection with the assignment state.
- Use
'LOCAL' | 'HYBRID' | 'CLOUD'fordataModeConstraint. Source it fromRecipeAssignmentV1.dataModeConstraint; the current profile projection always reports'Hybrid'and cannot representLOCAL,CLOUD, or no constraint.- Do not map assignment
updatedAtfromcreatedAt. State transitions incrementrevisionwithout changingcreatedAt, so the dashboard timestamp becomes stale. Persist and updateupdatedAtduring state transitions.🤖 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 `@services/api/src/features/fa/api/folder-autopilot-dashboard.ts` at line 29, Update the dashboard projection’s dataModeConstraint to use the `'LOCAL' | 'HYBRID' | 'CLOUD'` values from RecipeAssignmentV1.dataModeConstraint, preserving an absent constraint instead of defaulting to `'Hybrid'`. In the assignment state transition flow, persist and update updatedAt independently from createdAt so dashboard timestamps reflect revision changes.apps/web/src/features/folder-autopilot/folder-autopilot-page.tsx-36-40 (1)
36-40: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winLocalize the remaining engine reason codes.
reasonLabellocalizes onlyDESTINATION_COLLISION. The engine also emitsDESTINATION_COLLISION_SKIPPEDandMOVE_REQUIRES_APPROVAL(seeservices/engine/src/databreeze_engine/processors/folder_autopilot_plan.py:64-137). Those codes reach the previewreasonCodeslist and render as raw uppercase tokens in the Vietnamese default locale. Add message keys for the codes the engine can produce, and keep the raw token only as a last-resort fallback.As per coding guidelines: "Keep Vietnamese the default product locale and English complete."
🤖 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 `@apps/web/src/features/folder-autopilot/folder-autopilot-page.tsx` around lines 36 - 40, Update reasonLabel to localize DESTINATION_COLLISION_SKIPPED and MOVE_REQUIRES_APPROVAL in addition to DESTINATION_COLLISION, using appMessage with new message keys. Add Vietnamese default translations and complete English translations for all engine reason codes, while preserving the raw value only as the fallback for unknown codes.Source: Coding guidelines
apps/web/src/features/folder-autopilot/folder-autopilot-page.tsx-90-90 (1)
90-90: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winLocalize the hardcoded English UI strings.
Line 90 renders
JRA profile facade, Line 586 rendersHybrid, and lines 271 and 478 render the screen-reader labelActions. All four bypassappMessage, so they stay English in the Vietnamese default locale. The screen-reader labels matter most, because assistive-technology users on the default locale hear an English column name. Add message keys for these strings.As per coding guidelines: "Keep Vietnamese the default product locale and English complete."
🤖 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 `@apps/web/src/features/folder-autopilot/folder-autopilot-page.tsx` at line 90, Replace the hardcoded “JRA profile facade”, “Hybrid”, and both “Actions” labels in the folder autopilot page with appMessage lookups, adding corresponding entries to the message definitions with Vietnamese as the default locale and complete English translations.Source: Coding guidelines
apps/web/src/features/folder-autopilot/folder-autopilot-page.tsx-340-345 (1)
340-345: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winShow the empty state when every approval is filtered out.
Line 344 returns
nullfor an approval with no matching preview. The empty-state check at Line 340 testsapprovals.length, not the rendered count. If the dashboard returns approvals whose previews expired and were dropped from the projection, the panel renders an empty card list with no message. Derive the rendered list first, then branch on its length.♻️ Proposed refactor
+ const pairs = approvals.flatMap((approval) => { + const preview = previews.find((candidate) => candidate.previewId === approval.previewId); + return preview === undefined ? [] : [{ approval, preview }]; + });Then render from
pairsand testpairs.length === 0for the empty state.🤖 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 `@apps/web/src/features/folder-autopilot/folder-autopilot-page.tsx` around lines 340 - 345, Update the approval rendering flow in the folder autopilot page to derive the preview-matched approval pairs before rendering. Exclude approvals without a matching preview, render the card list from that derived collection, and use its length for the empty-state check so the message appears when all approvals are filtered out.apps/web/src/app/messages.ts-110-114 (1)
110-114: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winTranslate the remaining Vietnamese values.
Line 114 keeps the English word
Preview. Line 110 mixes Vietnamese and English in'Tạm dừng assignment'. Vietnamese is the default locale, so these strings appear untranslated to default users. Use the same Vietnamese term you already use elsewhere (Phân côngfor assignment, and a Vietnamese term for preview such asBản xem trước).🌐 Proposed wording fix
- 'autopilot.assignment.pause': 'Tạm dừng assignment', + 'autopilot.assignment.pause': 'Tạm dừng phân công', @@ - 'autopilot.approval.preview': 'Preview', + 'autopilot.approval.preview': 'Bản xem trước',As per coding guidelines: "Keep Vietnamese the default product locale and English complete."
🤖 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 `@apps/web/src/app/messages.ts` around lines 110 - 114, Update the Vietnamese locale values in the autopilot assignment and approval translations: replace the English “assignment” in autopilot.assignment.pause with the existing Vietnamese term “Phân công,” and translate autopilot.approval.preview using “Bản xem trước” or the established equivalent. Keep the remaining translations unchanged.Source: Coding guidelines
apps/web/src/features/folder-autopilot/folder-autopilot-page.tsx-605-607 (1)
605-607: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not render an epoch date when no profile exists.
With an empty
dashboard.profiles, the fallbacknew Date(0).toISOString()renders a formatted 1 January 1970 timestamp. The paragraph also carries no label, so the date appears with no explanation of what it represents. Omit the paragraph when there is no profile, and add a message key that names the value.🐛 Proposed fix
- <p className="authority-note"> - {dateLabel(locale, dashboard.profiles[0]?.updatedAt ?? new Date(0).toISOString())} - </p> + {dashboard.profiles[0] === undefined ? null : ( + <p className="authority-note"> + {dateLabel(locale, dashboard.profiles[0].updatedAt)} + </p> + )}🤖 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 `@apps/web/src/features/folder-autopilot/folder-autopilot-page.tsx` around lines 605 - 607, Update the profile date rendering in the folder autopilot page to omit the authority-note paragraph when dashboard.profiles has no profile instead of using an epoch-date fallback. When a profile exists, continue formatting its updatedAt value and add/use a localization message key that clearly labels the displayed date.packages/domain/src/folder-autopilot/v1.ts-129-135 (1)
129-135: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReject sparse binding-ID arrays.
Array.prototype.map()skips sparse indexes, soidentifiers()accepts a sparse array and returns an array containingundefined. UseArray.from(input, stable)and add tests for sparseinputBindingIdsandoutputBindingIds.🤖 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 `@packages/domain/src/folder-autopilot/v1.ts` around lines 129 - 135, The identifiers function currently allows sparse arrays because map skips missing indexes; replace the values construction with Array.from(input, stable) so every index is validated and sparse input is rejected. Add coverage for sparse inputBindingIds and outputBindingIds, ensuring both are rejected.apps/android/app/src/main/java/com/databreeze/android/folderautopilot/FolderAutopilotScreen.kt-49-57 (1)
49-57: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winComplete the Vietnamese default product UI.
The screen inserts English enum names into default resource strings. The default resource also retains
WatcherandPreview, and omits the file-content privacy statement fromautopilot_body.
apps/android/app/src/main/java/com/databreeze/android/folderautopilot/FolderAutopilotScreen.kt#L49-L57: map assignment and watcher states to localized resources.apps/android/app/src/main/java/com/databreeze/android/folderautopilot/FolderAutopilotScreen.kt#L89-L92: map approval decisions to localized resources.apps/android/app/src/main/java/com/databreeze/android/folderautopilot/FolderAutopilotScreen.kt#L117-L123: map outcome and undo states to localized resources.apps/android/app/src/main/java/com/databreeze/android/folderautopilot/FolderAutopilotScreen.kt#L137-L141: map exception severity to localized resources while retaining technical reason codes.apps/android/app/src/main/res/values/strings.xml#L13-L20: translate the remaining labels and preserve the source-path and file-content privacy meaning.As per coding guidelines, keep Vietnamese the default product locale and English complete.
🤖 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 `@apps/android/app/src/main/java/com/databreeze/android/folderautopilot/FolderAutopilotScreen.kt` around lines 49 - 57, Complete Vietnamese localization across all listed sites: in FolderAutopilotScreen.kt lines 49-57 map assignment and watcher states to localized resources; lines 89-92 map approval decisions; lines 117-123 map outcome and undo states; and lines 137-141 map exception severity while retaining technical reason codes. In apps/android/app/src/main/res/values/strings.xml lines 13-20 translate remaining labels, including Watcher and Preview, and restore both source-path and file-content privacy meaning. Keep Vietnamese as the default product locale and provide complete English translations.Source: Coding guidelines
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
services/engine/src/databreeze_engine/processors/folder_autopilot.py (1)
49-66: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winNormalize stable-key validation failures.
At Line 49, an invalid
modified_at_nsfails instable_execution_keybefore thetryblock. This bypasses theINVALID_OBSERVATIONerror normalization used for invalid model fields.Move stable-key derivation into the
tryblock. Add a regression test for an invalid timestamp.🤖 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 `@services/engine/src/databreeze_engine/processors/folder_autopilot.py` around lines 49 - 66, Move the _stable_execution_key() call and stable_key assignment from outside the try block into the beginning of the try block so that validation failures in the stable-key derivation (such as invalid modified_at_ns) are caught by the same exception handler and normalized to ValueError("INVALID_OBSERVATION") like FileObservation constructor failures. This ensures consistent error normalization for all observation-validation failures.apps/desktop/src/features/folder-autopilot/local-actions.ts (1)
274-279: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve receipts for every later execution failure.
The
catchonly wraps write failures. If a later operation fails stale-source validation at Line 230, prior operations can already be applied but the caller receives noappliedReceiptsfor journaling or compensation.Wrap later
LocalActionErrorfailures after the first receipt inLocalActionFailure. Preserve the original error code. Add coverage for a successful first operation followed by a stale second operation.🤖 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 `@apps/desktop/src/features/folder-autopilot/local-actions.ts` around lines 274 - 279, Expand the try-catch block to encompass all operations beyond the initial copyExclusive or renameExclusive calls, including stale-source validation and other logic that may throw LocalActionError. When catching a LocalActionError that occurs after a receipt has been recorded, wrap it in LocalActionFailure while preserving the original error code from the LocalActionError and including the accumulated receipts. This ensures that callers receive appliedReceipts even when later operations fail after earlier operations succeed.services/api/src/features/fa/api/folder-autopilot.controller.ts (1)
304-327: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve rejected approval and undo HTTP statuses.
Both facade methods can return
accepted: false, but these handlers force HTTP 200. Add@Res({ passthrough: true }),@applyFolderAutopilotOutcomeResponses(), andpreserveFolderAutopilotStatus. MapFA_JRA_APPROVAL_FACADE_UNAVAILABLEandFA_JRA_UNDO_FACADE_UNAVAILABLEto503; the current default maps them to400.🤖 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 `@services/api/src/features/fa/api/folder-autopilot.controller.ts` around lines 304 - 327, Update decideApproval and requestUndo to preserve rejected-outcome HTTP statuses instead of always returning 200. Add passthrough response handling, applyFolderAutopilotOutcomeResponses, and preserveFolderAutopilotStatus to both handlers, mapping FA_JRA_APPROVAL_FACADE_UNAVAILABLE and FA_JRA_UNDO_FACADE_UNAVAILABLE to 503 while retaining the existing outcome mappings.
🤖 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 `@services/api/test/features/fa/folder-autopilot.service.test.ts`:
- Around line 132-134: Update the setup in the affected autopilot service test
to assert successful completion of both createBinding calls and createAssignment
instead of discarding their results. Before the sibling lookup assertion, use
the owner context to read the assignment by the same ID and assert that it
succeeds, ensuring the sibling rejection is validated against established data.
---
Outside diff comments:
In `@apps/desktop/src/features/folder-autopilot/local-actions.ts`:
- Around line 274-279: Expand the try-catch block to encompass all operations
beyond the initial copyExclusive or renameExclusive calls, including
stale-source validation and other logic that may throw LocalActionError. When
catching a LocalActionError that occurs after a receipt has been recorded, wrap
it in LocalActionFailure while preserving the original error code from the
LocalActionError and including the accumulated receipts. This ensures that
callers receive appliedReceipts even when later operations fail after earlier
operations succeed.
In `@services/api/src/features/fa/api/folder-autopilot.controller.ts`:
- Around line 304-327: Update decideApproval and requestUndo to preserve
rejected-outcome HTTP statuses instead of always returning 200. Add passthrough
response handling, applyFolderAutopilotOutcomeResponses, and
preserveFolderAutopilotStatus to both handlers, mapping
FA_JRA_APPROVAL_FACADE_UNAVAILABLE and FA_JRA_UNDO_FACADE_UNAVAILABLE to 503
while retaining the existing outcome mappings.
In `@services/engine/src/databreeze_engine/processors/folder_autopilot.py`:
- Around line 49-66: Move the _stable_execution_key() call and stable_key
assignment from outside the try block into the beginning of the try block so
that validation failures in the stable-key derivation (such as invalid
modified_at_ns) are caught by the same exception handler and normalized to
ValueError("INVALID_OBSERVATION") like FileObservation constructor failures.
This ensures consistent error normalization for all observation-validation
failures.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 57535106-acc4-4eed-a227-99fd2c53e2fe
📒 Files selected for processing (42)
apps/android/app/src/main/java/com/databreeze/android/MainActivity.ktapps/android/app/src/main/java/com/databreeze/android/folderautopilot/FolderAutopilotModels.ktapps/android/app/src/main/java/com/databreeze/android/folderautopilot/FolderAutopilotOfflineActionQueue.ktapps/android/app/src/main/java/com/databreeze/android/folderautopilot/FolderAutopilotScreen.ktapps/android/app/src/main/res/values-en/strings.xmlapps/android/app/src/main/res/values/strings.xmlapps/android/app/src/test/java/com/databreeze/android/folderautopilot/FolderAutopilotOfflineQueueTest.ktapps/desktop/src/features/folder-autopilot/file-observation.tsapps/desktop/src/features/folder-autopilot/local-actions.tsapps/desktop/src/features/folder-autopilot/local-journal.tsapps/desktop/test/folder-autopilot-journal.test.tsapps/desktop/test/folder-autopilot-local-actions.test.tsapps/desktop/test/folder-autopilot-observation.test.tsapps/web/src/app/messages.tsapps/web/src/features/folder-autopilot/folder-autopilot-api.tsapps/web/src/features/folder-autopilot/folder-autopilot-page.tsxapps/web/test/folder-autopilot-api.test.tsapps/web/test/folder-autopilot-page.test.tsxdocs/operations/coderabbit-pr-51-disposition.mdpackages/domain/src/folder-autopilot/v1.tspackages/domain/test/folder-autopilot-v1.test.mjsservices/api/openapi/v1.jsonservices/api/prisma/migrations/20260804120000_fa_assignment_scope_key/migration.sqlservices/api/prisma/schema/fa.prismaservices/api/src/features/fa/adapter/in-memory-folder-autopilot-repository.adapter.tsservices/api/src/features/fa/adapter/prisma-folder-autopilot-repository.adapter.tsservices/api/src/features/fa/api/folder-autopilot-dashboard.tsservices/api/src/features/fa/api/folder-autopilot.controller.tsservices/api/src/features/fa/api/folder-autopilot.dto.tsservices/api/src/features/fa/application/folder-autopilot.service.tsservices/api/src/features/fa/fa.module.tsservices/api/test/features/fa/folder-autopilot-dashboard.test.tsservices/api/test/features/fa/folder-autopilot.controller.test.tsservices/api/test/features/fa/folder-autopilot.service.test.tsservices/api/test/prisma-foundation.test.mjsservices/engine/src/databreeze_engine/folder_autopilot_contracts.pyservices/engine/src/databreeze_engine/processors/folder_autopilot.pyservices/engine/src/databreeze_engine/processors/folder_autopilot_plan.pyservices/engine/src/databreeze_engine/registry.pyservices/engine/tests/test_folder_autopilot_action.pyservices/engine/tests/test_folder_autopilot_observation.pyservices/engine/tests/test_folder_autopilot_plan.py
🚧 Files skipped from review as they are similar to previous changes (27)
- apps/web/src/app/messages.ts
- apps/android/app/src/main/res/values-en/strings.xml
- packages/domain/test/folder-autopilot-v1.test.mjs
- apps/desktop/test/folder-autopilot-observation.test.ts
- services/api/test/prisma-foundation.test.mjs
- services/api/prisma/schema/fa.prisma
- services/api/src/features/fa/api/folder-autopilot-dashboard.ts
- apps/android/app/src/main/res/values/strings.xml
- apps/web/test/folder-autopilot-api.test.ts
- services/engine/tests/test_folder_autopilot_observation.py
- services/api/test/features/fa/folder-autopilot.controller.test.ts
- services/engine/tests/test_folder_autopilot_action.py
- apps/web/src/features/folder-autopilot/folder-autopilot-page.tsx
- services/api/src/features/fa/adapter/in-memory-folder-autopilot-repository.adapter.ts
- services/api/test/features/fa/folder-autopilot-dashboard.test.ts
- services/engine/tests/test_folder_autopilot_plan.py
- apps/desktop/src/features/folder-autopilot/file-observation.ts
- apps/android/app/src/main/java/com/databreeze/android/MainActivity.kt
- services/engine/src/databreeze_engine/processors/folder_autopilot_plan.py
- packages/domain/src/folder-autopilot/v1.ts
- apps/android/app/src/main/java/com/databreeze/android/folderautopilot/FolderAutopilotOfflineActionQueue.kt
- services/engine/src/databreeze_engine/registry.py
- services/api/src/features/fa/application/folder-autopilot.service.ts
- apps/desktop/src/features/folder-autopilot/local-journal.ts
- services/api/openapi/v1.json
- services/api/src/features/fa/adapter/prisma-folder-autopilot-repository.adapter.ts
- apps/web/src/features/folder-autopilot/folder-autopilot-api.ts
1 participant
Summary
Scope and safety
This PR is a testable module slice, not a claim that the full FA P0/P1 specification is complete. Remaining preview, reconciliation, full execution, approval, recovery, export, and monitoring projections remain fail-closed and are documented in the release evidence. Local paths, bytes, previews, and source values do not cross the Web/API boundary.
Verification
corepack pnpm repo:checkcorepack pnpm repo:buildReview notes
mainper the current delivery flowSummary by CodeRabbit