intersystems / intersystems/ipm
History log "committed" column is inaccurate
- Dominant language
- ObjectScript
- Stars
- 41
- Forks
- 29
- Avg merge
- 23h 54m
- Merged PRs (30d)
- 4
Description
## Describe the bug
The `Committed` column on history records does not reliably indicate whether an action's changes survived. It is computed from the action's status and whether developer mode was on, rather than from what actually happened to the transaction:
`%IPM.General.AbstractHistory:Finalize()` (`src/cls/IPM/General/AbstractHistory.cls:124`):
```objectscript
set ..Committed = (dev = 1) || $$$ISOK(status)
```
This is wrong in both directions.
**Dev-mode failures are always marked committed, even when fully rolled back.**
In `%IPM.Utils.Module:LoadModuleFromDirectory()`, the transaction opens at line 1160 and the first developer-mode `tcommit` is at line 1272. `tDeveloperMode` is not even resolved until line 1259. Everything in that window runs inside a transaction that the `catch` block rolls back at lines 1313-1315, regardless of developer mode.
So a `-dev` action that fails before line 1272 (for example a malformed `module.xml` rejected in `GetModuleObjectFromPath`) is rolled back in full, but its history record still reads `Committed`.
The intended dev-mode behavior for phases *after* line 1272 is correct and is not at issue here. The early-failure window is.
**Non-transactional failures are marked uncommitted despite keeping their changes.**
`tUseTransactions` is false when `-DNoTransaction=1` or `-DNoJournal=1` is passed (line 1137). No transaction is opened, so a failure leaves partial changes in the namespace. Because the status is an error and developer mode is off, the record reads `Uncommitted`.
## To Reproduce
Rolled-back action reported as committed:
1. `load /path/to/malformed-module -dev` where the directory contains a `module.xml` with a syntax error
2. `history find -limit 5`
3. The record shows an error in `Success` and `Committed` in `Committed`, even though the transaction was rolled back and nothing was written
Retained changes reported as uncommitted:
1. `install some-module -DNoTransaction=1` with a module whose `Activate` phase fails
2. `history find -limit 5`
3. The record reads `Uncommitted`, but partial changes remain in the namespace
## Expected behavior
`Committed` should reflect whether the action's changes were actually retained.
Options, roughly in increasing order of effort:
1. Compare `$tlevel` against the level captured at entry, or have the rollback path signal that it ran, and pass that to `Finalize()` instead of inferring from `dev`.
2. Set `Committed` at each commit point rather than once at the end, so the flag tracks how far the action actually got.
3. If the current value is intentional as a coarse signal, rename it or document it as "changes may have been retained" and stop treating `dev = 1` as sufficient.
Option 1 is the smallest change that fixes both directions, since it makes the flag observed rather than derived.
## System information
- IPM version: 0.10.5+
Contributor guide
Research direction
Start with %IPM.General.AbstractHistory:Finalize() at src/cls/IPM/General/AbstractHistory.cls:124 and trace the transaction setup, developer-mode resolution, commit, and rollback paths in %IPM.Utils.Module:LoadModuleFromDirectory() around lines 1137-1315. Reproduce both malformed module.xml loading with -dev and installation with -DNoTransaction=1; done means Committed matches whether changes were actually retained.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- xml
- Domain
- backend, cli
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100