packages/engine/internal/repo/sync/poller.go (1)

83-96: 💤 Low value

time.NewTicker delays the first sync by a full interval.

With the production 60s default (or 10s floor), no sync runs until the first interval elapses after startup. If a prompt initial sync on boot is desired, fire once before entering the loop.

🤖 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/engine/internal/repo/sync/poller.go` around lines 83 - 96, The
poller currently creates a time.NewTicker and waits a full interval before the
first SyncRepo call, delaying initial sync; to fix, invoke SyncRepo(ctx, p.DB,
p.Store, r, p.Driver) once immediately after creating the ticker (and before the
for/select) and call p.OnSync(r, report, syncErr) if set, while still honoring
ctx.Done; keep the existing ticker loop intact for subsequent periodic syncs so
NewTicker/ticker.Stop and the select remain unchanged.

packages/engine/internal/cli/serve.go (2)

168-179: 💤 Low value

Poller goroutine isn't joined on shutdown.

go poller.Run(ctx) is fire-and-forget; when srv.Start(ctx) returns after ctx cancellation, the process can exit before the poller's in-flight SyncRepo completes. If clean drain of an in-progress sync matters, wait for Run to return (e.g., via a channel/WaitGroup) before returning from RunE.

🤖 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/engine/internal/cli/serve.go` around lines 168 - 179, The poller
goroutine started with go poller.Run(ctx) is not awaited, so when srv.Start(ctx)
returns on cancellation the process may exit before reposync.Poller.Run
completes; modify the serve command (RunE) to start Run in a managed goroutine
and wait for it to finish before returning: create a sync.WaitGroup or a done
channel, increment it before calling poller.Run(ctx) (or run poller.Run in a
goroutine that signals the done channel), and defer or select to wait for that
signal after srv.Start(ctx) returns (and/or when ctx is canceled) so
reposync.Poller.Run has completed; reference reposync.Poller, poller.Run, and
srv.Start in your changes.

---

158-167: 💤 Low value

Encryptor is constructed twice from the same key.

Lines 82-90 already build eng.Resolver (with its own secret.Encryptor) when cfg.Encryption.Key != "". This block repeats the same NewEncryptor + Resolver construction. Reuse a single resolver to avoid the duplicate decryption setup and keep the two wirings from drifting.

🤖 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/engine/internal/cli/serve.go` around lines 158 - 167, The code is
creating a second secret.Encryptor and secret.Resolver when cfg.Encryption.Key
!= "" (using secret.NewEncryptor and instantiating secret.Resolver/secret.Store)
which duplicates the earlier eng.Resolver construction; instead reuse the
already-created eng.Resolver (or its Store/Encryptor) rather than calling
secret.NewEncryptor again — replace the duplicate NewEncryptor +
secret.Resolver/secret.Store construction with an assignment from the existing
eng.Resolver (e.g., secretResolver = eng.Resolver or secretResolver =
&eng.Resolver.Store as appropriate) so only one Encryptor/Resolver is
instantiated and wired throughout.

packages/engine/internal/repo/store.go (1)

279-296: ⚖️ Poor tradeoff

UpdateSyncState writes state without an in-transaction audit event, diverging from the Store's stated invariant.

The Store doc (lines 15-17) promises every state-changing method emits an audit event in the same transaction "so audit log and state never drift." UpdateSyncState mutates git_repos but emits nothing — the engine emits git.sync.* events in a separate transaction via its own emit helper, so this particular write isn't covered by the same-tx guarantee. If that's an intentional exception (sync state is operational, not security-relevant), consider tightening the Store comment to scope the invariant; otherwise fold the audit emission into this write.

As per coding guidelines: "Emit audit events via the AuditEmitter interface for every state-changing 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 `@packages/engine/internal/repo/store.go` around lines 279 - 296,
UpdateSyncState currently updates git_repos without emitting an audit event in
the same DB transaction, violating the Store invariant; change it to perform the
update and the audit emission inside a single transaction: start a tx (e.g.,
s.DB.BeginTx(ctx,...)), run the UPDATE using tx.ExecContext, call the Store's
AuditEmitter method (AuditEmitter.EmitAudit or the concrete emit helper) to
record the git.sync.* audit event with repo id, sha and syncErr using the same
tx, rollback on any error and commit on success, and preserve the existing
error-wrapping behavior (keep function name UpdateSyncState and use the same
parameters). Ensure the audit emission uses the AuditEmitter interface so the
audit and state change cannot drift.

Fix all unresolved CodeRabbit comments on this PR:

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