Stop boot from unlinking every room's messages. - #18
Merged
Merged
Conversation
`daemon::run` called `drop_channel` for each configured room on every start, and `drop_key` unlinks `channels/<id>.jsonl`. A room with a month of history opened empty after a restart, and the file was gone. Rooms had the boot and create sides swapped. Bots have it right: boot reads back with `load_agent`, and `insert_spawned_agent` drops first so an id freed by a delete cannot open onto the old conversation. The comment sitting in the boot loop — with the indentation still broken from the paste — is the one that belongs on `add_channel`, which was calling `load_channel` instead. Both sides now match the bot ones, and `restore_transcripts` puts boot's two loops in one place so the next edit cannot swap one without the other.
From review of #18. The test only passed a room, so inverting the bot arm of `restore_transcripts` — the exact mirror of the bug — left it green. It now carries one of each and asserts both files survive. `add_channel`'s drop had no test at all, which is the half that deletes a file. `a_new_room_does_not_open_onto_a_freed_id` pins it. That drop also ran while `channels()` was held, so every RPC that reads the roster waited on a disk unlink. It runs between the existence check and the insert now, holding nothing.
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.
TL;DR
데몬이 시작할 때마다 설정된 모든 채널의 대화 기록 파일을 삭제하고 있었어요. 봇 쪽과 부팅/생성 처리가 정확히 뒤바뀌어 있었어요.
증상
채널에 쌓인 대화가 앱을 껐다 켜면 사라져요. 메모리에서만 비는 게 아니라
~/Library/Application Support/crew/channels/<id>.jsonl파일 자체가 지워져서 복구가 안 돼요.원인
daemon::run()부팅 루프가 채널마다drop_channel()을 호출해요.drop_channel → drop_key → fs::remove_file라 실제 언링크예요.봇 쪽은 올바른 모양이에요:
load_agent✅insert_spawned_agent→drop_agent✅drop_channel❌add_channel→load_channel❌둘이 맞바뀐 거예요. 부팅 루프에 있던 주석(
Same as a new agent: a room id freed by a deleted channel must not open onto that channel's messages)은 생성 쪽 주석이고, 붙여넣기 때 깨진 들여쓰기가 그대로 남아 있어요 —rustfmt가 그 줄을 계속 잡고 있었어요.들어온 시점은
db7bd8f(#4,Keep a dead session out of a new bot of the same name.) 예요:그 PR 본문이 "
add_channel은 아직 load 라 채널 쪽이 남아 있다"고 적어 둔 그 수정이,add_channel이 아니라 부팅 루프에 들어갔어요. 즉 #4 이후로 계속 지워지고 있었어요.고친 것
load_channel,add_channel은drop_channel— 봇과 같은 모양으로 맞췄어요.restore_transcripts()하나로 모았어요. 봇과 채널이 나란히 있어서 한쪽만 뒤집는 편집이 눈에 띄어요.add_channel의 unlink 를channels()락 밖으로 뺐어요. 로스터를 읽는 모든 RPC 가 디스크 unlink 를 기다리고 있었어요.remove_channel의drop_channel은 원래 맞아서 그대로예요.검증
cargo test --locked -p crew— 210 passedboot_keeps_the_messages_a_room_and_a_bot_already_have— 방과 봇 양쪽 을 넣어서, 어느 한쪽 arm 만 뒤집어도 실패해요restore_transcripts를 원래 버그대로 되돌리면FAILED, 되돌리면oka_new_room_does_not_open_onto_a_freed_id— 파일을 지우는 쪽(add_channel)도 테스트로 고정cargo fmt --check— 이 PR 이 깨진 들여쓰기 한 건을 없애요 (daemon.rs 의 남은 2건은 기존 drift, 범위 밖)영향
지금 채널이 0개면 루프가 안 돌아서 피해가 없어요. 채널을 하나라도 만드는 순간부터 매 실행마다 지워져요.
범위 밖 (리뷰에서 나온 기존 문제)
daemon::run이 소켓을 bind 하기 전에 전체 봇을 spawn 하고, bind 실패 시shutdown_agents()없이 early return 해요 — 데몬 두 개가 같은 transcript 를 덮어쓸 수 있어요.load_key가 파싱 실패한 줄을 조용히 버리고, 다음persist()가 메모리 기준으로 파일을 통째로 다시 써요 — 깨진 한 줄이 영구 삭제돼요.persist()/Config::save()가fs::write라 쓰는 도중 죽으면 잘린 파일이 남아요 (temp + rename 아님).