Persist app ERROR cause in status_message and expose via API (#129)#177
Open
ClaydeCode wants to merge 4 commits into
Open
Persist app ERROR cause in status_message and expose via API (#129)#177ClaydeCode wants to merge 4 commits into
ClaydeCode wants to merge 4 commits into
Conversation
First migration after the init schema. Establishes the yoyo -- depends: naming convention (shard-core-NNNN-<slug>). Column is nullable so it applies cleanly to existing databases and defaults to no message. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
update_app_status now writes the message to the new status_message column
when a status becomes ERROR, and clears it (NULL) on any other transition.
The InstalledApp model gains a status_message field, so both
GET /protected/apps and GET /protected/apps/{name} return the reason,
which now survives a page reload instead of only reaching the UI via the
transient on_app_install_error websocket event.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Covers: ERROR message persisted, cleared on leaving ERROR, ignored for
non-ERROR status, and still returned by GET /protected/apps/{name} after
a reload. Documents the shard-core-NNNN-<slug> yoyo migration convention
in agents.md.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
- test_message_ignored_for_non_error_status now pre-seeds an ERROR message so it proves the STOPPED transition actively clears it, not just that a message was never set. - API round-trip test now also asserts the list endpoint (GET /protected/apps), which the issue names explicitly, and uses the lighter app_client fixture (no Docker/lifespan) instead of api_client. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
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
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.
Closes #129.
What & why
update_app_status(name, status, message=...)only logged the error message — it was never persisted. Error reasons reached the UI solely through the transienton_app_install_errorwebsocket event, so after a page reload an errored app showed a bareERRORstatus with no cause.This persists the cause in the database so it survives a reload and can be rendered by the web-terminal / Sundial.
Changes
shard-core-0002-app-status-message.sql— adds a nullablestatus_message TEXTcolumn toinstalled_apps. First migration after the init schema; establishes theshard-core-NNNN-<slug>.sql+-- depends:naming convention (now documented inagents.md).update_app_statuswritesstatus_messagewhen a status becomesERRORand clears it (NULL) on any other transition. The directupdate_statuscalls inapp_tools.py(RUNNING/PAUSED/STOPPED/DOWN) also clear it via the new default-None param — which is exactly the "clear on leaving ERROR" behavior.InstalledAppmodel gainsstatus_message: Optional[str] = None, so it flows automatically intoGET /protected/appsandGET /protected/apps/{name}(both use theInstalledAppWithMetaresponse model) and the websocket app push.tests/test_app_status_message.py): persisted on ERROR; cleared on leaving ERROR; ignored (and actively cleared) for non-ERROR; and survives a reload via both the single-app and list API endpoints.Acceptance criteria
test_error_message_survives_reload_via_api, asserting both endpoints).-- depends: shard-core-0001-init, no table rewrite, no backfill. Verified: yoyo resolves the dependency chain and the migration runs on every test DB boot.Recommended reading order
migrations/shard-core-0002-app-status-message.sqlshard_core/data_model/app_meta.py(model field)shard_core/database/installed_apps.py(update_statuscolumn write)shard_core/service/app_installation/util.py(update_app_statusERROR-guard)tests/test_app_status_message.pyagents.mdVerification
tests/test_app_status_message.py— 4 passed. Proven falsifiable: forcingstatus_message = Noneinupdate_app_statusmakestest_error_message_is_persistedfail.test_app_installation(12), plustest_model,test_db_snapshot,test_traefik_dyn_spec,test_app_error.ruff check .clean;just cleanupapplied.Review panel
Five reviewer subagents ran (adversarial, test adversary, DevEx, DB/migration, API-contract), each given only the diff + issue. No blocking findings from any reviewer. Advisory findings addressed:
GET /protected/apps.api_clienttier: fixed — demoted to the lighterapp_clientfixture (no Docker/lifespan).docker_shutdown_all_apps(force=True)flips apps with an existing compose file toDOWN, which clears the message. The persisted message inherits exactly the same restart-durability as theERRORstatus itself; the issue's acceptance criterion is browser reload, which is fully satisfied. Broadening restart-survival is out of scope for Persist app error causes so the UI can show why an app is in ERROR #129.db_snapshotrestore, only reachable on an unsupported version downgrade; migrate-runs-before-restore makes normal fresh restores always target the latest schema. Not introduced here.🤖 Generated with Claude Code