Skip to content

teach and push read and keep sealed artifacts - #1786

Merged
abrichr merged 33 commits into
mainfrom
feat/sealed-teach-push
Nov 4, 2026
Merged

abrichr merged 33 commits into
mainfrom
feat/sealed-teach-push

Conversation

@abrichr

@abrichr abrichr commented Nov 3, 2026 •

Copy link
Copy Markdown
Member

Last consumers in #1751. Stacked on #1777.

teach reads sealed input and keeps its output sealed whenever any input is sealed, or in strict mode:

  • the taught bundle, including crops and repair/candidate.json, under sealed_set.output_bundle_key();
  • the skill library, as a sealed set of the new kind library;
  • _teach_compiled, as its own sealed bundle inside an encrypted recording.

push, sanitize, review-sanitized and approve-sanitized:

  • Review copy. For a sealed source, or in strict mode, the sanitized review copy is a sealed set of the new kind sanitized. It is staged in a 0700 temp dir that is removed in a finally.
  • Approved archive. Stored AES-GCM-encrypted and decrypted in memory for the upload.
  • Scrubbing. The scrubbing, second pass and approval rescan are unchanged.

All of these join _SEALED_READY_COMMANDS. The reader lint test covers the converted functions, and the inventory doc test covers both new kinds.

Tests:

  • test_teach_reads_sealed_inputs_and_writes_sealed, test_teach_in_strict_mode_seals_plain_inputs_outputs and test_teach_refuses_a_plaintext_library_for_sealed_input.
  • test_push_of_a_sealed_recording_keeps_the_review_copy_sealed: the full push, review, redact, approve and upload flow, scanning the test directory for plaintext.
  • test_push_in_strict_mode_seals_the_review_copy_of_a_plain_recording.
  • test_artifact_reads.py: 18 passed.
  • With the teach, hosted, sanitized-artifact, repair-lifecycle and learn-loop suites: 369 passed, 1 skipped. Two new teach assertions were corrected afterwards and passed on re-run.

Open question for review: a sealed bundle that sanitization changes is re-saved through the sealed-set path, so the uploaded copy's workflow.json may carry encrypted: true. No test covers it yet.

About 440 code lines plus 290 test lines.

Review fixes (bb478b5, 017b1fd):

  • Teach refusals keep sealed content out. When the run is sealed or strict mode is on, a provenance refusal (wrong workflow, or a halt step the base does not hold) and the "stopped after a step acted" refusals print a generic reason with the halt id only. They no longer print the workflow name or the step intent.
  • Teach never writes over an input. check_out_is_new refuses before anything is written when --out or the skill library resolves to an input: the base bundle, the halted run, or the fix. "Resolves" means the same realpath, one path inside the other, or the same inode (os.path.samefile). A symlink alias of the base is refused too. Branch teach does the same check against all of its inputs.
  • Push refusals keep sealed content out. When the input is sealed or strict mode is on, a push failure caused by another error prints only that error's type. A Pydantic error, for example, quotes input_value. A failure push raises itself prints as before. Plain input outside strict mode keeps the full message.

New real-filesystem tests in test_artifact_reads.py:

  • test_teach_refusal_on_a_sealed_run_never_prints_its_contents[state|workflow]. The plain run files are deleted, a private marker is planted in the sealed report, and strict mode is on. The test checks that the marker appears in neither stdout nor stderr, and in no file under the test directory.
  • test_teach_never_writes_over_the_sealed_base[same path|symlink|inside]. The test checks exit 2, that the base files are unchanged, that no library or _teach_compiled is written, and that the base still loads.
  • test_teach_refuses_an_out_that_is_another_input[run|fix].
  • test_push_refusal_on_a_sealed_bundle_never_prints_its_contents[strict|sealed input]. The input is an authenticated sealed bundle whose action is the marker. The test checks that the marker appears in neither output nor any file.
  • test_push_refusal_on_a_plain_bundle_keeps_its_details, a control.

Test runs:

  • test_artifact_reads, test_sealed_at_rest, test_teach_cli, test_teach_branch, test_teach_branch_narration, test_agent_teach_branch and test_teach_placement: 96 passed, before the push fix.
  • test_artifact_reads, test_sealed_at_rest, test_teach_cli and test_teach_branch: 74 passed, after the push fix.
  • test_hosted, test_sanitized_artifact and the branch-teach and placement suites: 282 passed, 1 skipped.
  • ruff check and format pass on the changed files. The I001 at __main__.py (silent_saves import) is already on main.

Review fixes, round 3 (20b420a):

  • One shared place strips the input. New openadapt_flow/safe_errors.py. message() drops Pydantic's input_value from a validation error, and withheld() gives only the error type with a generic reason. Teach, branch teach, push and every REFUSED line in main print through it.
  • Teach summaries on sealed or strict input are generic. TeachResult has a private flag, set when the run is sealed. The summary then prints only the verdict and action with "withheld because the input is sealed or strict mode is on". No clusters, skill id or workflow name are shown, and gate failures appear only as a count.
  • The generic handler fails closed. If the input is sealed or strict mode is on, an unexpected exception in main prints FAILED: with the withheld reason to stderr and exits 1, with no traceback. If the check itself fails, it counts as private. Plain input re-raises as before.
  • A sealed skill library reads back with its own key (Codex P2). SkillLibrary._load reads through the library's own sealed-set writer, which checks each file's hash. The reader lint now allows those two calls, with that reason.
  • Docs: phi_at_rest.md states the --out overlap rule and that refusals never quote what they read.

Tests added or tightened in test_artifact_reads.py. The marker tests now capture stdout and stderr at the file-descriptor level (capfd) and the logs (caplog).

  • test_refusal_text_never_quotes_a_validation_input
  • test_an_unexpected_failure_on_sealed_input_prints_no_traceback
  • test_a_sealed_library_reads_back_with_its_own_key: a library sealed with one key reads back under that key, and is not readable under another.
  • test_teach_reads_sealed_inputs_and_writes_sealed now checks that the workflow name is absent from the LEARNED and no-change output, and that "withheld" is present. The plain-bundle control checks that the details are still shown.

Runs:

  • 20 teach, push, sealed, at-rest, learning, hosted and repair suites: 593 passed, 51 skipped, 1 failed. The failure was the reader lint on _load, which is now allowed as above.
  • After the fix, test_artifact_reads: 32 passed. test_skills_registry, test_teach_cli, test_sealed_at_rest, test_continuous_learning, test_mcp_teach_cli and test_teach_branch: 100 passed, 10 skipped.
  • ruff check and format pass on the changed files, apart from the I001 at __main__.py (silent_saves import), which is already on main.

Review fixes, round 4 (70a9cb2):

  • Success summaries show names only as a short hash. New safe_errors.label(name, private=...) is the one place teach and push summaries name a skill or workflow. For sealed or strict-mode input it prints "withheld, sha256 <12 hex>" instead of the name. The teach summary uses it for the skill id (which defaults to the decrypted workflow name). The push summary uses it for the server's workflow_name. Plain input outside strict mode still prints the name.
  • test_strict_teach_success_never_prints_a_name_from_its_input: plain inputs whose workflow name is the private marker, taught in strict mode. Both the LEARNED run and the second (no_change) run are checked. The marker appears in neither stdout nor stderr (capfd), nor in the logs (caplog), nor in any file of the taught bundle, the library or _teach_compiled.
  • test_teach_success_on_plain_input_names_its_skill: the control.
  • test_push_success_names_the_workflow_only_for_plain_input[strict|plain]: hosted.push is replaced with a stub that returns a workflow named with the marker. This tests only the summary line.
  • Runs: the new tests, 5 passed. test_artifact_reads, test_hosted, the teach, branch-teach, placement, skills, learning, learn-loop, sealed and sanitized suites (14 files): 476 passed, 51 skipped. ruff passes on the changed files.

Round 5 (fce3ec8), for the review in internal #100:

  • A teach or push refusal for sealed or strict-mode input now prints only the error's type. The one exception is an error marked public, meaning its text was built from the command line alone. TeachError(..., public=True) is set in three places: an --out or library that overlaps an input, a run that did not stop in the given bundle (that message no longer names the halt's state id), and opening the skill library. All three go through one helper, _refusal_text in __main__.py. Teach, teach-branch and push use it. This covers the library-mismatch refusal, which named the skill id, and unchained push errors.
  • branch_teach.py: the input loop's variable is renamed from ex to example, so mypy no longer sees ex typed as two different things. mypy is clean on teach.py and branch_teach.py.
  • New tests:
    • A strict library-mismatch test on a real sealed library. Its first teach succeeds; a second teach against a library holding another program for the same skill id returns 2, and the marker is absent from stdout, stderr and the logs.
    • A strict teach refusal prints its text only when the error is public.
    • A push failure with no cause is withheld in strict mode and printed as is for plain input.
    • The stale-state test now asserts that the state id is not printed.
  • Tests run: test_artifact_reads, test_teach_cli and test_hosted: 254 passed, 1 skipped. Branch-teach, sealed, sanitized, skills and learning suites: 233 passed. Doc and PHI suites: 21 passed. ruff is clean apart from the I001 at __main__.py:5785, which is already on main.
  • teach-branch is refused in strict mode and for sealed input before it runs. Its use of the helper is defensive only, and no test covers it.

Not covered:

  • The learn-loop outcome reasons and gate failures are withheld whole for private input, not audited one by one.
  • Open question above: the uploaded copy of a re-saved sealed bundle may carry encrypted: true.

With no key configured, Flow generates a local key (0600) and encrypts web
recordings, compiled and induced bundles (also from a plaintext recording)
and durable checkpoints. OPENADAPT_FLOW_PLAINTEXT_AT_REST opts out, with a
warning and a ledger line. OPENADAPT_FLOW_AT_REST=strict encrypts every run
directory and refuses what can't be encrypted yet; outside strict mode those
writes are named on stderr and in the ledger, and Desktop shows a notice.
Compile-agent screenshots are redacted before they reach the model, and heal
artifacts go through the sealed-set helpers. Remaining work: #1751.
…sections and check mark read sealed input

Add openadapt_flow.artifacts (thin wrapper over the sealed_set path helpers)
and route the artifact reads of these commands through it. for-each of a
sealed body writes a sealed loop bundle; emit-skill copies a sealed bundle as
ciphertext; record --with-check marks a check in an encrypted recording.
A lint test fails when a converted function reads an artifact with pathlib
or open(). Refs #1751.
…e read sealed children

Route the composition, process-contract and variant-set reads (parent files,
admission envelopes, child bundles, repair-candidate records) through
openadapt_flow.artifacts. compose copies sealed children as ciphertext;
process, variants and route read sealed children in place. Each command has a
sealed-input test, and the lint in test_artifact_reads covers the converted
functions. Refs #1751.
…s, deployment-key commands, ledger failures stop writes
…w copy sealed

teach of a sealed run, fix recording or bundle (and every teach in strict
mode) writes the taught bundle under the output bundle key, its skill library
as a sealed set of the new kind 'library', and the compiled fix as a sealed
bundle inside the recording. push, sanitize, review-sanitized and
approve-sanitized of a sealed source keep the sanitized review copy as a
sealed set of the new kind 'sanitized', staged 0700 and removed on failure,
with the approved archive encrypted beside it and decrypted in memory for the
upload. Scrubbing, the second pass and the approval rescan are unchanged.
Sealed-input tests for each, and the artifact-read lint covers the new
functions. Refs #1751.
@abrichr

abrichr commented Nov 3, 2026

Copy link
Copy Markdown
Member Author

Open question resolved: Workflow.load treats encrypted as true only with an encrypted envelope on disk (a sealed set or workflow.json.enc). A plaintext copy of a sealed bundle, such as the decrypted upload, loads with encrypted and manifest.encrypted set to False. Test: test_a_plaintext_copy_of_a_sealed_bundle_never_loads_as_encrypted. The stack's suites (reader, encryption-at-rest, run gate, governed-run validation, hosted, sanitized artifact, at-rest default, sealed-at-rest) ran on the zenbook lane: 394 passed, 1 skipped.

…only; read-only key resolver; post-run reports survive a ledger failure
# Conflicts:
#	packages/desktop/CHANGELOG.md
#	packages/desktop/openadapt_desktop/__init__.py
#	packages/desktop/package-lock.json
#	packages/desktop/package.json
#	packages/desktop/pyproject.toml
#	packages/desktop/src-tauri/Cargo.lock
#	packages/desktop/src-tauri/Cargo.toml
#	packages/desktop/src-tauri/tauri.conf.json
#	packages/desktop/uv.lock
@abrichr abrichr added the review-needed Needs an independent review from another vendor's agent label Nov 4, 2026
# Conflicts:
#	CAPABILITIES.md
#	llms-full.txt
# Conflicts:
#	CAPABILITIES.md
#	llms-full.txt
#	packages/flow/openadapt_flow/__main__.py
# Conflicts:
#	CAPABILITIES.md
#	llms-full.txt
# Conflicts:
#	CAPABILITIES.md
#	llms-full.txt
#	packages/flow/docs/phi_at_rest.md
#	packages/flow/openadapt_flow/__main__.py
#	packages/flow/tests/test_artifact_reads.py
# Conflicts:
#	CAPABILITIES.md
#	llms-full.txt
#	packages/flow/docs/phi_at_rest.md
Main's strict gate requires encrypted input for sealed-ready commands that
do not seal their own input. teach and push do seal it under strict mode
(their strict tests check no plaintext is left), so they join the
self-sealing set. sanitize stays gated: under strict it still needs
encrypted input and its output check still refuses.
@abrichr

abrichr commented Nov 4, 2026 •

Copy link
Copy Markdown
Member Author

Independent adversarial review: APPROVE

From: Codex. Re-reviewed actual head ffec567 after the approved 3c27d04 privacy fix and main integration. The other five privacy classifier/refusal helpers remain AST-identical.

The narrow Windows change treats only invalid-name/path codes 123 and 161 as no such path; other unreadable-path errors remain private. Those constants match Microsoft's Win32 error documentation. Nine independent error probes pass, including exact-parent controls, access/I/O/sharing errors, sealed ancestors beneath invalid logical names, and a sealed argument beside a URL. All 17 earlier real-filesystem/encryption/path probes also pass.

Full affected suite on repeat: 447 passed, 1 skipped. Full Flow Ruff/format/mypy pass (592 sources); exact-head CI is green apart from neutral CodeQL. The initial run had one fail-closed resume refusal because a stored checkpoint timestamp was later than its pause. Both timestamps were retained correctly; the unchanged isolated test and metadata-observed full repeat pass. Its initial ordering cause remains unestablished. Durable core code is unchanged from 3c27; no freshness guard or assertion was relaxed.

Synthetic fixtures, real crypto/filesystem, headless Linux resource gate; Windows errors were simulated rather than exercised on a native desktop. Bytecode/cache writes were disabled; runtime disk watchers and test guards remained active. No reviewed source was edited or pushed.

@abrichr abrichr removed the review-needed Needs an independent review from another vendor's agent label Nov 4, 2026
@abrichr abrichr added the review-needed Needs an independent review from another vendor's agent label Nov 4, 2026
Base automatically changed from feat/sealed-name-path to main November 4, 2026 10:05
Validation errors are reported through one shared formatter that drops
the input value. Teach summaries on sealed or strict input give only the
verdict and a withheld reason, and an unexpected failure on such input
prints no traceback. A sealed skill library now reads back with its own
key.
Teach and push success summaries name the skill and the workflow
through one shared helper. For sealed or strict-mode input it prints a
short sha256 instead of the name read from that input.
@abrichr abrichr removed the review-needed Needs an independent review from another vendor's agent label Nov 4, 2026
@abrichr abrichr added review-needed Needs an independent review from another vendor's agent and removed review-needed Needs an independent review from another vendor's agent labels Nov 4, 2026
@abrichr

abrichr commented Nov 4, 2026

Copy link
Copy Markdown
Member Author

Pushed 1e13e50..3c27d04 for the remaining P1 in Codex's review (comment 6627641331).

What changed:

  • _collect_sealed no longer skips any string by pattern. The newline, :// and length checks are gone. Every non-empty string (and the part after the first =) is checked against the filesystem.
  • New _sealed_root_named resolves the string with os.path.abspath and stats the sealed header in that directory and in each parent. ENOENT, ENOTDIR and ENAMETOOLONG mean no such path, so nothing sealed can be read through it. An embedded NUL is also not a path. Any other error (for example EACCES on a parent) raises _UnclassifiedArgument. The argument is then unclassified, so the command is treated as private and refused before it runs. Active in-process writers are still found through sealed_set.sealed_root.

How it was tested:

  • New tests in tests/test_artifact_reads.py:
    • Codex's two probes. teach-branch gets a sealed example under a directory with a newline in its name, and under a url: directory spelled with a redundant slash (.../url://sealed). Each is refused before teach_branch runs, the marker is in no stdout, stderr or log text, and no output bundle is written.
    • Text that names no local path (a URL, a string with a newline, a 5000-character string) stays public.
    • A path under a directory that can't be searched (mode 0) is private. Skipped as root.
  • Control: with 1e13e50's __main__.py, all four new cases fail.
  • Suites: test_artifact_reads, test_encryption_at_rest, test_run_gate, test_governed_run_validation, test_hosted, test_sanitized_artifact, test_at_rest_default and test_sealed_at_rest ran together: 437 passed, 1 skipped. mypy is clean on 589 sources. ruff check and format are clean on both changed files, apart from the I001 at __main__.py:5785, which is already on main.

Not tested: Linux and Windows. This ran only on macOS. The unreadable-directory test depends on POSIX permissions.

@abrichr abrichr added review-needed Needs an independent review from another vendor's agent and removed review-needed Needs an independent review from another vendor's agent labels Nov 4, 2026
# Conflicts:
#	CAPABILITIES.md
#	llms-full.txt
#	packages/flow/docs/phi_at_rest.md
#	packages/flow/openadapt_flow/__main__.py
#	packages/flow/tests/test_artifact_reads.py
@abrichr
abrichr merged commit 9f026da into main Nov 4, 2026
48 checks passed
@abrichr
abrichr deleted the feat/sealed-teach-push branch November 4, 2026 13:45
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.

1 participant