feat(publishing): LinkedIn integration publication#99
Merged
Conversation
…ment - Add support for new publication statuses: PENDING, DISABLED, REQUIRES_RECONNECT, DELETED, and BLOCKED. - Introduce NotificationEvent model and repository for tracking LinkedIn publication outcomes. - Enhance LinkedInCredentialGateway with refresh token metadata. - Implement ListPublicationsHandler for querying publications with optional filters. - Update API to include new endpoints for managing publication statuses and notifications. - Create Liquibase migration for new database columns related to publications and notifications.
This comment has been minimized.
This comment has been minimized.
Contributor
|
Warning Review limit reached
More reviews will be available in 39 minutes and 55 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughImplements LinkedIn publication MVP as a BLOCKED lifecycle extension to the existing publishing bounded context. Adds refresh-aware credential resolution with token refresh and reconnect flows, a periodic blocked-recovery worker scan, durable notification events, a list-publications API endpoint, and frontend reconnect and BLOCKED status UI prompts. Includes Liquibase schema migrations and a full OpenSpec documentation archive covering exploration, proposal, risk analysis, specifications, design, tasks, and verification. ChangesLinkedIn Integration Publication
Sequence Diagram(s)sequenceDiagram
participant Worker as PublishingWorker
participant Executor as PublishingJobExecutor
participant PubRepo as PublicationRepository
participant Resolver as RefreshAwareCredentialResolverImpl
participant LinkedIn as LinkedIn REST API
participant NotifRepo as NotificationEventRepository
Worker->>Executor: executeClaim(job)
Executor->>PubRepo: load SocialConnection status
alt DISABLED or REQUIRES_RECONNECT
Executor->>PubRepo: markBlocked(publication)
Executor->>NotifRepo: record(PUBLICATION_BLOCKED)
Executor-->>Worker: job completed
else DELETED
Executor->>PubRepo: markFailed(publication)
Executor->>NotifRepo: record(PUBLICATION_FAILED)
Executor-->>Worker: job completed
else ACTIVE
Executor->>Resolver: resolve(account)
alt token expired or approaching refresh-ahead
Resolver->>LinkedIn: POST /oauth/v2/accessToken
LinkedIn-->>Resolver: new access token
Resolver->>PubRepo: storeForOwner(credentials)
end
Resolver-->>Executor: access token
Executor->>LinkedIn: POST /rest/posts (publish)
LinkedIn-->>Executor: postUrn
Executor->>PubRepo: markPublished(publicUrl)
Executor->>NotifRepo: record(PUBLICATION_SUCCESS)
Executor-->>Worker: job completed
end
Note over Worker: Later... scheduled blockedRecoveryInterval
Worker->>Worker: scanBlockedForRecovery()
Worker->>PubRepo: findBlockedForRecovery(maxRetries)
loop each eligible blocked publication
Worker->>Worker: prepareBlockedRetry (exponential backoff)
Worker->>PubRepo: updateEditableDraft
Worker->>Worker: replaceForPublication (new PENDING job)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Deploying profiletailors with
|
| Latest commit: |
041ac48
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4ed0196a.profiletailors-com.pages.dev |
| Branch Preview URL: | https://feature-linkedin-integration.profiletailors-com.pages.dev |
github-actions
Bot
added
area:backend
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
server/smp/src/main/resources/db/changelog/dev/001-seed-test-data.yaml (1)
3-5:⚠️ Potential issue | 🔴 CriticalConfigure Liquibase to activate the
devcontext only when the dev profile is active.The changeset specifies
context: dev(line 5), but the Liquibase configuration inapplication.yamldoes not set acontextsproperty. Without this configuration, thecontext: devspecification in the changeset is ignored, and the seed data will execute in all environments including production.Add the following to the Liquibase configuration in
application.yaml:Required configuration change
spring: liquibase: enabled: ${SMP_LIQUIBASE_ENABLED:true} change-log: classpath:db/changelog/db.changelog-master.yaml url: ${SMP_LIQUIBASE_JDBC_URL:jdbc:postgresql://localhost:5432/profiletailors_smp} user: ${SMP_DB_USERNAME:profiletailors} password: ${SMP_DB_PASSWORD:CHANGE_ME} contexts: ${SMP_LIQUIBASE_CONTEXTS:}Then, in the dev profile section (after line 116), add:
spring: liquibase: contexts: dev🤖 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 `@server/smp/src/main/resources/db/changelog/dev/001-seed-test-data.yaml` around lines 3 - 5, The changeset with id dev-001-seed-test-data specifies context: dev but the Liquibase configuration in application.yaml does not define a contexts property, causing the context specification to be ignored and seed data to execute in all environments including production. Add a contexts property to the main Liquibase configuration in application.yaml that uses a parameterized default (empty), then override this in the dev profile section of application.yaml to set contexts: dev. This ensures that the dev context is only activated when the dev profile is active, preventing unwanted seed data execution in production environments.openspec/specs/publishing/spec.md (1)
211-225:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUnify the LinkedIn media upload contract across both spec copies.
Both spec files still carry the archived
/assetsupload flow, while the new delta below moves media ingestion to/rest/images//rest/posts. Please either mark the older block as historical/non-normative or update it in both places; otherwise the archive and canonical spec tell implementers to use incompatible upload APIs.🤖 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 `@openspec/specs/publishing/spec.md` around lines 211 - 225, The LinkedIn asset upload contract is defined inconsistently across two spec files: the anchor in openspec/specs/publishing/spec.md (lines 211-225) and the sibling in openspec/changes/archive/2026-06-16-linkedin-integration-publication/specs/publishing/spec.md (lines 211-225). Both currently document the older POST /assets and PUT upload URL flow, but the new implementation uses POST /rest/images and /rest/posts endpoints. Either mark the archived `/assets` flow documentation as historical and non-normative in both locations to clarify it is legacy, or update both spec files to document the current `/rest/images`/`/rest/posts` API contract so implementers receive consistent guidance on which LinkedIn API to use.openspec/changes/archive/2026-06-16-linkedin-integration-publication/design.md (1)
320-333:⚠️ Potential issue | 🟡 MinorCode implementation is correct; operational runbook documentation missing.
The
LinkedIn-Versionheader is properly configurable:
- ✓ Configured in
application.yamlviaapi-version: ${SMP_LINKEDIN_API_VERSION:202606}- ✓ Read from
LinkedInPublishingProperties.apiVersionin the adapter- ✓ Applied to outbound requests:
RealLinkedInPublisher(line 245) andRealLinkedInAssetUploader(lines 100, 139)- ✓ No hardcoded version strings in the LinkedIn adapter code
However, the operational runbook documented in the design (quarterly review cadence, deprecation monitoring, deployment procedure) is not present in the repository. Create the operational runbook per design requirements before deployment.
🤖 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 `@openspec/changes/archive/2026-06-16-linkedin-integration-publication/design.md` around lines 320 - 333, The code implementation correctly makes the LinkedIn-Version header configurable through application.yaml and LinkedInPublishingProperties without hardcoding, but the operational runbook specified in the design document is missing from the repository. Create an operational runbook document that includes the quarterly review cadence for checking LinkedIn API version deprecations, the process for updating the version value when new LinkedIn API versions are released, monitoring procedures for LinkedIn API response headers and deprecation warnings, and the deployment procedure for updating the version without downtime. This runbook should be added to the repository as part of the deployment documentation to fulfill the design requirements before this feature goes live.server/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/scheduling/PublishingSchedulingConfiguration.kt (1)
35-37:⚠️ Potential issue | 🟠 Major | ⚡ Quick winIncrease scheduler pool size for dual-loop execution.
Line 35 sets
poolSize = 1, but the lifecycle now runs both polling and blocked-recovery fixed-rate tasks. A long poll cycle can starve recovery scans. Use at least 2 threads (or separate schedulers) to avoid starvation.Suggested patch
- poolSize = 1 + poolSize = 2🤖 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 `@server/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/scheduling/PublishingSchedulingConfiguration.kt` around lines 35 - 37, The poolSize is currently set to 1 in the PublishingSchedulingConfiguration, but the scheduler now executes both polling and blocked-recovery fixed-rate tasks. With only one thread, a long poll cycle will starve the recovery scans from executing. Increase the poolSize value to at least 2 threads to allow concurrent execution of both tasks, preventing starvation of the recovery scan operations.server/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/scheduling/PublishingWorker.kt (1)
349-356:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEmit a notification event on terminal publish failure.
The terminal failure path marks publication/job failed but does not record a
NotificationEvent. This leaves durable outcome tracking incomplete for provider failures and retry exhaustion.Suggested patch
} else { publicationRepository.markFailed( publication.id, now, exception::class.simpleName, exception.message ) publicationJobRepository.fail(claim.jobId, now) + recordNotificationEvent( + workspaceId = publication.workspaceId, + socialAccountId = publication.socialAccountId, + publicationId = publication.id, + category = NotificationCategory.PUBLICATION_FAILED, + message = exception.message ?: "Publication failed", + now = now, + provider = publication.provider, + ) }🤖 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 `@server/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/scheduling/PublishingWorker.kt` around lines 349 - 356, The terminal failure path in PublishingWorker marks the publication and job as failed using publicationRepository.markFailed() and publicationJobRepository.fail() but does not emit a NotificationEvent to complete durable outcome tracking. After both repository calls in this failure block (around lines 349-356), add a call to emit a NotificationEvent that captures the terminal failure state, ensuring the outcome is durably recorded for provider failures and retry exhaustion scenarios.server/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/linkedin/LinkedInPublishingAdapters.kt (1)
164-188:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
enabledBundlesis currently a no-op, so capability gating is bypassed.
LinkedInCapabilityValidatoracceptsenabledBundlesbut never checks them invalidate(...). That allows gated capabilities (e.g., video) to pass validation even when only personal text/image bundles are configured.Proposed fix
class LinkedInCapabilityValidator( private val enabledBundles: Set<com.profiletailors.smp.publishing.domain.LinkedinCapabilityBundle> = setOf( com.profiletailors.smp.publishing.domain.LinkedinCapabilityBundle.PERSONAL_PROFILE_TEXT, com.profiletailors.smp.publishing.domain.LinkedinCapabilityBundle.PERSONAL_PROFILE_IMAGE, ), ) : ProviderCapabilityValidator { override fun validate(input: ProviderCapabilityValidationInput) { @@ validateMediaTypes(input) + validateEnabledCapability(input) validateFileSizes(input) } + + private fun validateEnabledCapability(input: ProviderCapabilityValidationInput) { + val requiredBundle = when { + input.assets.isEmpty() -> + com.profiletailors.smp.publishing.domain.LinkedinCapabilityBundle.PERSONAL_PROFILE_TEXT + input.assets.any { it.mediaType.equals("video/mp4", ignoreCase = true) } -> + com.profiletailors.smp.publishing.domain.LinkedinCapabilityBundle.VIDEO + input.assets.all { it.mediaType.startsWith("image/", ignoreCase = true) } -> + com.profiletailors.smp.publishing.domain.LinkedinCapabilityBundle.PERSONAL_PROFILE_IMAGE + else -> return + } + if (requiredBundle !in enabledBundles) { + throw PublicationValidationException("LinkedIn capability not enabled: $requiredBundle") + } + }🤖 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 `@server/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/linkedin/LinkedInPublishingAdapters.kt` around lines 164 - 188, The LinkedInCapabilityValidator class accepts an enabledBundles parameter but does not use it in the validate method, which bypasses capability gating and allows unsupported asset types to pass validation. Add validation logic within the validate method that checks each asset in input.assets against the enabledBundles collection and throws a PublicationValidationException if any asset type is not supported by the currently enabled bundles. Map the asset media types to their corresponding LinkedinCapabilityBundle types and verify they are present in the enabledBundles set before allowing validation to proceed.
🤖 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 `@apps/web/app/src/stores/publishing.ts`:
- Line 33: The Channel.status type definition is missing legacy reconnect
statuses (REVOKED and EXPIRED) that the backend still emits. Add REVOKED and
EXPIRED to the Channel.status union type on line 33. Additionally, update the
reconnect detection logic in the selector around lines 265-270 to check for
these legacy statuses (REVOKED and EXPIRED) in addition to REQUIRES_RECONNECT
when determining if reconnection is needed. The type assertion on line 155 will
be automatically corrected once the Channel.status type properly includes these
statuses.
- Around line 51-52: The Publication interface declares the blockedReason field
but the calendar mapper that hydrates Publication objects from API responses
does not extract and populate this field. Locate the calendar mapper function
that transforms API response data into Publication objects (likely in a mapping
or transformation utility), and add the mapping logic to populate the
blockedReason field from the corresponding API response field whenever a
publication has a BLOCKED status. This ensures that server-loaded BLOCKED
publications retain the blockedReason information needed by dependent UI logic.
In
`@openspec/changes/archive/2026-06-16-linkedin-integration-publication/verify-report.md`:
- Around line 69-70: The verification report marks the "Expiring access token
refreshes automatically through resolver" row as COMPLIANT, but the cited test
RefreshAwareCredentialResolverTest.returns access token when token is not
expired only validates the non-expired path, not an actual automatic refresh
scenario. Change the compliance status from COMPLIANT to PARTIAL or UNTESTED for
this row unless a test exists that actually exercises the refresh path, and
update the final verdict wording to accurately reflect that automatic refresh
behavior is not verified by the current test evidence. This same alignment issue
also affects lines 159-163, so apply the same corrections there as well.
In
`@server/smp/src/main/kotlin/com/profiletailors/smp/publishing/application/PublishingHandlers.kt`:
- Around line 624-635: The pagination is currently being performed in memory
after fetching all matching publications, which makes list operations O(N) and
creates a performance bottleneck for large workspaces. Modify the
`publicationRepository.findInDateRange()` method signature to accept offset and
limit parameters (or create a separate paginated variant), pass `query.offset`
and `query.limit` directly to the repository call instead of using in-memory
`.drop()` and `.take()` operations, and ensure the method returns both the
paginated results and the total count of all matching records separately so only
the requested page of data is fetched from the database. Apply this same pattern
to the other affected location mentioned in the comment.
- Around line 639-641: The code instantiating the `now` variable uses
`java.time.Clock.systemUTC().instant()` directly instead of leveraging a
dependency-injected clock, reducing testability and determinism. Replace the
direct Clock.systemUTC() call with the injected clock instance that should be
available in the PublishingHandlers class (likely as a constructor parameter or
class property), then call .instant() on that injected clock to maintain the
same assignment pattern for the `now` variable.
In
`@server/smp/src/main/kotlin/com/profiletailors/smp/publishing/domain/PublishingPolicies.kt`:
- Around line 153-160: The `markBlocked` function in PublishingPolicies.kt
currently only validates that the publication is not in terminal states, but it
should restrict transitions to only in-flight publishing states. The current
check allows invalid state transitions from DRAFT and FAILED states into
BLOCKED. Add an additional validation condition that ensures the publication
status is in an in-flight publish state (such as actively publishing or
in-progress states) before allowing the transition to BLOCKED. This will prevent
the lifecycle from accepting invalid state paths outside the normal publish
pipeline.
In
`@server/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/credentials/RefreshAwareCredentialResolverImpl.kt`:
- Around line 105-109: The issue is that executeRefresh() returns null for both
transient failures (timeouts, IO errors, interruptions) and permanent failures
(invalid credentials), but attemptRefresh() treats all null returns as
INVALID_GRANT errors, forcing unnecessary reconnect flows. To fix this, modify
the error handling logic in the executeRefresh() method to distinguish between
transient and permanent failures by either catching and re-raising specific
exceptions for transient errors, or by having executeRefresh() return an error
type (like a Result/Either wrapper) that captures whether the failure is
transient or permanent, then update the code that calls executeRefresh() to
route transient failures to retry/backoff logic instead of immediately throwing
ReconnectRequiredException with INVALID_GRANT. This applies to all call sites
where executeRefresh() is used and its null return is converted to
ReconnectRequiredException.
- Around line 104-123: The credentialReference (treated as a credential row id
at the start of the method in RefreshAwareCredentialResolverImpl) is being
incorrectly passed as an ownerId to credentialGateway.storeForOwner() at the
end. Since storeForOwner() upserts by (owner_type, owner_id) and generates a new
random id on insert, this breaks the optimistic locking pattern and can create
divergent credential records. Replace the storeForOwner call with a method that
updates by the actual credential row id (credentialReference) and includes
version-based optimistic locking to ensure you are updating the correct row that
was read at the beginning of the refresh operation.
In
`@server/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/http/PublishingControllers.kt`:
- Around line 285-286: The limit and offset request parameters in this
controller method lack validation, allowing arbitrary integers including
negative values or unbounded sizes that can cause expensive queries and
downstream failures. Add validation at the controller boundary (within the
method containing these parameters) to enforce reasonable bounds: ensure limit
is positive and within a maximum threshold (for example, reject values less than
or equal to 0 or greater than 1000), and ensure offset is non-negative (greater
than or equal to 0). Apply these validations immediately after the parameters
are received but before they are used in any service calls.
In
`@server/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/persistence/R2dbcPublishingRepositories.kt`:
- Around line 284-310: The findBlockedForRecovery method returns
PublicationDraft objects through toPublicationDraft() which sets assetIds to an
empty list, causing data loss when these drafts are later passed to
updateEditableDraft where replaceAssetLinks deletes and reinserts asset links
based on the empty assetIds. Load and hydrate the asset IDs for each
PublicationDraft returned by findBlockedForRecovery before returning them to the
caller, ensuring the assetIds field is populated with actual linked asset
identifiers so that replaceAssetLinks preserves the existing asset links during
recovery.
In
`@server/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/scheduling/PublishingWorker.kt`:
- Around line 402-414: The current code makes two separate repository calls
where publicationRepository.updateEditableDraft(prepared) is executed before
publicationJobRepository.replaceForPublication(), which violates atomicity. If
the second call fails, the draft is left updated with no corresponding job,
stranding the publication. Create a new atomic repository method that performs
both the draft update and job replacement in a single transaction, then replace
these two separate calls with this single atomic operation. This ensures either
both mutations succeed together or both fail together, preventing inconsistent
state.
- Around line 387-389: At
server/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/scheduling/PublishingWorker.kt
lines 387-389, wrap the requeueBlockedPublication(publication) call inside the
forEach iteration with a try-catch block to handle exceptions per-publication,
ensuring the loop continues processing remaining publications instead of
aborting on the first error. Apply the same per-item exception handling pattern
at lines 436-443 for the scheduled task callbacks with coroutines to prevent
uncaught exceptions from halting the scheduler. Additionally, configure an error
handler on the ThreadPoolTaskScheduler bean to gracefully handle any uncaught
exceptions in scheduled executions and prevent executor shutdown.
In
`@server/smp/src/main/resources/db/changelog/publishing/013-linkedin-integration-publication.yaml`:
- Around line 6-41: The changeSet with id
publishing-013-add-publications-columns adds columns to the publications table
but does not create an index to support the findBlockedForRecovery query. Add a
new createIndex change within this same changeSet to create an index on the
publications table covering the status, retry_count, and blocked_at columns (in
that order) to prevent full-table scans. Name the index
idx_publications_blocked_recovery. Also add the corresponding dropIndex to the
rollback section to drop this index when the migration is rolled back.
In
`@server/smp/src/test/kotlin/com/profiletailors/smp/publishing/domain/PublicationLifecyclePolicyTest.kt`:
- Around line 293-305: The test function prepareBlockedRetry caps delay at 60
minutes has a misleading name that does not match its actual assertions. The
test currently uses retryCount = 4, which produces a 16-minute delay that is
under the 60-minute cap, not testing the cap itself. Either rename the test
function to accurately reflect that it tests a delay under the cap (such as
prepareBlockedRetry uses exponential backoff under cap), or change the
retryCount to a higher value (such as 10 or more) that would trigger the
60-minute cap behavior and update the assertion to expect 60 minutes instead of
16 minutes.
In
`@server/smp/src/test/kotlin/com/profiletailors/smp/publishing/infrastructure/credentials/RefreshAwareCredentialResolverTest.kt`:
- Around line 223-226: The StubLinkedInHttpTransport class always throws an
exception, preventing any testing of the actual refresh logic in executeRefresh
and attemptRefresh methods. Create new test cases that use a controllable stub
(that can return different HTTP responses instead of always throwing) to
validate three key scenarios: successful refresh response parsing, transient
transport failure mapping, and non-2xx response handling. This ensures the
refresh execution outcomes are properly validated in the test suite.
In
`@server/smp/src/test/kotlin/com/profiletailors/smp/publishing/infrastructure/http/PublishingControllersTest.kt`:
- Around line 302-313: The test method `list publications delegates to mediator`
only verifies the response returned by the controller but doesn't assert that
the correct query was dispatched to the mediator. Add an assertion after the
response is obtained to verify that the CapturingMediator captured the expected
ListPublicationsQuery object. This ensures the controller properly maps the
request to the appropriate query command sent to the mediator, rather than just
relying on the canned response the mediator returns.
In
`@server/smp/src/test/kotlin/com/profiletailors/smp/publishing/infrastructure/scheduling/PublishingWorkerTest.kt`:
- Around line 340-444: Replace the `notificationEventRepository = null`
parameter with an in-memory implementation (like
InMemoryNotificationEventRepository) in all three test methods: the DISABLED
account test, the REQUIRES_RECONNECT account test, and the DELETED account test.
Then add assertions after each worker.pollOnce() call to verify that the correct
notification events were emitted — assert for PUBLICATION_BLOCKED events in the
first two tests and PUBLICATION_FAILED events in the third test. This ensures
regressions in event emission for each account status path will be caught by the
tests.
---
Outside diff comments:
In
`@openspec/changes/archive/2026-06-16-linkedin-integration-publication/design.md`:
- Around line 320-333: The code implementation correctly makes the
LinkedIn-Version header configurable through application.yaml and
LinkedInPublishingProperties without hardcoding, but the operational runbook
specified in the design document is missing from the repository. Create an
operational runbook document that includes the quarterly review cadence for
checking LinkedIn API version deprecations, the process for updating the version
value when new LinkedIn API versions are released, monitoring procedures for
LinkedIn API response headers and deprecation warnings, and the deployment
procedure for updating the version without downtime. This runbook should be
added to the repository as part of the deployment documentation to fulfill the
design requirements before this feature goes live.
In `@openspec/specs/publishing/spec.md`:
- Around line 211-225: The LinkedIn asset upload contract is defined
inconsistently across two spec files: the anchor in
openspec/specs/publishing/spec.md (lines 211-225) and the sibling in
openspec/changes/archive/2026-06-16-linkedin-integration-publication/specs/publishing/spec.md
(lines 211-225). Both currently document the older POST /assets and PUT upload
URL flow, but the new implementation uses POST /rest/images and /rest/posts
endpoints. Either mark the archived `/assets` flow documentation as historical
and non-normative in both locations to clarify it is legacy, or update both spec
files to document the current `/rest/images`/`/rest/posts` API contract so
implementers receive consistent guidance on which LinkedIn API to use.
In
`@server/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/linkedin/LinkedInPublishingAdapters.kt`:
- Around line 164-188: The LinkedInCapabilityValidator class accepts an
enabledBundles parameter but does not use it in the validate method, which
bypasses capability gating and allows unsupported asset types to pass
validation. Add validation logic within the validate method that checks each
asset in input.assets against the enabledBundles collection and throws a
PublicationValidationException if any asset type is not supported by the
currently enabled bundles. Map the asset media types to their corresponding
LinkedinCapabilityBundle types and verify they are present in the enabledBundles
set before allowing validation to proceed.
In
`@server/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/scheduling/PublishingSchedulingConfiguration.kt`:
- Around line 35-37: The poolSize is currently set to 1 in the
PublishingSchedulingConfiguration, but the scheduler now executes both polling
and blocked-recovery fixed-rate tasks. With only one thread, a long poll cycle
will starve the recovery scans from executing. Increase the poolSize value to at
least 2 threads to allow concurrent execution of both tasks, preventing
starvation of the recovery scan operations.
In
`@server/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/scheduling/PublishingWorker.kt`:
- Around line 349-356: The terminal failure path in PublishingWorker marks the
publication and job as failed using publicationRepository.markFailed() and
publicationJobRepository.fail() but does not emit a NotificationEvent to
complete durable outcome tracking. After both repository calls in this failure
block (around lines 349-356), add a call to emit a NotificationEvent that
captures the terminal failure state, ensuring the outcome is durably recorded
for provider failures and retry exhaustion scenarios.
In `@server/smp/src/main/resources/db/changelog/dev/001-seed-test-data.yaml`:
- Around line 3-5: The changeset with id dev-001-seed-test-data specifies
context: dev but the Liquibase configuration in application.yaml does not define
a contexts property, causing the context specification to be ignored and seed
data to execute in all environments including production. Add a contexts
property to the main Liquibase configuration in application.yaml that uses a
parameterized default (empty), then override this in the dev profile section of
application.yaml to set contexts: dev. This ensures that the dev context is only
activated when the dev profile is active, preventing unwanted seed data
execution in production environments.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b5f7a7f0-b00a-499b-98f1-accbda9a4862
⛔ Files ignored due to path filters (1)
server/smp/src/main/resources/db/changelog/data/dev/local_password_credentials_dev.csvis excluded by!**/*.csv
📒 Files selected for processing (40)
.codegraph/.gitignore.env.exampleapps/web/app/src/stores/publishing.tsapps/web/app/src/views/SchedulerView.vueopenspec/changes/archive/2026-06-16-linkedin-integration-publication/design.mdopenspec/changes/archive/2026-06-16-linkedin-integration-publication/exploration.mdopenspec/changes/archive/2026-06-16-linkedin-integration-publication/proposal.mdopenspec/changes/archive/2026-06-16-linkedin-integration-publication/risk-analysis.mdopenspec/changes/archive/2026-06-16-linkedin-integration-publication/specs/publishing/spec.mdopenspec/changes/archive/2026-06-16-linkedin-integration-publication/state.yamlopenspec/changes/archive/2026-06-16-linkedin-integration-publication/tasks.mdopenspec/changes/archive/2026-06-16-linkedin-integration-publication/verify-report.mdopenspec/specs/publishing/spec.mdserver/smp/src/main/kotlin/com/profiletailors/smp/publishing/application/PublishingApi.ktserver/smp/src/main/kotlin/com/profiletailors/smp/publishing/application/PublishingHandlers.ktserver/smp/src/main/kotlin/com/profiletailors/smp/publishing/domain/NotificationEvent.ktserver/smp/src/main/kotlin/com/profiletailors/smp/publishing/domain/PublishingModels.ktserver/smp/src/main/kotlin/com/profiletailors/smp/publishing/domain/PublishingPolicies.ktserver/smp/src/main/kotlin/com/profiletailors/smp/publishing/domain/PublishingProviderPorts.ktserver/smp/src/main/kotlin/com/profiletailors/smp/publishing/domain/PublishingRepositories.ktserver/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/credentials/LinkedInCredentialGateway.ktserver/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/credentials/RefreshAwareCredentialResolverImpl.ktserver/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/http/PublishingControllers.ktserver/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/linkedin/LinkedInAssetUploaderAdapters.ktserver/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/linkedin/LinkedInPublishingAdapters.ktserver/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/persistence/R2dbcNotificationEventRepository.ktserver/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/persistence/R2dbcPublishingConnectionRepositories.ktserver/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/persistence/R2dbcPublishingRepositories.ktserver/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/scheduling/PublishingSchedulingConfiguration.ktserver/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/scheduling/PublishingWorker.ktserver/smp/src/main/resources/application.yamlserver/smp/src/main/resources/db/changelog/db.changelog-master.yamlserver/smp/src/main/resources/db/changelog/dev/001-seed-test-data.yamlserver/smp/src/main/resources/db/changelog/publishing/013-linkedin-integration-publication.yamlserver/smp/src/test/kotlin/com/profiletailors/smp/publishing/application/PublishingHandlersTest.ktserver/smp/src/test/kotlin/com/profiletailors/smp/publishing/domain/PublicationLifecyclePolicyTest.ktserver/smp/src/test/kotlin/com/profiletailors/smp/publishing/infrastructure/credentials/RefreshAwareCredentialResolverTest.ktserver/smp/src/test/kotlin/com/profiletailors/smp/publishing/infrastructure/http/PublishingControllersTest.ktserver/smp/src/test/kotlin/com/profiletailors/smp/publishing/infrastructure/linkedin/LinkedInPublishingAdaptersTest.ktserver/smp/src/test/kotlin/com/profiletailors/smp/publishing/infrastructure/scheduling/PublishingWorkerTest.kt
💤 Files with no reviewable changes (1)
- server/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/persistence/R2dbcPublishingConnectionRepositories.kt
Comment on lines
+51
to
+52
| /** Reason the publication was blocked (e.g., account DISABLED or REQUIRES_RECONNECT). */ | ||
| blockedReason?: string |
Contributor
There was a problem hiding this comment.
blockedReason is declared but not populated from API results.
Lines 51-52 add Publication.blockedReason, but the calendar mapper never hydrates it. Server-loaded BLOCKED publications therefore lose this field and dependent UI logic won’t receive the reason.
Proposed fix
export interface CalendarPublicationResult {
id: string
workspaceId: string
socialAccountId: string
provider: string
status: string
scheduleMode: string
priority: boolean
title: string | null
bodyText: string | null
scheduledFor: string | null
+ blockedReason?: string | null
hasConflict: boolean
conflictingPublicationIds: string[]
}
function apiResultToPublication(api: CalendarPublicationResult): Publication {
return {
@@
hasConflict: api.hasConflict,
conflictingPublicationIds: api.conflictingPublicationIds,
accountId: api.socialAccountId,
+ blockedReason: api.blockedReason ?? undefined,
}
}🤖 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/app/src/stores/publishing.ts` around lines 51 - 52, The Publication
interface declares the blockedReason field but the calendar mapper that hydrates
Publication objects from API responses does not extract and populate this field.
Locate the calendar mapper function that transforms API response data into
Publication objects (likely in a mapping or transformation utility), and add the
mapping logic to populate the blockedReason field from the corresponding API
response field whenever a publication has a BLOCKED status. This ensures that
server-loaded BLOCKED publications retain the blockedReason information needed
by dependent UI logic.
Comment on lines
+624
to
+635
| val publications = publicationRepository.findInDateRange( | ||
| workspaceId = workspaceId, | ||
| from = from, | ||
| to = to, | ||
| statuses = statuses, | ||
| socialAccountIds = accountIds, | ||
| ) | ||
| val items = publications.drop(query.offset).take(query.limit).map { it.toListItem() } | ||
| return ListPublicationsResponse( | ||
| publications = items, | ||
| total = publications.size, | ||
| ) |
Contributor
There was a problem hiding this comment.
Push pagination/counting into the repository instead of paginating in memory.
Line 624 and Line 644 fetch the full range and only then apply offset/limit. That makes list calls O(N) per request and can become a latency/memory hotspot for large workspaces.
Suggested direction
- val publications = publicationRepository.findInDateRange(...)
- val items = publications.drop(query.offset).take(query.limit).map { it.toListItem() }
- return ListPublicationsResponse(publications = items, total = publications.size)
+ val total = publicationRepository.countInDateRange(...)
+ val publications = publicationRepository.findInDateRangePaged(
+ ...,
+ limit = query.limit,
+ offset = query.offset,
+ )
+ return ListPublicationsResponse(
+ publications = publications.map { it.toListItem() },
+ total = total,
+ )Also applies to: 644-655
🤖 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
`@server/smp/src/main/kotlin/com/profiletailors/smp/publishing/application/PublishingHandlers.kt`
around lines 624 - 635, The pagination is currently being performed in memory
after fetching all matching publications, which makes list operations O(N) and
creates a performance bottleneck for large workspaces. Modify the
`publicationRepository.findInDateRange()` method signature to accept offset and
limit parameters (or create a separate paginated variant), pass `query.offset`
and `query.limit` directly to the repository call instead of using in-memory
`.drop()` and `.take()` operations, and ensure the method returns both the
paginated results and the total count of all matching records separately so only
the requested page of data is fetched from the database. Apply this same pattern
to the other affected location mentioned in the comment.
Comment on lines
+639
to
+641
| val now = java.time.Clock.systemUTC().instant() | ||
| val broadFrom = from ?: now.minus(java.time.Duration.ofDays(90)) | ||
| val broadTo = to ?: now.plus(java.time.Duration.ofDays(30)) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Use the injected clock here instead of Clock.systemUTC().
Line 639 bypasses dependency-injected time, making behavior less deterministic and harder to test consistently with the rest of the handlers.
Proposed adjustment
internal class ListPublicationsHandler(
private val resourceContextProvider: ResourceContextProvider,
private val publicationRepository: PublicationRepository,
+ private val clock: Clock,
) : QueryHandler<ListPublicationsQuery, ListPublicationsResponse> {
- val now = java.time.Clock.systemUTC().instant()
+ val now = clock.instant()🤖 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
`@server/smp/src/main/kotlin/com/profiletailors/smp/publishing/application/PublishingHandlers.kt`
around lines 639 - 641, The code instantiating the `now` variable uses
`java.time.Clock.systemUTC().instant()` directly instead of leveraging a
dependency-injected clock, reducing testability and determinism. Replace the
direct Clock.systemUTC() call with the injected clock instance that should be
available in the PublishingHandlers class (likely as a constructor parameter or
class property), then call .instant() on that injected clock to maintain the
same assignment pattern for the `now` variable.
Comment on lines
+340
to
+444
| fun `preflight blocks DISABLED account without calling publisher`() = runTest { | ||
| val disabledAccount = successAccount().copy(status = SocialConnectionStatus.DISABLED) | ||
| val publication = successPublication() | ||
| val publicationRepository = InMemoryPublicationRepository(publication) | ||
| val jobRepository = InMemoryJobRepository(PublicationJobClaim("job-1", "pub-1", "workspace-1", 1, fixedClock.instant())) | ||
| val publisher = NeverPublishesPublisher() | ||
| val executor = PublishingJobExecutor( | ||
| publicationJobRepository = jobRepository, | ||
| publicationRepository = publicationRepository, | ||
| socialAccountRepository = InMemoryAccountRepository(disabledAccount), | ||
| publicationAssetRepository = InMemoryAssetRepository(emptyList()), | ||
| deliveryAttemptRepository = InMemoryAttemptRepository(), | ||
| notificationEventRepository = null, | ||
| providerCapabilityValidator = AcceptingCapabilityValidator(), | ||
| socialPublisher = publisher, | ||
| retryPolicy = DeliveryRetryPolicy(3, Duration.ofMinutes(5)), | ||
| clock = fixedClock, | ||
| ) | ||
| val worker = PublishingWorker( | ||
| publicationJobRepository = jobRepository, | ||
| publicationRepository = publicationRepository, | ||
| executor = executor, | ||
| clock = fixedClock, | ||
| workerId = "worker-1", | ||
| ) | ||
|
|
||
| worker.pollOnce() | ||
|
|
||
| // Publisher was never called | ||
| assertEquals(false, publisher.called) | ||
| // Publication was marked blocked | ||
| assertEquals("pub-1", publicationRepository.blockedPublicationId) | ||
| // Job was completed (not left hanging) | ||
| assertEquals("job-1", jobRepository.completedJobId) | ||
| } | ||
|
|
||
| @Test | ||
| fun `preflight blocks REQUIRES_RECONNECT account without calling publisher`() = runTest { | ||
| val reconnectAccount = successAccount().copy(status = SocialConnectionStatus.REQUIRES_RECONNECT) | ||
| val publication = successPublication() | ||
| val publicationRepository = InMemoryPublicationRepository(publication) | ||
| val jobRepository = InMemoryJobRepository(PublicationJobClaim("job-1", "pub-1", "workspace-1", 1, fixedClock.instant())) | ||
| val publisher = NeverPublishesPublisher() | ||
| val executor = PublishingJobExecutor( | ||
| publicationJobRepository = jobRepository, | ||
| publicationRepository = publicationRepository, | ||
| socialAccountRepository = InMemoryAccountRepository(reconnectAccount), | ||
| publicationAssetRepository = InMemoryAssetRepository(emptyList()), | ||
| deliveryAttemptRepository = InMemoryAttemptRepository(), | ||
| notificationEventRepository = null, | ||
| providerCapabilityValidator = AcceptingCapabilityValidator(), | ||
| socialPublisher = publisher, | ||
| retryPolicy = DeliveryRetryPolicy(3, Duration.ofMinutes(5)), | ||
| clock = fixedClock, | ||
| ) | ||
| val worker = PublishingWorker( | ||
| publicationJobRepository = jobRepository, | ||
| publicationRepository = publicationRepository, | ||
| executor = executor, | ||
| clock = fixedClock, | ||
| workerId = "worker-1", | ||
| ) | ||
|
|
||
| worker.pollOnce() | ||
|
|
||
| assertEquals(false, publisher.called) | ||
| assertEquals("pub-1", publicationRepository.blockedPublicationId) | ||
| assertEquals("job-1", jobRepository.completedJobId) | ||
| } | ||
|
|
||
| @Test | ||
| fun `preflight fails DELETED account terminally without calling publisher`() = runTest { | ||
| val deletedAccount = successAccount().copy(status = SocialConnectionStatus.DELETED) | ||
| val publication = successPublication() | ||
| val publicationRepository = InMemoryPublicationRepository(publication) | ||
| val jobRepository = InMemoryJobRepository(PublicationJobClaim("job-1", "pub-1", "workspace-1", 1, fixedClock.instant())) | ||
| val publisher = NeverPublishesPublisher() | ||
| val executor = PublishingJobExecutor( | ||
| publicationJobRepository = jobRepository, | ||
| publicationRepository = publicationRepository, | ||
| socialAccountRepository = InMemoryAccountRepository(deletedAccount), | ||
| publicationAssetRepository = InMemoryAssetRepository(emptyList()), | ||
| deliveryAttemptRepository = InMemoryAttemptRepository(), | ||
| notificationEventRepository = null, | ||
| providerCapabilityValidator = AcceptingCapabilityValidator(), | ||
| socialPublisher = publisher, | ||
| retryPolicy = DeliveryRetryPolicy(3, Duration.ofMinutes(5)), | ||
| clock = fixedClock, | ||
| ) | ||
| val worker = PublishingWorker( | ||
| publicationJobRepository = jobRepository, | ||
| publicationRepository = publicationRepository, | ||
| executor = executor, | ||
| clock = fixedClock, | ||
| workerId = "worker-1", | ||
| ) | ||
|
|
||
| worker.pollOnce() | ||
|
|
||
| // Publisher was never called | ||
| assertEquals(false, publisher.called) | ||
| // Publication was failed terminally (not blocked — DELETED is terminal) | ||
| assertEquals("pub-1", publicationRepository.failedPublicationId) | ||
| assertEquals("job-1", jobRepository.failedJobId) | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Cover notification-event emission in preflight tests.
These tests validate publication/job state transitions, but they pass notificationEventRepository = null, so regressions in emitted categories/messages (PUBLICATION_BLOCKED / PUBLICATION_FAILED) won’t be caught. Add an in-memory event repository and assert emitted events per status path.
🤖 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
`@server/smp/src/test/kotlin/com/profiletailors/smp/publishing/infrastructure/scheduling/PublishingWorkerTest.kt`
around lines 340 - 444, Replace the `notificationEventRepository = null`
parameter with an in-memory implementation (like
InMemoryNotificationEventRepository) in all three test methods: the DISABLED
account test, the REQUIRES_RECONNECT account test, and the DELETED account test.
Then add assertions after each worker.pollOnce() call to verify that the correct
notification events were emitted — assert for PUBLICATION_BLOCKED events in the
first two tests and PUBLICATION_FAILED events in the third test. This ensures
regressions in event emission for each account status path will be caught by the
tests.
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. The branch was updated while autofix was in progress. Please try again. |
![kody-ai[bot]](https://avatars.githubusercontent.com/in/413034?s=60&v=4){kind=small}
Comment on lines
+35
to
+36
| # Dev-only: hardcoded hash for "S3cr3tP@ssw0rd*123" — do not use in production. | ||
| - column: { name: password_hash, value: "$2b$12$Lh38sBH4uUHd6hKDM4dTreK7l/KaMo9wQKZijLzfvDWVic7qZS2Z2" } |
There was a problem hiding this comment.
Hardcoded secret in 001-seed-test-data.yaml, RefreshAwareCredentialResolverTest.kt, and LinkedInPublishingAdapters.kt replaces the ${dev-password-hash} parameter with a literal bcrypt hash. Revert to the parameterized value to comply with the prohibition of hardcoded secrets in source control.
Kody rule violation: Prohibit hardcoded secrets in source code
- column: { name: password_hash, value: "${dev-password-hash}" }Prompt for LLM
File server/smp/src/main/resources/db/changelog/dev/001-seed-test-data.yaml:
Line 35 to 36:
Hardcoded password hash committed in source control violates 'Prohibit hardcoded secrets in source code'. The previous parameterized value `${dev-password-hash}` was replaced with a literal bcrypt hash.
Suggested Code:
- column: { name: password_hash, value: "${dev-password-hash}" }
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
1. SchedulerView.vue: handle async reconnect with try/catch - Unhandled promise rejection on network failure when reconnecting - Wrapped connectLinkedInPersonalProfile() in handleReconnect() 2. R2dbcPublishingRepositories.kt: add row locking to recovery scan - Race condition: multiple workers could claim same BLOCKED rows - Added FOR UPDATE SKIP LOCKED to findBlockedForRecovery query 3. PublishingWorker.kt: prevent orphaned QUEUED publications - Non-atomic requeue: if job creation failed, publication left without job - Added try/catch with rollback to BLOCKED on failure - Also added missing notification event on terminal failure
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
server/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/persistence/R2dbcPublishingRepositories.kt (2)
287-301:⚠️ Potential issue | 🔴 CriticalMove the row-lock transaction boundary to include the entire requeue loop.
The
FOR UPDATE OF p SKIP LOCKEDat line 301 holds row locks only for the duration of the SELECT statement's transaction. OncefindBlockedForRecovery()returns and the transaction ends, the locks are released. The subsequentrequeueBlockedPublication()calls inscanBlockedForRecovery()(PublishingWorker.kt:392–403) execute in separate transactions without transaction boundaries, creating a race window where a second worker can invokefindBlockedForRecovery()and claim the same BLOCKED rows. This results in duplicate recovery jobs enqueued for the same publications.Wrap the entire recovery workflow—from
findBlockedForRecovery()through the completion of allrequeueBlockedPublication()calls—in a single transaction to ensure the row locks persist across the select-and-requeue cycle.🤖 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 `@server/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/persistence/R2dbcPublishingRepositories.kt` around lines 287 - 301, The row locks acquired by the FOR UPDATE OF p SKIP LOCKED clause in the SQL query are released immediately after findBlockedForRecovery() returns, allowing concurrent workers to claim the same rows. Extend the transaction boundary to encompass the entire recovery workflow including both the findBlockedForRecovery() call and all subsequent requeueBlockedPublication() invocations in the scanBlockedForRecovery() method in PublishingWorker.kt. Ensure that the transaction scope wraps the select-and-requeue cycle as a single atomic operation so that row locks persist throughout the entire recovery process, preventing duplicate job enqueuing.
293-300:⚠️ Potential issue | 🟠 MajorAdd a scan to terminally fail BLOCKED publications whose accounts transition to DELETED.
The existing
findBlockedForRecoveryrecovery query only returns BLOCKED publications with ACTIVE accounts. This means BLOCKED publications targeting DELETED accounts are never retrieved by the recovery scan and remain BLOCKED indefinitely instead of terminally failing as required.While the preflight validation does terminally fail publications when their account is DELETED (via
failPublicationTerminalinPublishingWorker), this check only applies to publications in claimable job states (PENDING, RETRY_WAITING). BLOCKED publications have completed jobs and are never reclaimed by normal polling, so they bypass the preflight check entirely.A separate scan or query is needed to find BLOCKED publications with DELETED accounts and transition them to FAILED status with terminal failure notification.
🤖 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 `@server/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/persistence/R2dbcPublishingRepositories.kt` around lines 293 - 300, The current `findBlockedForRecovery` query only retrieves BLOCKED publications with ACTIVE accounts due to the filter condition `a.status = :activeStatus`, which means BLOCKED publications associated with DELETED accounts are never found by the recovery scan and remain stuck in BLOCKED status instead of failing terminally as required. Create a separate query method that specifically finds BLOCKED publications linked to DELETED accounts, then implement a corresponding scan or recovery handler that transitions these publications to FAILED status and sends the appropriate terminal failure notification. This ensures BLOCKED publications whose accounts have been deleted are properly cleaned up by the recovery mechanism rather than being ignored indefinitely.
🤖 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
`@server/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/persistence/R2dbcPublishingRepositories.kt`:
- Around line 315-334: The SQL query for assetLinks does not specify an
ordering, causing asset links to be returned in arbitrary database row order.
Modify the SELECT query in the databaseClient.sql() call to include ORDER BY
position_index at the end of the statement, ensuring that when
assetsByPublication groups these links by publication_id, the asset ordering is
preserved according to their position index, preventing asset reordering when
recovery drafts are saved or retried.
In
`@server/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/scheduling/PublishingWorker.kt`:
- Around line 414-435: The updateEditableDraft method is called before
replaceForPublication, which persists the incremented retryCount and updated
schedule to the database. If the replaceForPublication call fails and
markBlocked is invoked, the retry metadata has already been persisted but no job
was created, effectively consuming a retry attempt without scheduling any work.
To fix this, either wrap the updateEditableDraft and replaceForPublication calls
in a database transaction so both operations atomically succeed or fail
together, or alternatively, before calling markBlocked in the exception handler,
explicitly restore the original publication's retryCount and schedule fields to
their pre-update state so that the failed retry attempt does not consume the
retry budget.
In
`@server/smp/src/test/kotlin/com/profiletailors/smp/publishing/application/PublishingHandlersTest.kt`:
- Around line 1217-1233: The test `list publications open-ended uses broad date
range` uses hardcoded fixture dates in June 2026, but the
ListPublicationsHandler.handle method derives its date window from
Clock.systemUTC().instant() at runtime, causing the test to fail once the actual
current date drifts beyond those fixed dates. Either replace the hardcoded
calendarPublication timestamps with runtime-relative dates (e.g., derived from
the current system clock), or capture and assert on the actual from/to query
bounds that the publicationRepository receives when handle is called, rather
than just asserting the result count. This ensures the test remains
deterministic and won't become flaky as real time progresses.
In
`@server/smp/src/test/kotlin/com/profiletailors/smp/publishing/infrastructure/credentials/RefreshAwareCredentialResolverTest.kt`:
- Around line 238-280: The tests in RefreshAwareCredentialResolverTest are
incorrectly treating all refresh failures uniformly as requiring reconnect
(INVALID_GRANT). Update the test methods to distinguish between different
failure types: only actual OAuth invalid_grant errors (expired/revoked refresh
tokens) should throw ReconnectRequiredException with INVALID_GRANT reason, while
other failures like invalid_client, transport/IO errors, timeouts, and HTTP
5xx/429 responses should be tested as retryable failures that do not require
reconnect. Specifically, for the test starting at line 238 (testing
invalid_client error with 401 status), change the assertion from expecting
ReconnectRequiredException to expecting a retryable error response instead.
Apply the same distinction across all affected test methods in the file to
properly separate transient/configuration failures from actual token revocation
scenarios.
---
Outside diff comments:
In
`@server/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/persistence/R2dbcPublishingRepositories.kt`:
- Around line 287-301: The row locks acquired by the FOR UPDATE OF p SKIP LOCKED
clause in the SQL query are released immediately after findBlockedForRecovery()
returns, allowing concurrent workers to claim the same rows. Extend the
transaction boundary to encompass the entire recovery workflow including both
the findBlockedForRecovery() call and all subsequent requeueBlockedPublication()
invocations in the scanBlockedForRecovery() method in PublishingWorker.kt.
Ensure that the transaction scope wraps the select-and-requeue cycle as a single
atomic operation so that row locks persist throughout the entire recovery
process, preventing duplicate job enqueuing.
- Around line 293-300: The current `findBlockedForRecovery` query only retrieves
BLOCKED publications with ACTIVE accounts due to the filter condition `a.status
= :activeStatus`, which means BLOCKED publications associated with DELETED
accounts are never found by the recovery scan and remain stuck in BLOCKED status
instead of failing terminally as required. Create a separate query method that
specifically finds BLOCKED publications linked to DELETED accounts, then
implement a corresponding scan or recovery handler that transitions these
publications to FAILED status and sends the appropriate terminal failure
notification. This ensures BLOCKED publications whose accounts have been deleted
are properly cleaned up by the recovery mechanism rather than being ignored
indefinitely.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7041a7bd-0e39-4862-8c22-ff81923b3cd7
📒 Files selected for processing (7)
apps/web/app/src/views/SchedulerView.vueserver/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/persistence/R2dbcPublishingRepositories.ktserver/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/scheduling/PublishingWorker.ktserver/smp/src/test/kotlin/com/profiletailors/smp/publishing/application/PublishingApiTest.ktserver/smp/src/test/kotlin/com/profiletailors/smp/publishing/application/PublishingHandlersTest.ktserver/smp/src/test/kotlin/com/profiletailors/smp/publishing/domain/NotificationEventTest.ktserver/smp/src/test/kotlin/com/profiletailors/smp/publishing/infrastructure/credentials/RefreshAwareCredentialResolverTest.kt
Comment on lines
+414
to
+435
| // Update publication first; if job replacement fails, revert to BLOCKED to avoid orphaning | ||
| // a QUEUED publication with no matching job (recovery scans filter by BLOCKED). | ||
| publicationRepository.updateEditableDraft(prepared) | ||
| try { | ||
| publicationJobRepository.replaceForPublication( | ||
| com.profiletailors.smp.publishing.domain.PublicationJob( | ||
| id = "pjob-${UUID.randomUUID()}", | ||
| publicationId = prepared.id, | ||
| workspaceId = prepared.workspaceId, | ||
| status = com.profiletailors.smp.publishing.domain.JobStatus.PENDING, | ||
| dueAt = prepared.scheduledFor ?: now, | ||
| priorityRank = 0, | ||
| attemptCount = 0, | ||
| maxAttempts = 1, | ||
| ), | ||
| ) | ||
| } catch (@Suppress("TooGenericExceptionCaught") e: Exception) { | ||
| // Revert publication to BLOCKED so next recovery scan can retry | ||
| log.warn("Failed to replace job for blocked publication ${publication.id}, reverting to BLOCKED", e) | ||
| publicationRepository.markBlocked(publication.id, now, "Retry failed: ${e.message}") | ||
| return | ||
| } |
Contributor
There was a problem hiding this comment.
Rollback must restore retry metadata or be transactional.
updateEditableDraft(prepared) persists the incremented retryCount and retry schedule before job replacement. If replaceForPublication fails, markBlocked only flips status/reason, so failed requeue attempts still consume the max retry budget and can leave a BLOCKED publication with no retry ever scheduled. Put the draft update and job replacement in one transaction, or restore the original draft’s retry/schedule fields on failure.
🤖 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
`@server/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/scheduling/PublishingWorker.kt`
around lines 414 - 435, The updateEditableDraft method is called before
replaceForPublication, which persists the incremented retryCount and updated
schedule to the database. If the replaceForPublication call fails and
markBlocked is invoked, the retry metadata has already been persisted but no job
was created, effectively consuming a retry attempt without scheduling any work.
To fix this, either wrap the updateEditableDraft and replaceForPublication calls
in a database transaction so both operations atomically succeed or fail
together, or alternatively, before calling markBlocked in the exception handler,
explicitly restore the original publication's retryCount and schedule fields to
their pre-update state so that the failed retry attempt does not consume the
retry budget.
Comment on lines
+238
to
+280
| @Test | ||
| fun `executeRefresh returns null on non-2xx causing ReconnectRequiredException`() = runTest { | ||
| val account = testAccount() | ||
| val connectionId = UUID.randomUUID() | ||
| val expiredCredentials = LinkedInCredentials( | ||
| accessToken = "old-token", | ||
| refreshToken = "refresh-token-123", | ||
| expiresAtEpochSeconds = fixedNow.epochSecond - 100, | ||
| scope = "w_member_social", | ||
| ) | ||
| fakeConnectionRepository.addConnection( | ||
| SocialConnection( | ||
| id = "conn-1", | ||
| workspaceId = "ws-1", | ||
| provider = SocialProvider.LINKEDIN, | ||
| providerConnectionRef = "linkedin-member-123", | ||
| status = SocialConnectionStatus.ACTIVE, | ||
| credentialReference = connectionId.toString(), | ||
| ), | ||
| ) | ||
| fakeCredentialGateway.store("linkedin:user", connectionId, expiredCredentials) | ||
|
|
||
| val fakeTransport = ControllableFakeLinkedInHttpTransport( | ||
| response = com.profiletailors.smp.publishing.infrastructure.linkedin.LinkedInHttpResponse( | ||
| statusCode = 401, | ||
| headers = java.net.http.HttpHeaders.of(emptyMap()) { _, _ -> true }, | ||
| body = """{"error":"invalid_client"}""", | ||
| ), | ||
| ) | ||
| resolver = RefreshAwareCredentialResolverImpl( | ||
| credentialGateway = fakeCredentialGateway, | ||
| socialConnectionRepository = fakeConnectionRepository, | ||
| properties = properties, | ||
| httpTransport = fakeTransport, | ||
| objectMapper = objectMapper, | ||
| clock = clock, | ||
| ) | ||
|
|
||
| val exception = assertThrows(ReconnectRequiredException::class.java) { | ||
| kotlinx.coroutines.runBlocking { resolver.resolve(account) } | ||
| } | ||
| assertEquals(ReconnectReason.INVALID_GRANT, exception.reason) | ||
| } |
Contributor
There was a problem hiding this comment.
Classify refresh failures instead of asserting all failures require reconnect.
These tests map invalid_client, timeouts, IOException, and unexpected exceptions to ReconnectReason.INVALID_GRANT. That would force user reconnects for transient LinkedIn/network failures or app misconfiguration; reserve reconnect for expired/revoked refresh tokens such as OAuth invalid_grant, and surface transport/5xx/429 failures as retryable refresh/publish failures.
Also applies to: 282-400
🤖 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
`@server/smp/src/test/kotlin/com/profiletailors/smp/publishing/infrastructure/credentials/RefreshAwareCredentialResolverTest.kt`
around lines 238 - 280, The tests in RefreshAwareCredentialResolverTest are
incorrectly treating all refresh failures uniformly as requiring reconnect
(INVALID_GRANT). Update the test methods to distinguish between different
failure types: only actual OAuth invalid_grant errors (expired/revoked refresh
tokens) should throw ReconnectRequiredException with INVALID_GRANT reason, while
other failures like invalid_client, transport/IO errors, timeouts, and HTTP
5xx/429 responses should be tested as retryable failures that do not require
reconnect. Specifically, for the test starting at line 238 (testing
invalid_client error with 401 status), change the assertion from expecting
ReconnectRequiredException to expecting a retryable error response instead.
Apply the same distinction across all affected test methods in the file to
properly separate transient/configuration failures from actual token revocation
scenarios.
- Add ORDER BY publication_id, position_index ASC to findBlockedForRecovery asset links query to preserve asset ordering (matching findByWorkspaceAndId pattern) - Replace hardcoded June 2026 dates in open-ended list test with runtime-relative dates to prevent flakiness as time progresses
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
|
|
PR title violation of the ': ' pattern occurs when using the conventional-commit 'feat(publishing):' format. Remove the 'feat(' prefix and ')' characters so the scope appears directly before the colon. Kody rule violation: Write a clear, scoped PR title |
Comment on lines
+288
to
+291
| require(limit in MIN_LIMIT..MAX_LIMIT) { | ||
| "limit must be between $MIN_LIMIT and $MAX_LIMIT, got $limit" | ||
| } | ||
| require(offset >= 0) { "offset must be non-negative, got $offset" } |
There was a problem hiding this comment.
IllegalArgumentException from require() causes HTTP 500 errors for out-of-range limit or negative offset, breaking the API contract by masking client errors. Replace require() with explicit ResponseStatusException(HttpStatus.BAD_REQUEST, ...) checks to match the timezone validation pattern already used in the same controller.
if (limit !in MIN_LIMIT..MAX_LIMIT) {
throw ResponseStatusException(
HttpStatus.BAD_REQUEST,
"limit must be between $MIN_LIMIT and $MAX_LIMIT, got $limit",
)
}
if (offset < 0) {
throw ResponseStatusException(
HttpStatus.BAD_REQUEST,
"offset must be non-negative, got $offset",
)
}Prompt for LLM
File server/smp/src/main/kotlin/com/profiletailors/smp/publishing/infrastructure/http/PublishingControllers.kt:
Line 288 to 291:
WHAT: require() throws IllegalArgumentException that is not mapped to an HTTP status, causing Spring to return HTTP 500 for out-of-range limit or negative offset. WHY: Clients sending invalid pagination parameters receive 500 instead of 400, breaking the API contract and masking client errors as server failures. HOW: Replace require() with explicit ResponseStatusException(HttpStatus.BAD_REQUEST, ...) checks, matching the timezone validation pattern already used in the same controller.
Suggested Code:
if (limit !in MIN_LIMIT..MAX_LIMIT) {
throw ResponseStatusException(
HttpStatus.BAD_REQUEST,
"limit must be between $MIN_LIMIT and $MAX_LIMIT, got $limit",
)
}
if (offset < 0) {
throw ResponseStatusException(
HttpStatus.BAD_REQUEST,
"offset must be non-negative, got $offset",
)
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a comprehensive set of features and improvements for LinkedIn integration within the publishing system, focusing on robust publication lifecycle management, enhanced credential handling, and improved user feedback.
Key changes include:
BLOCKEDstatus for publications. Publications are now markedBLOCKEDif their target social account isDISABLED,REQUIRES_RECONNECT, orPENDING.BLOCKEDpublications are automatically retried with exponential backoff once the associated account's status is restored toACTIVE. Publications targetingDELETEDaccounts will fail terminally.REQUIRES_RECONNECTand preventing further publishing attempts until re-authentication. Optimistic locking is used to prevent concurrent token refresh attempts.notification_eventstable and associated domain/infrastructure to record durable events for critical publication and connection outcomes (e.g., success, failure, blocked, reconnect required). This lays the groundwork for future user notification features.BLOCKEDstatus indicator on individual publications in both calendar and list views, along with a direct reconnect action./publications) to list publications, supporting filtering by status, social account, and date range, enabling more flexible display of publication history in the UI.publicationstable (public_url,blocked_at,blocked_reason,retry_count) and thesecure_credentialstable (versionfor optimistic locking), and creates the newnotification_eventstable.