feat(cli): add ark context command#5
Conversation
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
📝 WalkthroughWalkthroughAdds a new read-only Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant CLI as ark context<br/>(crates/ark-cli)
participant Discover as Layout::discover_from
participant Gather as gather_context
participant IO as io/git.rs + io/fs.rs
participant Parse as PRD/Specs Parser
participant Project as project()<br/>(projection)
participant Render as JSON/Text Encoder
User->>CLI: ark context --scope session --format json
CLI->>Discover: discover_from(cwd)
Discover->>IO: check ancestors for .ark/
IO-->>Discover: .ark/ path
Discover-->>CLI: Layout { root }
CLI->>Gather: gather_context(&layout)
Gather->>IO: run_git(...) & read .ark/tasks/, specs files
IO-->>Gather: git output, file contents
Gather->>Parse: parse INDEX.md / managed blocks / PRD related-specs
Parse-->>Gather: spec rows, related-specs
Gather-->>CLI: Context { git, tasks, specs, archive }
CLI->>Project: project(ctx, Scope::Session)
Project-->>CLI: ProjectedContext
CLI->>Render: format as JSON (pretty) or Text
Render-->>CLI: output string
CLI-->>User: print stdout (SessionStart wrapper if session+json)
sequenceDiagram
participant User as User
participant Init as ark init
participant FS as io/fs.rs
participant Settings as .claude/settings.json
User->>Init: ark init
Init->>FS: update_settings_hook(path, canonical_entry)
FS->>Settings: read existing hooks (or {})
Settings-->>FS: current JSON
FS->>FS: normalize hooks.SessionStart, upsert by command identity
FS->>Settings: write pretty JSON if changed
Settings-->>FS: OK
FS-->>Init: updated = true/false
Init-->>User: initialization completed (hook installed/skipped)
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Claude finished @Anekoique's task in 4m 31s —— View job PR Review —
|
There was a problem hiding this comment.
Pull request overview
Adds a new top-level ark context command (plus supporting snapshot/hook/git plumbing) and updates the Ark workflow/slash-command templates and specs to use ark context as the primary source of project/phase state.
Changes:
- Introduce
ark context(JSON/text output, session/phase projections) and wire it intoark-cli. - Add
.claude/settings.jsonSessionStart hook management (install/upgrade/unload/load/remove) and snapshot support for capturing the Ark hook entry. - Update workflow docs/specs/templates to reference
ark context, and bump installed version metadata/hashes.
Reviewed changes
Copilot reviewed 49 out of 49 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| templates/claude/commands/ark/quick.md | Updates template quick command to use ark context for design-phase context. |
| templates/claude/commands/ark/design.md | Updates design template to refresh context per phase and clarifies self-contained ## Spec rule. |
| templates/claude/commands/ark/archive.md | Updates archive template to use verify-phase context snapshot instead of reading task.toml directly. |
| templates/ark/workflow.md | Rewrites workflow doc to center ark context calls and clarifies PLAN/REVIEW/VERIFY gates. |
| docs/ROADMAP.md | Marks ark context, ark unload, and ark upgrade items as shipped and updates syntax. |
| crates/ark-core/src/state/snapshot.rs | Adds hook_bodies to snapshot + SnapshotHookBody model and tests for backward compat/round-trip. |
| crates/ark-core/src/state/mod.rs | Re-exports new snapshot types. |
| crates/ark-core/src/lib.rs | Re-exports context command API types and entrypoint. |
| crates/ark-core/src/layout.rs | Adds project-spec paths, .claude/settings.json path, and Layout::discover_from with tests. |
| crates/ark-core/src/io/mod.rs | Adds git module and exports settings-hook helpers/constants. |
| crates/ark-core/src/io/git.rs | Introduces the single sanctioned git subprocess helper with tests. |
| crates/ark-core/src/io/fs.rs | Adds settings-hook CRUD helpers (update/remove/read_settings_hook) and canonical hook entry builder + tests. |
| crates/ark-core/src/error.rs | Adds Error::GitSpawn for subprocess spawn failures. |
| crates/ark-core/src/commands/upgrade.rs | Re-applies SessionStart hook on every upgrade and adds upgrade idempotence/re-add tests. |
| crates/ark-core/src/commands/unload.rs | Captures + removes Ark SessionStart hook entry into snapshot; reports hook entry count. |
| crates/ark-core/src/commands/remove.rs | Removes the Ark SessionStart hook entry during ark remove. |
| crates/ark-core/src/commands/mod.rs | Exposes context module and re-exports context-facing types. |
| crates/ark-core/src/commands/load.rs | Restores settings hook entries from snapshot and ensures canonical hook for legacy snapshots; adds round-trip tests. |
| crates/ark-core/src/commands/init.rs | Installs SessionStart hook on init and adds tests for hook install/re-add behavior. |
| crates/ark-core/src/commands/context/mod.rs | Implements ark context entrypoint, JSON envelope wrapping for SessionStart, and enforcement test for Command::new locality. |
| crates/ark-core/src/commands/context/gather.rs | Gathers git/tasks/specs/archive/current-task state; parses project/features spec indices. |
| crates/ark-core/src/commands/context/model.rs | Defines the JSON/text data model and caps/constants for context output. |
| crates/ark-core/src/commands/context/projection.rs | Projects full context into session/phase-scoped payloads and filters related feature specs. |
| crates/ark-core/src/commands/context/render.rs | Text rendering for projections with locked section headings and omission rules. |
| crates/ark-core/src/commands/context/related_specs.rs | Parses PRD [**Related Specs**] section into feature spec paths. |
| crates/ark-cli/src/main.rs | Adds ark context subcommand + arg validation; adds target discovery logic (resolve_with_discovery). |
| README.md | Documents ark context and updates lifecycle command list. |
| AGENTS.md | Updates repository layout and workflow/lifecycle table to include context and hook behavior. |
| .claude/settings.json | Adds repo’s Claude Code SessionStart hook invoking ark context --scope session --format json. |
| .claude/commands/ark/quick.md | Syncs shipped slash command to use ark context (quick). |
| .claude/commands/ark/design.md | Syncs shipped slash command to use ark context across phases (design). |
| .claude/commands/ark/archive.md | Syncs shipped slash command to use ark context (archive). |
| .ark/workflow.md | Syncs live workflow doc from template changes. |
| .ark/tasks/archive/2026-04/spec-extract-self-contained/task.toml | Adds archived task artifact documenting spec-extract self-contained requirement. |
| .ark/tasks/archive/2026-04/spec-extract-self-contained/PRD.md | Adds archived PRD artifact describing the self-contained ## Spec rule motivation. |
| .ark/tasks/archive/2026-04/ark-context/task.toml | Adds archived ark-context task metadata. |
| .ark/tasks/archive/2026-04/ark-context/VERIFY.md | Adds archived verify artifact documenting behavior and follow-ups. |
| .ark/tasks/archive/2026-04/ark-context/PRD.md | Adds archived ark-context PRD describing goals/outcomes. |
| .ark/tasks/archive/2026-04/ark-context/02_REVIEW.md | Adds archived review iteration 02 artifact. |
| .ark/tasks/archive/2026-04/ark-context/02_PLAN.md | Adds archived plan iteration 02 artifact. |
| .ark/tasks/archive/2026-04/ark-context/01_REVIEW.md | Adds archived review iteration 01 artifact. |
| .ark/tasks/archive/2026-04/ark-context/01_PLAN.md | Adds archived plan iteration 01 artifact. |
| .ark/tasks/archive/2026-04/ark-context/00_REVIEW.md | Adds archived review iteration 00 artifact. |
| .ark/specs/features/ark-upgrade/SPEC.md | Extends upgrade spec C-8 to cover settings hook behavior and adds changelog entry. |
| .ark/specs/features/ark-context/SPEC.md | Adds the extracted ark-context feature spec. |
| .ark/specs/features/ark-agent-namespace/SPEC.md | Clarifies spec extract copies latest ## Spec verbatim and requires self-contained ## Spec. |
| .ark/specs/features/INDEX.md | Registers ark-context in the managed features index. |
| .ark/.installed.json | Bumps installed version to 0.1.2 and updates hashes for changed files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let header_pos = text.find(SECTION_HEADER)?; | ||
| // Skip past the header line itself (everything up to and including | ||
| // the first newline after it). | ||
| let body_start = match text[header_pos..].find('\n') { | ||
| Some(nl) => header_pos + nl + 1, | ||
| None => return Some(""), |
There was a problem hiding this comment.
locate_section uses text.find(SECTION_HEADER) which will match [**Related Specs**] anywhere in the document (including mid-line), but the contract in the module doc says “Locate the line starting with [**Related Specs**]”. This can cause false positives if the token appears in prose. Consider scanning line-by-line for a header line whose trimmed content equals SECTION_HEADER (or starts with it) before extracting the section body.
| let header_pos = text.find(SECTION_HEADER)?; | |
| // Skip past the header line itself (everything up to and including | |
| // the first newline after it). | |
| let body_start = match text[header_pos..].find('\n') { | |
| Some(nl) => header_pos + nl + 1, | |
| None => return Some(""), | |
| let mut offset = 0usize; | |
| let mut body_start = None; | |
| for line in text.split_inclusive('\n') { | |
| let trimmed = line.trim_start(); | |
| if trimmed.starts_with(SECTION_HEADER) { | |
| body_start = Some(offset + line.len()); | |
| break; | |
| } | |
| offset += line.len(); | |
| } | |
| let body_start = match body_start { | |
| Some(start) => start, | |
| None => { | |
| let trimmed = text.trim_start(); | |
| if trimmed.starts_with(SECTION_HEADER) && !text.contains('\n') { | |
| return Some(""); | |
| } | |
| return None; | |
| } |
| /// Per-entry hook bodies captured by `unload`, restored by `load` via | ||
| /// `update_settings_hook`. New in v0.1.3 — older `.ark.db` files lack | ||
| /// this key and deserialize to `vec![]`. Per ark-context C-27. |
There was a problem hiding this comment.
The doc comment says hook_bodies is “New in v0.1.3”, but this PR bumps the workspace/package version to 0.1.2 and uses that version in snapshot fixtures/tests. Please correct the referenced version (or rephrase without pinning a specific version) to avoid confusion when reading older snapshots.
| // 3. Capture + remove the Ark-owned SessionStart hook entry. Per | ||
| // ark-context C-18: only the Ark entry is captured; user siblings are | ||
| // user-owned content outside Ark's owned dirs and do not survive a | ||
| // round-trip. |
There was a problem hiding this comment.
The comment claims user sibling hooks in .claude/settings.json “do not survive a round-trip”, but the actual implementation only removes the Ark entry (via remove_settings_hook) and leaves the rest of the file intact. That means sibling hooks can persist on disk across unload → load (and your load.rs tests explicitly assert this). Please update this comment to reflect the real behavior (only Ark entry is captured into the snapshot; siblings are not captured but are preserved on disk because unload is a surgical edit).
| //! - [`git`] — the only sanctioned `Command::new("git")` site (per | ||
| //! ark-context C-26). Kept module-private; callers route through | ||
| //! [`crate::commands::context::gather`] rather than the raw helper. | ||
|
|
||
| pub mod fs; | ||
| pub mod git; |
There was a problem hiding this comment.
io::git is documented here as “Kept module-private”, but it’s declared as pub mod git;, making it public to external crates. Either make it pub(crate) mod git (or mod git + re-export only what’s needed internally) to match the stated constraint, or update the module docs to reflect that it’s a public module.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41b469cdef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let root = if a.force { | ||
| a.target.resolve() | ||
| } else { | ||
| a.target.resolve_with_discovery()? |
There was a problem hiding this comment.
ark load target without requiring .ark
ark load now calls resolve_with_discovery() in the non---force path, which only succeeds when an ancestor already contains .ark/. That prevents the documented restore/scaffold flow in common cases: a fresh project (no .ark yet) and a project right after ark unload (only .ark.db exists). In both scenarios the CLI exits with NotLoaded before load() can restore from snapshot or fall back to init.
Useful? React with 👍 / 👎.
| } | ||
| Self::Remove(a) => { | ||
| let root = a.resolve(); | ||
| let root = a.resolve_with_discovery()?; |
There was a problem hiding this comment.
ark remove run without .ark discovery
ark remove now resolves its root via resolve_with_discovery(), so it refuses to run unless .ark/ exists in the cwd ancestry. This breaks the unconditional cleanup behavior (notably deleting .ark.db after ark unload), because unload removes .ark/ first and remove now fails before reaching remove()'s no-op-safe cleanup logic.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (1)
templates/claude/commands/ark/quick.md (1)
20-34:⚠️ Potential issue | 🟡 MinorSame step numbering inconsistency as noted in
.claude/commands/ark/quick.md.This is the source template — applying the fix here will propagate to the generated file. Steps jump from 1 to 3.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/claude/commands/ark/quick.md` around lines 20 - 34, The template has a step-numbering gap: after the heading "### 1. Pull project context" the next heading is "### 3. Create the task"; update the template so steps are sequential (either renumber "### 3. Create the task" to "### 2. Create the task" or insert the missing step 2 content if intended), ensuring the headings in templates/claude/commands/ark/quick.md follow 1, 2, 3 order to propagate the correct numbering to generated files.
🧹 Nitpick comments (4)
crates/ark-core/src/commands/remove.rs (1)
106-115: Consider adding assertion forremoved_hook_entryin the test.The test
remove_after_init_wipes_everythingverifies most summary fields but doesn't assert on the newremoved_hook_entryflag. Sinceinitnow writes the SessionStart hook, this test should also verify that the hook was removed.💡 Proposed addition
let summary = remove(RemoveOptions::new(tmp.path())).unwrap(); assert!(summary.removed_ark_dir); assert!(summary.removed_claude_commands); assert_eq!(summary.blocks_removed, 1); + assert!(summary.removed_hook_entry); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/ark-core/src/commands/remove.rs` around lines 106 - 115, The test remove_after_init_wipes_everything must assert the new removed_hook_entry flag; after calling remove(RemoveOptions::new(tmp.path())) and obtaining summary, add an assertion checking summary.removed_hook_entry (e.g., assert!(summary.removed_hook_entry)) so the test verifies that the SessionStart hook written by init(...) was removed; update the test function remove_after_init_wipes_everything accordingly, referencing the existing init, remove, RemoveOptions, and the summary variable.crates/ark-core/src/io/mod.rs (1)
7-12: Makeio::gitprivate to enforce the intended API boundary.The docstring states this module is "module-private" with callers routing through [
crate::commands::context::gather], butpub mod git;exposes it as part ofark-core's public API. No code outsidecrates/ark-coreusesark_core::io::git, and internal callers (likegather.rs) work identically withmod git;. Change tomod git;to match the documented design and keep the git-spawning boundary tight.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/ark-core/src/io/mod.rs` around lines 7 - 12, The io::git module is publicly exported but should be module-private per the docstring; change the declaration from a public export to a private module by replacing the `pub mod git;` export with `mod git;` in the io module, leaving `pub mod fs;` as-is, and verify internal callers such as the gather implementation (`crate::commands::context::gather` / gather.rs) continue to reference `crate::io::git` (they can access it because it's in the same crate) or import the private module directly; no external API should reference `ark_core::io::git`.crates/ark-core/src/io/fs.rs (1)
293-300: Consider using actual path in error context instead of placeholder.The error is constructed with a placeholder path
"<settings.json>"which loses the actual file path context. Sinceupsert_session_start_entrydoesn't receive the path, consider either:
- Passing the path as a parameter
- Using a more specific error variant
This is minor since the error message itself is descriptive.
💡 Alternative: propagate path from caller
fn upsert_session_start_entry( root: &mut serde_json::Value, entry: serde_json::Value, + path: &Path, ) -> Result<()> { let identity = identity_command(&entry).ok_or_else(|| { Error::io( - std::path::PathBuf::from("<settings.json>"), + path.to_path_buf(), std::io::Error::other( "hook entry missing inner `hooks[*].command` (or top-level `command`)", ), ) })?;Then update the caller at line 197.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/ark-core/src/io/fs.rs` around lines 293 - 300, The error uses a placeholder PathBuf "<settings.json>" when identity_command(&entry) returns None inside upsert_session_start_entry; change this by either adding a path parameter to upsert_session_start_entry (propagate the real file path from its caller) and use that PathBuf in the Error::io construction, or switch to a more specific error variant that does not require a PathBuf (e.g., a custom Error::config or Error::invalid_entry) and include the descriptive message; update the caller that invokes upsert_session_start_entry to pass the real path if you choose the first option so the error includes accurate file context.crates/ark-core/src/commands/context/projection.rs (1)
221-243: Testplan_phase_filters_features_to_relatedpasses due to exact-match coincidence.The test uses
related = ["specs/features/foo/SPEC.md"]and features with paths like.ark/specs/features/foo/SPEC.md. After normalization, both becomespecs/features/foo/SPEC.md, sor.ends_with(&f_norm)succeeds only because they're identical strings.Consider adding a test case where
related_specscontains a path like"./specs/features/foo/SPEC.md"or".ark/specs/features/foo/SPEC.md"to exercise the normalization + matching logic more thoroughly and catch the predicate direction issue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/ark-core/src/commands/context/projection.rs` around lines 221 - 243, The test plan_phase_filters_features_to_related currently only exercises exact-match normalization; add an additional assertion case that supplies a CurrentTask.related_specs entry using a non-exact path (e.g. "./specs/features/foo/SPEC.md" or ".ark/specs/features/foo/SPEC.md") when building ctx via ctx_with and calling project(ctx, Scope::Phase(PhaseFilter::Plan)), then verify the resulting pj.specs.features still contains only "foo" and other projections (archive/tasks) remain None; this ensures the normalization + matching logic used by project (and the predicate comparing related_specs to feature paths) is exercised for prefixed/relative paths as well as exact ones.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.ark/tasks/archive/2026-04/ark-context/02_PLAN.md:
- Around line 78-85: The archived "## Spec" section currently uses "carried
forward from `01_PLAN.md`" which leaves the spec non-self-contained and will
break `ark agent spec extract`; fix by replacing that delta form in the "##
Spec" block with the full, inlined final spec text (i.e., expand G-1..G-12,
NG-1..NG-11, and C-1..C-29 as they should appear) or, if you intend this file to
be historical only, add a clear machine-readable marker and human notice at the
top of the file (e.g., "ARCHIVED: DO NOT USE FOR EXTRACTION" and a brief note
that this file is historical evidence only) so tools like `ark agent spec
extract` will skip it. Ensure the change targets the "## Spec" section that
currently contains the carried-forward phrase and update any references to
C-27/C-28/C-29 to match the inlined content or archival marker.
In @.ark/tasks/archive/2026-04/ark-context/VERIFY.md:
- Line 26: The summary sentence that reads "All 12 Goals (G-1 through G-12) and
26 Constraints (C-1 through C-26 + C-27 through C-29)" is internally
inconsistent with the enumerated constraints C-1..C-29; update that sentence to
say "29 Constraints" (or rephrase to "Constraints C-1 through C-29") so the
count matches the enumeration, and make the same change anywhere else in
VERIFY.md where the old "26 Constraints" appears.
In @.claude/commands/ark/quick.md:
- Around line 20-34: The numbered headings in .claude/commands/ark/quick.md skip
from "### 1. Pull project context" to "### 3. Create the task"; update the
heading "### 3. Create the task" to "### 2. Create the task" and incrementally
renumber all subsequent step headings (e.g., 4→3, 5→4, etc.) so numbering is
sequential; also search the same file for any in-text references to those step
numbers and update them to match the new numbering (look for the heading text
"Create the task" and other subsequent step titles to locate all occurrences).
In `@AGENTS.md`:
- Around line 33-53: The Markdown module-map fenced block starting with triple
backticks lacks a language tag which triggers MD040; update the opening fence to
include a language identifier (e.g., change the block that contains the module
tree under the module-map comment to start with ```text) so the code block is
explicitly marked as plain text and the MD040 warning is resolved.
In `@crates/ark-core/src/commands/context/mod.rs`:
- Around line 89-94: Replace the call to layout.ark_dir().exists() with
try_exists() and map any I/O error into your crate error type instead of
treating it as “not loaded”; call layout.ark_dir().try_exists(), and on Err(e)
return Err(Error::io(opts.project_root.clone(), e)) (or equivalent using the
same path value), while only returning Err(Error::NotLoaded { path:
opts.project_root }) when try_exists() returns Ok(false); keep references to
Layout::new, layout.ark_dir(), Error::NotLoaded and Error::io to locate and
implement the change.
- Around line 99-123: Change the two serde_json::to_string_pretty calls to
return errors instead of panicking: add (or reuse) an Error variant for
serialization failures, change wrap_session_start_envelope(payload: &str) ->
String to wrap_session_start_envelope(payload: &str) -> Result<String, Error>
and use serde_json::to_string_pretty(...)? (instead of .expect), and change the
caller that builds ContextSummary::Json to propagate the error (use ? when
serializing projected and when calling wrap_session_start_envelope) so context()
(or the enclosing function) returns Result<ContextSummary, Error>; update any
downstream call sites accordingly.
In `@crates/ark-core/src/commands/context/projection.rs`:
- Around line 144-148: The filter predicate is inverted: inside the closure that
computes f_norm (from f.path) and uses normalized_related.iter().any(...),
change the any(...) test so it checks whether each normalized related path ends
with the feature normalized path (i.e., r.ends_with(&f_norm)) instead of
checking f_norm.ends_with(r); locate the closure where f_path_str, f_norm, and
normalized_related are used (the filter on the iterator over features) and swap
the arguments to ends_with to match the docstring/spec (or update the docstring
if the opposite behavior is intended).
In `@crates/ark-core/src/commands/context/related_specs.rs`:
- Around line 33-52: locate_section() currently uses text.find(SECTION_HEADER)
which treats inline mentions as headers; change the logic to scan the input
line-by-line to find an actual header line whose trimmed contents equal
SECTION_HEADER, then set body_start to the first byte after that header line
(same newline-skip behavior) and proceed as before; update the function using
the existing helpers (is_section_header_line) and keep the rest of the loop that
detects the next section intact so extract() and current_task.related_specs get
the correct block.
In `@crates/ark-core/src/commands/unload.rs`:
- Around line 87-102: Update the misleading comment that says "user siblings...
do not survive a round-trip" to correctly state that user siblings are not
captured into the snapshot but remain on disk and therefore survive the
round-trip; reference the relevant symbols to clarify behavior: the
capture/remove sequence in unload.rs (layout.claude_settings(),
read_settings_hook(), SnapshotHookBody, remove_settings_hook(),
ARK_CONTEXT_HOOK_COMMAND) only captures/removes Ark's entry for the snapshot
while user siblings remain and are preserved when load (see test
roundtrip_preserves_user_pretooluse_sibling in load.rs) re-applies Ark's entry
without deleting other sibling entries.
In `@templates/claude/commands/ark/archive.md`:
- Around line 30-35: The pre-archive precheck queries ark context without
scoping to the resolved slug and assumes current_task and
current_task.summary.phase exist; update the archive precheck to call the
context command bound to the slug resolved in Step 1 (so the context is for the
target slug), then defensively check that current_task exists and that
current_task.summary and summary.phase are present before reading phase; if
absent, halt with a clear error. Also ensure phase validation logic uses the
correct expected phase per tier (standard/deep => "verify", quick => "execute"),
and only attempt to read VERIFY.md and inspect the "Verdict" line when the
artifact exists; if Verdict is "Rejected" halt and report.
---
Duplicate comments:
In `@templates/claude/commands/ark/quick.md`:
- Around line 20-34: The template has a step-numbering gap: after the heading
"### 1. Pull project context" the next heading is "### 3. Create the task";
update the template so steps are sequential (either renumber "### 3. Create the
task" to "### 2. Create the task" or insert the missing step 2 content if
intended), ensuring the headings in templates/claude/commands/ark/quick.md
follow 1, 2, 3 order to propagate the correct numbering to generated files.
---
Nitpick comments:
In `@crates/ark-core/src/commands/context/projection.rs`:
- Around line 221-243: The test plan_phase_filters_features_to_related currently
only exercises exact-match normalization; add an additional assertion case that
supplies a CurrentTask.related_specs entry using a non-exact path (e.g.
"./specs/features/foo/SPEC.md" or ".ark/specs/features/foo/SPEC.md") when
building ctx via ctx_with and calling project(ctx,
Scope::Phase(PhaseFilter::Plan)), then verify the resulting pj.specs.features
still contains only "foo" and other projections (archive/tasks) remain None;
this ensures the normalization + matching logic used by project (and the
predicate comparing related_specs to feature paths) is exercised for
prefixed/relative paths as well as exact ones.
In `@crates/ark-core/src/commands/remove.rs`:
- Around line 106-115: The test remove_after_init_wipes_everything must assert
the new removed_hook_entry flag; after calling
remove(RemoveOptions::new(tmp.path())) and obtaining summary, add an assertion
checking summary.removed_hook_entry (e.g., assert!(summary.removed_hook_entry))
so the test verifies that the SessionStart hook written by init(...) was
removed; update the test function remove_after_init_wipes_everything
accordingly, referencing the existing init, remove, RemoveOptions, and the
summary variable.
In `@crates/ark-core/src/io/fs.rs`:
- Around line 293-300: The error uses a placeholder PathBuf "<settings.json>"
when identity_command(&entry) returns None inside upsert_session_start_entry;
change this by either adding a path parameter to upsert_session_start_entry
(propagate the real file path from its caller) and use that PathBuf in the
Error::io construction, or switch to a more specific error variant that does not
require a PathBuf (e.g., a custom Error::config or Error::invalid_entry) and
include the descriptive message; update the caller that invokes
upsert_session_start_entry to pass the real path if you choose the first option
so the error includes accurate file context.
In `@crates/ark-core/src/io/mod.rs`:
- Around line 7-12: The io::git module is publicly exported but should be
module-private per the docstring; change the declaration from a public export to
a private module by replacing the `pub mod git;` export with `mod git;` in the
io module, leaving `pub mod fs;` as-is, and verify internal callers such as the
gather implementation (`crate::commands::context::gather` / gather.rs) continue
to reference `crate::io::git` (they can access it because it's in the same
crate) or import the private module directly; no external API should reference
`ark_core::io::git`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 34f65fed-cdcb-4dee-ba24-ae744e408c77
📒 Files selected for processing (49)
.ark/.installed.json.ark/specs/features/INDEX.md.ark/specs/features/ark-agent-namespace/SPEC.md.ark/specs/features/ark-context/SPEC.md.ark/specs/features/ark-upgrade/SPEC.md.ark/tasks/archive/2026-04/ark-context/00_PLAN.md.ark/tasks/archive/2026-04/ark-context/00_REVIEW.md.ark/tasks/archive/2026-04/ark-context/01_PLAN.md.ark/tasks/archive/2026-04/ark-context/01_REVIEW.md.ark/tasks/archive/2026-04/ark-context/02_PLAN.md.ark/tasks/archive/2026-04/ark-context/02_REVIEW.md.ark/tasks/archive/2026-04/ark-context/PRD.md.ark/tasks/archive/2026-04/ark-context/VERIFY.md.ark/tasks/archive/2026-04/ark-context/task.toml.ark/tasks/archive/2026-04/spec-extract-self-contained/PRD.md.ark/tasks/archive/2026-04/spec-extract-self-contained/task.toml.ark/workflow.md.claude/commands/ark/archive.md.claude/commands/ark/design.md.claude/commands/ark/quick.md.claude/settings.jsonAGENTS.mdREADME.mdcrates/ark-cli/src/main.rscrates/ark-core/src/commands/context/gather.rscrates/ark-core/src/commands/context/mod.rscrates/ark-core/src/commands/context/model.rscrates/ark-core/src/commands/context/projection.rscrates/ark-core/src/commands/context/related_specs.rscrates/ark-core/src/commands/context/render.rscrates/ark-core/src/commands/init.rscrates/ark-core/src/commands/load.rscrates/ark-core/src/commands/mod.rscrates/ark-core/src/commands/remove.rscrates/ark-core/src/commands/unload.rscrates/ark-core/src/commands/upgrade.rscrates/ark-core/src/error.rscrates/ark-core/src/io/fs.rscrates/ark-core/src/io/git.rscrates/ark-core/src/io/mod.rscrates/ark-core/src/layout.rscrates/ark-core/src/lib.rscrates/ark-core/src/state/mod.rscrates/ark-core/src/state/snapshot.rsdocs/ROADMAP.mdtemplates/ark/workflow.mdtemplates/claude/commands/ark/archive.mdtemplates/claude/commands/ark/design.mdtemplates/claude/commands/ark/quick.md
| ## Spec | ||
|
|
||
| *Carried forward from `01_PLAN.md` §Spec, with the following deltas:* | ||
|
|
||
| - **G-1 through G-12:** unchanged. | ||
| - **NG-1 through NG-11:** unchanged. | ||
| - *Constraints:* C-1 through C-26 are inherited from 01_PLAN (C-3, C-11, C-21, C-26 with their 01-revised wording, plus the C-21 narrowing in §Changed above). C-27, C-28, C-29 added in this iteration. | ||
|
|
There was a problem hiding this comment.
The archived final ## Spec is still non-self-contained.
Lines 78-85 still use the “carried forward from 01_PLAN.md” delta form. That means rerunning ark agent spec extract on this task would reproduce the same unusable SPEC shape this PR is trying to prevent. Please either inline the full final spec here or clearly mark this file as pre-fix historical evidence that must not be used as extraction input.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 78-78: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.ark/tasks/archive/2026-04/ark-context/02_PLAN.md around lines 78 - 85, The
archived "## Spec" section currently uses "carried forward from `01_PLAN.md`"
which leaves the spec non-self-contained and will break `ark agent spec
extract`; fix by replacing that delta form in the "## Spec" block with the full,
inlined final spec text (i.e., expand G-1..G-12, NG-1..NG-11, and C-1..C-29 as
they should appear) or, if you intend this file to be historical only, add a
clear machine-readable marker and human notice at the top of the file (e.g.,
"ARCHIVED: DO NOT USE FOR EXTRACTION" and a brief note that this file is
historical evidence only) so tools like `ark agent spec extract` will skip it.
Ensure the change targets the "## Spec" section that currently contains the
carried-forward phrase and update any references to C-27/C-28/C-29 to match the
inlined content or archival marker.
|
|
||
| ## Summary | ||
|
|
||
| The implementation delivers what `02_PLAN.md` (carrying forward `01_PLAN.md`) promised. All 12 Goals (G-1 through G-12) and 26 Constraints (C-1 through C-26 + C-27 through C-29) are satisfied as specified, with one deliberate scope adjustment (V-IT-15 negative case became a positive case — see V-001 below). 191 unit/integration tests pass; the CLI smoke test (init → context → unload → load → remove) round-trips cleanly with the SessionStart hook captured and restored. Build, fmt, clippy, and test gates are all green. Three non-blocking follow-ups documented below: V-IT-15 plan/reality mismatch (better behavior than planned, plan should be amended), Phase 4 documentation is partial (READMAP marker added but the SPEC body amendment is in the live SPEC only — `templates/ark/specs/...` is unchanged because that file isn't templated), and the slash-command G-10 row remained "manual review" without a per-file mechanical check (carried forward from 01's Unresolved §1, no regression). |
There was a problem hiding this comment.
Fix the constraint count in the summary.
Line 26 says “26 Constraints” but then enumerates C-1 through C-29. Please either update the count to 29 or simplify the sentence so the archive summary stays internally consistent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.ark/tasks/archive/2026-04/ark-context/VERIFY.md at line 26, The summary
sentence that reads "All 12 Goals (G-1 through G-12) and 26 Constraints (C-1
through C-26 + C-27 through C-29)" is internally inconsistent with the
enumerated constraints C-1..C-29; update that sentence to say "29 Constraints"
(or rephrase to "Constraints C-1 through C-29") so the count matches the
enumeration, and make the same change anywhere else in VERIFY.md where the old
"26 Constraints" appears.
| ├── layout.rs # Layout + project-root constants + discover_from | ||
| ├── templates.rs # include_dir!() trees + walker | ||
| ├── io/ | ||
| │ ├── path_ext.rs # PathExt trait wrapping std::fs | ||
| │ └── fs.rs # write_file, walk_files, managed-block ops | ||
| │ ├── fs.rs # write_file, walk_files, managed-block ops, settings-hook | ||
| │ └── git.rs # the only sanctioned Command::new("git") site | ||
| ├── state/ | ||
| │ ├── manifest.rs # .ark/.installed.json | ||
| │ └── snapshot.rs # .ark.db capture/restore | ||
| │ └── snapshot.rs # .ark.db capture/restore (incl. SnapshotHookBody) | ||
| └── commands/ | ||
| ├── init.rs # scaffold from templates | ||
| ├── load.rs # restore from .ark.db OR scaffold | ||
| ├── unload.rs # capture into .ark.db, remove live files | ||
| ├── remove.rs # unconditional wipe | ||
| ├── upgrade.rs # refresh embedded templates to current CLI version | ||
| ├── context/ # `ark context` — read-only state snapshot | ||
| │ ├── model.rs # Context + sub-structs, schema=1 | ||
| │ ├── gather.rs # one-pass collection (git + tasks + specs) | ||
| │ ├── projection.rs # Scope / PhaseFilter / project() | ||
| │ ├── render.rs # text-mode Display | ||
| │ └── related_specs.rs # PRD [**Related Specs**] parser |
There was a problem hiding this comment.
Add a language identifier to the module-map fenced block.
Line 29 opens a fenced code block without a language, which triggers MD040.
💡 Proposed fix
-```
+```text
ark-core/src/
├── lib.rs # public re-exports
...
└── template.rs # internal helper: extract embedded templates</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 52-52: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @AGENTS.md around lines 33 - 53, The Markdown module-map fenced block
starting with triple backticks lacks a language tag which triggers MD040; update
the opening fence to include a language identifier (e.g., change the block that
contains the module tree under the module-map comment to start with ```text) so
the code block is explicitly marked as plain text and the MD040 warning is
resolved.
</details>
<!-- fingerprinting:phantom:poseidon:hawk:9b3bb1b3-2d1d-47dc-83b0-4e8d9ea33fc3 -->
<!-- d98c2f50 -->
<!-- This is an auto-generated comment by CodeRabbit -->
| let raw = | ||
| serde_json::to_string_pretty(&projected).expect("ProjectedContext serializes"); | ||
| let body = if matches!(opts.scope, Scope::Session) { | ||
| wrap_session_start_envelope(&raw) | ||
| } else { | ||
| raw | ||
| }; | ||
| Ok(ContextSummary::Json(format!("{body}\n"))) | ||
| } | ||
| Format::Text => Ok(ContextSummary::Text(Box::new(projected))), | ||
| } | ||
| } | ||
|
|
||
| /// Wrap a JSON payload in Claude Code's SessionStart envelope. The payload | ||
| /// is embedded as a stringified value of `additionalContext` because the | ||
| /// hook contract requires that field to be a string. | ||
| fn wrap_session_start_envelope(payload: &str) -> String { | ||
| serde_json::to_string_pretty(&serde_json::json!({ | ||
| "hookSpecificOutput": { | ||
| "hookEventName": "SessionStart", | ||
| "additionalContext": payload, | ||
| } | ||
| })) | ||
| .expect("envelope serializes") | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether serde_json errors can be propagated with `?` in ark-core error type.
fd -i 'error*.rs' crates/ark-core/src
rg -n "enum Error|type Result|From<serde_json::Error>|serde_json::Error" crates/ark-core/srcRepository: Anekoique/Ark
Length of output: 285
🏁 Script executed:
cat -n crates/ark-core/src/error.rsRepository: Anekoique/Ark
Length of output: 3768
🏁 Script executed:
sed -n '85,130p' crates/ark-core/src/commands/context/mod.rsRepository: Anekoique/Ark
Length of output: 1685
🏁 Script executed:
rg -B 3 "fn wrap_session_start_envelope" crates/ark-core/src/commands/context/mod.rsRepository: Anekoique/Ark
Length of output: 319
🏁 Script executed:
rg -n "serde_json::to_string" crates/ark-core/src --context=2Repository: Anekoique/Ark
Length of output: 4771
🏁 Script executed:
rg -n "ManifestCorrupt|SerializationError|JsonError" crates/ark-core/srcRepository: Anekoique/Ark
Length of output: 224
Propagate serde_json serialization errors instead of panicking.
Lines 100 and 116 use expect() for fallible operations that can realistically fail (out of memory, I/O constraints). Per the coding guideline, "Never use expect() outside tests; reserve expect("…") for documented invariants only," these should propagate as Result errors.
Additionally, this pattern appears elsewhere (state/snapshot.rs:128, state/manifest.rs:105, io/fs.rs:368), suggesting a systematic issue. To fix this, the Error enum needs a variant for serialization failures (or reuse an existing variant), then update both wrap_session_start_envelope() to return Result<String> and propagate errors in context() with the ? operator.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/ark-core/src/commands/context/mod.rs` around lines 99 - 123, Change
the two serde_json::to_string_pretty calls to return errors instead of
panicking: add (or reuse) an Error variant for serialization failures, change
wrap_session_start_envelope(payload: &str) -> String to
wrap_session_start_envelope(payload: &str) -> Result<String, Error> and use
serde_json::to_string_pretty(...)? (instead of .expect), and change the caller
that builds ContextSummary::Json to propagate the error (use ? when serializing
projected and when calling wrap_session_start_envelope) so context() (or the
enclosing function) returns Result<ContextSummary, Error>; update any downstream
call sites accordingly.
Anekoique
force-pushed
the
feat/ark-context
branch
from
41b469c to
a0acbe3
Compare
|
Claude finished @Anekoique's task in 3m 42s —— View job PR Review —
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0acbe3faa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // targets the Ark entry by command, leaving the rest of the file | ||
| // intact for the next `load` to find. | ||
| let settings = layout.claude_settings(); | ||
| if let Some(entry) = read_settings_hook(&settings, ARK_CONTEXT_HOOK_COMMAND)? { |
There was a problem hiding this comment.
If .claude/settings.json is malformed JSON, read_settings_hook(...)? returns an error here and unload exits before snapshot.write(). By this point step 2 has already removed managed blocks (remove_managed_block), so the command can fail after destructive edits without producing a restorable .ark.db, which breaks unload’s preservation guarantee and can drop user-managed block content.
Useful? React with 👍 / 👎.
| // 4. Ark-owned hook entry in .claude/settings.json (sibling user | ||
| // entries are preserved). | ||
| summary.removed_hook_entry = | ||
| remove_settings_hook(layout.claude_settings(), ARK_CONTEXT_HOOK_COMMAND)?; |
There was a problem hiding this comment.
ark remove is documented as unconditional cleanup, but this new call hard-fails on malformed .claude/settings.json because JSON parse errors bubble through ?. In that case the command returns failure after already deleting .ark/ and .ark.db, leaving only a stale hook entry and forcing users to hand-fix settings before remove can succeed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
.ark/tasks/archive/2026-04/ark-context/02_PLAN.md (1)
78-85:⚠️ Potential issue | 🟠 Major
## Specis still non-self-contained and remains extraction-hostile.This section is still delta-style (“carried forward”) rather than a standalone spec, so extracting from this plan cannot produce a complete artifact. Please either inline the full final
## Speccontent here or add a clear machine-readable archival marker so extract tooling skips this file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.ark/tasks/archive/2026-04/ark-context/02_PLAN.md around lines 78 - 85, The "## Spec" section is currently delta-style and not self-contained; update the section labeled "## Spec" so it is extraction-friendly by either inlining the full, final spec text instead of "Carried forward ..." deltas or adding a clear machine‑readable archival marker (e.g., JSON/YAML frontmatter or a skip token) immediately under the "## Spec" header that signals extract tools to ignore this file; ensure the change is applied to the "## Spec" block and that any references to G-*, NG-*, or C-* constraints remain intact or are fully expanded when you inline.crates/ark-core/src/commands/context/gather.rs (1)
104-106:⚠️ Potential issue | 🟠 MajorDon't collapse unreadable task directories into an "empty" snapshot.
Both guards use
exists(), which turns I/O failures intofalse. If.ark/tasks/or.ark/tasks/archive/is unreadable,ark contextwill silently report no active/archive tasks instead of surfacing the permission problem. Usetry_exists()and map failures withError::io(...).Suggested fix
fn gather_tasks(layout: &Layout) -> Result<TasksState> { let tasks_dir = layout.tasks_dir(); - if !tasks_dir.exists() { + if !tasks_dir + .try_exists() + .map_err(|source| crate::error::Error::io(&tasks_dir, source))? + { return Ok(TasksState::default()); } @@ fn gather_archive(layout: &Layout) -> Result<ArchiveState> { let archive_root = layout.tasks_archive_dir(); - if !archive_root.exists() { + if !archive_root + .try_exists() + .map_err(|source| crate::error::Error::io(&archive_root, source))? + { return Ok(ArchiveState::default()); }As per coding guidelines: "
**/*.rs: Every fallible operation in Rust code must returncrate::error::Result<T>; wrapstd::io::ErrorviaError::io(path, source)and never useunwrap()outside tests; reserveexpect(\"…\")for documented invariants only`."Also applies to: 147-149
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/ark-core/src/commands/context/gather.rs` around lines 104 - 106, The code currently treats unreadable task directories as non-existent by calling exists(); change to use try_exists() on layout.tasks_dir() and layout.tasks_archive() and map any std::io::Error into the crate error via Error::io(path, source) so the function returns Err(...) instead of silently returning an empty TasksState; update the early-return branches that reference tasks_dir and archive_dir (variables from gather.rs) to propagate IO errors using the crate::error::Result<T> convention rather than collapsing failures into false.crates/ark-core/src/commands/context/mod.rs (1)
99-107:⚠️ Potential issue | 🟠 MajorPropagate JSON serialization failures instead of panicking.
Both
serde_json::to_string_prettycalls still useexpect()in production code. Return aResult<String>fromwrap_session_start_envelopeand bubble serialization failures throughcontext()soark contextfails with a regularErrorinstead of aborting the process.As per coding guidelines: "
**/*.rs: Every fallible operation in Rust code must returncrate::error::Result<T>; wrapstd::io::ErrorviaError::io(path, source)and never useunwrap()outside tests; reserveexpect(\"…\")for documented invariants only`."Also applies to: 116-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/ark-core/src/commands/context/mod.rs` around lines 99 - 107, The code currently calls serde_json::to_string_pretty(...).expect(...) which will abort on serialization failure; change both places (the Format::Json branch and the other JSON call at 116-123) to propagate errors by returning crate::error::Result<String> from wrap_session_start_envelope and by replacing expect(...) with ? on serde_json::to_string_pretty(&projected) (or map_err to crate::Error if needed), then pass the resulting String into wrap_session_start_envelope? and propagate the Result up through context() so context() returns crate::error::Result<ContextSummary> instead of panicking; update function signatures (wrap_session_start_envelope, context and any callers) to use the crate::error::Result type and convert serde_json::Error into the crate error per project conventions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.ark/specs/features/ark-context/SPEC.md:
- Line 40: Several fenced code blocks in SPEC.md are missing language
identifiers which triggers markdownlint MD040; update each bare triple-backtick
fence used for plain text to include a language tag (e.g., change ``` to
```text) for the three occurrences referenced (the fence at the snippet around
line 40 and the other two occurrences around the noted locations) so the blocks
become ```text ... ``` and satisfy the linter.
In @.ark/tasks/archive/2026-04/ark-context/01_PLAN.md:
- Around line 27-28: The document's contract is inconsistent about whether
sibling hook entries are preserved byte-for-byte versus only structurally;
reconcile this by making the behavior explicit and aligning code and docs:
update the Snapshot struct description to state that Snapshot.hook_bodies only
captures Ark-owned entries (matched via ARK_CONTEXT_HOOK_COMMAND) and that
unload/load (see unload and load command implementations) will remove/reapply
only Ark-owned entries using update_settings_hook which may reformat the JSON,
so sibling/user-owned hook entries are preserved semantically but not guaranteed
byte-identical; then adjust the text around the settings-hook preservation
(including the lines referencing hook_bodies and update_settings_hook) to
reflect this precise rule and ensure tests expect semantic preservation not
byte-for-byte identity.
- Line 350: The plan step referencing `.ark/workflow.md` is stale because the
Mechanics table was moved to "## 6"; update the instruction in this file to
target the current Mechanics section by changing the step to add the row `|
Session / phase orientation (read-only) | ark context [--scope …] [--for …] |`
into the Mechanics table now under "## 6", placing that row immediately above
the existing `ark agent` rows; ensure the file name `.ark/workflow.md`, section
header "## 6" (Mechanics), and the `ark agent` rows are used to locate the
correct insertion point.
---
Duplicate comments:
In @.ark/tasks/archive/2026-04/ark-context/02_PLAN.md:
- Around line 78-85: The "## Spec" section is currently delta-style and not
self-contained; update the section labeled "## Spec" so it is
extraction-friendly by either inlining the full, final spec text instead of
"Carried forward ..." deltas or adding a clear machine‑readable archival marker
(e.g., JSON/YAML frontmatter or a skip token) immediately under the "## Spec"
header that signals extract tools to ignore this file; ensure the change is
applied to the "## Spec" block and that any references to G-*, NG-*, or C-*
constraints remain intact or are fully expanded when you inline.
In `@crates/ark-core/src/commands/context/gather.rs`:
- Around line 104-106: The code currently treats unreadable task directories as
non-existent by calling exists(); change to use try_exists() on
layout.tasks_dir() and layout.tasks_archive() and map any std::io::Error into
the crate error via Error::io(path, source) so the function returns Err(...)
instead of silently returning an empty TasksState; update the early-return
branches that reference tasks_dir and archive_dir (variables from gather.rs) to
propagate IO errors using the crate::error::Result<T> convention rather than
collapsing failures into false.
In `@crates/ark-core/src/commands/context/mod.rs`:
- Around line 99-107: The code currently calls
serde_json::to_string_pretty(...).expect(...) which will abort on serialization
failure; change both places (the Format::Json branch and the other JSON call at
116-123) to propagate errors by returning crate::error::Result<String> from
wrap_session_start_envelope and by replacing expect(...) with ? on
serde_json::to_string_pretty(&projected) (or map_err to crate::Error if needed),
then pass the resulting String into wrap_session_start_envelope? and propagate
the Result up through context() so context() returns
crate::error::Result<ContextSummary> instead of panicking; update function
signatures (wrap_session_start_envelope, context and any callers) to use the
crate::error::Result type and convert serde_json::Error into the crate error per
project conventions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e0c5716e-f277-416a-966a-929a0eaefd53
📒 Files selected for processing (49)
.ark/.installed.json.ark/specs/features/INDEX.md.ark/specs/features/ark-agent-namespace/SPEC.md.ark/specs/features/ark-context/SPEC.md.ark/specs/features/ark-upgrade/SPEC.md.ark/tasks/archive/2026-04/ark-context/00_PLAN.md.ark/tasks/archive/2026-04/ark-context/00_REVIEW.md.ark/tasks/archive/2026-04/ark-context/01_PLAN.md.ark/tasks/archive/2026-04/ark-context/01_REVIEW.md.ark/tasks/archive/2026-04/ark-context/02_PLAN.md.ark/tasks/archive/2026-04/ark-context/02_REVIEW.md.ark/tasks/archive/2026-04/ark-context/PRD.md.ark/tasks/archive/2026-04/ark-context/VERIFY.md.ark/tasks/archive/2026-04/ark-context/task.toml.ark/tasks/archive/2026-04/spec-extract-self-contained/PRD.md.ark/tasks/archive/2026-04/spec-extract-self-contained/task.toml.ark/workflow.md.claude/commands/ark/archive.md.claude/commands/ark/design.md.claude/commands/ark/quick.md.claude/settings.jsonAGENTS.mdREADME.mdcrates/ark-cli/src/main.rscrates/ark-core/src/commands/context/gather.rscrates/ark-core/src/commands/context/mod.rscrates/ark-core/src/commands/context/model.rscrates/ark-core/src/commands/context/projection.rscrates/ark-core/src/commands/context/related_specs.rscrates/ark-core/src/commands/context/render.rscrates/ark-core/src/commands/init.rscrates/ark-core/src/commands/load.rscrates/ark-core/src/commands/mod.rscrates/ark-core/src/commands/remove.rscrates/ark-core/src/commands/unload.rscrates/ark-core/src/commands/upgrade.rscrates/ark-core/src/error.rscrates/ark-core/src/io/fs.rscrates/ark-core/src/io/git.rscrates/ark-core/src/io/mod.rscrates/ark-core/src/layout.rscrates/ark-core/src/lib.rscrates/ark-core/src/state/mod.rscrates/ark-core/src/state/snapshot.rsdocs/ROADMAP.mdtemplates/ark/workflow.mdtemplates/claude/commands/ark/archive.mdtemplates/claude/commands/ark/design.mdtemplates/claude/commands/ark/quick.md
✅ Files skipped from review due to trivial changes (15)
- .claude/settings.json
- .ark/tasks/archive/2026-04/spec-extract-self-contained/task.toml
- crates/ark-core/src/state/mod.rs
- .ark/specs/features/INDEX.md
- .ark/tasks/archive/2026-04/ark-context/task.toml
- README.md
- .ark/tasks/archive/2026-04/ark-context/PRD.md
- .ark/.installed.json
- .ark/tasks/archive/2026-04/ark-context/VERIFY.md
- .ark/tasks/archive/2026-04/ark-context/02_REVIEW.md
- .ark/specs/features/ark-agent-namespace/SPEC.md
- .claude/commands/ark/archive.md
- crates/ark-core/src/lib.rs
- crates/ark-core/src/commands/context/render.rs
- .claude/commands/ark/design.md
🚧 Files skipped from review as they are similar to previous changes (13)
- crates/ark-core/src/commands/context/projection.rs
- crates/ark-core/src/commands/mod.rs
- crates/ark-core/src/commands/remove.rs
- crates/ark-core/src/io/git.rs
- crates/ark-core/src/commands/context/model.rs
- .claude/commands/ark/quick.md
- .ark/specs/features/ark-upgrade/SPEC.md
- AGENTS.md
- crates/ark-core/src/io/fs.rs
- docs/ROADMAP.md
- crates/ark-cli/src/main.rs
- crates/ark-core/src/commands/upgrade.rs
- .ark/tasks/archive/2026-04/spec-extract-self-contained/PRD.md
|
|
||
| [**Architecture**] | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Add language tags to fenced blocks to satisfy markdownlint.
The three fences opened on these lines are missing a language identifier (MD040).
Suggested patch
-```
+```text
@@
-```
+```text
@@
-```
+```textAlso applies to: 91-91, 111-111
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 40-40: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.ark/specs/features/ark-context/SPEC.md at line 40, Several fenced code
blocks in SPEC.md are missing language identifiers which triggers markdownlint
MD040; update each bare triple-backtick fence used for plain text to include a
language tag (e.g., change ``` to ```text) for the three occurrences referenced
(the fence at the snippet around line 40 and the other two occurrences around
the noted locations) so the blocks become ```text ... ``` and satisfy the
linter.
| - **C-17:** `.claude/settings.json` is **not hash-tracked.** The Ark-owned `SessionStart` hook entry is re-applied on every successful `init`, `load`, and `upgrade` via `update_settings_hook`, mirroring the `CLAUDE.md` managed-block treatment (upgrade SPEC C-8). The file is not listed in `manifest.files` and has no entry in `manifest.hashes`. User-owned siblings (other `hooks.*` keys, unrelated top-level keys) are preserved byte-identically where JSON object-key order is stable. | ||
| - **C-18:** `Snapshot` gains a `hook_bodies: Vec<SnapshotHookBody>` slot. `unload` captures the Ark-owned `SessionStart` entry (matched by `command == ARK_CONTEXT_HOOK_COMMAND`) into this slot. `load` restores it by re-applying `update_settings_hook` after scaffolding. User-edits to *unrelated* hook entries are NOT captured by `hook_bodies` and do NOT survive a round-trip — they are user-owned content outside Ark's owned dirs. Documented trade-off; matches the existing rule that `.claude/` (apart from `commands/ark/`) is user space. |
There was a problem hiding this comment.
The settings-hook preservation contract is internally inconsistent.
These sections currently claim three different behaviors: sibling entries are preserved "byte-identically"/"verbatim", sibling entries do not survive unload/load, and update_settings_hook rewrites the file by parsing + pretty-serializing the whole JSON document. The unload/load flow shown in crates/ark-core/src/commands/unload.rs:70-130 and crates/ark-core/src/commands/load.rs:96-125 only removes/reapplies the Ark-owned entry, so sibling hooks survive structurally on disk, but they are neither captured in hook_bodies nor preserved byte-for-byte. Tighten this contract before the tests lock in the wrong behavior.
Also applies to: 42-44, 192-199
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.ark/tasks/archive/2026-04/ark-context/01_PLAN.md around lines 27 - 28, The
document's contract is inconsistent about whether sibling hook entries are
preserved byte-for-byte versus only structurally; reconcile this by making the
behavior explicit and aligning code and docs: update the Snapshot struct
description to state that Snapshot.hook_bodies only captures Ark-owned entries
(matched via ARK_CONTEXT_HOOK_COMMAND) and that unload/load (see unload and load
command implementations) will remove/reapply only Ark-owned entries using
update_settings_hook which may reformat the JSON, so sibling/user-owned hook
entries are preserved semantically but not guaranteed byte-identical; then
adjust the text around the settings-hook preservation (including the lines
referencing hook_bodies and update_settings_hook) to reflect this precise rule
and ensure tests expect semantic preservation not byte-for-byte identity.
| 1. Add `io::fs::update_settings_hook` + `remove_settings_hook` + `ARK_CONTEXT_HOOK_COMMAND`. V-UT-20, V-UT-21, V-UT-22. | ||
| 2. Wire `init.rs` to call `update_settings_hook` after scaffold. Add `Snapshot::hook_bodies` capture in `unload.rs` + restore in `load.rs`. Wire `remove.rs` to call `remove_settings_hook`. Wire `upgrade.rs` to call `update_settings_hook` unconditionally after template writes. | ||
| 3. Update `templates/claude/commands/ark/{quick,design,archive}.md` to prepend `ark context --scope phase --for <phase> --format json` recipe. | ||
| 4. Update `.ark/workflow.md` §7 mechanics table to add: `| Session / phase orientation (read-only) | ark context [--scope …] [--for …] |` placed above the `ark agent` rows. |
There was a problem hiding this comment.
This workflow reference is stale.
.ark/workflow.md in this PR moved Mechanics to ## 6 and removed the old mechanics table, so this step now points at a section that no longer exists.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.ark/tasks/archive/2026-04/ark-context/01_PLAN.md at line 350, The plan step
referencing `.ark/workflow.md` is stale because the Mechanics table was moved to
"## 6"; update the instruction in this file to target the current Mechanics
section by changing the step to add the row `| Session / phase orientation
(read-only) | ark context [--scope …] [--for …] |` into the Mechanics table now
under "## 6", placing that row immediately above the existing `ark agent` rows;
ensure the file name `.ark/workflow.md`, section header "## 6" (Mechanics), and
the `ark agent` rows are used to locate the correct insertion point.
This pull request updates the Ark tool's installed version and feature specifications, and clarifies the behavior of the
spec extractoperation. The most important changes are grouped below:Ark version and feature registration updates:
0.1.1to0.1.2in.ark/.installed.json.ark-contextfeature in.ark/specs/features/INDEX.md.Specification and documentation clarifications:
.ark/specs/features/ark-agent-namespace/SPEC.mdthatspec extractcopies the latest## Specsection verbatim from the PLAN and does not merge across iterations; the PLAN must keep## Specself-contained.File hash updates (as a result of the above changes):
.ark/.installed.jsonto reflect new or changed files and specifications.Summary by CodeRabbit
New Features
ark context— read-only snapshot of git + workflow state (JSON/text) with session/phase scoping and phase-specific--forfilters.ark initnow installs a SessionStart hook that auto-invokesark contextat session start.Behavior Changes
ark loadrestores andark unloadcaptures the SessionStart hook;ark removedeletes the Ark hook;ark upgradere-applies the hook idempotently.Documentation
ark contextfor phase-based state queries and tightened SPEC iteration rules.