feat: Cursor harness support (2.5.63) - #661
Conversation
leandrodamascena
left a comment
leandrodamascena
left a comment
There was a problem hiding this comment.
The Cursor harness is well structured and covers the nominal path, but several reproducible issues could break real workflows or overwrite existing project configuration.
1. High: The global subagent ledger conflates concurrent conversations
Location: harness/cursor/hooks/aidlc-cursor-adapter.ts:135-175,287-300
The adapter stores only one {agent, parent} entry per project. While conversation A has an active subagent, calls from conversation B have a different conversation_id and are incorrectly attributed to the subagent from A.
I reproduced this by starting a reviewer in A and invoking Task from B. B was denied as nested delegation even though it was an independent parent conversation. Reads and writes from B may also receive the reviewer restrictions from A. The ledger should track entries by parent conversation or Task ID rather than assuming every conversation different from the recorded parent is its subagent.
2. High: Multi-root workspaces can run hooks against the wrong repository
Location: harness/cursor/hooks/aidlc-cursor-adapter.ts:73-82
The adapter selects workspace_roots[0] before process.cwd(). Cursor explicitly supports multiple workspace roots, and the first root may not own the .cursor/hooks.json that launched the hook.
I reproduced this with [repoA, repoB] while running the adapter from repoB, where the active workflow existed. No session context was produced when repoB was second; reversing the array made it work. This also redirects audit, sensors, reviewer-scope enforcement, and runtime compilation. The adapter should prefer the project-hook cwd or resolve the root that owns the executing adapter.
3. High: Installation overwrites existing Cursor configuration
Locations: README.md:247-251, docs/guide/harnesses/cursor.md:50-54
The documented commands overwrite existing .cursor/hooks.json, .cursor/cli.json, and AGENTS.md. I reproduced this with existing content in all three files; none was preserved after applying the documented copy operations. Overwriting hooks.json may silently remove existing security or auditing hooks. The installation flow needs explicit structural merging or an installer that refuses unresolved collisions.
4. Medium: Failed Tasks leave stale subagent attribution
Locations: harness/cursor/hooks.json:13-19, harness/cursor/hooks/aidlc-cursor-adapter.ts:330-337
The ledger is created during preToolUse Task but cleared only by successful postToolUse Task. Cursor provides postToolUseFailure for failed, denied, timed-out, and interrupted calls, but it is not registered. A failed Task therefore leaves attribution active for up to 30 minutes, causing unrelated conversations to be treated as that subagent. Register postToolUseFailure and clear only the corresponding Task entry.
5. Medium: Default-space initialization modifies 11 committed agent files
Locations: core/tools/aidlc-includes.ts:122-128,181-215, core/hooks/aidlc-session-start.ts:52-60
The distributed agents contain <active-space> paths. On the first sessionStart, the rewriter replaces these with default, dirtying 11 files even though the active space has never changed. This contradicts the documented zero-churn guarantee. The active-space test compares restored bytes for the rule file but not the agent file, so it misses the regression.
6. Medium: Intent rebind depends on an optional identifier
Location: harness/cursor/hooks/aidlc-cursor-adapter.ts:213-217,250-256
Cursor documents conversation_id as the stable conversation identifier, but the rebind implementation only works when session_id exists. I reproduced this by retaining conversation_id while removing session_id: switching from intent A to B produced no rebind warning. The adapter should fall back to conversation_id.
7. Medium: Generated Cursor plugin hooks require a Unix shell
Location: scripts/package.ts:906-917
The generated hook invokes sh -c. Native Windows Cursor installations do not necessarily provide sh, so plugin composition fails before Bun runs. The Cursor hook should invoke a portable Bun script directly and perform executable discovery in TypeScript.
Verification
- Cursor packaging and adapter tests: 24 passed.
- Active-space tests: 18 passed.
- Cursor package drift check: passed.
- Plugin integration test was blocked locally by missing
smol-toml. - Worktree remained clean.
|
Thanks for the thorough review - all reproducible, all real. Everything is addressed in cb45ad4. 1. Concurrent-conversation conflation (High) - You were right that the single-entry ledger conflated independent conversations, and your suggested direction (track by parent conversation / Task ID) surfaced a deeper problem when I went to implement it: Cursor's payloads carry no parent lineage at all. 2. Multi-root workspaces (High) - Fixed. The adapter no longer consults 3. Install overwrites existing configuration (High) - Fixed with a merge-aware installer, 4. Failed Tasks leave stale attribution (Medium) - 5. Default-space init dirties 11 committed files (Medium) - Fixed at the packager: shipped Cursor persona bodies now carry the concrete default-space path, so the first 6. Rebind depends on optional session_id (Medium) - Fixed, 7. Plugin hooks require a Unix shell (Medium) - Fixed for Cursor: the emitted hook command is now Two adjacent hardenings that fell out of the same adapter work: a background agent's prompts no longer mint human presence ( Verification: full smoke+unit tier green (179 files), packaging drift check green across all six trees, and a live end-to-end run on cursor-agent 2026.07.23 - a scoped reviewer subagent denied on a sibling-unit read and on a nested Task, allowed on its own unit, an independent conversation never conflated, ledger empty after session end. |
leandrodamascena
left a comment
leandrodamascena
left a comment
There was a problem hiding this comment.
Hey Arden, thanks for round 1. The attribution-registry rework and installer are meaningful progress. I ran the updated implementation through an adversarial review and executable probes. Four confirmed defects still block merge.
Blockers
[P1] Stale Task records disable reviewer-scope enforcement
harness/cursor/hooks/aidlc-cursor-adapter.ts:284-307
Cursor CLI does not deliver postToolUse for Task, so an earlier lead-agent spawn remains in the ledger when the reviewer is dispatched. With records for different agents, activeSubagent() returns an empty identity. The reviewer-scope hook then treats the reviewer as an ordinary conversation and permits sibling-unit reads and deletes.
I reproduced this against the shipped Cursor distribution: a single reviewer record correctly denies the sibling read, while sequential developer and reviewer records allow it. The synthetic Task completion used by t251 does not occur under the verified CLI behavior.
This also prevents SUBAGENT_COMPLETED from being recorded. Please clear or conclusively attribute completed Task records before reviewer dispatch, or deny reviewer-scoped operations when attribution is ambiguous.
[P1] The installer cannot perform its documented upgrade
harness/cursor/install.ts:167-177
Every existing framework file whose bytes differ from the refreshed distribution is classified as an unresolved collision. Normal use already changes managed files such as aidlc/active-space, .cursor/rules/aidlc.mdc, and persona memory paths. Any genuine framework upgrade also changes shipped files.
I reproduced a fresh install, space repoint, and reinstall; it fails on:
.cursor/rules/aidlc.mdc
aidlc/active-space
The current test covers only a byte-identical reinstall. The installer needs to distinguish managed files, runtime-mutated files, and genuinely project-owned collisions.
[P1] Cursor plugin composition silently becomes a no-op when aidlc is installed
scripts/plugin-hooks-template/aidlc-plugin-compose.ts:11-20
The preferred branch runs aidlc plugin sync without setting AIDLC_PLUGIN_ROOT, PLUGIN_ROOT, or CLAUDE_PLUGIN_ROOT. plugin sync discovers plugins exclusively through those variables, so it prints "no installed plugins; nothing to sync" and exits successfully. The launcher then exits before executing sibling compose.ts.
This is confirmed through the actual control path. The integration test misses it by clearing PATH, which forces the fallback branch. Please pass the launcher's resolved plugin root into plugin sync, or fall back when sync composes zero plugins.
[P1] Security-critical guards remain fail-open on failure
harness/cursor/hooks.json:13-15
harness/cursor/hooks/aidlc-cursor-adapter.ts:469-477
The preToolUse hook omits Cursor's supported failClosed: true, and the adapter denies only when a child guard exits exactly 2. A missing, crashing, or invalid guard returns another status that is ignored, after which the adapter exits successfully and Cursor permits the operation.
I reproduced this by removing the reviewer-scope hook: an otherwise protected operation exits 0 with no denial. Please configure failClosed: true and convert unexpected child failures into a denial inside the adapter.
Additional Finding
[P2] Generated "opt-in" skills remain automatically model-invocable
harness/cursor/manifest.ts:85-86
Cursor automatically applies relevant skills unless they declare:
disable-model-invocation: trueThe generated runners use user-invocable: true, which Cursor does not interpret as explicit-only. Consequently, an ordinary coding request can activate a state-mutating stage runner without the user invoking /aidlc-*.
Please emit Cursor-specific explicit-only frontmatter for these runner skills.
Housekeeping
PR #613 also currently claims version 2.5.14. Whichever PR merges second must rebump and rename its changelog heading.
The IDE payload contract remains a testing gap: the fixture corpus contains CLI captures only. I would treat that as residual risk rather than a blocker until real IDE payloads demonstrate a behavioral difference.
Validation
- 112 tests passed with 602 assertions.
bun scripts/package.ts cursor --checkpassed.git diff --check origin/v2...HEADpassed.- Reviewed remote head:
cb45ad4b640066c6e7245ec836f31c5868b317ed.
Recommendation: Request Changes.
|
Review round 2 is addressed in
Verification:
|
49d21dd to
f4db441
Compare
apackeer
changed the title
|
Rebased onto the current Validation completed locally:
@leandrodamascena, rereview requested. |
f4db441 to
8654977
Compare
|
Pushed Rebase + version. Rebased onto v2 at New commit: native workflow shortcuts, integrating ideas from #685 (credit to @rauldiaz for the design direction):
Live verification. #685's agent-decided rule type was doc-verified only, so I probed it on cursor-agent 2026.07.23 headless before adopting it: planted distinct codewords in two phase memory files, then ran a construction-phase question (surfaced only the construction codeword), an ideation-phase question (only the ideation codeword), and an off-topic control (neither). Per-phase selectivity works as documented; the manifest and harness guide now carry the live-verified note. Gates on Note for other open PRs: 2.5.31 is also claimed elsewhere; whichever merges second re-bumps per the changelog policy. |
v2 shipped 2.5.30 while this branch still declared 2.5.27. Scanned the open PRs by their actual AIDLC_VERSION diff rather than their titles: 2.5.31 (awslabs#535, awslabs#661, awslabs#686), 2.5.32 (awslabs#660, awslabs#687) and 2.5.33 (awslabs#689) are claimed, so this takes 2.5.34. The CHANGELOG entry was rebuilt from v2's file with this branch's block reinserted, so no upstream heading is lost. Its sensor-cache bullet now describes the engine-path match rather than the leaf-name one, and a new bullet covers the clean-filter binding. Coverage registry regenerated with the tool, not hand-edited.
v2 shipped 2.5.30 while this branch still declared 2.5.27. Scanned the open PRs by their actual AIDLC_VERSION diff rather than their titles: 2.5.31 (awslabs#535, awslabs#661, awslabs#686), 2.5.32 (awslabs#660, awslabs#687) and 2.5.33 (awslabs#689) are claimed, so this takes 2.5.34. The CHANGELOG entry was rebuilt from v2's file with this branch's block reinserted, so no upstream heading is lost. Its sensor-cache bullet now describes the engine-path match rather than the leaf-name one, and a new bullet covers the clean-filter binding. Coverage registry regenerated with the tool, not hand-edited.
|
@leandrodamascena, could you please take another review pass on the current head? |
8654977 to
df3a104
Compare
apackeer
changed the title
77ba660 to
b7e279b
Compare
apackeer
changed the title
|
Rebased onto the current I verified #707 rather than relying on ancestry alone: its merge commit is the direct parent of this branch, the overlapping behavioral hunks remain intact, and Cursor now carries the project-facing voice/narration contract, The rebase also forward-ports the current safety seams into Cursor: plan approval runs before Task attribution is recorded, and review-freeze joins the fail-closed PreToolUse guard chain. Verification on
@leandrodamascena, rereview requested. |
b7e279b to
8b7133b
Compare
leandrodamascena
left a comment
leandrodamascena
left a comment
There was a problem hiding this comment.
The current head is structurally strong and all existing checks pass, but adversarial testing found five blocking correctness/security issues and two additional reliability gaps.
1. [P1] Shell working directory bypasses reviewer scope and review freeze
harness/cursor/hooks/aidlc-cursor-adapter.ts:376-383
Cursor supplies the effective shell directory through tool_input.cwd in the captured fixture and documents tool_input.working_directory in the current hook contract. The adapter forwards those fields only inside tool_input; the core reviewer-scope and review-freeze hooks resolve relative shell paths from top-level cwd, which is empty in the captured payload and therefore falls back to the project root.
I reproduced a reviewer scoped to unit-a successfully passing the guards for:
{
"tool_name": "Shell",
"cwd": "",
"tool_input": {
"command": "cat design.md",
"cwd": "<record>/construction/unit-b"
}
}Cursor executes the command in unit-b, while the guards evaluate design.md relative to the project root and allow it. Relative writes can bypass review freeze through the same mismatch. Normalize tool_input.cwd / working_directory into the top-level cwd expected by the shared guards and add sibling-read plus protected-write regressions.
2. [P1] A reviewer can delete its own identity ledger and escape enforcement
harness/cursor/hooks/aidlc-cursor-adapter.ts:171-215,285-348
Subagent attribution lives in predictable, writable /tmp/aidlc-cursor-subagent-*.json files. A reviewer can run rm against its ledger because reviewer scope permits paths outside construction/. Once the file is gone, activeSubagent() returns an empty identity and the next sibling-unit operation is treated as an ordinary conversation.
I reproduced the full sequence through the shipped adapter: reviewer spawn, permitted removal of the ledger file, then a permitted Read of unit-b despite a dispatch record binding the reviewer to unit-a. Ledger write/read failures also deliberately degrade to the same fail-open state.
The attribution store needs protection from delegated tools, and failures to establish or recover identity must not silently disable reviewer enforcement while a reviewer dispatch is active.
3. [P1] Receipt-backed reinstall removes the selected-plugin configuration
harness/cursor/install.ts:254-264
The installer stores hashes in .cursor/aidlc-install.json but never compares the current file with the previous receipt hash. The mere presence of a path in priorReceipt.managedFiles grants unconditional overwrite permission.
I reproduced a normal install, added "plugins": ["aidlc"] to .cursor/tools/data/harness.json, and reran the installer. The reinstall succeeded and removed the selection. Since an absent selection means all installed plugins are enabled, an upgrade can silently change an explicit opt-out into broad enablement. The same overwrite path temporarily strips composed graph/stage state.
Preserve runtime-owned fields/state during upgrades or distinguish unchanged prior framework bytes from files modified after installation. Add a receipt-backed reinstall test after select-plugins.
4. [P1] The emitted Cursor plugin hook can succeed without composing the project
scripts/plugin-hooks-template/aidlc-plugin-compose.ts:11-34
scripts/plugin-hooks-template/compose.ts:41-48,354-357
The launcher never reads the SessionStart payload and exports no project root. When it executes from the plugin directory, both aidlc plugin sync and the fallback composer resolve the project from the plugin cwd/PWD. The composer finds no .cursor/tools/aidlc-graph.ts there and returns success as a deliberate non-AIDLC no-op.
I reproduced the emitted launcher with a real SessionStart payload whose workspace_roots named an installed Cursor project. It exited 0 while the target project received no plugin stage. The integration test masks this by setting AIDLC_PROJECT_DIR explicitly and running with the project as cwd, neither of which is established by the emitted hook.
Parse the hook payload and export an unambiguous project root, including an explicit multi-root policy. Test the emitted launcher from the plugin directory without injecting AIDLC_PROJECT_DIR.
5. [P1] Cursor discovers duplicate plugin agents with conflicting projections
scripts/package.ts:902-918,995-1017
scripts/plugin-hooks-template/compose.ts:1311-1322
Cursor officially auto-discovers agents/ when .cursor-plugin/plugin.json does not override that component. The emitted plugin therefore exposes agents/test-pro-metrics-agent.md directly, while compose copies the same native identity into project .cursor/agents/.
The plugin copy retains unresolved {{HARNESS_DIR}} text and model: sonnet; the project copy competes under the same name. Resolution is host-dependent, and the named model also contradicts this PR’s documented lower-plan portability rule for Cursor agents.
Emit one authoritative native agent surface, apply the Cursor token/model projection to it, and add plugin-agent assertions covering duplicate discovery, token substitution, and absence of named model pins.
6. [P2] Stop forwarding expires after Cursor’s default five continuations
harness/cursor/hooks.json:29-30
Cursor documents a default loop_limit of five for stop hooks. The current registration omits it, so a workflow needing more than five forwarding nudges can terminate while the core still reports pending work. Existing coverage verifies only one follow-up. Set and test an intentional limit suitable for the AIDLC forwarding contract.
7. [P2] Restoring a missing native surface points it at the wrong active space
harness/cursor/install.ts:255-256
Existing differing rules/agents pass through managedContent(), but a missing managed file uses raw copy. I reproduced an install on active space team-b, deleted a phase rule, and reinstalled. The restored rule pointed to aidlc/spaces/default/memory/ while aidlc/active-space remained team-b. Apply active-space rewriting to both copy and update paths.
Verification
- Official focused unit slice: 88 tests passed.
- Plugin composition integration: 77 tests passed.
bun scripts/package.ts --check: all six harnesses passed.- TypeScript and Biome checks passed.
- GitHub CI is green.
- Six custom adversarial probes confirmed the reported behaviors.
- Branch is current with
v2;git diff --checkand worktree status are clean.
The green suite covers nominal packaging and adapter paths but does not exercise the effective shell cwd, hostile ledger mutation, receipt-backed runtime state, real plugin-hook cwd, duplicate Cursor plugin discovery, or stop-loop exhaustion. Recommendation: keep CHANGES_REQUESTED until the five P1 paths are fixed and regression-tested.
|
Addressed the independent follow-up review and pushed the rewritten head at Follow-up fixes
The previously verified fixes remain intact: shell cwd normalization, fail-closed reviewer attribution, plugin selection/composed-state preservation, SessionStart project-root discovery, one authoritative Cursor plugin-agent surface, Validation
|
leandrodamascena
left a comment
leandrodamascena
left a comment
There was a problem hiding this comment.
Re-review of ef53481f: the earlier fixes address several findings, but three blockers remain.
-
P1 - harmless delegated Shell calls are denied from the project root (
harness/cursor/hooks/aidlc-cursor-adapter.ts:538).Object.values(toolInput)treatsworking_directoryas a path being accessed. BecauseoverlapsProtectedPath()also matches ancestors, the project root overlaps the attribution ledger beneath it. A reviewer Shell call such asecho okwithworking_directoryset to the project root is therefore denied as an attempt to access reviewer attribution state. Cursor commonly supplies this field, so normal reviewer execution can be blocked. Restrict this check to actual path operands/targets rather than cwd metadata. -
P1 - delegated code can still erase attribution state and bypass reviewer scope (
harness/cursor/hooks/aidlc-cursor-adapter.ts:503). The protection is lexical: a Bun/Node command whose Base64 payload removes.aidlc-cursor-subagentsand.aidlc-reviewer-dispatch.jsonpasses the guard because those paths are not visible in the command text. After executing it, the adapter loses reviewer attribution and allows a read from a sibling unit. If this state is a security boundary, arbitrary interpreter execution needs to be denied for delegated reviewers or isolated with a real sandbox; token/path matching cannot enforce the stated invariant. -
P1 - selected-plugin reinstall freezes stale compiled routing data across upgrades (
harness/cursor/install.ts:204). Wheneverharness.jsoncontains an explicitpluginsselection,managedContent()returns the existingstage-graph.jsonandscope-grid.json. Reinstalling a newer release can therefore update stage source files while retaining the previous compiled graph used for routing. The current test attests/unit/t250-cursor-packaging.test.ts:492codifies byte-for-byte graph preservation but does not model a release upgrade. Recompose the selected plugin set against the newly installed core data instead of preserving old compiled outputs.
The branch is also currently two commits behind v2, and GitHub reports it as conflicting (mergeable_state: dirty), so it must be rebased and revalidated before approval.
Verification completed on this head: t250, t251, t188, package parity, typecheck, lint, git diff --check, and focused adversarial reproductions.
ef53481 to
1cfab2d
Compare
|
Addressed the August 10 re-review and the follow-up rebase reconciliation at
Validation:
|
leandrodamascena
left a comment
leandrodamascena
left a comment
There was a problem hiding this comment.
Thanks for the latest updates. I performed another end-to-end review of the current head, including the installer, adapter enforcement, plugin upgrade behavior, generated distribution, documentation parity, and focused tests.
Several earlier issues were addressed, particularly the working_directory false positive and parts of plugin routing regeneration. However, the PR is still not ready for approval. The remaining issues include two deterministic CI failures and multiple enforcement or upgrade paths not covered by the current tests.
1. Fresh-session behavior is still missing
Location: harness/cursor/skills/aidlc/SKILL.md:79,144
The Cursor orchestrator still uses run-then-continue and instructs the current conversation to execute the next intent after completing the current one.
That conflicts with the current AI-DLC fresh-session contract: when an intent completes and another queued intent exists, the current session must stop and instruct the user to start a fresh session. Continuing in the existing conversation carries stale context, assumptions, tool history, and potentially the previous intent's role state into the next intent.
This is also a deterministic CI failure in t181-conductor-skill-parity.test.ts.
Expected: Cursor should use the same stop-and-handoff semantics as the other harnesses.
Requested change: Update both Cursor orchestration paths to stop after completion and present the fresh-session continuation instruction. Add Cursor to all corresponding parity assertions.
2. Reviewer protection can still be bypassed through shell expansion
Location: harness/cursor/hooks/aidlc-cursor-adapter.ts:528-582
The new shell guard blocks literal references to protected reviewer-attribution files and blocks direct interpreter use. That closes some paths, but it does not resolve shell expansion before execution.
For example, a command can construct the protected paths dynamically:
ledger="aidlc/.aidlc-cursor-subagents"
dispatch="aidlc/spaces/default/intents/<intent>/.aidlc-reviewer-dispatch.json"
rm -rf "$ledger" "$dispatch"The adapter evaluates the literal command tokens before the shell expands $ledger and $dispatch. The protected paths therefore do not appear in the parsed operands, but the shell still deletes them.
If the attribution ledger and reviewer dispatch marker are removed, subsequent calls can lose reviewer attribution and avoid reviewer-only restrictions. Blocking python, node, perl, and similar interpreters does not close this path because expansion is performed by the shell itself.
Expected: Delegated agents must not be able to mutate reviewer attribution state, regardless of whether a protected path is written literally or assembled dynamically.
Requested change: Use an enforcement boundary that does not depend only on lexical command inspection. At minimum, account for shell assignments and expansions and add regression tests using dynamically constructed paths.
3. Partial ledger loss does not fail closed
Location: harness/cursor/hooks/aidlc-cursor-adapter.ts:339-378
The current fail-closed fallback only consults the active reviewer dispatch when no active ledger records remain.
A problematic case remains:
- A reviewer record and an unrelated developer record are active.
- The reviewer record becomes missing, malformed, unreadable, or otherwise excluded by
readRecord(). - The developer record remains readable.
- The reviewer conversation produces another protected tool call.
- The adapter sees one remaining agent and attributes the call to the developer.
- The active reviewer dispatch fallback is not consulted because the active-record array is non-empty.
This means partial ledger corruption can weaken enforcement more than total ledger loss. The existing test around t251-cursor-adapter.test.ts:998-1015 covers the zero-record case but not this partial-loss case.
Expected: While a reviewer dispatch is active, uncertainty involving an unknown conversation must never resolve to a non-reviewer identity merely because an unrelated record remains.
Requested change: Incorporate the active reviewer dispatch into ambiguous attribution even when non-reviewer records exist. Add a regression test with one missing reviewer record and one surviving developer record.
4. Plugin-modified stages can remain permanently pinned to stale core content
Location: harness/cursor/install.ts:171-177,419,528-530
The installer preserves an existing stage whenever its contents differ from the framework source. That preservation also applies to stages generated by plugin composition.
The subsequent plugin-routing refresh recompiles the selected graph, but it uses the already-preserved stage as its input. It does not reconstruct the stage from the new core version and reapply the plugin contribution.
The resulting upgrade sequence is:
- Version A installs a core stage.
- A plugin contribution modifies the installed stage.
- Version B ships an important update to the same core stage.
- The installer sees the composed stage differs from the Version B source and preserves it.
- The receipt is updated.
- Plugin routing is regenerated from the preserved Version A-derived stage.
- The Version B core fix never reaches that stage.
t188-plugin-compose.test.ts:327-350 currently expects the old composed content to remain, so the test codifies this stale-stage behavior rather than detecting it.
Expected: Framework-owned plugin composition should be reproducible from the new core source plus the installed plugin contribution. Only genuine user edits should be preserved.
Requested change: Track enough provenance to distinguish generated plugin composition from user modification, then reconstruct composed stages during upgrades. Update the test to verify that new core content and plugin contributions are both present after upgrade.
5. Pre-receipt migration can overwrite user-modified files
Location: harness/cursor/install.ts:71-77,395,446-451
For an older installation without an ownership receipt, the installer treats the presence of three sentinel files as sufficient evidence that framework files are safe to overwrite.
That establishes that AI-DLC was previously installed, but it does not establish that every framework-shaped file still matches the original distribution. A user may have customized rules, agents, stages, hooks, or tools before receipts existed.
Under the current migration path, those files can be considered framework-owned and overwritten without a hash comparison or backup.
Expected: Lack of historical ownership metadata should not be interpreted as proof that every existing file is unmodified.
Requested change: Use a conservative migration strategy, such as comparing against known prior distribution hashes, preserving unverified files, creating backups, or requiring explicit confirmation before overwriting uncertain content. Add a migration test with a modified pre-receipt framework file.
6. Cursor runtime state is not fully excluded from Git
Location: harness/cursor/dot-gitignore:48-49
Cursor's generated .gitignore is missing:
aidlc/spaces/*/intents/.aidlc-*This allows hidden intent-level runtime state to appear as untracked or staged content. Besides repository noise, some of these files can contain transient framework state that should remain local.
This omission causes the deterministic failure in t157-workspace-shell-seed.test.ts.
Expected: Cursor should emit the same runtime ignore coverage as the other harnesses.
Requested change: Add the missing pattern to the authored Cursor template, regenerate dist/, and keep t157 green.
7. Real Task completion is not reliably audited
Location: harness/cursor/hooks/aidlc-cursor-adapter.ts:630-641,778-785
The implementation notes that Cursor CLI does not emit a Task postToolUse event. The adapter compensates by retiring an earlier record when the same parent starts another Task, but that does not cover all completed tasks.
If a parent launches one Task and never launches another, sessionEnd removes the remaining records without emitting the same completion audit event. The explicit SUBAGENT_COMPLETED path appears to depend on a synthetic event rather than the normal Cursor lifecycle.
This can leave the audit trail with a dispatch/start record but no corresponding completion record.
Expected: Every delegated Task should eventually have a deterministic terminal audit event, including the final Task in a session.
Requested change: Emit an appropriately qualified completion or inferred-completion event when retiring records at session end, or document and test another reliable lifecycle signal.
8. Question-rendering parity is incomplete
Location: harness/cursor/skills/aidlc/question-rendering.md:110-132
Cursor's question-rendering guidance does not include the current fresh-numbering and visible-key mapping requirements.
The relevant parity assertions in t181-conductor-skill-parity.test.ts:396-411 also omit Cursor, so this divergence is not detected by that portion of the test suite.
Without these rules, rendered option numbers can drift after filtering, and the visible option number may not map deterministically to the stored answer key.
Expected: Cursor should follow the same deterministic numbering and answer-key contract as the other harnesses.
Requested change: Port the current shared guidance to Cursor and include Cursor in the parity assertions.
9. PR title and shipped version do not match
The PR title still refers to version 2.5.58, while the implementation, changelog, and generated outputs are now at 2.5.63.
This is not a functional blocker, but it makes review history and release scope harder to understand.
Requested change: Update the PR title to reflect the actual release version or remove the version from the title.
Validation results
I reproduced the current test state locally:
FAIL tests/unit/t157-workspace-shell-seed.test.ts
FAIL tests/unit/t181-conductor-skill-parity.test.ts
The following focused validation passed:
t250-cursor-packaging.test.ts
t251-cursor-adapter.test.ts
t188-plugin-compose.test.ts
bun scripts/package.ts --check
Those passing tests confirm that the generated package is internally consistent with the authored source. They do not resolve the behavioral gaps above; in particular, the plugin test currently expects the stale-stage outcome, and the adapter tests do not cover dynamic shell expansion or partial reviewer-ledger loss.
Review conclusion
The working_directory false positive appears resolved. The interpreter restrictions and plugin-routing recompilation are useful improvements, but they only partially address the original reviewer-enforcement and upgrade concerns.
I recommend keeping this review at Request changes until:
t157andt181pass.- Reviewer attribution fails closed under dynamic path construction and partial ledger loss.
- Plugin-composed stages receive new core updates without discarding plugin contributions.
- Pre-receipt migration handles uncertain ownership conservatively.
- The missing parity and lifecycle cases have regression coverage.
apackeer
changed the title
1cfab2d to
85c64bd
Compare
|
Addressed the August 10 review at Fixes
Verification
|
leandrodamascena
left a comment
leandrodamascena
left a comment
There was a problem hiding this comment.
Requesting changes. The latest revision resolves the earlier fresh-session, gitignore, question-rendering, partial-ledger, and inferred Task-completion findings. The following issues remain.
1. [P1] Reject symlinked installer targets
harness/cursor/install.ts:919-958
The installer reads and writes project paths without checking whether the target or one of its parent directories is a symlink. This allows installation into a project containing a symlinked AGENTS.md, .gitignore, or .cursor directory to modify files outside the requested project root.
I reproduced this with AGENTS.md symlinked to an external file. The installer exited successfully and appended the AI-DLC section to the external target.
This matters when installing into a cloned or otherwise untrusted repository: a repository-controlled symlink can redirect writes to any location writable by the user.
Please reject symlinked targets and symlinked parent components, or resolve each destination and verify that its real path remains under the intended project root before writing.
2. [P1] Shell wrappers can remove reviewer attribution state
harness/cursor/hooks/aidlc-cursor-adapter.ts:591-624
The new attribution-state protection can still be bypassed by combining a shell wrapper with quote concatenation:
command rm -f /project/.../.aidlc-reviewer-dispatch.jso''nThe shell resolves jso''n to json, but the adapter does not recognize the operation:
- The serialized-input substring check does not contain the complete protected filename.
shellWriteTargets()treatscommandas the executable and does not inspect the nestedrm.shellWords()reconstructs the path, but the follow-up check only evaluates glob prefixes.- The dynamic-evaluation guard does not classify
commandas an interpreter or expansion.
After this command executes, the reviewer ledger still identifies the delegated reviewer, but aidlc-reviewer-scope.ts intentionally fails open when the dispatch record is missing. The reviewer can therefore access sibling-unit construction artifacts.
Please unwrap recognized shell command prefixes such as command before classifying the underlying operation, and check every reconstructed concrete shell word against the protected paths rather than checking only wildcard prefixes.
3. [P2] Reconcile plugin ownership when core adopts a contributed value
harness/cursor/install.ts:546-615
The installer reconstructs a plugin-composed stage by removing sidecar-owned values from the installed stage and applying them to the new core source. However, it does not update the corresponding plugin-contrib-*.json ownership record.
A problematic upgrade sequence is:
- Core v1 does not declare
artifact-x. - Plugin P contributes
artifact-xand records it in its contribution sidecar. - Core v2 adds
artifact-xnatively. - The installer upgrades the stage and retains
artifact-x, but the sidecar still identifies it as plugin-owned. - The user disables plugin P.
stripDisabledPluginContributions()removesartifact-x, even though it is now required by core v2.
The installed stage then diverges from the upgraded core definition.
Please remove values already present in the new core source from plugin ownership records, or regenerate the contribution sidecars so they describe only contributions that remain additive after the upgrade.
4. [P2] Remove obsolete receipt-owned files during upgrades
harness/cursor/install.ts:845-907
managedFiles begins with every entry from the previous receipt, but installation only iterates files present in the new distribution. There is no deletion action for files that were previously managed but have since been removed upstream.
I reproduced this by adding a receipt-owned .cursor/rules/obsolete.mdc and reinstalling. The installer exited successfully, left the obsolete rule active, and retained its receipt entry.
This can preserve removed rules, hooks, agents, or tools indefinitely. In the case of hooks and rules, obsolete behavior remains active even though the installed release no longer ships it.
Please compare the previous receipt with the new managed-file set. A removed path can be deleted safely when its current hash still matches the previous receipt. If it was modified locally, preserve it and report the ownership conflict instead of silently retaining it as framework-managed.
5. [P2] Shell wrappers bypass the review-freeze hook
harness/cursor/hooks/aidlc-cursor-adapter.ts:814-829
core/hooks/aidlc-review-freeze.ts:334-448
The review-freeze parser recognizes direct mutation commands:
truncate -s 0 requirements.mdIt does not recognize the same command through a standard shell wrapper:
command truncate -s 0 requirements.mdFor the wrapped form, shellWriteTargets() sees command as the executable and returns no mutation targets. The adapter consequently permits modification of an artifact covered by a fresh READY receipt.
The artifact fingerprint still prevents final acceptance of the modified bytes, so this is not a bypass of the final receipt-validation floor. It does bypass the deterministic freeze, invalidates the terminal receipt, and can return the workflow to the review/rewrite loop that the freeze was introduced to prevent.
Please unwrap supported shell prefixes before mutation classification and add wrapper cases for each protected mutator.
6. [P2] Interpreter detection rejects ordinary command arguments
harness/cursor/hooks/aidlc-cursor-adapter.ts:580-588
shellInvokesDynamicEvaluation() applies the interpreter regular expression to every parsed shell word rather than only executable positions. This rejects ordinary search and output commands when their data happens to name an interpreter.
Examples:
rg node README.md
printf '%s\n' python
rg 'source' README.mdThe first two are rejected because node or python appears anywhere in the argument list. The final command is rejected by the quote-unaware source expression.
This creates avoidable false positives for delegated reviewers and can block legitimate read-only investigation.
Please identify executable positions per command segment, including supported wrapper handling, and apply eval/source detection only where those tokens have shell semantics rather than where they appear as quoted data or arguments.
7. [P2] Expired main markers can misattribute top-level conversations
harness/cursor/hooks/aidlc-cursor-adapter.ts:244-255
harness/cursor/hooks/aidlc-cursor-adapter.ts:339-386
isKnownMain() checks the marker TTL before refreshing it. A legitimate top-level conversation that has been idle for more than 30 minutes is therefore no longer recognized as main on its next event.
If another conversation currently has a live reviewer task, the resumed top-level conversation can be attributed as that reviewer. Its normal operations may then be denied by reviewer-scope, dynamic-evaluation, or nested-delegation controls.
The existing refresh prevents expiry during continuous activity, but not after an ordinary idle period.
Please avoid using inactivity alone to revoke a known top-level identity, or introduce a separate lifecycle signal that distinguishes a genuinely stale conversation from a valid conversation resuming after inactivity.
Validation
The following local validation passed:
241focused unit and integration tests- Cursor adapter and installer suites
- Reviewer-scope and review-freeze suites under the repository test-runner guard environment
- Version/changelog synchronization
- Workspace-shell seed tests
bun scripts/package.ts --checkfor every harness
All current CI checks are green. Findings 1 and 2 remain merge-blocking because they permit writes outside the installation root and deterministic removal of reviewer-scope enforcement, respectively.
2 participants
Summary
Adds the sixth harness: Cursor (
dist/cursor/), serving the Cursor IDE and the Cursor CLI (agent) from one tree. Ships as2.5.14with the CHANGELOG entry, new guide chapter (docs/guide/harnesses/cursor.md), and full roster/docs sweep.Every behavioral claim was live-verified against cursor-agent 2026.07.23 on Linux (spike corpus + probes; fixture payloads captured field-verbatim into
tests/fixtures/cursor-hook-payloads/).Design
Cursor is the most "native" port so far: no
emit.ts- the standard core projection is consumed directly..cursor/carries the engine plus every native surface: the orchestrator + generated stage runners inskills/(invoked as/aidlc, args forwarded inline), the 14 personas inagents/as live native subagents (thetasktool targets them by name - no emitted twins), the method rule inrules/aidlc.mdc,hooks.json, andcli.json(pre-approvesShell(bun)only).cursor: { model: null }for every tier): Cursor model availability is plan-dependent (Free accounts reject every named model with rc 0), so a pinned id would hard-fail lower-plan installs. Agents inherit the session model.@-imports (live-verified), sorules/aidlc.mdc(alwaysApply) carries a read instruction naming the method files, and the sessionStart hook injects live workflow context./aidlc space <name>re-points the rule in place (new.cursorarm inaidlc-includes.ts).aidlc-cursor-adapter.ts, the one authored code file) normalizes Cursor's camelCase events into the 13 byte-shared core hooks:{"permission":"deny","agent_message"}channel (live-verified to block and relay).decision: blockbecomes an advisoryfollowup_messagenudge (the opencode posture).Shellmaps toBash; Cursor's first-classDeletetool (unique to this harness) is presented to the reviewer-scope guard as a path-shaped write so a unit-scoped reviewer cannot delete a sibling unit's artifacts - the audit path keeps the real name so a deletion is never logged as a write.subagentStart/Stopnever fire on the CLI). Nested Task delegation from a delegated conversation is denied without clobbering the parent's ledger entry.beforeSubmitPrompt's blockinguser_message(Cursor's sessionStart carries no resume discriminator); the core session-start hook gains arebind_checkprobe mode that emits no session events. Interactive-only:agent -pnever firesbeforeSubmitPrompt.kind: "cursor"projects Cursor's flat camelCasehooks.sessionStart[].commandschema with the requiredversionfield - Cursor silently delivers zero hook events for ahooks.jsonwithout it (probe-verified) - andcompose.tsfalls back to its own parent directory for the plugin root (Cursor sets no env var).RUNTIME_DISTRIBUTIONS;aidlc adapter cursor <target>routes through the packaged adapter; build gates cover both.Documented constraints
beforeSubmitPrompt, which only fires interactively, soagent -precords noHUMAN_TURNand a gated stage refuses - the presence guard working as intended, documented in the guide + shipped AGENTS.md rather than worked around. Headless suits the read-only utilities and autonomous Construction.Tests
t250-cursor-packaging(9): dist drift guard, core byte-parity,.mdc-only rules dir, camelCase hooks wiring with adapter-arm cross-check, no-model-pin agent frontmatter,cli.jsonshape, foreign-engine-dir sweep, doctor recognition, install-idiom docs pin.t251-cursor-adapter(15): live-captured payload corpus through the adapter - context re-key, guard deny conversion, identity-ledger spawn/attribute/clear, nested-delegation deny, rebind offer + consumption + quiet path, Delete reviewer-scoping (sibling denied, own unit allowed) with the state-transition guard keeping the real tool name, audit/mint/session-end rows, advisory stop, malformed-stdin fail-open.t-run-cursor-status.seriale2e (gateAIDLC_CURSOR_RUN_LIVE=1):/aidlc --statuson the shipped tree viaagent -p- ran live green on Linux.cursor-rule/cursor-hookscapability arms), t220 tier pin, t221 registration case, t157 include branch, t239 roster, t-active-space-includes repoint cases, t188 plugin schema + real compose run, t228/t230/t238 adapter export/dispatcher route/binary gates, coverage registry.Gate evidence
2026-07-26T21-54-57Z)./tmp/package.jsonmodule-resolution trap, green after clearing.bun scripts/package.ts --checkclean on all six trees + plugin projections;bun run typecheckclean.--status(e2e),--version,--doctor39/39 pass.Notes for reviewers
2.5.14assumes the open2.5.12/2.5.13claims land first; will re-bump on rebase if merge order differs.aidlc-tiers.ts,manifest-types.ts,harness-matrix.ts, t239 roster); second-to-merge rebases.