Repository navigation
Scope preflight override to owned commands #67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
9f8f304
Scope preflight override to owned commands
unbraind 4ec5f22
Keep the deprecated linear-sync alias inside the credential gate
unbraind 6ee9838
Retitle the preflight item to name the defect rather than the remedy
unbraind 339e887
Gate the Linear preflight on network access, not on mutation
unbraind fc94ef8
Pin the host CLI exactly and declare the manifest floor where the loa…
unbraind 95ae136
State the dry-run network contract accurately in help, README and rec…
unbraind File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| {"ts":"2026-08-14T06:44:19.151Z","author":"claude","author_source":"asserted","agent_harness":"claude-code","agent_instance":"70140e5dd5b320efbbb8d5d3","agent_provenance":{"model":null,"effort":{"value":"xhigh","source":"environment"},"role":{"value":"implementer","source":"argv"},"topic":null},"op":"create","patch":[{"op":"add","path":"/metadata/id","value":"pm-linear-ydo4"},{"op":"add","path":"/metadata/title","value":"The deprecated linear-sync importer alias loses its credential gate when the preflight override is scoped"},{"op":"add","path":"/metadata/description","value":"Scoping registerPreflight to a command list narrows which commands the override runs for. The scope listed linear sync, linear import and linear export, but registerImporter also exposes the deprecated linear-sync alias, which reaches the same write path. The previously global registration covered it by accident. Under the scoped form the alias ran with no credential validation, and its handler did not call assertPreflightOk either, so a mutating legacy import could bypass the gate entirely. The alias is now in the scope list, classified as mutating by commandMutatesLinear on the same dry-run terms as linear import, and its handler asserts the preflight sentinel like every other mutating handler. A test binds the scope list to the classifier so the two cannot drift apart again."},{"op":"add","path":"/metadata/type","value":"Issue"},{"op":"add","path":"/metadata/status","value":"open"},{"op":"add","path":"/metadata/priority","value":2},{"op":"add","path":"/metadata/tags","value":[]},{"op":"add","path":"/metadata/created_at","value":"2026-08-14T06:44:19.151Z"},{"op":"add","path":"/metadata/updated_at","value":"2026-08-14T06:44:19.151Z"},{"op":"add","path":"/metadata/author","value":"claude"}],"before_hash":"3cc22dff72be7b14824654a7a64ea62b04799939b2fee54c1b5f52ca60bf6df0","after_hash":"925fc1b5e6ccd4cf36a54132c2d5d480ff4486be567172cf37fc6c428d971c2e","item_hash_version":2,"message":"","context":{"agent_provenance_outcomes":{"model":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"},"version":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"}}}} | ||
| {"ts":"2026-08-14T06:44:19.702Z","author":"claude","author_source":"asserted","agent_harness":"claude-code","agent_instance":"70140e5dd5b320efbbb8d5d3","agent_provenance":{"model":null,"effort":{"value":"xhigh","source":"environment"},"role":{"value":"implementer","source":"argv"},"topic":null},"op":"update","patch":[{"op":"replace","path":"/metadata/updated_at","value":"2026-08-14T06:44:19.702Z"},{"op":"replace","path":"/metadata/status","value":"in_progress"}],"before_hash":"925fc1b5e6ccd4cf36a54132c2d5d480ff4486be567172cf37fc6c428d971c2e","after_hash":"2713fbac94a22c05ea72ca7264f7335a319ae1fd0cc1271b6d0783b93ceefdec","item_hash_version":2,"context":{"agent_provenance_outcomes":{"model":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"},"version":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"}}}} | ||
| {"ts":"2026-08-14T06:44:20.339Z","author":"claude","author_source":"asserted","agent_harness":"claude-code","agent_instance":"70140e5dd5b320efbbb8d5d3","agent_provenance":{"model":null,"effort":{"value":"xhigh","source":"environment"},"role":null,"topic":null},"op":"files_add","patch":[{"op":"replace","path":"/metadata/updated_at","value":"2026-08-14T06:44:20.339Z"},{"op":"add","path":"/metadata/files","value":[{"path":"index.ts","scope":"project"}]}],"before_hash":"2713fbac94a22c05ea72ca7264f7335a319ae1fd0cc1271b6d0783b93ceefdec","after_hash":"864d030f69da6bf531cb6461c5b05419a6dd221125b94f84d4207bde58e5ed78","item_hash_version":2,"context":{"agent_provenance_outcomes":{"model":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"},"version":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"}}}} | ||
| {"ts":"2026-08-14T06:44:20.853Z","author":"claude","author_source":"asserted","agent_harness":"claude-code","agent_instance":"70140e5dd5b320efbbb8d5d3","agent_provenance":{"model":null,"effort":{"value":"xhigh","source":"environment"},"role":null,"topic":null},"op":"files_add","patch":[{"op":"add","path":"/metadata/files/1","value":{"path":"test/smoke.test.ts","scope":"project"}},{"op":"replace","path":"/metadata/updated_at","value":"2026-08-14T06:44:20.853Z"}],"before_hash":"864d030f69da6bf531cb6461c5b05419a6dd221125b94f84d4207bde58e5ed78","after_hash":"e754797f530ba2ea3f220b794d5fc96c9f06c8148040d1560b0d409d0ce54bb0","item_hash_version":2,"context":{"agent_provenance_outcomes":{"model":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"},"version":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"}}}} | ||
| {"ts":"2026-08-14T06:44:21.393Z","author":"claude","author_source":"asserted","agent_harness":"claude-code","agent_instance":"70140e5dd5b320efbbb8d5d3","agent_provenance":{"model":null,"effort":{"value":"xhigh","source":"environment"},"role":{"value":"implementer","source":"argv"},"topic":null},"op":"close","patch":[{"op":"replace","path":"/metadata/updated_at","value":"2026-08-14T06:44:21.393Z"},{"op":"replace","path":"/metadata/status","value":"closed"},{"op":"add","path":"/metadata/closed_at","value":"2026-08-14T06:44:21.377Z"},{"op":"add","path":"/metadata/completed_at","value":"2026-08-14T06:44:21.377Z"},{"op":"add","path":"/metadata/resolution","value":"linear-sync import added to the preflight scope and to commandMutatesLinear, and the legacy importer handler now calls assertPreflightOk."},{"op":"add","path":"/metadata/expected_result","value":"A mutating legacy linear-sync import refuses without credentials, exactly as linear import does."},{"op":"add","path":"/metadata/actual_result","value":"Scope, classifier and handler all cover the alias; a loop test asserts every scoped command is classified mutating so the lists cannot drift."},{"op":"add","path":"/metadata/close_reason","value":"completed"}],"before_hash":"e754797f530ba2ea3f220b794d5fc96c9f06c8148040d1560b0d409d0ce54bb0","after_hash":"6a489b99336c852ee06fd397bb030f2da87834470a11330ef81b251caa70a967","item_hash_version":2,"context":{"agent_provenance_outcomes":{"model":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"},"version":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"}}}} | ||
| {"ts":"2026-08-15T15:45:29.345Z","author":"claude-agent","author_source":"asserted","agent_harness":"claude-code","agent_instance":"2f5451bfe1c87748ced6bd1e","agent_provenance":{"model":null,"effort":{"value":"xhigh","source":"environment"},"role":{"value":"implementer","source":"argv"},"topic":null},"op":"update","patch":[{"op":"replace","path":"/metadata/expected_result","value":"A linear-sync import that reaches Linear refuses without credentials, exactly as linear import does. With --dry-run alone it is offline and runs without a key; with --atomic --dry-run it fetches issues and so refuses without credentials."},{"op":"replace","path":"/metadata/resolution","value":"linear-sync import is in the preflight scope and in the access classifier, and the legacy importer handler calls assertPreflightOk so the sentinel becomes a clean USAGE error. Review round 2 renamed the classifier from commandMutatesLinear to commandNeedsLinearAccess and changed what it asks: the preflight guards credentials and reachability, so the question is whether an invocation REACHES Linear, not whether it mutates. That distinction changes two edges. linear export --push --dry-run writes nothing and makes no network call, so it no longer demands a key. linear sync, linear import and linear-sync import with --atomic --dry-run also write nothing but DO fetch issues to build the plan, so they now require the gate they previously skipped. A scoped command is therefore not always gated: the gate depends on the option combination."},{"op":"replace","path":"/metadata/updated_at","value":"2026-08-15T15:45:29.345Z"}],"before_hash":"6a489b99336c852ee06fd397bb030f2da87834470a11330ef81b251caa70a967","after_hash":"a13db0051e516793cbd275a806f832866eed2ce3a3338a3f61df0ef49df382bb","item_hash_version":2,"message":"Correct the record to the access-based classifier contract","context":{"agent_provenance_outcomes":{"model":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"},"version":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"}}}} | ||
| {"ts":"2026-08-15T15:45:29.801Z","author":"claude-agent","author_source":"asserted","agent_harness":"claude-code","agent_instance":"2f5451bfe1c87748ced6bd1e","agent_provenance":{"model":null,"effort":{"value":"xhigh","source":"environment"},"role":null,"topic":null},"op":"comment_add","patch":[{"op":"replace","path":"/metadata/updated_at","value":"2026-08-15T15:45:29.801Z"},{"op":"add","path":"/metadata/comments","value":[{"created_at":"2026-08-15T15:45:29.801Z","author":"claude-agent","text":"Correction to this item's recorded contract (CodeRabbit, PR #67) — appended, not rewritten.\n\nThe resolution and expected-result recorded here describe `commandMutatesLinear` and mutation-only scope behaviour. Both are superseded by review round 2 on PR #67, which renamed the classifier to `commandNeedsLinearAccess` and changed the question it asks.\n\nThe preflight guards CREDENTIALS AND REACHABILITY, so the right question is whether an invocation reaches Linear, not whether it mutates it. Asking the mutation question was wrong at both `--dry-run` edges, in opposite directions:\n\n- `linear export --push --dry-run` writes nothing and makes no network call, yet the mutation test saw `--push` and demanded `LINEAR_API_KEY`, so a documented-offline preview failed on any host without one.\n- `linear sync`, `linear import` and `linear-sync import` with `--atomic --dry-run` also write nothing, but they are NOT offline: they fetch issues to build the plan they report. The mutation test saw `--dry-run` and skipped the gate, so a missing key surfaced as a raw fetch failure inside the sync core instead of the clean USAGE error.\n\nThe consequence for this item's own subject: being inside the preflight scope no longer implies being gated. `linear-sync import` is in scope, and whether the gate runs for a given invocation now depends on the option combination — offline with `--dry-run` alone, gated with `--atomic --dry-run` or with neither.\n\nThe `--dry-run` flag help and the README \"Offline vs live\" section both promised offline behaviour unconditionally and have been corrected in the same commit, since a user reading them would have been told a command needs no key when it does.\n"}]}],"before_hash":"a13db0051e516793cbd275a806f832866eed2ce3a3338a3f61df0ef49df382bb","after_hash":"fb7ccb3c2918aca3270533d4963af6e1db574c3b949f76ea2a5c1c26d51b6a53","item_hash_version":2,"message":"Record the access-classifier correction","context":{"agent_provenance_outcomes":{"model":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"},"version":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"}}}} |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| id: pm-linear-hwol | ||
| title: "A globally-scoped preflight override collides with every other installed package, so pm health can never be green" | ||
| description: "registerPreflight used the bare-function (global) form, which collides pairwise with every other installed package's preflight override (pm health reports extension_preflight_override_collision). Rewrite it to the scoped object form declaring exactly the mutating linear command paths commandMutatesLinear recognizes (linear sync, linear import, linear export). The runtime matches a command against the commands array by exact normalized path, so the array lists full command paths, not bare top-level names (which would never match linear sync and silently disable the credential gate). The authoritative gate (sentinel option consumed by the handlers) stays in the override body; this only narrows its scope." | ||
| type: Issue | ||
| status: closed | ||
| priority: 2 | ||
| tags: [] | ||
| created_at: "2026-08-14T06:22:39.772Z" | ||
| updated_at: "2026-08-15T15:46:21.215Z" | ||
| closed_at: "2026-08-14T06:23:38.357Z" | ||
| completed_at: "2026-08-14T06:23:38.357Z" | ||
| author: pi-agent | ||
| comments[2]{created_at,author,text}: | ||
| "2026-08-15T14:13:34.363Z",claude-agent,"Correction to this item's description, recorded as an append-only event rather than a rewrite (CodeRabbit, PR #67).\n\nThe description above names the scoped command paths as \"linear sync, linear import, linear export\" and refers to the classifier as `commandMutatesLinear`. Both are now out of date:\n\n1. **The scope is four paths, not three.** `linear-sync import` — the deprecated importer alias — reaches the same write path as `linear import`. Under the previous global registration it was covered by accident; narrowing the override to a command list made the omission load-bearing, so it is explicitly in scope. The registered array is `[\"linear sync\", \"linear import\", \"linear export\", \"linear-sync import\"]`.\n\n2. **The classifier is now `commandNeedsLinearAccess`.** Review round 2 showed the mutation question was the wrong one to ask, and got both edges wrong in opposite directions — the preflight guards *credentials and reachability*, not writes:\n - `linear export --push --dry-run` writes nothing and is documented as making no network call, yet the mutation test saw `--push` and demanded `LINEAR_API_KEY`, so the offline preview failed on any host without a key.\n - `linear sync|import --atomic --dry-run` also writes nothing but is *not* offline — it fetches issues to build the atomic plan. The mutation test saw `--dry-run` and skipped the preflight, so a missing key surfaced as a raw fetch failure inside the sync core instead of the clean USAGE error.\n\n Renamed and rewritten to answer \"does this invocation reach Linear\", which is what the preflight actually gates.\n\n3. **The scope↔classifier binding is now bidirectional.** The original test walked `override.commands` only, so it could never detect the drift that matters most: a command the classifier requires credentials for while the scope omits it, which silently loses its credential gate entirely. The test now enumerates thirteen invocations with their expected verdict, asserts scope membership for every credential-requiring one, and asserts that every scoped command has a declared expectation.\n\nBoth classifier fixes verified non-vacuous: reverting either edge fails the test. 71/71 pass.\n" | ||
| "2026-08-15T14:59:28.053Z",claude-agent,"CI failure on PR #67, and what it exposed.\n\nThe `test (22)` / `test (26)` jobs failed on `Verify tracked pm project health`, reporting:\n\n```\nhistory_drift_hash_mismatch:pm-linear-hwol\nhistory_drift_chain_mismatch:pm-linear-hwol\n```\n\nNot a corrupt history. The append-only comment recording the description correction was written by pm **2026.8.15** (the CLI on PATH), while this package pinned `@unbrained/pm-cli` as `^2026.8.6` and CI resolved **2026.8.6** — a version that cannot verify the newer hash chain. Reproduced both ways on this host against the same bytes:\n\n- `./node_modules/.bin/pm health` (2026.8.6) → both drift warnings\n- `pm health` (2026.8.15) → no drift\n\nUpstream has this class already (unbraind/pm-cli#976, closed), so the fix belongs on our side: agree on one CLI version.\n\nFixing it turned up that this package was behind the fleet convention in **three** places at once, each stating a host-compatibility fact to a different consumer, and all three stale:\n\n1. `devDependencies` was `^2026.8.6` — a caret range, not a pin. That is precisely how the 2026.8.14 truncated-`list-all` build reached packages with no diff to review, and here it also meant CI could disagree with the history the tracker writes. Now exactly `2026.8.15`.\n2. `peerDependencies` was `>=2026.7.29`, below the floor the fleet standardised on. Now `>=2026.8.15`.\n3. `manifest.json` declared the floor as `pm.pm_min_version` — nested one level below where the host's version gate looks, which reads top-level `pm_min_version`. Verified against the SDK's own `checkExtensionManifestCompatibility`: the manifest returned `compatible: true` for every host tested back to 2026.1.1, including 2026.8.14. **The floor was not merely stale, it was never enforced.** Hoisted to top level; the gate now refuses 2026.8.14 with `pm_min_version_unmet` and still accepts later hosts.\n\nThis is the third distinct spelling of the same unenforced-floor defect found across the fleet today — pm-context used `compatibility: { pm: ... }`, pm-beads had the right field with a stale value, pm-linear nested it under `pm`. All three packages' suites were fully green throughout. Reported upstream as unbraind/pm-cli#1032, asking for an advisory finding when a manifest declares no enforceable bound and for unrecognized manifest keys to be reported.\n\nTwo regression tests added, matching the ones now in pm-beads and pm-context: one binds `manifest.json` `pm_min_version` to the package.json peer floor and asserts the dev pin is exact rather than a range; the other runs the SDK host gate against the manifest bytes on disk so a floor moved back under a nested key fails loudly instead of shipping inert.\n\nVerified after the change: `pm health` under the pinned 2026.8.15 reports no drift and `--strict-exit` returns 0. `release:check` green, 71/71 tests.\n" | ||
| files[5]{path,scope}: | ||
| index.ts,project | ||
| manifest.json,project | ||
| package-lock.json,project | ||
| package.json,project | ||
| test/smoke.test.ts,project | ||
| tests[2]: | ||
| - command: npx tsx --test test/smoke.test.ts | ||
| scope: project | ||
| timeout_seconds: 300 | ||
| assert_stdout_contains[1]: preflight override is scoped to | ||
| assert_stdout_regex[1]: fail 0$ | ||
| note: "Bidirectional scope-classifier binding over 13 invocations. Non-vacuous: reverting either dry-run edge fails (verified 2026-08-15)." | ||
| - command: npm test | ||
| scope: project | ||
| timeout_seconds: 300 | ||
| assert_stdout_regex[1]: fail 0$ | ||
| note: "Asserts registerPreflight is a scoped object whose commands array equals the four command paths pm-linear owns (linear sync, linear import, linear export, linear-sync import) and that run is a function. Scope membership is bound to commandNeedsLinearAccess in BOTH directions over 13 invocations, so a command the classifier requires credentials for cannot be missing from the scope and a scoped command cannot lack a declared verdict. Note the scope is ownership, not gating: whether the gate runs for a given invocation depends on the options, since --atomic --dry-run reaches Linear while --dry-run alone and export --push --dry-run do not. Test title: preflight override is scoped to pm-linear s owned command paths" | ||
| close_reason: completed | ||
| body: "" | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.