Skip to content

Log kli witness start failures and prevent startup stalls (#238) — v1.3 backport - #1486

Merged
pfeairheller merged 3 commits into
WebOfTrust:v1.3.5from
provenant-dev:bug-238-witness-logging-1.3
Jul 31, 2026
Merged

Log kli witness start failures and prevent startup stalls (#238) — v1.3 backport#1486
pfeairheller merged 3 commits into
WebOfTrust:v1.3.5from
provenant-dev:bug-238-witness-logging-1.3

Conversation

@dhh1128

@dhh1128 dhh1128 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Backport of #1477 to the 1.3 line (base v1.3.5). kli witness start could
stall during startup and did not reliably log error conditions (#238). This PR
makes the command log every startup/runtime failure visibly, eliminates the
interactive-prompt stall, and deprecates the mis-named --logfile in favor of the
accurate --logdir. Fixes #238.

The change is split into three commits so pieces can be dropped independently:

  1. Close boot Keeper before Habery re-open (fix: close boot Keeper and reuse Reger to prevent LMDB double-open on Linux #1367). v1.3.5 is missing the
    ks.close() fix that the 1.2 and 2.0 lines already carry, so on Linux LMDB
    raises environment is already open in this process and the witness cannot
    start once runWitness reaches the Habery/setupHby re-open. This commit
    restores it. Note: on 1.3.5 this is a functional prerequisite, not merely
    optional — the Ensure kli witness start logs errors correctly #238 logging fix can't be exercised without it, so dropping
    this commit would also require adjusting one test.
  2. Log failures + prevent non-TTY stalls (Ensure kli witness start logs errors correctly #238). Startup/runtime failures are
    logged at CRITICAL with a traceback (so they surface at the default
    --loglevel), the Habery is closed in a finally so a failed start leaves
    no stale LMDB lock, and an encrypted keystore started with no passcode on a
    non-TTY fails fast with a logged AuthError instead of blocking on an
    interactive getpass. --loglevel is normalized with .upper() so a
    lowercase level (e.g. debug) is honored.
  3. Deprecate --logfile in favor of --logdir (Ensure kli witness start logs errors correctly #238). The logging backend
    derives the file name from --name and the subdirectory from its own prefix,
    so only the directory is meaningful; the old code mis-assigned the whole file
    path as the log head directory. --logfile is retained as a deprecated alias
    whose directory portion is used, with a stderr deprecation notice. Droppable
    if you'd rather not change the CLI surface in a patch release.

Tests

Regression + unit tests added to tests/app/cli/test_kli_commands.py, covering
the boot-Keeper close ordering, CRITICAL failure logging + Habery cleanup, the
non-TTY fast-fail (both the no-passcode and wrong-passcode paths), --loglevel
normalization, and --logdir/--logfile directory handling + the deprecation
notice. Each assertion was mutation-checked — reverting the corresponding fix
turns the targeted test red. The 1.3 CI does not run pytest under -n auto, so
the shared-store xdist isolation fix from #1477 is not needed here.

Relationship to #1477

Same fix as the approved #1477 (base main, 2.0), adapted to the 1.3 CLI layout
(keri.app.cli.*, absolute imports, help.ogler). Opened per your request on
#1477.


This code was developed with Claude (using the Opus 4.8 model, mostly). However,
I (Daniel the human) have read, reviewed, and worked the code, and I stand behind
it and take responsibility for it.

dhh1128 added 3 commits July 10, 2026 15:15
…ust#1367)

runWitness opened the boot Keeper to read the aeid but never closed it
before Habery/setupHby re-opened the same keystore. On Linux LMDB then
raised "environment is already open in this process" and the witness
could not start. Close the boot Keeper immediately after reading the
aeid, matching the fix already present on the 1.2 and 2.0 lines.

Regression test spies on Keeper.close to confirm it runs before the
re-open; reverting the fix turns the test red with the LMDB error.

Signed-off-by: Daniel Hardman <daniel.hardman@gmail.com>
…bOfTrust#238)

Startup/runtime failures in kli witness start are now logged at CRITICAL
with a traceback so they surface even at the default --loglevel, and the
Habery is closed in a finally block so a failed start leaves no stale
LMDB lock. An encrypted keystore started with no passcode on a non-TTY
now fails fast with a logged AuthError (threaded through a new setupHby
noPrompt argument and a --no-prompt flag) instead of blocking on an
interactive getpass prompt. --loglevel is normalized with .upper() so a
lowercase level such as debug is honored.

Backport of WebOfTrust#1477 to the 1.3 line. Fixes WebOfTrust#238.

Signed-off-by: Daniel Hardman <daniel.hardman@gmail.com>
…rust#238)

--logfile is renamed to the accurately-named --logdir: the logging
backend derives the log file name from --name and the log subdirectory
from its own prefix, so only the directory is meaningful. The old code
mis-assigned the whole file path as the log head directory. --logfile is
retained as a deprecated alias whose directory portion is used, and a
deprecation notice reporting the resolved log path is printed to stderr.

Backport of WebOfTrust#1477 to the 1.3 line. Part of WebOfTrust#238.

Signed-off-by: Daniel Hardman <daniel.hardman@gmail.com>
@kentbull

Copy link
Copy Markdown
Contributor

I reviewed the main PR and this PR. They are both ready to go.

@dhh1128

dhh1128 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

@kentbull — approved on 2026-07-11 and still open. Could this land on v1.3.5?

Worth flagging an inversion this has created: the 1.2 backport (#1487) merged on 2026-07-14, so the 1.2 line now carries the #238 fix while 1.3.5 and main (#1477) do not. Anyone moving 1.2 → 1.3 currently loses witness startup logging.

Unchanged from your review: three individually-droppable commits, the first being the boot-Keeper ks.close() restore (#1367) that 1.3.5 is missing — on this line that is a functional prerequisite, not optional, since the witness cannot reach a successful Habery re-open without it.

@pfeairheller
pfeairheller merged commit 6fd851e into WebOfTrust:v1.3.5 Jul 31, 2026
7 checks passed
@dhh1128 dhh1128 mentioned this pull request Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants