[barbatdev]

Implements the directory projection mode from the maintainer-approved scoped plan (Phase A revision v2): cortex install --target-dir <path> projects skills as plain SKILL.md files into an existing directory, inside the existing transaction (manifest, ownership, backup, rollback, readback).

• New RuntimeID directory + RootKind directory, deliberately outside the probed runtime order; explicit DirectoryTargetMatrix constructor.
• Threaded through every validation seam (skilldest, skillroot, installplan, installstate, adapterplan, projection, skillartifact, installtxn, ownership, installobserve) with the three probed runtimes untouched.
• Fail-closed diagnostics: relative, missing, and non-directory paths are refused with error=invalid_target before any write.
• Review size: 503 lines, above the 400-line budget — approved exception: one behavior unit that does not split into working intermediate states.

Full go test ./internal/... green; go vet clean.

Fixes #341

[barbatdev]

Fleet review (glm)

Lens: requirements + adversarial. The directory projection mode is threaded through every validation seam with the probed three-runtime shape preserved, and the end-to-end test exercises the production dispatch. Two blocking findings: the install argument parser silently drops every argument after a valid --target-dir value, and the PR claims "Fixes #341" while its own code comment defers the QA-actor half of that issue's scope ("full capability set") to #344/#356. Two questions cover the promised manifest-hash verification and the unwritable-path diagnostic, which the tests do not actually exercise.

Blocking (2)

• parseTargetDir silently ignores every argument after --target-dir (internal/cli/install.go:96): The for loop body returns unconditionally on its first iteration, so only args[1] is ever inspected. Any argument appearing after a consumed --target-dir value is dropped without a diagnostic, while a non-flag first argument is refused with invalid_command. This contradicts the fail-closed argument handling the same stack adds for doctor and uninstall (runUninstallCommand/runDoctorCommand reject every unknown argument with invalid_command).
  • Failure scenario: User runs cortex install --target-dir /a --target-dir /b (or appends any extra token after the value). The command exits 0 and projects the catalog into /a; /b is silently ignored with no diagnostic, so capabilities land in a directory the user did not ask for, with no error to notice.
• Claims "Fixes feat: project capabilities to a user-selected directory target on install #341" but defers the QA-actor half of that issue's scope (internal/cli/install.go:140): Issue feat: project capabilities to a user-selected directory target on install #341 scope: "cortex install --target-dir projects skills and QA actors as plain SKILL.md files into the chosen directory" and acceptance criterion: "Install into an empty target directory writes the full capability set with a manifest that records provenance and hashes." The buildDirectoryRequests comment states the opposite: "skill-only (QA actors render as skills in feat: render QA actors as skills for directory targets and document the consumption contract #344, not here)", yet the PR body says "Fixes feat: project capabilities to a user-selected directory target on install #341". The criterion is unsatisfied at the head that claims to close the issue.
  • Failure scenario: Run cortex install --target-dir <empty dir> at this head: skills are projected but QA actors are not (they are deferred to feat: render QA actors as skills for directory targets and document the consumption contract #344/feat(qa): project QA actors as skills for directory targets and document the mode #356), so the directory does not contain the full capability set that issue feat: project capabilities to a user-selected directory target on install #341's acceptance criterion requires, while the PR is marked as fixing it.

Suggestion (1)

• Agent Skills consumption contract not yet documented where the mode is documented (internal/cli/doctor.go:172): Issue feat: project capabilities to a user-selected directory target on install #341 scope: "The Agent Skills specification ... is the consumption contract for the output; Cortex's metadata.cortex-* frontmatter passes through. Document this contract where the mode is documented." The rootHelp addition only names "Agent Skills layout" and says nothing about the frontmatter pass-through. If this documentation is planned for feat(qa): project QA actors as skills for directory targets and document the mode #356's operating-guide section, nothing is missing in the stack — but at this head the scope item is only partially addressed.

Question (2)

• Manifest hash "verification" only checks the hash is 64 characters (internal/cli/install_directory_test.go:103): Issue feat: project capabilities to a user-selected directory target on install #341's Checks promise "manifest hash verification". The test's readback loop asserts only len(artifact.SHA256) == 64 and len(data) > 0; it never compares sha256(data) against the recorded hash, so a wrong hash value would pass.
• Unwritable existing directory is untested despite being named in the acceptance criteria (internal/cli/install_directory_test.go:96): Acceptance criterion: "Refusing or failing paths (nonexistent, unwritable) fail closed with an explicit diagnostic." TestInstallDirectoryTargetFailClosedOnInvalidPaths covers relative, missing, file, and empty, but not an existing directory without write permission. It is unverified what explicit diagnostic installFailureReason produces for a permission error on the target, and whether the transaction refuses before any write.

Verdict: not ready

[barbatdev]

Post-merge QA (glm)

Directory projection mode for install (issue #341). The transactional path is genuinely reused (adapterplan validateDirectoryTarget, single directory result, all-or-nothing enforced), fail-closed diagnostics cover relative/missing/file/empty targets, the manifest records sha256 per artifact, and the directory runtime stays out of the probed runtimeOrder. Two non-blocking test-quality gaps: the readback test only checks that recorded hashes are 64 chars long rather than verifying file content against them, and parseTargetDir returns after the first recognized flag so arguments after --target-dir's value are silently ignored.

Suggestion (2)

• Readback test checks hash length, not content hash (internal/cli/install_directory_test.go:99): The test comments claim 'Readback: every manifest artifact exists on disk with the recorded hash' but only asserts len(artifact.SHA256) != 64. It never computes sha256 of the file bytes, so a projection that wrote corrupted content with a correct manifest would pass. Production readback may be stricter, but the end-to-end test does not prove it.
• parseTargetDir silently ignores arguments after the --target-dir value (internal/cli/install.go:83): The parse loop returns unconditionally at the end of its first iteration, so once --target-dir (or --target-dir=) is consumed any remaining arguments are discarded: cortex install --target-dir /x --bogus runs a directory install and never sees --bogus. Probed install keeps its zero-argument shape, so only the directory mode is affected, but the parse contract differs from parseUpdateArgs, which rejects unknown flags.

Verdict: clean

[barbatdev]

Post-merge QA (deepseek)

Lens: evidence + test design.

The merge implements #341 convincingly in breadth: the directory RuntimeID and RootKind are threaded through every validation seam (skilldest, skillroot, installplan, installstate, adapterplan, projection, skillartifact, installtxn, ownership, installobserve) without touching the probed three-runtime shape, the new TestDirectoryTargetIsExplicitAndUnprobed pins that Decide still rejects a directory observation and that the runtime stays outside runtimeOrder, and the main install test drives the production dispatch against a temp target and then does a real readback of the recorded manifest — every artifact present, non-empty, with a 64-hex digest. That is representative evidence for the first, third, and fourth acceptance criteria.

Two evidence gaps and one defect. The defect is the fail-closed promise for relative paths: the PR body and the parseTargetDir comment both say a relative path is refused with error=invalid_target before any write, but only a missing relative path is. A relative path that exists passes os.Stat, is then rejected by skillroot.ResolveIntoDirectory (which requires filepath.IsAbs), and the failure is reported as install_composition_failed with exit 1. The test that is supposed to cover this uses "relative/target", which does not exist, so it asserts the missing-path diagnostic and never exercises the relativity refusal at all. The unwritable half of the same acceptance criterion has no directory-mode test, and the mid-install rollback criterion is explicitly deferred to the filetxn suite even though the directory mode adds its own .cortex state write.

Blocking (1)

• A relative path that exists fails with install_composition_failed, not invalid_target (internal/cli/install.go:111): runDirectoryInstall validates only existence and IsDir through os.Stat, then hands the path to buildDirectoryRequests → skillroot.ResolveIntoDirectory, whose validPath requires filepath.IsAbs. A non-absolute root therefore returns an error that the caller reports as install_composition_failed (exit 1). The PR body states "relative, missing, and non-directory paths are refused with error=invalid_target before any write", and the comment above parseTargetDir (line 91) repeats it, so the code contradicts its own documented contract for exactly the case the docs later use as the example invocation.
  • Failure scenario: mkdir agent-skills then cortex install --target-dir ./agent-skills in that parent directory. os.Stat succeeds, ResolveIntoDirectory rejects the non-absolute root, and the command prints error=install_composition_failed and exits 1 — not error=invalid_target, and with no mention of the path, so the operator reads it as an internal bug rather than a usage error.

Suggestion (3)

• The relative fail-closed case passes for the wrong reason (internal/cli/install_directory_test.go:111): failClosedCases uses "relative/target", a relative path that also does not exist. The refusal it observes is the os.IsNotExist branch, not the relativity check, so the suite cannot distinguish "relative is refused" from "missing is refused" and stayed green while the code reports the wrong diagnostic. A fixture that creates the relative directory first (os.Mkdir a path relative to the test's working directory, plus cleanup) would exercise the claim the PR makes.
• unwritable and mid-install rollback have no directory-mode evidence (internal/cli/install_directory_test.go:93): Issue feat: project capabilities to a user-selected directory target on install #341 lists "Refusing or failing paths (nonexistent, unwritable) fail closed with an explicit diagnostic" and "A failed write mid-install leaves the directory unchanged (transactional, all-or-nothing)". The first has no unwritable target case at all; the second is deferred with a comment to the filetxn suite, but the directory mode also writes .cortex/install-state.json and stages a backup under the user's home, so a mid-transaction failure has directory-specific consequences the filetxn suite does not observe. Both deserve a case here: a 0o500 target (skipped for root, as the doctor test already does) and one induced failure that asserts the target holds no partial files and no state file.
• parseTargetDir silently ignores arguments after the first --target-dir (internal/cli/install.go:76): The loop returns on the first --target-dir it finds, so install --target-dir /a --target-dir /b or install --target-dir /a --apply installs into /a and never reports the extra argument, while doctor and update both refuse duplicate or unknown flags. Silent acceptance of a second destination is the one place a typo could install into the wrong tree without a diagnostic.

Verdict: blocking