Repository navigation
refactor: start every message of the binary with amem: - #50
Merged
Merged
Conversation
The binary is amem, but 74 messages started with memory: and 10 with amem:. Now all 84 start with amem:. The demo filters the lines of amem work by this prefix, so the three demo scripts change too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bhj4Ck4y5iwZUsjpgQx8A2
The pass found two hook messages that started with "memory hook:", a spike script that still removed "memory: " lines, and an AppArmor comment that showed the old output. Now they use amem:. A new test fails on a message that starts with the old name. The web error for the record was "memory:", which named the record, not the binary. Now it is "record:". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bhj4Ck4y5iwZUsjpgQx8A2
…ones
The second adversarial pass found spellings that the test let through:
no space after the colon, a tab, a leading newline escape, a single
quote, and a string continued on the next line. It also found that the
test refused comments and fixtures that hold the old name. Now the test
finds those spellings, skips comment lines, and accepts a line that
says "old name allowed". A new test holds each case. With the old list
of spellings, it fails on eprintln!("memory:{e}").
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bhj4Ck4y5iwZUsjpgQx8A2
There was a problem hiding this comment.
🟡 Changes recommended
The checker incorrectly flags comments and misses message-prefix violations in string continuations spanning blank source lines.
2 open issues
What changed in this PR
This PR aligns diagnostic prefixes with the amem command name, following the naming corrections in #37.
Changes:
- Replace old diagnostic prefixes with
amem:and update dependent shell filters. - Use
record:for the web record error and correct the AppArmor sample comment. - Add source checks for messages that use the old name.
| File | Description |
|---|---|
spikes/harness-containers/seatbelt-steps.sh |
Update the diagnostic exclusion filter. |
scripts/demo/7-platform.sh |
Match the new worker prefix. |
scripts/demo/4-fabric.sh |
Update worker output filters. |
scripts/demo/2-curation.sh |
Update the curation output filter. |
packaging/apparmor/amem |
Correct sample output in a comment. |
crates/web/src/lib.rs |
Label record errors with record:. |
crates/server/src/tape.rs |
Rename the tape diagnostic prefix. |
crates/server/src/projection.rs |
Rename projection diagnostic prefixes. |
crates/server/src/mcp.rs |
Rename the tape recording diagnostic prefix. |
crates/sandbox/src/measure.rs |
Rename the process termination prefix. |
crates/sandbox/src/linux.rs |
Rename sandbox and relay diagnostic prefixes. |
crates/sandbox/src/lib.rs |
Rename harness and OpenCode plugin prefixes. |
crates/memory/tests/message_prefix.rs |
Add old-prefix detection and regression tests. |
crates/memory/src/wslc.rs |
Rename WSL diagnostic prefixes. |
crates/memory/src/wslc_api.rs |
Rename session and image diagnostic prefixes. |
crates/memory/src/work.rs |
Rename worker diagnostic prefixes. |
crates/memory/src/transcripts.rs |
Rename transcript lifecycle diagnostic prefixes. |
crates/memory/src/taps.rs |
Rename tap diagnostic prefixes. |
crates/memory/src/stops.rs |
Rename the shutdown diagnostic prefix. |
crates/memory/src/sources.rs |
Rename the source release prefix. |
crates/memory/src/reenact.rs |
Rename re-enactment diagnostic prefixes. |
crates/memory/src/mine.rs |
Rename the mining diagnostic prefix. |
crates/memory/src/main.rs |
Rename launcher, usage, and hook prefixes. |
crates/memory/src/image.rs |
Rename image diagnostic prefixes. |
crates/memory/src/gofannon.rs |
Rename Gofannon worker diagnostic prefixes. |
crates/memory/src/eventlog.rs |
Rename the event log diagnostic prefix. |
Review effort: Balanced
| let mut continues = false; | ||
| for (n, line) in text.lines().enumerate() { | ||
| let trimmed = line.trim_start(); | ||
| let comment = trimmed.starts_with("//"); |
Contributor
Author
There was a problem hiding this comment.
Fixed in 260b44b. The checker now blanks line and block comments before the scan. It keeps string, raw string, byte string and char literals whole, so // or /* inside a literal opens no comment. Block comments nest. The new cases include your two examples. A trailing comment failed on the old code: "flagged: let x = 1; // formerly "memory: x"". A later commit adds a case with a message after a line comment, and a floor on the number of messages the scan sees.
Posted by Claude on behalf of @sstriker
| if held && !comment && !line.contains(&allowed) { | ||
| found.push((n + 1, trimmed.to_string())); | ||
| } | ||
| continues = !comment && line.ends_with('\\'); |
Contributor
Author
The check now blanks every comment before it scans a line. A trailing comment or a block comment no longer makes the check fail. A string literal stays whole, so "//" or "/*" in a string does not start a comment. A string continuation now continues through blank lines, as in Rust. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bhj4Ck4y5iwZUsjpgQx8A2
A line comment that hid the rest of a file still passed both checks. A new case puts a message after a line comment. Another case puts an escaped quote in a char before a message. The check of the real sources also fails if it sees 70 or fewer messages outside comments. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bhj4Ck4y5iwZUsjpgQx8A2
# Conflicts: # crates/memory/src/wslc_api.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The binary is
amem, but 74 messages incrates/started withmemory:, 2 started withmemory hook:, and 10 started withamem:. Copilot found this on #37. Now all of them start withamem:. The two hook messages start withamem: hook:.The change also corrects:
crates/sandbox/src/lib.rs);grep -E '^memory: (...)'filters inscripts/demo/2-curation.sh,4-fabric.shand7-platform.sh. These filters select the lines ofamem workthat the demo shows. With the new prefix and the old filters, the acts would show no worker lines;grep -v '^memory: 'filter inspikes/harness-containers/seatbelt-steps.sh;packaging/apparmor/amem.The web error for the record was
memory: {0}. There,memorynamed the record, not the binary. Now it isrecord: {0}.The change does not touch the file name
/opt/memory/memoryinside the sandbox and the image, or the prefixesrelay:,error:andlaunch debug:.Tests
crates/memory/tests/message_prefix.rsreads the sources undercrates/. It fails on a message that starts with the old name after a quote, a single quote, a backtick, a byte string or\n, and on a continued string. It does not fail on comments, or on a line that says "old name allowed".no_message_starts_with_the_old_namememory hook:messages incrates/memory/src/main.rsevery_spelling_of_the_old_name_is_found_and_only_thosemissed: eprintln!("memory:{e}");Checks
make checkpasses: 739 tests.Adversarial pass
Two fresh agents attacked the branch, one after each of the first two pushes.
The first pass found:
memory hook:messages.The second pass found that the new test missed five spellings, and refused comments and fixtures that hold the old name. The last commit corrects these. No pass ran after it.
Two findings of the second pass are not changed:
crates/. The demo scripts also depend on the prefix, but a check of shellgreppatterns is a separate change.relay:andlaunch debug:stay. They name a part of the binary, not an old name of it.Second review round
Copilot found two problems in the checker, and 260b44b fixes both. A separate agent reviewed the full branch, because Copilot had run out of quota. Its verdict was merge. It found two test gaps, and 9c2485f closes them. A message after a line comment is now a test case. An escaped quote in a char literal is also a test case. The scan of the real sources fails if it finds 70 or fewer messages outside comments. Each new check failed on a version of the code with that fault.
Three old-prefix spellings are still not detected:
memory:followed by a letter, a$, or a closing quote. No message in the code has these forms.make checkpasses on the branch withmainmerged: 739 tests.Posted by Claude on behalf of @sstriker