Fix of PR#8: only one process of a machine runs the Matrix client, the one holding sync.lock; another one reads messages.json along and takes over when the first ends. matrix login refuses while another process syncs.
PR document: doc/prs/2026-10-10-PR#11-matrix-one-client.md. 68 tests green, no compiler warnings. Version 0.4.3.
Fix of PR#8: only one process of a machine runs the Matrix client, the one holding `sync.lock`; another one reads `messages.json` along and takes over when the first ends. `matrix login` refuses while another process syncs.
PR document: `doc/prs/2026-10-10-PR#11-matrix-one-client.md`. 68 tests green, no compiler warnings. Version 0.4.3.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
claude-code
marked the pull request as ready for review 2026-10-10 21:55:50 +02:00
ORANGE 1 finding(s) that Jev rates as probably real and severe: dashboard/src/main/kotlin/de/hoennig/werkregie/dashboard/sources/MatrixSource.kt:170-190
hint 3 changed file(s) that neither weaken the checks nor touch security according to Jev
hint 1 build change(s) of low risk according to Jev: dashboard/build.gradle.kts (weakens checks 0.02, risk 0.00, security 0.01)
hint 1 finding(s) of low severity or rated minor by Jev: doc/prs/2026-10-10-PR#11-matrix-one-client.md:38-44
hint model recommends merge with caution
Findings
Lock leak on error path at dashboard/src/main/kotlin/de/hoennig/werkregie/dashboard/sources/MatrixSource.kt:170-190, severity unknown, counted as medium, ORANGE, as a line comment
Unclear Given/When/Then at doc/prs/2026-10-10-PR#11-matrix-one-client.md:38-44, severity unknown, counted as medium, hint, as a line comment
werkrevisor: ORANGE / FINDINGS for a4164793637b9fbed846bdae69e5aa954c13228f.
The result comment of werkrevisor holds the details.
werkrevisor: ORANGE / FINDINGS for `a4164793637b9fbed846bdae69e5aa954c13228f`.
The result comment of werkrevisor holds the details.
<!-- werkrevisor-review:a4164793637b9fbed846bdae69e5aa954c13228f -->
Lock leak on error path, severity unknown, counted as medium (werkrevisor)
ORANGE; Jev: real 0.88, severity 2.6 of 3
In login(), the try { ... } finally { ... } only wraps the access to client.userId.full to client.deviceId. If baseUrl(...), classicLoginWithPassword(...).getOrThrow(), or openClient(...).getOrThrow() throws after the lock is acquired, the lock is never released, and sync.lock stays held by the still-open file channel until the process exits. The current CLI usage masks this because the process exits after login (and the OS reclaims the lock), but the bug will manifest if login() is ever called from a longer-lived process or retried in the same process. The fix is to move the lock.release() into a try-finally that also wraps the openClient call.
**Lock leak on error path**, severity unknown, counted as medium (werkrevisor)
ORANGE; Jev: real 0.88, severity 2.6 of 3
In `login()`, the `try { ... } finally { ... }` only wraps the access to `client.userId.full to client.deviceId`. If `baseUrl(...)`, `classicLoginWithPassword(...).getOrThrow()`, or `openClient(...).getOrThrow()` throws after the lock is acquired, the lock is never released, and `sync.lock` stays held by the still-open file channel until the process exits. The current CLI usage masks this because the process exits after login (and the OS reclaims the lock), but the bug will manifest if `login()` is ever called from a longer-lived process or retried in the same process. The fix is to move the `lock.release()` into a try-finally that also wraps the `openClient` call.
Fixed in f8895e2: the channel of the lock is closed on every path after it was taken (lock.channel().use { … }). Red/green: the test "a failed login frees the lock" failed without the fix and passes with it.
Fixed in `f8895e2`: the channel of the lock is closed on every path after it was taken (`lock.channel().use { … }`). Red/green: the test "a failed login frees the lock" failed without the fix and passes with it.
- [MatrixSourceTest "a second process reads the messages along, without a client of its own"](../../dashboard/src/test/kotlin/de/hoennig/werkregie/dashboard/sources/MatrixSourceTest.kt)
#### Scenario#11.02: the second process takes over
Unclear Given/When/Then, severity unknown, counted as medium (werkrevisor)
hint; Jev: real 0.75, severity 1.2 of 3
In Scenario#11.02, the Given clause "a process reads along" is a consequence of the lock being held by another process rather than the precondition itself, and the When "the syncing process ends" does not mention the lock becoming available. Compared to 11.01 and 11.03 (which use "Given one process of the machine syncs Matrix"), this scenario leaves the reader guessing why a reading process would suddenly start a client. A clearer Given would name the lock state (e.g. "Given one process of the machine holds sync.lock and another reads along from messages.json") and the When would name the trigger (e.g. "When the syncing process ends and the lock is released").
**Unclear Given/When/Then**, severity unknown, counted as medium (werkrevisor)
hint; Jev: real 0.75, severity 1.2 of 3
In Scenario#11.02, the Given clause "a process reads along" is a consequence of the lock being held by another process rather than the precondition itself, and the When "the syncing process ends" does not mention the lock becoming available. Compared to 11.01 and 11.03 (which use "Given one process of the machine syncs Matrix"), this scenario leaves the reader guessing why a reading process would suddenly start a client. A clearer Given would name the lock state (e.g. "Given one process of the machine holds `sync.lock` and another reads along from `messages.json`") and the When would name the trigger (e.g. "When the syncing process ends and the lock is released").
Fixed in f8895e2: Scenario#11.02 now names the relay and the local mode, "Given the relay syncs, and the local mode shows its messages", "When the relay stops"; Scenario#11.01 no longer names messages.json.
Fixed in `f8895e2`: Scenario#11.02 now names the relay and the local mode, "Given the relay syncs, and the local mode shows its messages", "When the relay stops"; Scenario#11.01 no longer names `messages.json`.
⚠️ werkrevisor: ERROR / review not completed, human action needed
Change kind: mixed.
Reasons
ERROR GiteaError: POST /repos/mi/werkregie/statuses/a4164793637b9fbed846bdae69e5aa954c13228f failed with HTTP 403
ORANGE 1 finding(s) that Jev rates as probably real and severe: dashboard/src/main/kotlin/de/hoennig/werkregie/dashboard/sources/MatrixSource.kt:170-190
hint 3 changed file(s) that neither weaken the checks nor touch security according to Jev
hint 1 build change(s) of low risk according to Jev: dashboard/build.gradle.kts (weakens checks 0.02, risk 0.00, security 0.01)
hint 1 finding(s) of low severity or rated minor by Jev: doc/prs/2026-10-10-PR#11-matrix-one-client.md:38-44
hint model recommends merge with caution
Findings
Lock leak on error path at dashboard/src/main/kotlin/de/hoennig/werkregie/dashboard/sources/MatrixSource.kt:170-190, severity unknown, counted as medium, ORANGE, as a line comment
Unclear Given/When/Then at doc/prs/2026-10-10-PR#11-matrix-one-client.md:38-44, severity unknown, counted as medium, hint, as a line comment
werkrevisor: YELLOW / HUMAN_REVIEW for f8895e2a4de2aa8a06b2c71967efa81630627af0.
The result comment of werkrevisor holds the details.
werkrevisor: YELLOW / HUMAN_REVIEW for `f8895e2a4de2aa8a06b2c71967efa81630627af0`.
The result comment of werkrevisor holds the details.
<!-- werkrevisor-review:f8895e2a4de2aa8a06b2c71967efa81630627af0 -->
Meta-reference in Solution section, severity unknown, counted as medium (werkrevisor)
hint; Jev: real 0.61, severity 0.9 of 3
The Solution bullet "matrix login frees the lock on every path, also when the login fails (finding of werkrevisor)." ends with a parenthetical "(finding of werkrevisor)" that points to the review process instead of describing the change itself; a PR document records what the change does, not how it was reviewed.
**Meta-reference in Solution section**, severity unknown, counted as medium (werkrevisor)
hint; Jev: real 0.61, severity 0.9 of 3
The Solution bullet "matrix login frees the lock on every path, also when the login fails (finding of werkrevisor)." ends with a parenthetical "(finding of werkrevisor)" that points to the review process instead of describing the change itself; a PR document records what the change does, not how it was reviewed.
Gate: approved by werkrevisor; merge waits for 2 open hint(s): reply to each line comment and resolve it, then restart the review or push a commit.
Change kind: mixed.
Reasons
hint 3 changed file(s) that neither weaken the checks nor touch security according to Jev
hint 1 build change(s) of low risk according to Jev: dashboard/build.gradle.kts (weakens checks 0.02, risk 0.00, security 0.01)
hint 2 finding(s) of low severity or rated minor by Jev: doc/werkregie.werkbaum:33, doc/prs/2026-10-10-PR#11-matrix-one-client.md:66-71
Findings
Werkbaum indentation at doc/werkregie.werkbaum:33, severity unknown, counted as medium, hint, as a line comment
Markdown tag rendered away at doc/prs/2026-10-10-PR#11-matrix-one-client.md:66-71, severity unknown, counted as medium, hint, as a line comment
Classifier
8 of 8 units passed all questions (1 PR document(s)).
3 answers ignored because of a confidence below [classifier] min_confidence.
Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 18842, calls 1, tokens in 18251 / out 8318 (reasoning included in out), duration 198.3 s, cost 0.0111 USD (reported), classifier `jev-1.13.0` calls 8, tokens in 6003, cost 0.0003 USD, finding judge `jev-1.13.0` calls 2, tokens in 2974, cost 0.0001 USD, build file judge `jev-1.13.0` calls 4, tokens in 4563, cost 0.0002 USD, phases: path_judge 0.2 s, classifier 0.5 s, context 0.3 s, model_review 196.7 s, finding_judge 0.3 s, config: defaults + host.
Reviewed head `960a4464f23c23b9d9f6ebabf3c60b287b49f031` against base `c2ec294e26491048e4e9bea389bcf1fd3c3f3de0`, werkrevisor 0.32.2.
<!-- werkrevisor:960a4464f23c23b9d9f6ebabf3c60b287b49f031 -->
<!-- werkrevisor-verdict:GREEN -->
<!-- werkrevisor-blocks:51bfe85133929d3bf08479702546fd42,23eefd8b33ea52fbbd7ca22f63b64b83,7cd1870616b8b3a353adc5a261477af3,b857a263eb0726c73fef64c2c9307f5d,4aa77f6c2848de927e12a03e6e3a1068,a0b4357cedd0fa4b53baf06c19fc6f1b,b0e89c8480186a61e9af588ca369021a,1818b18b5d5fdc32ad736c45a9d66c9c,3bbba91bcb5860dcffde0485afc2f1e0,96c3e457b53864ff7d22377aba996d81,bafb39596b0eb0e049aa341807141335,65d82c336f5237c23b8d13f170c8b96d,286d01dbfc87b87fa5f1f9ff1d933cb0,57a03453fc621e29e5038463a979cfd2,9a01133b6a875ac4e80d6fb3746af540,3526da7a131181c358bfea9b58722edb,f50ae9928d22dee565cef184636a8f23 -->
<!-- werkrevisor-folded -->
<details><summary>🟢 werkrevisor GREEN for <code>960a446</code>, superseded by <a href="http://git.javagil.de/mi/werkregie/pulls/11#issuecomment-3419">the result for <code>6cc5199</code></a></summary>
## 🟢 werkrevisor: GREEN / PASS
Gate: approved by werkrevisor; merge waits for 2 open hint(s): reply to each line comment and resolve it, then restart the review or push a commit.
Change kind: `mixed`.
### Reasons
- **hint** 3 changed file(s) that neither weaken the checks nor touch security according to Jev
- **hint** 1 build change(s) of low risk according to Jev: dashboard/build.gradle.kts (weakens checks 0.02, risk 0.00, security 0.01)
- **hint** 2 finding(s) of low severity or rated minor by Jev: doc/werkregie.werkbaum:33, doc/prs/2026-10-10-PR#11-matrix-one-client.md:66-71
### Findings
1. **Werkbaum indentation** at `doc/werkregie.werkbaum:33`, severity unknown, counted as medium, hint, as a line comment
2. **Markdown tag rendered away** at `doc/prs/2026-10-10-PR#11-matrix-one-client.md:66-71`, severity unknown, counted as medium, hint, as a line comment
### Classifier
8 of 8 units passed all questions (1 PR document(s)).
3 answers ignored because of a confidence below [classifier] min_confidence.
<sub>
Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 18842, calls 1, tokens in 18251 / out 8318 (reasoning included in out), duration 198.3 s, cost 0.0111 USD (reported), classifier `jev-1.13.0` calls 8, tokens in 6003, cost 0.0003 USD, finding judge `jev-1.13.0` calls 2, tokens in 2974, cost 0.0001 USD, build file judge `jev-1.13.0` calls 4, tokens in 4563, cost 0.0002 USD, phases: path_judge 0.2 s, classifier 0.5 s, context 0.3 s, model_review 196.7 s, finding_judge 0.3 s, config: defaults + host.<br>
Reviewed head `960a4464f23c23b9d9f6ebabf3c60b287b49f031` against base `c2ec294e26491048e4e9bea389bcf1fd3c3f3de0`, werkrevisor 0.32.2.
</sub>
</details>
werkrevisor: GREEN / PASS for 960a4464f23c23b9d9f6ebabf3c60b287b49f031.
The result comment of werkrevisor holds the details.
werkrevisor: GREEN / PASS for `960a4464f23c23b9d9f6ebabf3c60b287b49f031`.
The result comment of werkrevisor holds the details.
<!-- werkrevisor-review:960a4464f23c23b9d9f6ebabf3c60b287b49f031 -->
Markdown tag rendered away, severity unknown, counted as medium (werkrevisor)
hint; Jev: real 0.91, severity 1.1 of 3
The document uses <stateDir> in two places (lines 66 and 71). Markdown treats an unescaped <...> as an HTML tag and strips it, so the rendered path becomes just /matrix/sync.lock and /matrix/ instead of <stateDir>/matrix/sync.lock and <stateDir>/matrix/. In line 71 the closing > also appears after /matrix/ rather than after stateDir, so the markup is malformed and the directory name is unclear. Use backticks or escape the angle brackets so the placeholder is preserved.
**Markdown tag rendered away**, severity unknown, counted as medium (werkrevisor)
hint; Jev: real 0.91, severity 1.1 of 3
The document uses `<stateDir>` in two places (lines 66 and 71). Markdown treats an unescaped `<...>` as an HTML tag and strips it, so the rendered path becomes just `/matrix/sync.lock` and `/matrix/` instead of `<stateDir>/matrix/sync.lock` and `<stateDir>/matrix/`. In line 71 the closing `>` also appears after `/matrix/` rather than after `stateDir`, so the markup is malformed and the directory name is unclear. Use backticks or escape the angle brackets so the placeholder is preserved.
No change: both <stateDir> stand inside backticks, i.e. in code spans, where Markdown renders < and > literally; Gitea shows <stateDir>/matrix/sync.lock in full.
No change: both `<stateDir>` stand inside backticks, i.e. in code spans, where Markdown renders `<` and `>` literally; Gitea shows `<stateDir>/matrix/sync.lock` in full.
Werkbaum indentation, severity unknown, counted as medium (werkrevisor)
hint; Jev: real 0.79, severity 1.4 of 3
The new description " Werkregie PR#11: one client per machine; a second process reads along. is added one column to the left of its parent bullet - [^] Login as a device of its own, in the relay (S). The previous description on the same bullet is indented two columns further (under the bullet). With the new line at the parent level it reads as a description of #dash.matrix rather than of the Login item it actually explains, and the Werkbaum viewer will likely misassociate it.
**Werkbaum indentation**, severity unknown, counted as medium (werkrevisor)
hint; Jev: real 0.79, severity 1.4 of 3
The new description `" Werkregie PR#11: one client per machine; a second process reads along.` is added one column to the left of its parent bullet `- [^] Login as a device of its own, in the relay (S)`. The previous description on the same bullet is indented two columns further (under the bullet). With the new line at the parent level it reads as a description of `#dash.matrix` rather than of the Login item it actually explains, and the Werkbaum viewer will likely misassociate it.
Aligned in 6cc5199. The association was right before: a " line belongs to the preceding node whatever its indentation (Werkbaum SPEC, descriptions); the parser attaches both lines to "Login as a device of its own".
Aligned in `6cc5199`. The association was right before: a `"` line belongs to the preceding node whatever its indentation (Werkbaum SPEC, descriptions); the parser attaches both lines to "Login as a device of its own".
claude-code marked this conversation as resolved
mi
dismissed ai-reviewer's review 2026-10-10 22:13:58 +02:00
Reason:
New commits pushed, approval review dismissed automatically according to repository settings
ORANGE 1 finding(s) that Jev rates as probably real and severe: dashboard/src/main/kotlin/de/hoennig/werkregie/dashboard/sources/MatrixSource.kt:80-87
hint approval of werkrevisor for 960a446 not carried over: 1 approved block(s) are missing
hint 3 changed file(s) that neither weaken the checks nor touch security according to Jev
hint 1 build change(s) of low risk according to Jev: dashboard/build.gradle.kts (weakens checks 0.02, risk 0.00, security 0.01)
hint model recommends merge with caution
Findings
Possible Race Condition at dashboard/src/main/kotlin/de/hoennig/werkregie/dashboard/sources/MatrixSource.kt:80-87, severity unknown, counted as medium, ORANGE, as a line comment
Classifier
8 of 8 units passed all questions (1 PR document(s)).
3 answers ignored because of a confidence below [classifier] min_confidence.
Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 18841, calls 1, tokens in 18250 / out 19671 (reasoning included in out), duration 118.7 s, cost 0.0290 USD (reported), classifier `jev-1.13.0` calls 8, tokens in 6003, cost 0.0003 USD, finding judge `jev-1.13.0` calls 1, tokens in 1612, cost 0.0001 USD, build file judge `jev-1.13.0` calls 4, tokens in 4563, cost 0.0002 USD, phases: path_judge 0.3 s, classifier 0.6 s, context 0.2 s, model_review 116.9 s, finding_judge 0.2 s, config: defaults + host.
Reviewed head `6cc5199ba96a0dd34f00b7eba0ed3ec58cf6ba39` against base `c2ec294e26491048e4e9bea389bcf1fd3c3f3de0`, werkrevisor 0.32.2.
<!-- werkrevisor:6cc5199ba96a0dd34f00b7eba0ed3ec58cf6ba39 -->
<!-- werkrevisor-verdict:ORANGE -->
<!-- werkrevisor-blocks:51bfe85133929d3bf08479702546fd42,23eefd8b33ea52fbbd7ca22f63b64b83,7cd1870616b8b3a353adc5a261477af3,b857a263eb0726c73fef64c2c9307f5d,4aa77f6c2848de927e12a03e6e3a1068,a0b4357cedd0fa4b53baf06c19fc6f1b,b0e89c8480186a61e9af588ca369021a,1818b18b5d5fdc32ad736c45a9d66c9c,3bbba91bcb5860dcffde0485afc2f1e0,96c3e457b53864ff7d22377aba996d81,bafb39596b0eb0e049aa341807141335,65d82c336f5237c23b8d13f170c8b96d,286d01dbfc87b87fa5f1f9ff1d933cb0,57a03453fc621e29e5038463a979cfd2,9a01133b6a875ac4e80d6fb3746af540,3526da7a131181c358bfea9b58722edb,d596c48fd6acd464d353f022d4d0345c -->
<!-- werkrevisor-folded -->
<details><summary>🟠 werkrevisor ORANGE for <code>6cc5199</code>, superseded by <a href="http://git.javagil.de/mi/werkregie/pulls/11#issuecomment-3424">the result for <code>5ecf800</code></a></summary>
## 🟠 werkrevisor: ORANGE / FINDINGS
Change kind: `mixed`.
### Reasons
- **ORANGE** 1 finding(s) that Jev rates as probably real and severe: dashboard/src/main/kotlin/de/hoennig/werkregie/dashboard/sources/MatrixSource.kt:80-87
- **hint** approval of werkrevisor for `960a446` not carried over: 1 approved block(s) are missing
- **hint** 3 changed file(s) that neither weaken the checks nor touch security according to Jev
- **hint** 1 build change(s) of low risk according to Jev: dashboard/build.gradle.kts (weakens checks 0.02, risk 0.00, security 0.01)
- **hint** model recommends merge with caution
### Findings
1. **Possible Race Condition** at `dashboard/src/main/kotlin/de/hoennig/werkregie/dashboard/sources/MatrixSource.kt:80-87`, severity unknown, counted as medium, ORANGE, as a line comment
### Classifier
8 of 8 units passed all questions (1 PR document(s)).
3 answers ignored because of a confidence below [classifier] min_confidence.
<sub>
Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 18841, calls 1, tokens in 18250 / out 19671 (reasoning included in out), duration 118.7 s, cost 0.0290 USD (reported), classifier `jev-1.13.0` calls 8, tokens in 6003, cost 0.0003 USD, finding judge `jev-1.13.0` calls 1, tokens in 1612, cost 0.0001 USD, build file judge `jev-1.13.0` calls 4, tokens in 4563, cost 0.0002 USD, phases: path_judge 0.3 s, classifier 0.6 s, context 0.2 s, model_review 116.9 s, finding_judge 0.2 s, config: defaults + host.<br>
Reviewed head `6cc5199ba96a0dd34f00b7eba0ed3ec58cf6ba39` against base `c2ec294e26491048e4e9bea389bcf1fd3c3f3de0`, werkrevisor 0.32.2.
</sub>
</details>
werkrevisor: ORANGE / FINDINGS for 6cc5199ba96a0dd34f00b7eba0ed3ec58cf6ba39.
The result comment of werkrevisor holds the details.
werkrevisor: ORANGE / FINDINGS for `6cc5199ba96a0dd34f00b7eba0ed3ec58cf6ba39`.
The result comment of werkrevisor holds the details.
<!-- werkrevisor-review:6cc5199ba96a0dd34f00b7eba0ed3ec58cf6ba39 -->
Possible Race Condition, severity unknown, counted as medium (werkrevisor)
ORANGE; Jev: real 0.86, severity 2.9 of 3
In fetch(), the assignment if (lock == null) lock = tryLock(dir) is a non-atomic read-modify-write on a @Volatile field. If two threads call fetch() concurrently, both can read lock == null and both invoke tryLock(). Only one acquires the OS-level lock; the other receives null and then writes null back to lock, overwriting the successful acquisition. After the race, the field reports lock == null even though the first thread still holds the OS lock, so syncing returns false and the source falls back to readSaved() for the rest of the run, while sync is actually happening in the background and messages.json is being updated. The user-facing note() likewise misreports "liest mit". The existing synchronized(items) blocks and the new @Volatile on lock suggest concurrent fetch() is a realistic scenario; the assignment should be wrapped in synchronized or use AtomicReference.compareAndSet.
**Possible Race Condition**, severity unknown, counted as medium (werkrevisor)
ORANGE; Jev: real 0.86, severity 2.9 of 3
In `fetch()`, the assignment `if (lock == null) lock = tryLock(dir)` is a non-atomic read-modify-write on a `@Volatile` field. If two threads call `fetch()` concurrently, both can read `lock == null` and both invoke `tryLock()`. Only one acquires the OS-level lock; the other receives `null` and then writes `null` back to `lock`, overwriting the successful acquisition. After the race, the field reports `lock == null` even though the first thread still holds the OS lock, so `syncing` returns `false` and the source falls back to `readSaved()` for the rest of the run, while sync is actually happening in the background and `messages.json` is being updated. The user-facing `note()` likewise misreports "liest mit". The existing `synchronized(items)` blocks and the new `@Volatile` on `lock` suggest concurrent `fetch()` is a realistic scenario; the assignment should be wrapped in `synchronized` or use `AtomicReference.compareAndSet`.
Fixed in 5ecf800: the check and the assignment run in synchronized(this), so a taken lock is never overwritten with null. In the dashboard only one refresh per source runs at a time (Dashboard.refreshIfDue), so the race needed a caller outside it.
Fixed in `5ecf800`: the check and the assignment run in `synchronized(this)`, so a taken lock is never overwritten with null. In the dashboard only one refresh per source runs at a time (`Dashboard.refreshIfDue`), so the race needed a caller outside it.
werkrevisor: GREEN / PASS for 5ecf8006220d3016c73f26f8b1e7d9aab7eff210.
The result comment of werkrevisor holds the details.
werkrevisor: GREEN / PASS for `5ecf8006220d3016c73f26f8b1e7d9aab7eff210`.
The result comment of werkrevisor holds the details.
<!-- werkrevisor-review:5ecf8006220d3016c73f26f8b1e7d9aab7eff210 -->
Findings of werkrevisor: the lock leaked when the login failed;
Scenario#11.01 named implementation details, Scenario#11.02 was hard to read.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ai-review/assessment AI review passed; approved by werkrevisor; merge not scheduled: the protection of main does not require an approval and status checks
werkrevisor: GREEN / PASS for c8d10a76e8d5ac63c731200d28fc9e6d55884a0e.
The result comment of werkrevisor holds the details.
werkrevisor: GREEN / PASS for `c8d10a76e8d5ac63c731200d28fc9e6d55884a0e`.
The result comment of werkrevisor holds the details.
<!-- werkrevisor-review:c8d10a76e8d5ac63c731200d28fc9e6d55884a0e -->
claude-code
marked the pull request as work in progress 2026-10-11 06:33:46 +02:00
claude-code
marked the pull request as ready for review 2026-10-11 06:33:51 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fix of PR#8: only one process of a machine runs the Matrix client, the one holding
sync.lock; another one readsmessages.jsonalong and takes over when the first ends.matrix loginrefuses while another process syncs.PR document:
doc/prs/2026-10-10-PR#11-matrix-one-client.md. 68 tests green, no compiler warnings. Version 0.4.3.🤖 Generated with Claude Code
🟠 werkrevisor ORANGE for
a416479, superseded by the result forf8895e2🟠 werkrevisor: ORANGE / FINDINGS
Change kind:
mixed.Reasons
Findings
Lock leak on error path at
dashboard/src/main/kotlin/de/hoennig/werkregie/dashboard/sources/MatrixSource.kt:170-190, severity unknown, counted as medium, ORANGE, as a line commentUnclear Given/When/Then at
doc/prs/2026-10-10-PR#11-matrix-one-client.md:38-44, severity unknown, counted as medium, hint, as a line commentClassifier
6 of 8 units passed all questions (1 PR document(s)).
Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 17975, calls 1, tokens in 17412 / out 14727 (reasoning included in out), duration 67.6 s, cost 0.0229 USD (reported), classifier `jev-1.13.0` calls 8, tokens in 5955, cost 0.0003 USD, finding judge `jev-1.13.0` calls 2, tokens in 3372, cost 0.0001 USD, build file judge `jev-1.13.0` calls 4, tokens in 4046, cost 0.0002 USD, phases: path_judge 0.3 s, classifier 0.5 s, context 0.2 s, model_review 65.5 s, finding_judge 0.2 s, config: defaults + host.2 answers ignored because of a confidence below [classifier] min_confidence.
Reviewed head `a4164793637b9fbed846bdae69e5aa954c13228f` against base `c2ec294e26491048e4e9bea389bcf1fd3c3f3de0`, werkrevisor 0.32.2.
werkrevisor: ORANGE / FINDINGS for
a4164793637b9fbed846bdae69e5aa954c13228f.The result comment of werkrevisor holds the details.
@@ -166,5 +166,5 @@} finally {client.closeSuspending()}}Lock leak on error path, severity unknown, counted as medium (werkrevisor)
ORANGE; Jev: real 0.88, severity 2.6 of 3
In
login(), thetry { ... } finally { ... }only wraps the access toclient.userId.full to client.deviceId. IfbaseUrl(...),classicLoginWithPassword(...).getOrThrow(), oropenClient(...).getOrThrow()throws after the lock is acquired, the lock is never released, andsync.lockstays held by the still-open file channel until the process exits. The current CLI usage masks this because the process exits after login (and the OS reclaims the lock), but the bug will manifest iflogin()is ever called from a longer-lived process or retried in the same process. The fix is to move thelock.release()into a try-finally that also wraps theopenClientcall.Fixed in
f8895e2: the channel of the lock is closed on every path after it was taken (lock.channel().use { … }). Red/green: the test "a failed login frees the lock" failed without the fix and passes with it.@@ -0,0 +35,4 @@- [MatrixSourceTest "a second process reads the messages along, without a client of its own"](../../dashboard/src/test/kotlin/de/hoennig/werkregie/dashboard/sources/MatrixSourceTest.kt)#### Scenario#11.02: the second process takes overUnclear Given/When/Then, severity unknown, counted as medium (werkrevisor)
hint; Jev: real 0.75, severity 1.2 of 3
In Scenario#11.02, the Given clause "a process reads along" is a consequence of the lock being held by another process rather than the precondition itself, and the When "the syncing process ends" does not mention the lock becoming available. Compared to 11.01 and 11.03 (which use "Given one process of the machine syncs Matrix"), this scenario leaves the reader guessing why a reading process would suddenly start a client. A clearer Given would name the lock state (e.g. "Given one process of the machine holds
sync.lockand another reads along frommessages.json") and the When would name the trigger (e.g. "When the syncing process ends and the lock is released").Fixed in
f8895e2: Scenario#11.02 now names the relay and the local mode, "Given the relay syncs, and the local mode shows its messages", "When the relay stops"; Scenario#11.01 no longer namesmessages.json.⚠️ werkrevisor ERROR for
a416479, superseded by the result forf8895e2⚠️ werkrevisor: ERROR / review not completed, human action needed
Change kind:
mixed.Reasons
Findings
Lock leak on error path at
dashboard/src/main/kotlin/de/hoennig/werkregie/dashboard/sources/MatrixSource.kt:170-190, severity unknown, counted as medium, ORANGE, as a line commentUnclear Given/When/Then at
doc/prs/2026-10-10-PR#11-matrix-one-client.md:38-44, severity unknown, counted as medium, hint, as a line commentClassifier
6 of 8 units passed all questions (1 PR document(s)).
Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 17975, calls 1, tokens in 17412 / out 14727 (reasoning included in out), duration 67.6 s, cost 0.0229 USD (reported), classifier `jev-1.13.0` calls 8, tokens in 5955, cost 0.0003 USD, finding judge `jev-1.13.0` calls 2, tokens in 3372, cost 0.0001 USD, build file judge `jev-1.13.0` calls 4, tokens in 4046, cost 0.0002 USD, phases: path_judge 0.3 s, classifier 0.5 s, context 0.2 s, model_review 65.5 s, finding_judge 0.2 s, config: defaults + host.2 answers ignored because of a confidence below [classifier] min_confidence.
Reviewed head `a4164793637b9fbed846bdae69e5aa954c13228f` against base `c2ec294e26491048e4e9bea389bcf1fd3c3f3de0`, werkrevisor 0.32.2.
🟡 werkrevisor YELLOW for
f8895e2, superseded by the result for960a446🟡 werkrevisor: YELLOW / HUMAN_REVIEW
Change kind:
mixed.Reasons
Findings
doc/prs/2026-10-10-PR#11-matrix-one-client.md:72, severity unknown, counted as medium, hint, as a line commentClassifier
7 of 8 units passed all questions (1 PR document(s)).
Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 18795, calls 1, tokens in 18210 / out 23975 (reasoning included in out), duration 112.3 s, cost 0.0342 USD (reported), classifier `jev-1.13.0` calls 8, tokens in 6021, cost 0.0003 USD, finding judge `jev-1.13.0` calls 1, tokens in 1072, cost 0.0000 USD, build file judge `jev-1.13.0` calls 4, tokens in 4563, cost 0.0002 USD, phases: path_judge 0.3 s, classifier 0.7 s, context 0.2 s, model_review 110.5 s, finding_judge 0.2 s, config: defaults + host.1 answer ignored because of a confidence below [classifier] min_confidence.
Reviewed head `f8895e2a4de2aa8a06b2c71967efa81630627af0` against base `c2ec294e26491048e4e9bea389bcf1fd3c3f3de0`, werkrevisor 0.32.2.
werkrevisor: YELLOW / HUMAN_REVIEW for
f8895e2a4de2aa8a06b2c71967efa81630627af0.The result comment of werkrevisor holds the details.
@@ -0,0 +69,4 @@On each fetch it tries the lock again and starts its own client once it gets it.- `matrix login` takes the lock first and refuses while another process holds it.- The directory `<stateDir>/matrix/` gets mode 0700 also when the lock creates it first.- `matrix login` frees the lock on every path, also when the login fails (finding of werkrevisor).Meta-reference in Solution section, severity unknown, counted as medium (werkrevisor)
hint; Jev: real 0.61, severity 0.9 of 3
The Solution bullet "matrix login frees the lock on every path, also when the login fails (finding of werkrevisor)." ends with a parenthetical "(finding of werkrevisor)" that points to the review process instead of describing the change itself; a PR document records what the change does, not how it was reviewed.
Fixed in
960a446: the parenthetical is gone; Scenario#11.02 now has one condition and one result.🟢 werkrevisor GREEN for
960a446, superseded by the result for6cc5199🟢 werkrevisor: GREEN / PASS
Gate: approved by werkrevisor; merge waits for 2 open hint(s): reply to each line comment and resolve it, then restart the review or push a commit.
Change kind:
mixed.Reasons
Findings
Werkbaum indentation at
doc/werkregie.werkbaum:33, severity unknown, counted as medium, hint, as a line commentMarkdown tag rendered away at
doc/prs/2026-10-10-PR#11-matrix-one-client.md:66-71, severity unknown, counted as medium, hint, as a line commentClassifier
8 of 8 units passed all questions (1 PR document(s)).
Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 18842, calls 1, tokens in 18251 / out 8318 (reasoning included in out), duration 198.3 s, cost 0.0111 USD (reported), classifier `jev-1.13.0` calls 8, tokens in 6003, cost 0.0003 USD, finding judge `jev-1.13.0` calls 2, tokens in 2974, cost 0.0001 USD, build file judge `jev-1.13.0` calls 4, tokens in 4563, cost 0.0002 USD, phases: path_judge 0.2 s, classifier 0.5 s, context 0.3 s, model_review 196.7 s, finding_judge 0.3 s, config: defaults + host.3 answers ignored because of a confidence below [classifier] min_confidence.
Reviewed head `960a4464f23c23b9d9f6ebabf3c60b287b49f031` against base `c2ec294e26491048e4e9bea389bcf1fd3c3f3de0`, werkrevisor 0.32.2.
werkrevisor: GREEN / PASS for
960a4464f23c23b9d9f6ebabf3c60b287b49f031.The result comment of werkrevisor holds the details.
@@ -0,0 +63,4 @@## Solution- The process that holds the file lock `<stateDir>/matrix/sync.lock` runs the client.Markdown tag rendered away, severity unknown, counted as medium (werkrevisor)
hint; Jev: real 0.91, severity 1.1 of 3
The document uses
<stateDir>in two places (lines 66 and 71). Markdown treats an unescaped<...>as an HTML tag and strips it, so the rendered path becomes just/matrix/sync.lockand/matrix/instead of<stateDir>/matrix/sync.lockand<stateDir>/matrix/. In line 71 the closing>also appears after/matrix/rather than afterstateDir, so the markup is malformed and the directory name is unclear. Use backticks or escape the angle brackets so the placeholder is preserved.No change: both
<stateDir>stand inside backticks, i.e. in code spans, where Markdown renders<and>literally; Gitea shows<stateDir>/matrix/sync.lockin full.@@ -30,6 +30,7 @@" Werkregie PR#5: it does; the spike is in dashboard/spikes/matrix-sdk.- [^] Login as a device of its own, in the relay (S) :#dash.matrix.sdk" Werkregie PR#8: werkregie-dashboard matrix login; the session lives in an H2 file." Werkregie PR#11: one client per machine; a second process reads along.Werkbaum indentation, severity unknown, counted as medium (werkrevisor)
hint; Jev: real 0.79, severity 1.4 of 3
The new description
" Werkregie PR#11: one client per machine; a second process reads along.is added one column to the left of its parent bullet- [^] Login as a device of its own, in the relay (S). The previous description on the same bullet is indented two columns further (under the bullet). With the new line at the parent level it reads as a description of#dash.matrixrather than of the Login item it actually explains, and the Werkbaum viewer will likely misassociate it.Aligned in
6cc5199. The association was right before: a"line belongs to the preceding node whatever its indentation (Werkbaum SPEC, descriptions); the parser attaches both lines to "Login as a device of its own".New commits pushed, approval review dismissed automatically according to repository settings
🟠 werkrevisor ORANGE for
6cc5199, superseded by the result for5ecf800🟠 werkrevisor: ORANGE / FINDINGS
Change kind:
mixed.Reasons
960a446not carried over: 1 approved block(s) are missingFindings
dashboard/src/main/kotlin/de/hoennig/werkregie/dashboard/sources/MatrixSource.kt:80-87, severity unknown, counted as medium, ORANGE, as a line commentClassifier
8 of 8 units passed all questions (1 PR document(s)).
Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 18841, calls 1, tokens in 18250 / out 19671 (reasoning included in out), duration 118.7 s, cost 0.0290 USD (reported), classifier `jev-1.13.0` calls 8, tokens in 6003, cost 0.0003 USD, finding judge `jev-1.13.0` calls 1, tokens in 1612, cost 0.0001 USD, build file judge `jev-1.13.0` calls 4, tokens in 4563, cost 0.0002 USD, phases: path_judge 0.3 s, classifier 0.6 s, context 0.2 s, model_review 116.9 s, finding_judge 0.2 s, config: defaults + host.3 answers ignored because of a confidence below [classifier] min_confidence.
Reviewed head `6cc5199ba96a0dd34f00b7eba0ed3ec58cf6ba39` against base `c2ec294e26491048e4e9bea389bcf1fd3c3f3de0`, werkrevisor 0.32.2.
werkrevisor: ORANGE / FINDINGS for
6cc5199ba96a0dd34f00b7eba0ed3ec58cf6ba39.The result comment of werkrevisor holds the details.
@@ -66,1 +77,4 @@/** Whether this process runs the client, rather than reading along. */internal val syncing: Boolean get() = lock != nulloverride fun fetch(now: Instant, since: Instant): List<Item> {Possible Race Condition, severity unknown, counted as medium (werkrevisor)
ORANGE; Jev: real 0.86, severity 2.9 of 3
In
fetch(), the assignmentif (lock == null) lock = tryLock(dir)is a non-atomic read-modify-write on a@Volatilefield. If two threads callfetch()concurrently, both can readlock == nulland both invoketryLock(). Only one acquires the OS-level lock; the other receivesnulland then writesnullback tolock, overwriting the successful acquisition. After the race, the field reportslock == nulleven though the first thread still holds the OS lock, sosyncingreturnsfalseand the source falls back toreadSaved()for the rest of the run, while sync is actually happening in the background andmessages.jsonis being updated. The user-facingnote()likewise misreports "liest mit". The existingsynchronized(items)blocks and the new@Volatileonlocksuggest concurrentfetch()is a realistic scenario; the assignment should be wrapped insynchronizedor useAtomicReference.compareAndSet.Fixed in
5ecf800: the check and the assignment run insynchronized(this), so a taken lock is never overwritten with null. In the dashboard only one refresh per source runs at a time (Dashboard.refreshIfDue), so the race needed a caller outside it.🟢 werkrevisor GREEN for
5ecf800, superseded by the result forc8d10a7🟢 werkrevisor: GREEN / PASS
Change kind:
mixed.Reasons
Classifier
8 of 8 units passed all questions (1 PR document(s)).
Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 18894, calls 1, tokens in 18301 / out 4495 (reasoning included in out), duration 27.1 s, cost 0.0109 USD (reported), classifier `jev-1.13.0` calls 8, tokens in 6003, cost 0.0003 USD, build file judge `jev-1.13.0` calls 4, tokens in 4586, cost 0.0002 USD, phases: path_judge 0.2 s, classifier 0.5 s, context 0.3 s, model_review 25.8 s, config: defaults + host.3 answers ignored because of a confidence below [classifier] min_confidence.
Reviewed head `5ecf8006220d3016c73f26f8b1e7d9aab7eff210` against base `c2ec294e26491048e4e9bea389bcf1fd3c3f3de0`, werkrevisor 0.32.2.
werkrevisor: GREEN / PASS for
5ecf8006220d3016c73f26f8b1e7d9aab7eff210.The result comment of werkrevisor holds the details.
5ecf800622toc8d10a76e8New commits pushed, approval review dismissed automatically according to repository settings
🟢 werkrevisor: GREEN / PASS
Change kind:
mixed.Reasons
5ecf800not carried over: 2 approved block(s) are missingClassifier
8 of 8 units passed all questions (1 PR document(s)).
Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 18929, calls 1, tokens in 18333 / out 13292 (reasoning included in out), duration 73.7 s, cost 0.0214 USD (reported), classifier `jev-1.13.0` calls 8, tokens in 6003, cost 0.0003 USD, build file judge `jev-1.13.0` calls 4, tokens in 5908, cost 0.0002 USD, phases: path_judge 0.3 s, classifier 0.5 s, context 0.7 s, model_review 71.9 s, config: defaults + host + repository (.werkrevisor.toml at main3 answers ignored because of a confidence below [classifier] min_confidence.
09e0252).Reviewed head `c8d10a76e8d5ac63c731200d28fc9e6d55884a0e` against base `09e0252854adeda5d331455489f32ff84ee37ce7`, werkrevisor 0.32.2.
werkrevisor: GREEN / PASS for
c8d10a76e8d5ac63c731200d28fc9e6d55884a0e.The result comment of werkrevisor holds the details.