Skip to content

Keep a FOS report readable after its task is gone - #1237

Merged
mastacontrola merged 1 commit into
dev-branchfrom
tasklog-retention-15
Aug 20, 2026
Merged

Keep a FOS report readable after its task is gone#1237
mastacontrola merged 1 commit into
dev-branchfrom
tasklog-retention-15

Conversation

@mastacontrola

Copy link
Copy Markdown
Member

Port of 1.6's schema 341 (#1236). Writer half only — this branch has no Task Management log pane, so there is no reader to change, which is also what makes the defect worse here rather than milder.

The problem

taskLog stores no host and no task type of its own and reaches both through tasks. Nothing deletes taskLog rows — but Host::destroy() calls TaskManager->destroy() and taskLog is in no cascade at all. So deleting a host destroys its tasks and leaves the reports behind with nothing to join to, losing the host name at the same moment the host row that could have supplied it goes.

On 1.6 the log pane at least still shows the text with a blank host column. Here tasklog is in Route::$validClasses and the REST API is its only reader — it hands back a report whose taskID points at nothing and no way at all to learn which machine it came from.

The fix

Schema 283 adds logHostID, logHostName, logTaskTypeName and backfills the reports whose task is still there, so the history is not split between rows that can answer the question and rows that cannot. TaskError fills them; nothing else does.

Every other row in this table is a state transition written by TaskingElement::taskLog() on every transition — meaningless without its task anyway, so it leaves them empty. Same reasoning that gave logText no value on a state row in step 280.

The state a row records was never at risk: taskLog stores taskStateID itself, so that lookup survives its task.

Two column shapes, following what the writer can actually produce: save() omits an unset optional column whose key ends in id, so logHostID lands NULL, but writes '' for every other unset field — the trap step 282 had to repair for logType.

Why not block deleting a task that has reports: it inverts the dependency — a diagnostic artifact would constrain operational cleanup — and to be consistent it would have to block host deletion too, since that's the path that actually removes tasks.

Not ported

The manifest repair that was the other half of #1236. This branch has no schema-expected.php, no SchemaReconciler and no bin/schema-manifest.php, so there is no second description of taskLog to drift out of step.

Tests

tests/task-error-report.test.php gains four checks in its own source-level idiom: the writer stores each field, the schema step adds the columns, and it backfills. Mutation-verified — dropping the three set() calls, dropping a field from the model, misspelling a column in the ALTER, and removing the backfill each fail it.

Step 283 was executed against a 1.5 server over temporary shadow tables and produces the intended rows: report with a live task filled, report already orphaned left empty, state row untouched.

sh tests/run-all.sh — 19 passed, 0 failed.

Downstream

No route classes changed, so no FogApi sync is implied.

Port of 1.6's schema 341 (#1236). The writer half only: this branch has
no Task Management log pane, so there is no reader to change -- which is
also what makes the defect worse here rather than milder.

taskLog stores no host and no task type of its own and reaches both
through `tasks`. Nothing deletes taskLog rows -- but Host::destroy()
calls TaskManager->destroy() and taskLog is in no cascade at all, so
deleting a host destroys its tasks and leaves the reports behind with
nothing to join to. The host name goes at the same moment the host row
that could supply it does.

On 1.6 the log pane at least still shows the text. Here the REST API is
the only reader `tasklog` has, and it hands back a report whose taskID
points at nothing, with no way at all to learn which machine it came
from. The point of GH-1206 is that a failure message is findable later
instead of arriving as a phone photo of a wrapped console, and a foreign
key to a routinely-deleted row cannot deliver that.

Blocking deletion of a task that has reports was the alternative. It
inverts the dependency -- a diagnostic artifact would then constrain
operational cleanup -- and to be consistent it would have to block HOST
deletion too, since that is the path that actually removes tasks.
Refusing to delete a host because it once failed to image is a worse
product than losing a host name.

Schema 283 adds logHostID, logHostName and logTaskTypeName, backfilling
the reports whose task is still there so the history is not split
between rows that can answer and rows that cannot. TaskError fills them;
nothing else does. Every other row here is a state transition written by
TaskingElement::taskLog() on every transition, meaningless without its
task anyway -- the same reasoning that gave logText no value on a state
row in step 280.

The state a row records was never at risk: taskLog stores taskStateID
itself, so that lookup survives its task.

Two column shapes, following what the writer can produce: save() omits
an unset OPTIONAL column whose key ends in "id", so logHostID lands
NULL, but writes '' for every other unset field -- the trap step 282 had
to repair for logType. Declaring logHostName NOT NULL DEFAULT '' says
what will really be stored.

Not ported: the manifest repair that was the other half of #1236. This
branch has no schema-expected.php, no SchemaReconciler and no
bin/schema-manifest.php, so there is no second description of taskLog to
drift out of step.

tests/task-error-report.test.php gains four checks in its own idiom --
the writer stores each field, the schema step adds the columns, and it
backfills. Mutation-verified: dropping the three set() calls, dropping a
field from the model, misspelling a column in the ALTER and removing the
backfill each fail it. Step 283 was executed against a 1.5 server over
temporary shadow tables and produces the intended rows.

sh tests/run-all.sh -- 19 passed, 0 failed.

Co-Authored-By: Claude <noreply@anthropic.com>
@mastacontrola
mastacontrola merged commit 77dfb2c into dev-branch Aug 20, 2026
7 checks passed
@mastacontrola
mastacontrola deleted the tasklog-retention-15 branch August 20, 2026 01:50
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.

2 participants