Fix pnpm audit violation GHSA-gmq8-994r-jv83: yauzl contains an off-by-one error#325
Merged
leynos merged 1 commit intoimplement-revision-safe-update-strategy-i43x3pfrom Mar 13, 2026
Conversation
Reviewer's GuideImplements a revision-safe interests update flow across backend domain, HTTP, and persistence layers; removes hidden retry logic from the DieselUserInterestsCommand in favour of explicit optimistic concurrency using expectedRevision, extends the UserInterests model and HTTP schemas to carry revision, tightens error mapping to a standard revision_mismatch conflict envelope, updates tests and BDD flows accordingly, and adjusts tooling/docs including pg_worker provisioning and pnpm audit mitigation for yauzl. Sequence diagram for the revision-safe interests update HTTP flowsequenceDiagram
actor Client
participant UsersHandler as UsersHTTPHandler
participant UserInterestsPort as UserInterestsCommand
participant DieselInterests as DieselUserInterestsCommand
participant PrefRepo as UserPreferencesRepository
Client->>UsersHandler: PUT /api/v1/users/me/interests
activate UsersHandler
UsersHandler->>UsersHandler: parse InterestsRequest
UsersHandler->>UsersHandler: parse_interest_theme_ids()
UsersHandler->>UserInterestsPort: set_interests(UpdateUserInterestsRequest)
activate UserInterestsPort
UserInterestsPort->>DieselInterests: set_interests(request)
activate DieselInterests
DieselInterests->>PrefRepo: find_by_user_id(&request.user_id)
activate PrefRepo
PrefRepo-->>DieselInterests: existing_prefs: Option~UserPreferences~
deactivate PrefRepo
DieselInterests->>DieselInterests: build_preferences_for_interest_update(&request, existing_prefs)
alt first_write_and_no_expected_revision
DieselInterests-->>UserInterestsPort: Ok(UserInterests(revision=1))
else existing_prefs_and_missing_expected_revision
DieselInterests-->>UserInterestsPort: Err(Error::conflict revision_mismatch)
else expected_revision_mismatch
DieselInterests-->>UserInterestsPort: Err(Error::conflict revision_mismatch)
else expected_revision_matches
DieselInterests->>PrefRepo: save(&prefs, request.expected_revision)
activate PrefRepo
PrefRepo-->>DieselInterests: Ok(())
deactivate PrefRepo
DieselInterests-->>UserInterestsPort: Ok(UserInterests(revision=next_revision))
end
deactivate DieselInterests
UserInterestsPort-->>UsersHandler: Result<UserInterests, Error>
deactivate UserInterestsPort
alt success
UsersHandler-->>Client: 200 OK
note right of Client: Body: { userId, interestThemeIds, revision }
else conflict
UsersHandler-->>Client: 409 Conflict
note right of Client: Body: { code: conflict, details: { code: revision_mismatch, expectedRevision, actualRevision } }
end
deactivate UsersHandler
Updated class diagram for interests domain and portsclassDiagram
class UserId
class InterestThemeId
class UpdateUserInterestsRequest {
+UserId user_id
+Vec~InterestThemeId~ interest_theme_ids
+Option~u32~ expected_revision
}
class UserInterests {
-UserId user_id
-Vec~InterestThemeId~ interest_theme_ids
-u32 revision
+new(user_id: UserId, interest_theme_ids: Vec~InterestThemeId~, revision: u32) UserInterests
+user_id() &UserId
+interest_theme_ids() &Vec~InterestThemeId~
+revision() u32
}
class UserInterestsCommand {
<<trait>>
+set_interests(request: UpdateUserInterestsRequest) Result~UserInterests, Error~
}
class FixtureUserInterestsCommand {
+set_interests(request: UpdateUserInterestsRequest) Result~UserInterests, Error~
}
class DieselUserInterestsCommand {
-preferences_repository: Arc~UserPreferencesRepository~
+new(preferences_repository: Arc~UserPreferencesRepository~) DieselUserInterestsCommand
+set_interests(request: UpdateUserInterestsRequest) Result~UserInterests, Error~
-build_preferences_for_interest_update(request: &UpdateUserInterestsRequest, existing: Option~UserPreferences~) Result~UserPreferences, Error~
-map_preferences_persistence_error(error: UserPreferencesRepositoryError) Error
-revision_conflict(expected: Option~u32~, actual: u32) Error
}
class UserPreferences {
+UserId user_id
+Vec~uuid::Uuid~ interest_theme_ids
+Vec~uuid::Uuid~ safety_toggle_ids
+UnitSystem unit_system
+u32 revision
+builder(user_id: UserId) UserPreferencesBuilder
}
class UserPreferencesBuilder {
+interest_theme_ids(ids: Vec~uuid::Uuid~) UserPreferencesBuilder
+safety_toggle_ids(ids: Vec~uuid::Uuid~) UserPreferencesBuilder
+unit_system(unit_system: UnitSystem) UserPreferencesBuilder
+revision(revision: u32) UserPreferencesBuilder
+build() UserPreferences
}
class UserPreferencesRepository {
<<trait>>
+find_by_user_id(user_id: &UserId) Result~Option~UserPreferences~, UserPreferencesRepositoryError~
+save(prefs: &UserPreferences, expected_revision: Option~u32~) Result~(), UserPreferencesRepositoryError~
}
class UserPreferencesRepositoryError {
<<enum>>
+NotFound
+MissingForUpdate expected: u32
+RevisionMismatch expected: u32, actual: u32
+ConcurrentWriteConflict
+Query message: String
}
class Error {
+conflict(message: &str) Error
+internal(message: String) Error
+with_details(details: serde_json::Value) Error
}
UserInterestsCommand <|.. FixtureUserInterestsCommand
UserInterestsCommand <|.. DieselUserInterestsCommand
DieselUserInterestsCommand --> UserPreferencesRepository
DieselUserInterestsCommand ..> UpdateUserInterestsRequest
DieselUserInterestsCommand ..> UserInterests
DieselUserInterestsCommand ..> UserPreferences
DieselUserInterestsCommand ..> UserPreferencesRepositoryError
DieselUserInterestsCommand ..> Error
FixtureUserInterestsCommand ..> UpdateUserInterestsRequest
FixtureUserInterestsCommand ..> UserInterests
UpdateUserInterestsRequest --> UserId
UpdateUserInterestsRequest --> InterestThemeId
UserInterests --> UserId
UserInterests --> InterestThemeId
UserPreferencesRepositoryError ..> Error
Updated class diagram for HTTP interests request and schemaclassDiagram
class InterestsRequest {
+Vec~String~ interest_theme_ids
+Option~u32~ expected_revision
}
class ParsedInterests {
+Vec~InterestThemeId~ interest_theme_ids
+Option~u32~ expected_revision
}
class UserInterestsSchema {
+String user_id
+Vec~String~ interest_theme_ids
+u32 revision
}
class InterestsRequestError {
<<enum>>
+TooManyInterestThemeIds length: usize
+InvalidInterestThemeId index: usize
}
class UsersHandler {
+update_interests(state: AppState, session: UserSession, payload: InterestsRequest) ApiResult~UserInterestsSchema~
-parse_interest_theme_ids(payload: InterestsRequest) Result~ParsedInterests, InterestsRequestError~
}
class AppState {
+interests: Arc~UserInterestsCommand~
}
class UserSession {
+require_user_id() Result~UserId, Error~
}
class UpdateUserInterestsRequest {
+UserId user_id
+Vec~InterestThemeId~ interest_theme_ids
+Option~u32~ expected_revision
}
class UserInterests {
+UserId user_id
+Vec~InterestThemeId~ interest_theme_ids
+u32 revision
}
UsersHandler ..> InterestsRequest
UsersHandler ..> ParsedInterests
UsersHandler ..> InterestsRequestError
UsersHandler ..> AppState
UsersHandler ..> UserSession
UsersHandler ..> UpdateUserInterestsRequest
UsersHandler ..> UserInterests
UsersHandler ..> UserInterestsSchema
AppState --> UserInterestsCommand
ParsedInterests --> InterestThemeId
UpdateUserInterestsRequest --> UserId
UpdateUserInterestsRequest --> InterestThemeId
UserInterests --> UserId
UserInterests --> InterestThemeId
UserInterestsSchema --> UserInterests
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Caution Review failedFailed to post review comments Summary by CodeRabbitRelease Notes
WalkthroughThis pull request implements optimistic concurrency control for user interests updates by introducing revision tracking. The user_preferences aggregate now shares a revision counter across both full preference writes and partial interests updates. Clients must provide the expected revision when updating after the initial write; stale or missing revisions return HTTP 409 Conflict responses with detailed mismatch information. Changes
Sequence DiagramsequenceDiagram
participant Client
participant HTTP as HTTP Handler
participant Domain as Domain<br/>(UserInterests)
participant Persistence as Persistence<br/>(Diesel)
participant DB as Database
Client->>HTTP: PUT /api/v1/users/me/interests<br/>{interestThemeIds, expectedRevision?}
HTTP->>Domain: set_interests(UpdateUserInterestsRequest)
Domain->>Persistence: set_interests(request)
Persistence->>DB: SELECT user_preferences WHERE user_id
DB-->>Persistence: existing (Option<UserPreferences>)
alt expectedRevision matches existing.revision
Persistence->>Persistence: Compute next_revision
Persistence->>DB: UPDATE user_preferences<br/>SET interest_theme_ids=?, revision=?
DB-->>Persistence: Success
Persistence-->>Domain: Ok(UserInterests{user_id, themes, new_revision})
else expectedRevision mismatch
Persistence-->>Domain: Err(Conflict{expected, actual})
else expectedRevision missing when preferences exist
Persistence-->>Domain: Err(Conflict{missing, actual})
else First write (no existing preferences)
Persistence->>DB: INSERT user_preferences<br/>SET revision=1
DB-->>Persistence: Success
Persistence-->>Domain: Ok(UserInterests{..., revision: 1})
end
Domain-->>HTTP: Result<UserInterests, Error>
alt Success
HTTP->>Client: 200 {userId, interestThemeIds, revision}
else Conflict
HTTP->>Client: 409 {code: "revision_mismatch",<br/>expectedRevision, actualRevision}
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
There was a problem hiding this comment.
(1 files improve in Code Health)
Gates Failed
(2 files with Code Duplication)
Gates Passed
See analysis details in CodeScene
Reason for failure
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| user_interests_revision_conflicts_bdd.rs | 1 advisory rule | 9.39 | Suppress |
| flow_support.rs | 1 advisory rule | 9.39 | Suppress |
View Improvements
| File | Code Health Impact | Categories Improved |
|---|---|---|
| support.rs | 9.39 → 10.00 | Code Duplication |
Quality Gate Profile: Pay Down Tech Debt
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
Comment on lines
+60
to
+77
| fn existing_preferences_revision_1_with_preserved_safety_and_unit_settings(world: &mut World) { | ||
| if is_skipped(world) { | ||
| return; | ||
| } | ||
|
|
||
| let db = world.db.as_ref().expect("db context"); | ||
| seed_preferences( | ||
| db.database_url.as_str(), | ||
| SeedPreferences { | ||
| user_id: Uuid::parse_str(FIXTURE_AUTH_ID).expect("valid fixture UUID"), | ||
| interest_ids: &[FIRST_THEME_ID], | ||
| safety_ids: &[SAFETY_TOGGLE_ID], | ||
| unit_system: "imperial", | ||
| revision: 1, | ||
| }, | ||
| ) | ||
| .expect("seed user preferences"); | ||
| } |
There was a problem hiding this comment.
❌ New issue: Code Duplication
The module contains 6 functions with similar structure: existing_preferences_revision_1_with_preserved_safety_and_unit_settings,existing_preferences_revision_2,the_first_interests_response_includes_revision_1,the_response_is_a_conflict_with_expected_revision_1_and_actual_revision_2 and 2 more functions
Comment on lines
+121
to
+139
| async fn update_interests_snapshot<S>( | ||
| app: &S, | ||
| cookie: Cookie<'static>, | ||
| payload: InterestsRequest, | ||
| ) -> Snapshot | ||
| where | ||
| S: actix_web::dev::Service< | ||
| actix_http::Request, | ||
| Response = actix_web::dev::ServiceResponse<actix_web::body::BoxBody>, | ||
| Error = actix_web::Error, | ||
| >, | ||
| { | ||
| let req = actix_test::TestRequest::put() | ||
| .uri("/api/v1/users/me/interests") | ||
| .cookie(cookie) | ||
| .set_json(payload) | ||
| .to_request(); | ||
| capture_snapshot(actix_test::call_service(app, req).await, false).await | ||
| } |
There was a problem hiding this comment.
❌ New issue: Code Duplication
The module contains 5 functions with similar structure: preferences_snapshot,run_first_write,run_missing_revision_conflict,run_stale_revision_conflict and 1 more functions
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="backend/src/domain/ports/user_interests_command.rs" line_range="43-46" />
<code_context>
+ request: UpdateUserInterestsRequest,
) -> Result<UserInterests, Error> {
- Ok(UserInterests::new(user_id.clone(), interest_theme_ids))
+ Ok(UserInterests::new(
+ request.user_id,
+ request.interest_theme_ids,
+ request.expected_revision.map_or(1, |revision| revision + 1),
+ ))
}
</code_context>
<issue_to_address>
**issue:** Fixture implementation can overflow the revision counter when `expected_revision == u32::MAX`.
Here the increment can overflow when `expected_revision` is `u32::MAX`, panicking in debug and wrapping in release. The production code uses `checked_add(1)` and turns overflow into a domain error. To keep behaviour consistent and avoid surprising test/fixture behaviour, mirror that approach (e.g. `revision.checked_add(1).unwrap_or(revision)` or return an error-equivalent).
</issue_to_address>
### Comment 2
<location path="backend/src/outbound/persistence/diesel_user_interests_command/tests/mapping.rs" line_range="120" />
<code_context>
#[rstest]
#[case(StubFailure::Connection, ErrorCode::ServiceUnavailable)]
#[case(StubFailure::Query, ErrorCode::InternalError)]
</code_context>
<issue_to_address>
**suggestion (testing):** Assert that `save` is only called once for each failure variant to prove retries were removed
Since the retry loop was removed and `save_call_count` was added to `StubUserPreferencesRepository`, please extend this test to assert `repository.save_call_count() == 1` for each failure case (e.g. `RevisionMismatch`, `MissingForUpdate`, `ConcurrentWriteConflict`). This will codify the expectation that these failures do not trigger retries and help catch regressions if retries are reintroduced.
Suggested implementation:
```rust
// existing error mapping assertion
assert_eq!(expected_code, error.code());
// ensure that save is only called once and no retries occur
assert_eq!(
1,
repository.save_call_count(),
"save should be called exactly once for failure {:?}",
failure
);
```
Because I only see the attribute section of the test, you will need to ensure:
1. The test function already has (or can access) a `repository` variable of type `StubUserPreferencesRepository` (or a reference to it). If it’s created locally, keep the name `repository` to match the snippet above; otherwise, adjust the assertion to use the actual variable name.
2. The error value is bound to a variable named `error` and the expected code variable is `expected_code`. If your names differ, update the `assert_eq!(expected_code, error.code());` search pattern and the assertion accordingly.
3. The failure case argument is named `failure` to use in the assertion message. If it is named differently (e.g. `stub_failure`), update the format string argument accordingly.
4. This assertion is placed in the test that is using the `#[rstest]` block you showed (the one with cases for `Connection`, `Query`, `RevisionMismatch`, `MissingForUpdate`, and `ConcurrentWriteConflict`) so that it runs for each failure variant.
</issue_to_address>
### Comment 3
<location path="docs/wildside-testing-guide.md" line_range="34" />
<code_context>
+- Otherwise, the Makefile installs `pg-embed-setup-unpriv`'s `pg_worker` into
+ a cache under `target/pg-worker-root/` and then copies it to
+ `PG_WORKER_PATH`.
+- By default, `PG_WORKER_PATH` is `target/pg_worker`. Override it when you need
+ a different writable destination, for example:
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The phrase "when you need" uses a second-person pronoun, which the style guidelines disallow.
Consider rephrasing to avoid "you", for example:
- "Override it when a different writable destination is required" or
- "Override it when a different writable destination is needed".
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*.md`
**Instructions:**
Avoid 2nd person or 1st person pronouns ("I", "you", "we").
</details>
</issue_to_address>
### Comment 4
<location path="docs/investigations/2026-03-13-toolchain-errors.md" line_range="348" />
<code_context>
+- A later `cargo clippy` run reaches normal linting and reports unrelated,
+ comprehensible source issues instead.
+
+What I cannot prove from current evidence:
+
+- Whether the root cause was a nightly Rust/Clippy regression, a transient
</code_context>
<issue_to_address>
**suggestion (review_instructions):** This sentence and nearby ones use first-person pronouns ("I"), which the style rules disallow.
To follow the pronoun guideline, consider rephrasing here and in nearby sentences such as "I therefore assign the following confidence levels", "I re-ran the exact probe command", and "I also re-ran `cargo clippy`".
For example:
- "What cannot be proved from current evidence:" instead of "What I cannot prove...";
- "The following confidence levels are assigned:" instead of "I therefore assign...";
- "The exact probe command was re-run directly" instead of "I re-ran the exact probe command directly".
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*.md`
**Instructions:**
Avoid 2nd person or 1st person pronouns ("I", "you", "we").
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+43
to
+46
| Ok(UserInterests::new( | ||
| request.user_id, | ||
| request.interest_theme_ids, | ||
| request.expected_revision.map_or(1, |revision| revision + 1), |
There was a problem hiding this comment.
issue: Fixture implementation can overflow the revision counter when expected_revision == u32::MAX.
Here the increment can overflow when expected_revision is u32::MAX, panicking in debug and wrapping in release. The production code uses checked_add(1) and turns overflow into a domain error. To keep behaviour consistent and avoid surprising test/fixture behaviour, mirror that approach (e.g. revision.checked_add(1).unwrap_or(revision) or return an error-equivalent).
| assert_eq!(err.code(), expected_code); | ||
| } | ||
|
|
||
| #[rstest] |
There was a problem hiding this comment.
suggestion (testing): Assert that save is only called once for each failure variant to prove retries were removed
Since the retry loop was removed and save_call_count was added to StubUserPreferencesRepository, please extend this test to assert repository.save_call_count() == 1 for each failure case (e.g. RevisionMismatch, MissingForUpdate, ConcurrentWriteConflict). This will codify the expectation that these failures do not trigger retries and help catch regressions if retries are reintroduced.
Suggested implementation:
// existing error mapping assertion
assert_eq!(expected_code, error.code());
// ensure that save is only called once and no retries occur
assert_eq!(
1,
repository.save_call_count(),
"save should be called exactly once for failure {:?}",
failure
);Because I only see the attribute section of the test, you will need to ensure:
- The test function already has (or can access) a
repositoryvariable of typeStubUserPreferencesRepository(or a reference to it). If it’s created locally, keep the namerepositoryto match the snippet above; otherwise, adjust the assertion to use the actual variable name. - The error value is bound to a variable named
errorand the expected code variable isexpected_code. If your names differ, update theassert_eq!(expected_code, error.code());search pattern and the assertion accordingly. - The failure case argument is named
failureto use in the assertion message. If it is named differently (e.g.stub_failure), update the format string argument accordingly. - This assertion is placed in the test that is using the
#[rstest]block you showed (the one with cases forConnection,Query,RevisionMismatch,MissingForUpdate, andConcurrentWriteConflict) so that it runs for each failure variant.
| - Otherwise, the Makefile installs `pg-embed-setup-unpriv`'s `pg_worker` into | ||
| a cache under `target/pg-worker-root/` and then copies it to | ||
| `PG_WORKER_PATH`. | ||
| - By default, `PG_WORKER_PATH` is `target/pg_worker`. Override it when you need |
There was a problem hiding this comment.
suggestion (review_instructions): The phrase "when you need" uses a second-person pronoun, which the style guidelines disallow.
Consider rephrasing to avoid "you", for example:
- "Override it when a different writable destination is required" or
- "Override it when a different writable destination is needed".
Review instructions:
Path patterns: **/*.md
Instructions:
Avoid 2nd person or 1st person pronouns ("I", "you", "we").
| - A later `cargo clippy` run reaches normal linting and reports unrelated, | ||
| comprehensible source issues instead. | ||
|
|
||
| What I cannot prove from current evidence: |
There was a problem hiding this comment.
suggestion (review_instructions): This sentence and nearby ones use first-person pronouns ("I"), which the style rules disallow.
To follow the pronoun guideline, consider rephrasing here and in nearby sentences such as "I therefore assign the following confidence levels", "I re-ran the exact probe command", and "I also re-ran cargo clippy".
For example:
- "What cannot be proved from current evidence:" instead of "What I cannot prove...";
- "The following confidence levels are assigned:" instead of "I therefore assign...";
- "The exact probe command was re-run directly" instead of "I re-ran the exact probe command directly".
Review instructions:
Path patterns: **/*.md
Instructions:
Avoid 2nd person or 1st person pronouns ("I", "you", "we").
leynos
changed the base branch from
main
to
implement-revision-safe-update-strategy-i43x3p
leynos
merged commit 22c3e5a
into
implement-revision-safe-update-strategy-i43x3p
1 of 4 checks passed
💡 Codex ReviewLine 170 in f929017  Create PG_WORKER_PATH parent before copying pg_worker
 Use checked_add when deriving fixture interests revision
The fixture interests command derives the next revision with ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
leynos
added a commit
that referenced
this pull request
Mar 18, 2026
…#323) * docs(execplans): add ExecPlan for revision-safe interests update strategy Add a detailed 616-line execution plan for roadmap item 3.5.4 defining and implementing a revision-safe update strategy for user interests. This new document outlines the purpose, constraints, tolerances, risks, progress stages, surprises, decision log, outcomes, plan of work, interfaces, error mapping, and team ownership related to replacing the legacy last-write-wins interests update with an optimistic concurrency model aligned with the existing user preferences aggregate. No implementation is included yet; this draft locks the architectural and concurrency decisions to guide subsequent development and testing. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * feat(interests): add optimistic concurrency with revision to interests update - Introduce UpdateUserInterestsRequest to carry expectedRevision for concurrency control. - UserInterests now includes a revision field to track aggregate version. - Convert DieselUserInterestsCommand to enforce revision checks and reject stale writes. - Expose revision-safe update and conflict semantics in HTTP API for PUT /users/me/interests. - Remove implicit retries on concurrency conflicts; surface 409 Conflict with revision details. - Add extensive unit and integration tests covering revision conflicts and concurrency handling. - Update documentation to reflect that interests share aggregate and revision with user preferences, with optimistic locking. This change ensures safe concurrent updates on user interests by leveraging the aggregate revision from user preferences, providing explicit conflict feedback to clients when revisions are stale or missing after the first write. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * docs(investigations): add detailed investigation report on toolchain errors and failures Add a comprehensive 500-line investigation document describing three transient failures observed in the backend validation process. This report includes failure reproductions, environment snapshots, failure diagnoses, relevant code paths, and recommended next steps. The failures are analyzed in detail, identifying causes such as a broken /dev/null device and transient toolchain instabilities, with evidence ruling out persistent code defects. This addition aids in debugging and future failure mitigation related to the Rust toolchain and container environment. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * refactor(pg_worker): replace in-repo pg_worker binary with external one This change removes the local `backend/src/bin/pg_worker.rs` binary and the corresponding entry in `Cargo.toml`. Instead, the Makefile is updated to use the `pg_worker` binary provided by the `pg-embed-setup-unpriv` crate. - Makefile updated to check for an existing `pg_worker` on PATH and install it if missing. - Introduced `PG_WORKER_INSTALL_ROOT` and `PG_EMBED_SETUP_UNPRIV_VERSION` variables in Makefile. - Documentation updated to describe the new usage pattern for the embedded Postgres worker helper binary. This simplifies maintenance by relying on an external, versioned binary rather than one built and maintained in this repository. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * refactor(tests/user_interests_revision_conflicts_bdd): refactor test flow and DB support for user interests revision - Extracted common test logic into helper functions (e.g., with_world, seed_prefs) to reduce code duplication and improve readability. - Modified seed_preferences DB support function to accept user_id as explicit parameter enhancing clarity. - Simplified test step implementations by wrapping them with helper functions managing skip checks. - Refined flow_support functions to unify update operations and introduce reusable session handling (with_app_session), improving async control and reducing boilerplate. - Cleaned up conditional checks scattered in tests and standardized update call patterns. These changes improve maintainability and clarity of BDD test code for user preferences revision conflict scenarios. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * Fix GHSA-gmq8-994r-jv83: yauzl contains an off-by-one error (#325) * refactor(tests/user_interests_revision_conflicts_bdd): improve test helper functions and use consistent calls - Introduced reusable step helpers for interest and conflict assertions to reduce duplication - Updated seed_prefs to use a single parameter struct for clarity - Simplified then-step functions to use new helpers for clearer intent - Added generic call_endpoint helper in flow_support.rs to centralize request handling - Refactored test requests to use the new call_endpoint improving code reuse and readability Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * refactor(ci): simplify pg_worker build step by using make prepare-pg-worker Replaced the manual cargo build and install commands with a single make prepare-pg-worker command in the GitHub Actions CI workflow. This improves maintainability and reuses existing build scripts. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * fix(make): ensure target directory exists before installing pg_worker When pg_worker is found on PATH, the prepare-pg-worker target copies it to target/pg_worker without first creating the parent directory. On a clean checkout or after 'make clean', this fails with 'No such file or directory', blocking 'make test' in environments with a global pg_worker. This commit adds 'mkdir -p $(dirname $(PG_WORKER_PATH))' before the install command in both branches of the conditional to ensure the target directory always exists. Also removes invalid 'noImportCycles' Biome rule that was causing check-fmt to fail, and applies auto-formatting to test files. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * refactor: address code review findings for revision-safe interests update This commit addresses inline and outside-diff code review findings: Code changes: - Add Rustdoc examples to UserInterests::new and revision() methods demonstrating construction and revision assertion - Fix test in users/tests.rs to assert on HTTP DTO (InterestsRequest) instead of domain DTO for proper boundary testing - Add revision field assertion to user_interests_schema test - Add skip_serializing_if to InterestsRequest.expected_revision to omit None values from JSON serialization - Deduplicate mkdir command in prepare-pg-worker Makefile target by moving it before the conditional Documentation changes: - Update execplan, backend architecture, and PWA data model docs to correctly describe conflict response envelope shape: top-level code: "conflict" with nested details containing code: "revision_mismatch", expectedRevision, and actualRevision - Fix first-person narration in toolchain errors investigation doc, replacing "I re-ran" with passive constructions - Convert all headings to sentence case in toolchain errors doc per style guidelines - Update PWA data model doc timestamp to current date Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix(docs): wrap long line in toolchain errors investigation Fix MD013/line-length violation at line 312 by wrapping the sentence to stay within the 80-character limit. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix(docs): use sequential numbering for execplan execution steps Replace repeated "1." markers with sequential numbering (1., 2., 3., 4., 5., 6.) in the concrete execution steps section. Indent all list item content (code blocks and text) to maintain proper list continuity and satisfy MD029/ol-prefix markdownlint rule with "ordered" style. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * refactor(tests): eliminate duplication in schema has_expected_name tests Introduce assert_schema_contains helper to replace repeated pattern across all five has_expected_name test functions. The helper takes the expected schema name and a slice of field names to check, eliminating ~40 lines of duplicated test code. Also remove unused UpdateUserInterestsRequest import from users/tests.rs that was left over from the previous refactoring. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: address remaining code review findings Fix three remaining issues from code review: 1. Replace first-person phrasing "What I cannot prove" with neutral voice "What the available evidence does not establish" in toolchain errors investigation doc 2. Rename test function from update_user_interests_request_serializes_expected_revision_in_camel_case to interests_request_serializes_expected_revision_in_camel_case to match the actual DTO being tested (InterestsRequest) 3. Update execplan conflict-envelope description to accurately reflect the two-level structure: top-level code: "conflict" with nested details containing code: "revision_mismatch", expectedRevision, and actualRevision Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: address final code review findings Fix three remaining issues from code review: 1. Harden interests_request serialization test by adding assertion that snake_case key "expected_revision" is not present in JSON, ensuring only camelCase "expectedRevision" is serialized 2. Fix article/noun agreement in execplan: change "an existing preferences row" to "an existing preference row" (singular) 3. Update toolchain investigation doc to use neutral voice: change "What the available evidence does not establish:" to "What cannot be proved from current evidence:" to avoid first-person phrasing Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> --------- Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
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.
Summary by Sourcery
Implement revision-safe interests updates across backend domain, HTTP API, persistence, and tests, while updating tooling and docs, and add yauzl dependency to address the audit issue.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests:
Chores: