What happened?
next_review is accepted by the create and update DTOs of four registers, returns 2xx, and is then overwritten before the row is written. The value the caller sent never reaches the database.
The four register Update/Create paths call a recompute helper as their first statement:
| Register |
Write path |
Recompute |
| Risk |
db/risks.go:242 (create), :478 (update) |
CalculateScore → CalculateReviewDate (:135) |
| Supplier |
db/suppliers.go:132 (create), :317 (update) |
CalculateNextReview (:96) |
| System |
db/systems.go:171 (create), :365 (update) |
CalculateNextReview (:121) |
| Legal |
db/legal.go:151 (create), :349 (update) |
CalculateRiskScore (:89) → CalculateReviewDate (:72) |
| Asset |
db/assets.go:273 (update) |
none — the value is stored as sent |
The transactional twins used by suggestion-apply do the same: UpdateRiskTx (db/suggestions_tx.go:117), UpdateSupplierTx (:363), UpdateSystemTx (:605), UpdateLegalRequirementTx (:419) — while UpdateAssetTx (:628) does not.
The readings handlers are not at fault, which is what makes this confusing to read. All five of them branch correctly and comment the intent — "Next review: explicit user input wins, otherwise derive from …" (api_readings.go:266, :472, :572) and "Set next review date from explicit input, or compute from level" (:142, :372). Each assigns the caller's date, then hands the entity to a db update that recomputes it one call later. Only the asset path (:266-278) survives, because UpdateAssetTx leaves the field alone.
The doc comments claim the behaviour the code doesn't implement:
db/suppliers.go:94 — "sets next_review based on criticality (when not already set)". There is no such check; the body assigns unconditionally.db/legal.go:70 — "This is just a default suggestion — users can always override the date manually."db/systems.go:120 — "Cycle is purely derived — users override the date through readings/access reviews."
The one place that does honour "when not already set" is the cron: isms manager guards its backfill with if s.NextReview == nil || s.NextReview.IsZero() (cmd/isms/manager.go:97, :119) — and since every create path stamps the column, that guard can essentially never fire.
The sharper half — legal requirements cannot go overdue. Risk, supplier and system all anchor the computed date on LastReview when it is set:
base := time.Now()
if s.LastReview != nil && !s.LastReview.IsZero() {
base = s.LastReview.Time
}(db/suppliers.go:98-101, identically db/systems.go:123-126, db/risks.go:146-149)
LegalRequirement.CalculateReviewDate has no such anchor — it is always time.Now().AddDate(0, months, 0) (db/legal.go:82). So the workaround that exists for the other three (backdate last_review, let the cycle put next_review in the past) does not exist for legal. GetOverdueSummary selects on next_review < now() (db/overdue.go:115-125), so summary.Legal is structurally always empty, GET /overdue reports "legal": null, and CreateOverdueReviewTasks (db/overdue.go:296) — which drives review tasks off that same summary, and has a dedicated legal_review loop for them (:352-360) — can never create one. A register whose entire purpose is tracking regulatory obligations is the one register that cannot report a missed review.
Risk has a second guard the others lack: CalculateReviewDate early-returns when CurrentLevel == "" (db/risks.go:136-138). So an unassessed risk keeps an explicitly-set next_review, and an assessed one loses it. Legal's version has no early return and defaults to 12 months, so it fires even on a requirement with no likelihood/impact recorded.
Steps to reproduce
Against a server at $BASE/api/v1 as admin or manager. Today's date in the run below was 2026-08-06. Steps 1–3 use one set of throwaway entities; step 4 creates a fresh pair so that nothing has touched their dates in between.
1. The date is discarded on create.
POST /legal {"title":"…","jurisdiction":"EU","category":"privacy","status":"open",
"last_review":"2025-01-15","next_review":"2029-01-15"}
→ 201 last_review 2025-01-15 next_review 2027-08-06 ← 12 months from now, not 2029-01-15
POST /suppliers {"name":"…","supplier_type":"saas","criticality":"low",
"data_access":false,"status":"active","next_review":"2029-01-15"}
→ 201 next_review 2027-08-06
POST /systems {"name":"…","classification":"internal","criticality":"low",
"status":"active","next_review":"2029-01-15"}
→ 201 next_review 2027-08-06
POST /risks {…,"current_likelihood":4,"current_impact":4,"next_review":"2029-01-15"}
→ 201 level critical next_review 2026-09-06 ← 1-month critical cycle
POST /risks {…, no likelihood/impact, "next_review":"2029-01-15"}
→ 201 level "" next_review 2029-01-15 ← kept: the CurrentLevel=="" early return
POST /assets {"name":"…","asset_type":"software","status":"open","next_review":"2029-01-15"}
→ 201 next_review 2029-01-15 ← kept: no recompute at all
2. PUT is equally ineffective — and asset is the control.
PUT /legal/9 {"next_review":"2026-06-15"} → 200 GET → next_review 2027-08-06 (unchanged)
PUT /suppliers/7 {"next_review":"2025-07-15"} → 200 GET → next_review 2027-08-06 (unchanged)
PUT /systems/5 {"next_review":"2025-07-15"} → 200 GET → next_review 2027-08-06 (unchanged)
PUT /assets/13 {"next_review":"2025-07-15"} → 200 GET → next_review 2025-07-15 (stored)
3. A reading's next_review is honoured by the handler, then clobbered by the update.
POST /legal/9/readings {"current_likelihood":4,"current_impact":4,"next_review":"2026-09-15"}
→ 201 GET /legal/9 → last_review 2026-08-06, level critical, next_review 2026-09-06
2026-09-06 is one month from today (the critical cycle), not the 2026-09-15 that was sent. Note this is one month from today rather than from last_review, which the reading had just stamped to today as well — the two coincide here, but legal never reads LastReview at all, so the same clobber lands on a requirement whose last review was years ago.
4. The same backdate makes a supplier overdue and a legal requirement not. A fresh LEGAL-10 and SUPPLIER-8, both created with no dates, both then given the same last_review, with nothing in between and /overdue read immediately after:
PUT /legal/10 {"last_review":"2024-01-15"} → 200
PUT /suppliers/8 {"last_review":"2024-01-15"} → 200
GET /legal/10 → last_review 2024-01-15, next_review 2027-08-06 ← unmoved; 12 months from today
GET /suppliers/8 → last_review 2024-01-15, next_review 2025-01-15 ← recomputed off last_review
GET /overdue
"legal": null
"suppliers": [ {"entity_id":"SUPPLIER-8","days_late":568,…},
{"entity_id":"SUPPLIER-4","days_late":387,…} ]
Identical input, opposite outcome. Backdating last_review is the only way to mark a review as missed, it works for suppliers (and by the same anchor for risks and systems), and it does nothing at all for legal requirements.
legal_review is a first-class task type with its own loop in CreateOverdueReviewTasks (db/overdue.go:352-360), building "Legal review: <title> (<identifier>)" tasks from summary.Legal — a list that can never be non-empty.
Expected vs. actual
Expected: an explicit next_review on create, update, or a reading is stored. The computed cycle is a default for when the caller supplies nothing — which is what all three doc comments and the isms manager backfill guard already describe. And a legal requirement whose review has genuinely slipped shows up in GET /overdue.
Actual: the value is silently replaced on every write to a risk, supplier, system or legal requirement, with 2xx and a response body that already shows the substituted date. A legal requirement can never be overdue regardless of what is recorded on it, so GET /overdue and the review tasks derived from it are structurally incomplete.
Two things worth deciding together, since one fix touches both:
- Make the compute a default rather than an override. The narrow version is to apply the "when not already set" guard the comment already promises, in the four
Calculate* helpers. That is not sufficient on its own, because the recompute happens in the db layer where "did the caller ask for this?" is not knowable — existing.NextReview is already populated from the row. The DTOs carry the right signal and it is being dropped: NextReview **db.Epoch (api_legal.go:46, api_register_dtos.go:39, :78, :118, :178) distinguishes absent / set / explicitly-null, and the handlers already decode it correctly (api_legal.go:242-244). Threading "the caller set this" down to the update — or recomputing in the handler before the db call, as the readings paths already do — is what actually fixes it.
- Give
LegalRequirement.CalculateReviewDate the LastReview anchor the other three have (db/legal.go:82). Independent of (1), and on its own it makes legal overdue-able. Whether risk's CurrentLevel == "" early return should be the shared behaviour is a smaller call inside the same change.
Component / surface
Registers (risks, assets, suppliers, …)
Version
86ccf59 (0.7.x, unreleased master). Behaviour above reproduced live on a dd3b6b2 build; the files involved were last changed in a2ff9ac (2026-07-14), so the two are equivalent here.
Anything else?
Is this data-loss? Asking rather than asserting. The caller's date is discarded at write time rather than existing data being destroyed, which is the reasoning that kept #200 on bug alone. The argument for the stronger label is the second half: /overdue and the tasks generated from it are a compliance signal, and for legal requirements that signal is not merely wrong but structurally unreachable. Maintainers' call.
Scope: who this bites today. The Legal, Suppliers, Systems, Risks and Assets views only render next_review and derive an OVERDUE badge from it — Legal.vue:160,385,402, Suppliers.vue:171,343,374, Systems.vue:177,346,377, Risks.vue:172,587,616, Assets.vue:268,289,320. None has an input bound to the field, so no browser flow can hit the clobber. This lands on API, MCP and CLI clients, and on anyone reading GET /overdue. The DTOs' **db.Epoch set-vs-clear typing is fairly clear evidence the field was meant to be settable.
It travels into suggestion apply. applySupplierUpdate / applySystemUpdate / applyLegalUpdate / applyRiskUpdate route through the *Tx variants listed above, so an agent-authored suggestion proposing a review date is dropped by this bug rather than by #200's narrow unmarshal structs. Different cause, same outcome, and #200's fix will not recover it.
Org-configurable cycles do not help. riskReviewCycles (db/risks.go:221-235) lets an org set risk_review_cycle_<level> and feeds both risk and legal. It changes which wrong date gets written, not whether the caller's date survives. Supplier and system cycles are hard-coded (suppliers.go:81-92, systems.go:106-117) and not configurable at all — worth noting but not part of this issue.
Found during a multi-user manual test run on 2026-08-05 against a locally-built server (Postgres, file storage backend, iso27001 template scaffolded, five accounts across admin/manager/contributor/reader). Every code reference above was re-verified against 86ccf59, and every HTTP exchange was re-run live on 2026-08-06 against throwaway entities created for the purpose and deleted afterwards.
What happened?
next_reviewis accepted by the create and update DTOs of four registers, returns2xx, and is then overwritten before the row is written. The value the caller sent never reaches the database.The four register
Update/Createpaths call a recompute helper as their first statement:db/risks.go:242(create),:478(update)CalculateScore→CalculateReviewDate(:135)db/suppliers.go:132(create),:317(update)CalculateNextReview(:96)db/systems.go:171(create),:365(update)CalculateNextReview(:121)db/legal.go:151(create),:349(update)CalculateRiskScore(:89) →CalculateReviewDate(:72)db/assets.go:273(update)The transactional twins used by suggestion-apply do the same:
UpdateRiskTx(db/suggestions_tx.go:117),UpdateSupplierTx(:363),UpdateSystemTx(:605),UpdateLegalRequirementTx(:419) — whileUpdateAssetTx(:628) does not.The readings handlers are not at fault, which is what makes this confusing to read. All five of them branch correctly and comment the intent — "Next review: explicit user input wins, otherwise derive from …" (
api_readings.go:266,:472,:572) and "Set next review date from explicit input, or compute from level" (:142,:372). Each assigns the caller's date, then hands the entity to adbupdate that recomputes it one call later. Only the asset path (:266-278) survives, becauseUpdateAssetTxleaves the field alone.The doc comments claim the behaviour the code doesn't implement:
db/suppliers.go:94— "sets next_review based on criticality (when not already set)". There is no such check; the body assigns unconditionally.db/legal.go:70— "This is just a default suggestion — users can always override the date manually."db/systems.go:120— "Cycle is purely derived — users override the date through readings/access reviews."The one place that does honour "when not already set" is the cron:
isms managerguards its backfill withif s.NextReview == nil || s.NextReview.IsZero()(cmd/isms/manager.go:97,:119) — and since every create path stamps the column, that guard can essentially never fire.The sharper half — legal requirements cannot go overdue. Risk, supplier and system all anchor the computed date on
LastReviewwhen it is set:(
db/suppliers.go:98-101, identicallydb/systems.go:123-126,db/risks.go:146-149)LegalRequirement.CalculateReviewDatehas no such anchor — it is alwaystime.Now().AddDate(0, months, 0)(db/legal.go:82). So the workaround that exists for the other three (backdatelast_review, let the cycle putnext_reviewin the past) does not exist for legal.GetOverdueSummaryselects onnext_review < now()(db/overdue.go:115-125), sosummary.Legalis structurally always empty,GET /overduereports"legal": null, andCreateOverdueReviewTasks(db/overdue.go:296) — which drives review tasks off that same summary, and has a dedicatedlegal_reviewloop for them (:352-360) — can never create one. A register whose entire purpose is tracking regulatory obligations is the one register that cannot report a missed review.Risk has a second guard the others lack:
CalculateReviewDateearly-returns whenCurrentLevel == ""(db/risks.go:136-138). So an unassessed risk keeps an explicitly-setnext_review, and an assessed one loses it. Legal's version has no early return and defaults to 12 months, so it fires even on a requirement with no likelihood/impact recorded.Steps to reproduce
Against a server at
$BASE/api/v1as admin or manager. Today's date in the run below was 2026-08-06. Steps 1–3 use one set of throwaway entities; step 4 creates a fresh pair so that nothing has touched their dates in between.1. The date is discarded on create.
2.
PUTis equally ineffective — and asset is the control.3. A reading's
next_reviewis honoured by the handler, then clobbered by the update.2026-09-06 is one month from today (the critical cycle), not the 2026-09-15 that was sent. Note this is one month from today rather than from
last_review, which the reading had just stamped to today as well — the two coincide here, but legal never readsLastReviewat all, so the same clobber lands on a requirement whose last review was years ago.4. The same backdate makes a supplier overdue and a legal requirement not. A fresh
LEGAL-10andSUPPLIER-8, both created with no dates, both then given the samelast_review, with nothing in between and/overdueread immediately after:Identical input, opposite outcome. Backdating
last_reviewis the only way to mark a review as missed, it works for suppliers (and by the same anchor for risks and systems), and it does nothing at all for legal requirements.legal_reviewis a first-class task type with its own loop inCreateOverdueReviewTasks(db/overdue.go:352-360), building"Legal review: <title> (<identifier>)"tasks fromsummary.Legal— a list that can never be non-empty.Expected vs. actual
Expected: an explicit
next_reviewon create, update, or a reading is stored. The computed cycle is a default for when the caller supplies nothing — which is what all three doc comments and theisms managerbackfill guard already describe. And a legal requirement whose review has genuinely slipped shows up inGET /overdue.Actual: the value is silently replaced on every write to a risk, supplier, system or legal requirement, with
2xxand a response body that already shows the substituted date. A legal requirement can never be overdue regardless of what is recorded on it, soGET /overdueand the review tasks derived from it are structurally incomplete.Two things worth deciding together, since one fix touches both:
Calculate*helpers. That is not sufficient on its own, because the recompute happens in thedblayer where "did the caller ask for this?" is not knowable —existing.NextReviewis already populated from the row. The DTOs carry the right signal and it is being dropped:NextReview **db.Epoch(api_legal.go:46,api_register_dtos.go:39,:78,:118,:178) distinguishes absent / set / explicitly-null, and the handlers already decode it correctly (api_legal.go:242-244). Threading "the caller set this" down to the update — or recomputing in the handler before thedbcall, as the readings paths already do — is what actually fixes it.LegalRequirement.CalculateReviewDatetheLastReviewanchor the other three have (db/legal.go:82). Independent of (1), and on its own it makes legal overdue-able. Whether risk'sCurrentLevel == ""early return should be the shared behaviour is a smaller call inside the same change.Component / surface
Registers (risks, assets, suppliers, …)
Version
86ccf59(0.7.x, unreleasedmaster). Behaviour above reproduced live on add3b6b2build; the files involved were last changed ina2ff9ac(2026-07-14), so the two are equivalent here.Anything else?
Is this
data-loss? Asking rather than asserting. The caller's date is discarded at write time rather than existing data being destroyed, which is the reasoning that kept #200 onbugalone. The argument for the stronger label is the second half:/overdueand the tasks generated from it are a compliance signal, and for legal requirements that signal is not merely wrong but structurally unreachable. Maintainers' call.Scope: who this bites today. The Legal, Suppliers, Systems, Risks and Assets views only render
next_reviewand derive an OVERDUE badge from it —Legal.vue:160,385,402,Suppliers.vue:171,343,374,Systems.vue:177,346,377,Risks.vue:172,587,616,Assets.vue:268,289,320. None has an input bound to the field, so no browser flow can hit the clobber. This lands on API, MCP and CLI clients, and on anyone readingGET /overdue. The DTOs'**db.Epochset-vs-clear typing is fairly clear evidence the field was meant to be settable.It travels into suggestion apply.
applySupplierUpdate/applySystemUpdate/applyLegalUpdate/applyRiskUpdateroute through the*Txvariants listed above, so an agent-authored suggestion proposing a review date is dropped by this bug rather than by #200's narrow unmarshal structs. Different cause, same outcome, and #200's fix will not recover it.Org-configurable cycles do not help.
riskReviewCycles(db/risks.go:221-235) lets an org setrisk_review_cycle_<level>and feeds both risk and legal. It changes which wrong date gets written, not whether the caller's date survives. Supplier and system cycles are hard-coded (suppliers.go:81-92,systems.go:106-117) and not configurable at all — worth noting but not part of this issue.Found during a multi-user manual test run on 2026-08-05 against a locally-built server (Postgres, file storage backend,
iso27001template scaffolded, five accounts across admin/manager/contributor/reader). Every code reference above was re-verified against86ccf59, and every HTTP exchange was re-run live on 2026-08-06 against throwaway entities created for the purpose and deleted afterwards.