Skip to content

working_copy: Add LockedWorkingCopy::set_workspace_annotations hook - #9923

Draft
rayaq-siddiqui wants to merge 1 commit into
mainfrom
rayaq/set-workspace-annotations
Draft

working_copy: Add LockedWorkingCopy::set_workspace_annotations hook#9923
rayaq-siddiqui wants to merge 1 commit into
mainfrom
rayaq/set-workspace-annotations

Conversation

@rayaq-siddiqui

Copy link
Copy Markdown

This introduces a mechanism to pass metadata and annotations to a working-copy backend when its checkout is updated.

Specifically:

  • Adds a WorkspaceAnnotations struct to hold the checkout metadata (currently the associated CommitId).
  • Adds a set_workspace_annotations method to the LockedWorkingCopy trait, allowing backends to be notified of these updates. It defaults to a no-op for backends that do not track this state (like LocalWorkingCopy).
  • Invokes this new hook during snapshot_working_copy in the CLI after the working-copy commit is updated.

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added/updated tests to cover my changes
  • I fully understand the code that I am submitting (what it does,
    how it works, how it's organized), including any code drafted by an LLM.
  • For any prose generated by an LLM, I have proof-read and copy-edited with
    an eye towards deleting anything that is irrelevant, clarifying anything
    that is confusing, and adding details that are relevant. This includes,
    for example, commit descriptions, PR descriptions, and code comments.

@PhilipMetzger

PhilipMetzger commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

I guess this is the thing @hooper asked some questions about in the Discord. Anyway to adhere to our commit style guidelines drop the conventional commit feat(...) and just use working_copy: <title>

@rayaq-siddiqui
rayaq-siddiqui force-pushed the rayaq/set-workspace-annotations branch 3 times, most recently from b6668a6 to 44fc947 Compare August 5, 2026 23:25
@rayaq-siddiqui rayaq-siddiqui changed the title feat(annotations): Add LockedWorkingCopy::set_workspace_annotations hook working_copy: Add LockedWorkingCopy::set_workspace_annotations hook Aug 5, 2026
Custom working-copy backends (such as those used at Google) often maintain separate metadata stores alongside the repository. These backends need to record the checked-out commit ID (`@`) in synchronization with the working copy's operation ID. Maintaining this explicit mapping allows external tools, including developer environments and build systems, to accurately determine workspace state and verify code provenance.

Currently, when commands trigger automatic snapshotting of dirty working-copy files (e.g., during `jj status`), a new working-copy commit is created, but `LockedWorkingCopy` is not informed of the updated commit ID.

To address this, introduce a hook on `LockedWorkingCopy` that allows backends to be notified when checkout annotations or metadata are updated:

- Add `WorkspaceAnnotations` to encapsulate working-copy checkout metadata (currently holding the associated `CommitId`).
- Add `LockedWorkingCopy::set_workspace_annotations` with a default no-op implementation for backends that do not track this state (such as `LocalWorkingCopy`).
- Invoke `set_workspace_annotations` in `cli_util::snapshot_working_copy()` after updating the snapshot working-copy commit.
@rayaq-siddiqui
rayaq-siddiqui force-pushed the rayaq/set-workspace-annotations branch from 44fc947 to 8f6cd16 Compare August 5, 2026 23:32
@hooper

hooper commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

This is actually different from the problem I mentioned on Discord a few weeks ago, but it has a very similar shape. That older one (passing operation attributes from the working copy implementation) seemed less controversial, but I wanted to get some early feedback on this new one in case we missed a nicer way to do it. I haven't checked if this one is as tricky to test, but that's a consideration in addition to performance.

@PhilipMetzger

Copy link
Copy Markdown
Contributor

That older one (passing operation attributes from the working copy implementation) seemed less controversial

I agree.

but I wanted to get some early feedback on this new one in case we missed a nicer way to do it.

While I currently don't really have an idea on how to do it in a nicer way, the first thing which is surely a trade-off to make is that the Annotations statically encode all information (i.e the commit id) vs just having a set of strings downstreams can or cannot care about.

And if this is a static schema for all of Google I don't really think it should live upstream (that's a personal opinion).

Comment thread cli/src/cli_util.rs
};
locked_ws
.locked_wc()
.set_workspace_annotations(annotations)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have guessed that we could pass the commit id to LockedWorkingCopy::finish() instead of adding a new set_workspace_annotations() method. Why does that not work? I didn't quite follow the explanation in the commit description.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you suggesting to pass WorkspaceAnnotations into finish()? For example:

    async fn finish(
        self: Box<Self>,
        operation_id: OperationId,
        annotations: WorkspaceAnnotations<'_>,
    ) -> Result<Box<dyn WorkingCopy>, WorkingCopyStateError>;

I believe this would work. I have not tested it yet.

The initial motivation for set_workspace_annotations() was to keep finish() focused strictly on operation finalization. That said, bundling checkout metadata into finish() makes sense if we'd prefer to avoid adding a separate method to LockedWorkingCopy. Thoughts?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or even:

    async fn finish(
        self: Box<Self>,
        operation_id: OperationId,
        wc_commit_id: CommitId,
    ) -> Result<Box<dyn WorkingCopy>, WorkingCopyStateError>;

And removing the WorkspaceAnnotations struct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants