Skip to content

marks: patch up#5764

Merged
Fang- merged 5 commits into
developfrom
m/mark-fixes
Apr 29, 2026
Merged

marks: patch up#5764
Fang- merged 5 commits into
developfrom
m/mark-fixes

Conversation

@Fang-
Copy link
Copy Markdown
Member

@Fang- Fang- commented Apr 22, 2026

Summary

Many marks didn't compile into proper mark cores. Here, we make a bunch of changes to correct this.

These changes are necessary for #5668 to be able to incorporate all marks properly.

Changes

Mostly straightforward changes: making sure we're in type parsing mode, specifying a +grad, making sure we can +noun:grab, etc.

Special review attention request for the following:

  • Do /mar/chat/writs.hoon and /mar/group/invites.hoon pull in the right type versions?
  • What's up with /mar/group/create.hoon? The json conversion util it tried using doesn't seem to exist at all...
  • /mar/writ-diff.hoon aka %writ-diff seems to be completely unused. I think it's a very old mark. Should we just remove it?

How did I test?

Compiled all the marks for #5668.

Risks and impact

  • Technically safe to rollback without consulting PR author.
  • Affects important code area:
    • Message sync
    • Channel display

Rollback plan

Revert.

Many marks didn't compile into proper mark cores. Here, we make a bunch
of changes to correct this.
@Fang- Fang- requested review from arthyn and mikolajpp April 22, 2026 21:21
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5f0279b8a8

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread desk/mar/group/create.hoon Outdated
|%
++ noun create:v7:gv
++ json create:v7:dejs:j
++ json |=(^json *create:v7:gv) ::TODO REVIEW create:v7:dejs:j doesn't exist
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Decode group-create JSON instead of returning default noun

grab now defines ++ json as |=(^json *create:v7:gv) which ignores the incoming JSON payload and always returns the bunt/default create:v7:gv value. In any path that decodes %group-create JSON through this mark, user-supplied fields (name, metadata, members, etc.) are dropped, so downstream logic receives incorrect data and can create the wrong group state or fail validation unexpectedly.

Useful? React with 👍 / 👎.

@blacksmith-sh

This comment has been minimized.

Copy link
Copy Markdown
Collaborator

@mikolajpp mikolajpp left a comment

Choose a reason for hiding this comment

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

Good cleanup! A few group marks are remnants of old interfaces and could be removed altogether.

Edit: something's up with e2e tests though.

Comment thread desk/mar/group/create.hoon Outdated
Comment thread desk/mar/group/invites.hoon Outdated
Comment thread desk/mar/group/invites-1.hoon Outdated
Comment thread desk/mar/group/preview-update.hoon Outdated
Comment thread desk/mar/chat/writs.hoon Outdated
Comment thread desk/mar/writ-diff.hoon
@mikolajpp
Copy link
Copy Markdown
Collaborator

mikolajpp commented Apr 28, 2026

The e2e test fail here seems to be a client bug, but since we touch a whole bunch of marks let's wait until that gets fixed and merge develop to have a clean pass.

@Fang- Fang- merged commit b2bd0f5 into develop Apr 29, 2026
4 checks passed
@Fang- Fang- deleted the m/mark-fixes branch April 29, 2026 22:13
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.

2 participants