diff --git a/platform/android/app/build.gradle b/platform/android/app/build.gradle index 416e6e54..c0918fb5 100644 --- a/platform/android/app/build.gradle +++ b/platform/android/app/build.gradle @@ -79,8 +79,8 @@ android { applicationId = "cn.lineai" minSdk = huxeruiMinSdk targetSdk = huxeruiTargetSdk - versionCode = 37 - versionName = "1.3.3" + versionCode = 38 + versionName = "1.3.4" externalNativeBuild { cmake { diff --git a/resources/strings/default.properties b/resources/strings/default.properties index 99d03417..df59023b 100644 --- a/resources/strings/default.properties +++ b/resources/strings/default.properties @@ -93,6 +93,10 @@ tool_file_write_updated = "Successfully updated file {0} ({1} lines)" tool_file_write_is_directory = "Path is a directory, cannot write file: {0}\nTo create a file, specify the full file path." tool_file_write_mkdir_failed = "Failed to create parent directory: {0}" tool_file_write_failed = "Failed to write file: {0}" +tool_file_write_needs_read = "Cannot modify {0}: the file already exists but has not been read. Read it with file_read first, then try again." +tool_file_write_stale_read = "Cannot modify {0}: the file changed after it was read. Read it again with file_read, then try again." +tool_file_write_needs_full_read = "Cannot replace {0}: the file was never read in full, so replacing it could drop content that was never seen. Use file_edit for a targeted change." +tool_file_write_busy = "Cannot modify {0}: another tool call is already changing this file. Try again." tool_file_edit_no_match = "No matching text found" tool_file_edit_multiple_matches = "old_string matched {0} places. Provide a more unique old_string, or set replace_all=true to replace every occurrence." tool_file_edit_success = "Successfully edited {0} ({1} match(es) replaced)" diff --git a/resources/strings/zh.properties b/resources/strings/zh.properties index d0b51c8b..3f052b7a 100644 --- a/resources/strings/zh.properties +++ b/resources/strings/zh.properties @@ -93,6 +93,10 @@ tool_file_write_updated = "成功更新文件 {0} ({1} 行)" tool_file_write_is_directory = "路径是一个目录,无法写入文件: {0}\n如需创建文件,请指定完整文件路径。" tool_file_write_mkdir_failed = "无法创建父目录: {0}" tool_file_write_failed = "写入文件失败: {0}" +tool_file_write_needs_read = "无法修改 {0}:文件已存在但尚未读取。请先用 file_read 读取该文件,然后重试。" +tool_file_write_stale_read = "无法修改 {0}:文件在读取之后已被修改。请用 file_read 重新读取该文件,然后重试。" +tool_file_write_needs_full_read = "无法整体替换 {0}:该文件从未被完整读取,整体替换可能丢失未被看到的内容。请使用 file_edit 做局部修改。" +tool_file_write_busy = "无法修改 {0}:另一个工具调用正在修改该文件。请重试。" tool_file_edit_no_match = "未找到匹配的文本" tool_file_edit_multiple_matches = "old_string 匹配到 {0} 处。请提供更唯一的 old_string,或设置 replace_all=true 以替换全部匹配。" tool_file_edit_success = "成功编辑文件 {0} ({1} 处匹配已替换)" diff --git a/src/application/file_content_hash.cpp b/src/application/file_content_hash.cpp new file mode 100644 index 00000000..33adc63c --- /dev/null +++ b/src/application/file_content_hash.cpp @@ -0,0 +1,168 @@ +#include "application/file_content_hash.h" + +#include +#include +#include +#include +#include +#include + +namespace linecode::application { +namespace { + +// FIPS 180-4 round constants: the first 32 bits of the fractional parts of the +// cube roots of the first 64 primes. +constexpr std::array kRoundConstants{ + 0x428a2f98U, 0x71374491U, 0xb5c0fbcfU, 0xe9b5dba5U, 0x3956c25bU, + 0x59f111f1U, 0x923f82a4U, 0xab1c5ed5U, 0xd807aa98U, 0x12835b01U, + 0x243185beU, 0x550c7dc3U, 0x72be5d74U, 0x80deb1feU, 0x9bdc06a7U, + 0xc19bf174U, 0xe49b69c1U, 0xefbe4786U, 0x0fc19dc6U, 0x240ca1ccU, + 0x2de92c6fU, 0x4a7484aaU, 0x5cb0a9dcU, 0x76f988daU, 0x983e5152U, + 0xa831c66dU, 0xb00327c8U, 0xbf597fc7U, 0xc6e00bf3U, 0xd5a79147U, + 0x06ca6351U, 0x14292967U, 0x27b70a85U, 0x2e1b2138U, 0x4d2c6dfcU, + 0x53380d13U, 0x650a7354U, 0x766a0abbU, 0x81c2c92eU, 0x92722c85U, + 0xa2bfe8a1U, 0xa81a664bU, 0xc24b8b70U, 0xc76c51a3U, 0xd192e819U, + 0xd6990624U, 0xf40e3585U, 0x106aa070U, 0x19a4c116U, 0x1e376c08U, + 0x2748774cU, 0x34b0bcb5U, 0x391c0cb3U, 0x4ed8aa4aU, 0x5b9cca4fU, + 0x682e6ff3U, 0x748f82eeU, 0x78a5636fU, 0x84c87814U, 0x8cc70208U, + 0x90befffaU, 0xa4506cebU, 0xbef9a3f7U, 0xc67178f2U, +}; + +constexpr std::array kHexDigits{'0', '1', '2', '3', '4', '5', '6', + '7', '8', '9', 'a', 'b', 'c', 'd', + 'e', 'f'}; + +std::uint32_t RotateRight(const std::uint32_t value, + const std::uint32_t bits) noexcept { + return (value >> bits) | (value << (32U - bits)); +} + +// Big-endian 32-bit load, so the digest does not depend on host endianness. +std::uint32_t LoadBigEndian(const std::uint8_t *bytes) noexcept { + return (static_cast(bytes[0]) << 24U) | + (static_cast(bytes[1]) << 16U) | + (static_cast(bytes[2]) << 8U) | + static_cast(bytes[3]); +} + +} // namespace + +void Sha256Digest::Compress(const std::uint8_t *block) noexcept { + std::array schedule{}; + for (std::size_t index = 0; index < 16U; ++index) + schedule[index] = LoadBigEndian(block + index * 4U); + for (std::size_t index = 16U; index < schedule.size(); ++index) { + const std::uint32_t previous = schedule[index - 15U]; + const std::uint32_t recent = schedule[index - 2U]; + const std::uint32_t s0 = + RotateRight(previous, 7U) ^ RotateRight(previous, 18U) ^ (previous >> 3U); + const std::uint32_t s1 = + RotateRight(recent, 17U) ^ RotateRight(recent, 19U) ^ (recent >> 10U); + schedule[index] = schedule[index - 16U] + s0 + schedule[index - 7U] + s1; + } + + std::uint32_t a = state_[0]; + std::uint32_t b = state_[1]; + std::uint32_t c = state_[2]; + std::uint32_t d = state_[3]; + std::uint32_t e = state_[4]; + std::uint32_t f = state_[5]; + std::uint32_t g = state_[6]; + std::uint32_t h = state_[7]; + for (std::size_t index = 0; index < schedule.size(); ++index) { + const std::uint32_t sum1 = + RotateRight(e, 6U) ^ RotateRight(e, 11U) ^ RotateRight(e, 25U); + const std::uint32_t choice = (e & f) ^ (~e & g); + const std::uint32_t temporary1 = + h + sum1 + choice + kRoundConstants[index] + schedule[index]; + const std::uint32_t sum0 = + RotateRight(a, 2U) ^ RotateRight(a, 13U) ^ RotateRight(a, 22U); + const std::uint32_t majority = (a & b) ^ (a & c) ^ (b & c); + const std::uint32_t temporary2 = sum0 + majority; + h = g; + g = f; + f = e; + e = d + temporary1; + d = c; + c = b; + b = a; + a = temporary1 + temporary2; + } + + state_[0] += a; + state_[1] += b; + state_[2] += c; + state_[3] += d; + state_[4] += e; + state_[5] += f; + state_[6] += g; + state_[7] += h; +} + +void Sha256Digest::Update(std::string_view bytes) noexcept { + if (bytes.empty()) + return; + total_ += bytes.size(); + const auto *data = reinterpret_cast(bytes.data()); + std::size_t remaining = bytes.size(); + + // Complete a partially filled block before consuming whole blocks, so the + // digest is identical for any chunking of the same input. + if (buffered_ > 0) { + const std::size_t wanted = kBlockBytes - buffered_; + const std::size_t taken = std::min(wanted, remaining); + std::copy_n(data, taken, buffer_.begin() + + static_cast(buffered_)); + buffered_ += taken; + data += taken; + remaining -= taken; + if (buffered_ == kBlockBytes) { + Compress(buffer_.data()); + buffered_ = 0; + } + } + while (remaining >= kBlockBytes) { + Compress(data); + data += kBlockBytes; + remaining -= kBlockBytes; + } + if (remaining > 0) { + std::copy_n(data, remaining, buffer_.begin()); + buffered_ = remaining; + } +} + +std::string Sha256Digest::Hex() { + const std::uint64_t bit_length = total_ * 8U; + // Padding: the buffered message bytes, then a single 0x80 byte, zeros, and + // the 64-bit big-endian length. The tail is one block unless the message + // leaves fewer than 9 bytes free in it. + const std::size_t tail_bytes = buffered_ < 56U ? kBlockBytes : 2 * kBlockBytes; + std::array tail{}; + std::copy_n(buffer_.begin(), buffered_, tail.begin()); + tail[buffered_] = 0x80U; + for (std::size_t index = 0; index < 8U; ++index) + tail[tail_bytes - 1U - index] = + static_cast(bit_length >> (8U * index)); + for (std::size_t offset = 0; offset < tail_bytes; offset += kBlockBytes) + Compress(tail.data() + offset); + + std::string hex; + hex.reserve(64U); + for (const std::uint32_t word : state_) { + for (std::uint32_t shift = 28U;; shift -= 4U) { + hex.push_back(kHexDigits[(word >> shift) & 0x0fU]); + if (shift == 0U) + break; + } + } + return hex; +} + +std::string Sha256Hex(std::string_view bytes) { + Sha256Digest digest; + digest.Update(bytes); + return digest.Hex(); +} + +} // namespace linecode::application diff --git a/src/application/file_content_hash.h b/src/application/file_content_hash.h new file mode 100644 index 00000000..b4448a2e --- /dev/null +++ b/src/application/file_content_hash.h @@ -0,0 +1,47 @@ +#pragma once + +#include +#include +#include +#include +#include + +namespace linecode::application { + +// Incremental SHA-256 (FIPS 180-4) over raw bytes. +// +// The built-in file tools identify a file revision by the digest of its bytes, +// so the digest must be deterministic and independent of how those bytes were +// obtained. Update() takes arbitrarily sized chunks, which lets a caller stream +// a file in bounded reads instead of loading it, and the running digest stays +// valid across chunk boundaries. +// +// Hex() finalizes the digest. A hasher is single-use: finalization appends the +// padding, so a second Hex() call would hash the padding again. +class Sha256Digest final { +public: + Sha256Digest() noexcept = default; + + void Update(std::string_view bytes) noexcept; + // Lowercase, zero-padded, 64-character hex digest. + [[nodiscard]] std::string Hex(); + +private: + static constexpr std::size_t kBlockBytes = 64; + + void Compress(const std::uint8_t *block) noexcept; + + // FIPS 180-4 initial hash value. + std::array state_{ + 0x6a09e667U, 0xbb67ae85U, 0x3c6ef372U, 0xa54ff53aU, + 0x510e527fU, 0x9b05688cU, 0x1f83d9abU, 0x5be0cd19U}; + std::array buffer_{}; + // Bytes currently held in buffer_, and bytes consumed since construction. + std::size_t buffered_{}; + std::uint64_t total_{}; +}; + +// One-shot digest of an in-memory buffer. +[[nodiscard]] std::string Sha256Hex(std::string_view bytes); + +} // namespace linecode::application diff --git a/src/application/file_observation_store.cpp b/src/application/file_observation_store.cpp new file mode 100644 index 00000000..dbcb3278 --- /dev/null +++ b/src/application/file_observation_store.cpp @@ -0,0 +1,67 @@ +#include "application/file_observation_store.h" + +#include +#include +#include +#include +#include +#include +#include +#include + +namespace linecode::application { + +bool FileObservationStore::Claim(std::string_view absolute_path) { + const std::lock_guard lock{mutex_}; + return claimed_.insert(std::string{absolute_path}).second; +} + +void FileObservationStore::Release(std::string_view absolute_path) { + const std::lock_guard lock{mutex_}; + // Erasing through a heterogeneous lookup is not portable across the supported + // standard libraries, so the entry is found first and erased by iterator. + const auto found = claimed_.find(absolute_path); + if (found != claimed_.end()) + claimed_.erase(found); +} + +void FileObservationStore::Record(std::string absolute_path, + FileObservation observation) { + const std::lock_guard lock{mutex_}; + const auto found = observed_.find(absolute_path); + if (found == observed_.end() && + observed_.size() >= kMaximumObservedPaths) { + // The record is a hint, never a permission: dropping it can only require + // another read before the next mutation. + observed_.clear(); + } + observed_.insert_or_assign(std::move(absolute_path), std::move(observation)); +} + +void FileObservationStore::Forget(std::string_view absolute_path) { + const std::lock_guard lock{mutex_}; + const auto found = observed_.find(absolute_path); + if (found != observed_.end()) + observed_.erase(found); +} + +std::optional +FileObservationStore::Observed(std::string_view absolute_path) const { + const std::lock_guard lock{mutex_}; + const auto found = observed_.find(absolute_path); + if (found == observed_.end()) + return std::nullopt; + return found->second; +} + +std::size_t FileObservationStore::Size() const { + const std::lock_guard lock{mutex_}; + return observed_.size(); +} + +void FileObservationStore::Clear() { + const std::lock_guard lock{mutex_}; + observed_.clear(); +} + +} // namespace linecode::application diff --git a/src/application/file_observation_store.h b/src/application/file_observation_store.h new file mode 100644 index 00000000..cb20494b --- /dev/null +++ b/src/application/file_observation_store.h @@ -0,0 +1,84 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include + +namespace linecode::application { + +// What the tools know about one file revision, keyed by its resolved absolute +// path: the digest of the text that was read or written, plus whether the model +// received that revision in full. +struct FileObservation final { + // SHA-256 of the exact text the tool read or wrote for this revision. The + // digest never comes from a separate look at the path, so it always describes + // the bytes the model was given rather than a revision that appeared later. + std::string content_hash; + // True only when the model received every byte of that revision: a whole-file + // read whose output was not truncated, or content the tool itself just wrote. + // A partial observation still authorizes a localized edit, because the edit + // carries over every byte it does not replace, but never a whole-file + // replacement that would drop the content the model never saw. + bool complete{}; + + bool operator==(const FileObservation &) const = default; +}; + +// The in-memory record of which file revisions the built-in file tools have +// read, keyed by the resolved absolute path. +// +// Before a tool replaces or edits a file that already exists it consults this +// record: +// +// * no entry -> the file has not been read, so the model must read +// it before it may change it; +// * entry, digest equal to the text on disk -> the read is still current, so +// the mutation may proceed; +// * entry, digest unequal -> the file changed after it was read (another +// process, another Agent, or the user), so the model +// is told to read it again instead of silently +// overwriting content it never saw. +// +// The same record also hands out one in-process claim per path, which is what +// keeps two tool calls from both passing that check and then both writing: the +// second one is refused instead of dropping the first one's change. +// +// Nothing is persisted. A fresh session or process starts empty, which only +// ever makes the rule stricter: an unread file can never be overwritten. +// +// The record is bounded, so a long-lived process cannot accumulate one entry +// per file it has ever read. Dropping entries only ever costs another read. +class FileObservationStore final { +public: + // Number of paths kept before the record is dropped as a whole. + static constexpr std::size_t kMaximumObservedPaths = 4096; + + // Takes the in-process mutation claim for one path. False means another tool + // call is already changing that file, and the caller must not mutate it + // behind that call's back. Claims are held by MutationClaim, never recorded. + [[nodiscard]] bool Claim(std::string_view absolute_path); + void Release(std::string_view absolute_path); + + // Records the revision observed for `absolute_path`, replacing any earlier + // one. + void Record(std::string absolute_path, FileObservation observation); + // Drops the entry, if any: the next mutation must be preceded by a read. + void Forget(std::string_view absolute_path); + // Revision recorded for `absolute_path`, or nullopt when it was never read. + [[nodiscard]] std::optional + Observed(std::string_view absolute_path) const; + [[nodiscard]] std::size_t Size() const; + void Clear(); + +private: + mutable std::mutex mutex_; + // std::less<> keeps the string_view lookups allocation-free. + std::map> observed_; + std::set> claimed_; +}; + +} // namespace linecode::application diff --git a/src/application/file_tool_registry.cpp b/src/application/file_tool_registry.cpp index da602168..1ae9bce2 100644 --- a/src/application/file_tool_registry.cpp +++ b/src/application/file_tool_registry.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -23,6 +24,8 @@ #include +#include "application/file_content_hash.h" +#include "application/file_observation_store.h" #include "application/tool_file_path_policy.h" #include "application/tool_text_catalog.h" #include "infrastructure/archive_json.h" @@ -355,6 +358,10 @@ struct FileToolContext final { // `DiffRecorder` that wrapped FileWriteTool and FileEditTool. Optional: a // null store simply means "no review history". DiffStore *diffs{}; + // Remembers the content digest of every file the read tools have seen, so the + // write family can require a read and detect a file that changed afterwards. + // Optional only so a hand-built context (tests) can opt out. + FileObservationStore *observations{}; }; // Converts one catalog argument exactly like huxerui::StringVariant::Format: @@ -444,6 +451,121 @@ huxerui::Task> LastByteIsNewline(ToolFileAccess &files, co_return (*last == "\n"); } +// Records the revision a tool call observed: the exact text a read returned, or +// the text a write left behind. The digest is taken from those bytes and never +// from a later look at the path, so a record can only ever describe a revision +// the model was actually given. +void RememberObserved(const FileToolContext &context, + const std::string &absolute_path, std::string_view content, + const bool complete) { + if (context.observations == nullptr) + return; + context.observations->Record( + absolute_path, + FileObservation{.content_hash = Sha256Hex(content), .complete = complete}); +} + +// Holds the in-process mutation claim for one path, so the freshness check and +// the write it authorizes cannot interleave with another tool call changing the +// same file. Releasing on destruction covers every exit from the mutation, +// including a refusal and a provider failure. +// +// The claim cannot exclude a writer outside this process -- the file capability +// offers no compare-and-swap, so a file changed by another program between the +// read and the WriteText call is undetectable -- but it does keep the two tool +// calls that would otherwise both pass the check and then both report success +// from dropping each other's change. +class MutationClaim final { +public: + MutationClaim(FileObservationStore *store, std::string absolute_path) + : store_(store), absolute_path_(std::move(absolute_path)), + acquired_(store_ != nullptr && store_->Claim(absolute_path_)) {} + + ~MutationClaim() { + if (acquired_) + store_->Release(absolute_path_); + } + + MutationClaim(const MutationClaim &) = delete; + MutationClaim &operator=(const MutationClaim &) = delete; + + [[nodiscard]] bool acquired() const noexcept { return acquired_; } + +private: + FileObservationStore *store_{}; + std::string absolute_path_; + bool acquired_{}; +}; + +// Outcome of the read-before-write rule for a file that already exists. +enum class MutationGuard : std::uint8_t { + // The recorded read matches the bytes on disk: the mutation may proceed. + allowed, + // The file was never read, so overwriting it could discard unseen content. + not_read, + // The file changed after it was read, so the model is looking at a revision + // that no longer exists. + changed, + // The revision still matches, but the model never received all of it, so a + // whole-file replacement could drop content that was never seen. + partial, +}; + +// The revision the guard accepted, and whether the model received all of it. An +// edit hands `complete` on to the revision it produces, because the bytes it +// did not replace were seen exactly as often as the ones it did. +struct RevisionCheck final { + MutationGuard state{MutationGuard::allowed}; + bool complete{}; +}; + +// The read-before-write rule: an existing file may only be replaced or edited +// after the model read exactly the revision that is on disk now. `content` is +// the text the caller just read for this mutation and is about to act on, so +// the comparison can never be based on a different revision than the mutation +// itself. The caller consults this only for a target that exists; a missing +// target is a creation. +RevisionCheck CheckObservedRevision(const FileToolContext &context, + const std::string &absolute_path, + std::string_view content, + const bool require_complete) { + if (context.observations == nullptr) + return {}; + const auto observed = context.observations->Observed(absolute_path); + if (!observed) + return {.state = MutationGuard::not_read}; + if (observed->content_hash != Sha256Hex(content)) + return {.state = MutationGuard::changed}; + if (require_complete && !observed->complete) + return {.state = MutationGuard::partial, .complete = false}; + return {.state = MutationGuard::allowed, .complete = observed->complete}; +} + +// The model-visible refusal: it always names the remedy, so the next step is a +// read of the same path rather than a retry of the same mutation. +ToolInvocationResult MutationRefused(const FileToolContext &context, + const MutationGuard guard, + const std::string &input_path) { + if (guard == MutationGuard::not_read) { + return ToolFailure( + Text(context, ToolTextKey::tool_file_write_needs_read, input_path)); + } + if (guard == MutationGuard::partial) { + return ToolFailure(Text(context, ToolTextKey::tool_file_write_needs_full_read, + input_path)); + } + // `changed`, plus the `allowed` value no caller passes here. + return ToolFailure( + Text(context, ToolTextKey::tool_file_write_stale_read, input_path)); +} + +// The model-visible refusal for a path another tool call is already mutating. +ToolInvocationResult MutationBusy(const FileToolContext &context, + const std::string &input_path) { + return ToolFailure(Text(context, ToolTextKey::tool_file_write_busy, + input_path)); +} + // FileReadTool.appendDirectory(): a depth-first tree of "[DIR] /" and // "[FILE] " rows, capped at MAX_DIRECTORY_ITEMS entries. huxerui::Task> @@ -550,7 +672,13 @@ ExecuteFileRead(FileToolContext context, std::string arguments_json) { co_return ToolFailure(Text(context, ToolTextKey::tool_file_read_failed, content.error().message)); } - co_return ToolSuccess(TruncateContent(AddLineNumbers(*content, 1))); + // Record the digest of exactly the text this read returned, together with + // whether the model receives all of it: the rendered output is capped, and a + // truncated view must not authorize a whole-file replacement later. + auto rendered = AddLineNumbers(*content, 1); + const bool complete = rendered.size() <= kMaximumResultCharacters; + RememberObserved(context, target->absolute, *content, complete); + co_return ToolSuccess(TruncateContent(std::move(rendered))); } // Range read: only the requested bytes are fetched, so files larger than the @@ -622,6 +750,21 @@ ExecuteFileRead(FileToolContext context, std::string arguments_json) { --total_lines; result += Text(context, ToolTextKey::tool_file_read_range_info, total_lines, start_kb, end_kb, file_length / 1024U); + // A windowed read identifies the whole revision, which is what lets the model + // edit the file afterwards, but it is only complete -- and so only able to + // authorize a whole-file replacement -- when the window covered every byte and + // the rendered output was not truncated. Identifying the revision means + // reading the file as text once: a digest of the window alone could not tell + // whether the bytes outside it changed. + const bool complete = start_byte == 0 && end_byte == file_length && + result.size() <= kMaximumResultCharacters; + auto whole = co_await context.files->ReadText(target->absolute); + if (whole) + RememberObserved(context, target->absolute, *whole, complete); + else if (context.observations != nullptr) + // The revision cannot be identified, so a recorded one may describe + // different bytes: drop it rather than vouching for it. + context.observations->Forget(target->absolute); co_return ToolSuccess(TruncateContent(std::move(result))); } @@ -668,27 +811,55 @@ ExecuteFileWrite(FileToolContext context, std::string arguments_json) { target.error().message)); } const auto info = co_await context.files->Stat(target->absolute); + // Only a missing target is a creation. Any other failure -- a permission or + // I/O error, for example -- leaves the state of the path unknown, and writing + // anyway could overwrite a file that was never read. + if (!info && info.error().code != ToolFileErrorCode::not_found) { + co_return ToolFailure(Text(context, ToolTextKey::tool_file_write_failed, + info.error().message)); + } if (info && info->kind == ToolFileKind::directory) { co_return ToolFailure( Text(context, ToolTextKey::tool_file_write_is_directory, input_path)); } - // Legacy FileWriteTool creates the missing parent chain before writing. - const std::string parent = ParentPath(target->absolute); - if (!parent.empty()) { - const auto parent_info = co_await context.files->Stat(parent); - if (!parent_info || parent_info->kind != ToolFileKind::directory) { - auto created = co_await context.files->CreateDirectories(parent); - if (!created) { - co_return ToolFailure( - Text(context, ToolTextKey::tool_file_write_mkdir_failed, parent)); - } - } - } + // One mutation per path at a time in this process: two tool calls that both + // passed the freshness check would otherwise both write, and the later one + // would drop the earlier one's change while both reported success. + const MutationClaim claim{context.observations, target->absolute}; + if (!claim.acquired()) + co_return MutationBusy(context, input_path); + // Read before write: an existing file may only be overwritten after a read + // that still matches what is on disk, and that read replaces it whole, so it + // must also have shown the model every byte. The bytes read here are the ones + // the guard compares and the ones the diff records, so the check can never be + // based on a different revision than the write itself. A missing file is a + // creation, which cannot discard anything. std::string old_content; if (info.has_value()) { auto previous = co_await context.files->ReadText(target->absolute); - if (previous) - old_content = std::move(*previous); + if (!previous) { + co_return ToolFailure(Text(context, ToolTextKey::tool_file_write_failed, + previous.error().message)); + } + old_content = std::move(*previous); + const auto check = CheckObservedRevision(context, target->absolute, + old_content, + /*require_complete=*/true); + if (check.state != MutationGuard::allowed) + co_return MutationRefused(context, check.state, input_path); + } else { + // Legacy FileWriteTool creates the missing parent chain before writing. + const std::string parent = ParentPath(target->absolute); + if (!parent.empty()) { + const auto parent_info = co_await context.files->Stat(parent); + if (!parent_info || parent_info->kind != ToolFileKind::directory) { + auto created = co_await context.files->CreateDirectories(parent); + if (!created) { + co_return ToolFailure( + Text(context, ToolTextKey::tool_file_write_mkdir_failed, parent)); + } + } + } } const std::string content = StringArgument(*object, "content"); auto written = co_await context.files->WriteText(target->absolute, content); @@ -696,6 +867,9 @@ ExecuteFileWrite(FileToolContext context, std::string arguments_json) { co_return ToolFailure(Text(context, ToolTextKey::tool_file_write_failed, written.error().message)); } + // The tool supplied every byte it just wrote, so it knows the whole revision + // and the file counts as completely read instead of demanding a re-read. + RememberObserved(context, target->absolute, content, /*complete=*/true); auto result = ToolSuccess(Text(context, info.has_value() ? ToolTextKey::tool_file_write_updated @@ -740,6 +914,10 @@ ExecuteFileEdit(FileToolContext context, std::string arguments_json) { } const auto info = co_await context.files->Stat(target->absolute); if (!info) { + if (info.error().code != ToolFileErrorCode::not_found) { + co_return ToolFailure(Text(context, ToolTextKey::tool_file_edit_failed, + info.error().message)); + } co_return ToolFailure( Text(context, ToolTextKey::tool_file_edit_not_found, target->display)); } @@ -747,11 +925,25 @@ ExecuteFileEdit(FileToolContext context, std::string arguments_json) { co_return ToolFailure( Text(context, ToolTextKey::tool_file_edit_is_directory, input_path)); } + // One mutation per path at a time in this process, for the same reason as + // file_write: two edits derived from the same revision would otherwise both + // pass the check and the later one would drop the earlier one's change. + const MutationClaim claim{context.observations, target->absolute}; + if (!claim.acquired()) + co_return MutationBusy(context, input_path); + // Editing rewrites the file, so it follows the read-before-write rule as well. + // It does not need a complete view, though: the edit carries over every byte + // it does not replace, so the content the model never saw is preserved rather + // than dropped. auto content = co_await context.files->ReadText(target->absolute); if (!content) { co_return ToolFailure(Text(context, ToolTextKey::tool_file_edit_failed, content.error().message)); } + const auto check = CheckObservedRevision(context, target->absolute, *content, + /*require_complete=*/false); + if (check.state != MutationGuard::allowed) + co_return MutationRefused(context, check.state, input_path); if (content->find(old_string) == std::string::npos) { co_return ToolFailure(Text(context, ToolTextKey::tool_file_edit_no_match)); } @@ -771,6 +963,9 @@ ExecuteFileEdit(FileToolContext context, std::string arguments_json) { co_return ToolFailure(Text(context, ToolTextKey::tool_file_edit_failed, written.error().message)); } + // The revision the edit produced is as completely known as the one it started + // from: it kept every byte the model never saw. + RememberObserved(context, target->absolute, updated, check.complete); auto result = ToolSuccess(Text(context, ToolTextKey::tool_file_edit_success, target->display, replaced)); auto diff_id = co_await RecordChange(context, target->absolute, *content, @@ -1121,9 +1316,12 @@ FileToolRegistry::FileToolRegistry( std::shared_ptr settings, std::shared_ptr workspace, std::shared_ptr files, ToolTextLanguage language, - std::shared_ptr diffs) + std::shared_ptr diffs, + std::shared_ptr observations) : settings_(std::move(settings)), workspace_(std::move(workspace)), - files_(std::move(files)), language_(language), diffs_(std::move(diffs)) { + files_(std::move(files)), language_(language), diffs_(std::move(diffs)), + observations_(observations ? std::move(observations) + : std::make_shared()) { if (!settings_ || !workspace_ || !files_) { throw std::invalid_argument( "FileToolRegistry requires settings, project workspace and file access " @@ -1169,7 +1367,8 @@ FileToolRegistry::Invoke(std::string name, std::string arguments_json) { FileToolContext{.projects = workspace_.get(), .files = files_.get(), .language = language_, - .diffs = diffs_.get()}, + .diffs = diffs_.get(), + .observations = observations_.get()}, std::move(arguments_json)); } diff --git a/src/application/file_tool_registry.h b/src/application/file_tool_registry.h index a17d51c2..edbadbaa 100644 --- a/src/application/file_tool_registry.h +++ b/src/application/file_tool_registry.h @@ -4,6 +4,7 @@ #include #include +#include "application/file_observation_store.h" #include "application/mcp_execution_settings.h" #include "application/ports/project_workspace_controller.h" #include "application/ports/diff_store.h" @@ -21,6 +22,12 @@ namespace linecode::application { // branch. Enablement follows the "file_ops" group state and its local-only // execution mode, matching the legacy tool registry. // +// file_read records the digest of every file it reads in an in-memory +// FileObservationStore, and the write family refuses to replace or edit an +// existing file whose recorded digest is missing or no longer matches the bytes +// on disk. The model is then told to read the file again, so it never +// overwrites a revision it has not seen. +// // Tool output is localized through the application-owned text catalog rather // than app::strings::*, because the built-in tools run outside composition // where huxerui::UseString is unavailable. The language is fixed at @@ -31,7 +38,8 @@ class FileToolRegistry final : public ToolRegistry { std::shared_ptr workspace, std::shared_ptr files, ToolTextLanguage language = ToolTextLanguage::english, - std::shared_ptr diffs = {}); + std::shared_ptr diffs = {}, + std::shared_ptr observations = {}); [[nodiscard]] huxerui::Task> Refresh() override; @@ -46,6 +54,10 @@ class FileToolRegistry final : public ToolRegistry { std::shared_ptr files_; ToolTextLanguage language_{ToolTextLanguage::english}; std::shared_ptr diffs_; + // Shared by every tool call of this registry; an injected store lets callers + // that must agree on "already read" (for example a second file registry) pass + // the same record instead of keeping a private one. + std::shared_ptr observations_; std::vector tools_; }; diff --git a/src/application/tool_text_catalog.cpp b/src/application/tool_text_catalog.cpp index bd673ed9..a9789785 100644 --- a/src/application/tool_text_catalog.cpp +++ b/src/application/tool_text_catalog.cpp @@ -121,6 +121,30 @@ constexpr std::array kToolTextEntries{{ .english = "Failed to write file: {0}", .chinese = "写入文件失败: {0}", }, + { + .key = ToolTextKey::tool_file_write_needs_read, + .name = "tool_file_write_needs_read", + .english = "Cannot modify {0}: the file already exists but has not been read. Read it with file_read first, then try again.", + .chinese = "无法修改 {0}:文件已存在但尚未读取。请先用 file_read 读取该文件,然后重试。", + }, + { + .key = ToolTextKey::tool_file_write_stale_read, + .name = "tool_file_write_stale_read", + .english = "Cannot modify {0}: the file changed after it was read. Read it again with file_read, then try again.", + .chinese = "无法修改 {0}:文件在读取之后已被修改。请用 file_read 重新读取该文件,然后重试。", + }, + { + .key = ToolTextKey::tool_file_write_needs_full_read, + .name = "tool_file_write_needs_full_read", + .english = "Cannot replace {0}: the file was never read in full, so replacing it could drop content that was never seen. Use file_edit for a targeted change.", + .chinese = "无法整体替换 {0}:该文件从未被完整读取,整体替换可能丢失未被看到的内容。请使用 file_edit 做局部修改。", + }, + { + .key = ToolTextKey::tool_file_write_busy, + .name = "tool_file_write_busy", + .english = "Cannot modify {0}: another tool call is already changing this file. Try again.", + .chinese = "无法修改 {0}:另一个工具调用正在修改该文件。请重试。", + }, { .key = ToolTextKey::tool_file_edit_no_match, .name = "tool_file_edit_no_match", diff --git a/src/application/tool_text_catalog.h b/src/application/tool_text_catalog.h index cc389e93..f5b1460f 100644 --- a/src/application/tool_text_catalog.h +++ b/src/application/tool_text_catalog.h @@ -46,6 +46,10 @@ enum class ToolTextKey : std::uint8_t { tool_file_write_is_directory, tool_file_write_mkdir_failed, tool_file_write_failed, + tool_file_write_needs_read, + tool_file_write_stale_read, + tool_file_write_needs_full_read, + tool_file_write_busy, tool_file_edit_no_match, tool_file_edit_multiple_matches, tool_file_edit_success, @@ -110,7 +114,7 @@ enum class ToolTextKey : std::uint8_t { }; // Number of ToolTextKey values, and the row count of the generated table. -inline constexpr std::size_t kToolTextKeyCount = 76; +inline constexpr std::size_t kToolTextKeyCount = 80; // Packaged resource key of a catalog entry, such as "tool_file_read_failed". [[nodiscard]] std::string_view ToolTextName(ToolTextKey key) noexcept; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 734c4e97..9d8d9c43 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1766,6 +1766,8 @@ add_test(NAME web_tool_tests COMMAND web_tool_tests) add_executable(file_tool_tests file_tool_tests.cpp + ../src/application/file_content_hash.cpp + ../src/application/file_observation_store.cpp ../src/application/file_tool_registry.cpp ../src/application/tool_file_access.cpp ../src/application/tool_file_path_policy.cpp @@ -1790,6 +1792,26 @@ else () endif () add_test(NAME file_tool_tests COMMAND file_tool_tests) +add_executable(file_observation_tests + file_observation_tests.cpp + ../src/application/file_content_hash.cpp + ../src/application/file_observation_store.cpp +) +target_include_directories(file_observation_tests PRIVATE ../src) +target_compile_features(file_observation_tests PRIVATE cxx_std_23) +set_target_properties(file_observation_tests PROPERTIES + CXX_STANDARD 23 + CXX_STANDARD_REQUIRED ON + CXX_EXTENSIONS OFF +) +if (MSVC) + target_compile_options(file_observation_tests PRIVATE /W4 /permissive-) +else () + target_compile_options(file_observation_tests PRIVATE + -Wall -Wextra -Wpedantic -Werror) +endif () +add_test(NAME file_observation_tests COMMAND file_observation_tests) + add_executable(context_usage_tests context_usage_tests.cpp ../src/domain/context_usage.cpp diff --git a/tests/file_observation_tests.cpp b/tests/file_observation_tests.cpp new file mode 100644 index 00000000..70047323 --- /dev/null +++ b/tests/file_observation_tests.cpp @@ -0,0 +1,171 @@ +#include "gtest_support.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "application/file_content_hash.h" +#include "application/file_observation_store.h" + +namespace { + +using namespace linecode; + +// Reference digests produced by an independent SHA-256 implementation, so a +// mistake in the local one cannot be confirmed by the local one. +void CheckDigest(std::string_view bytes, std::string_view expected) { + EXPECT_EXPRESSION(application::Sha256Hex(bytes) == expected); +} + +TEST(Sha256Digest, MatchesTheKnownVectors) { + CheckDigest("", + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"); + CheckDigest("abc", + "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad"); + // 56 bytes: the shortest input that spans two compression blocks once the + // length padding is added. + CheckDigest("abcdbcdecdefdefgefghfghighijhijkijkljklmklmnlmnomnopnopq", + "248d6a61d20638b8e5c026930c3e6039a33ce45964ff2167f6ecedd419db06c1"); + // Exactly one block, so the padding block starts clean. + CheckDigest( + "1234567890123456789012345678901234567890123456789012345678901234", + "676491965ed3ec50cb7a63ee96315480a95c54426b0b72bca8a0d4ad1285ad55"); + CheckDigest(std::string(1000U, 'a'), + "41edece42d63e8d9bf515a9ba6932e1c20cbc9f5a5d134645adb5db1b9737ea3"); +} + +TEST(Sha256Digest, HashesArbitraryBytesNotJustText) { + std::string bytes; + for (std::uint32_t value = 0; value < 200U; ++value) + bytes.push_back(static_cast(value)); + EXPECT_EXPRESSION(application::Sha256Hex(bytes) == + "1901da1c9f699b48f6b2636e65cbf73abf99d0441ef67f5c540a42f7051dec6f"); +} + +TEST(Sha256Digest, IsIndependentOfChunking) { + std::string bytes; + bytes.reserve(4096U); + std::mt19937 generator{20260101U}; + for (std::size_t index = 0; index < 4096U; ++index) + bytes.push_back(static_cast(generator() & 0xffU)); + + const std::string expected = application::Sha256Hex(bytes); + for (const std::size_t chunk : {std::size_t{1}, std::size_t{63}, + std::size_t{64}, std::size_t{65}, + std::size_t{1000}}) { + application::Sha256Digest digest; + std::size_t offset = 0; + while (offset < bytes.size()) { + const std::size_t length = std::min(chunk, bytes.size() - offset); + digest.Update(std::string_view{bytes}.substr(offset, length)); + offset += length; + } + EXPECT_EXPRESSION(digest.Hex() == expected); + } + // An explicitly empty chunk must not disturb a running digest. + application::Sha256Digest digest; + digest.Update(std::string_view{}); + digest.Update("abc"); + EXPECT_EXPRESSION(digest.Hex() == + "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad"); +} + +TEST(FileObservationStore, TracksTheRevisionOfEveryReadPath) { + application::FileObservationStore store; + EXPECT_EXPRESSION(store.Size() == 0U); + EXPECT_EXPRESSION(!store.Observed("/workspace/a.txt").has_value()); + + const application::FileObservation whole{.content_hash = "first", + .complete = true}; + const application::FileObservation partial{.content_hash = "second", + .complete = false}; + store.Record("/workspace/a.txt", whole); + store.Record("/workspace/b.txt", partial); + EXPECT_EXPRESSION(store.Size() == 2U); + EXPECT_EXPRESSION(store.Observed("/workspace/a.txt") == + std::optional{whole}); + EXPECT_EXPRESSION(store.Observed("/workspace/b.txt") == + std::optional{partial}); + + // Reading the same path again replaces the recorded revision, including + // whether that revision was seen in full. + store.Record("/workspace/a.txt", application::FileObservation{ + .content_hash = "third", + .complete = false}); + EXPECT_EXPRESSION(store.Observed("/workspace/a.txt")->content_hash == "third"); + EXPECT_EXPRESSION(store.Observed("/workspace/a.txt")->complete == false); + EXPECT_EXPRESSION(store.Size() == 2U); + + // Paths are matched exactly: a sibling directory is a different file. + EXPECT_EXPRESSION(!store.Observed("/elsewhere/a.txt").has_value()); + + store.Forget("/workspace/a.txt"); + EXPECT_EXPRESSION(!store.Observed("/workspace/a.txt").has_value()); + EXPECT_EXPRESSION(store.Size() == 1U); + // Forgetting an unknown path is a no-op. + store.Forget("/workspace/missing.txt"); + EXPECT_EXPRESSION(store.Size() == 1U); + + store.Clear(); + EXPECT_EXPRESSION(store.Size() == 0U); + EXPECT_EXPRESSION(!store.Observed("/workspace/b.txt").has_value()); +} + +TEST(FileObservationStore, HandsOutOneMutationClaimPerPath) { + application::FileObservationStore store; + EXPECT_EXPRESSION(store.Claim("/workspace/a.txt")); + // A second tool call cannot claim the path while the first holds it. + EXPECT_EXPRESSION(!store.Claim("/workspace/a.txt")); + // A different path is unaffected. + EXPECT_EXPRESSION(store.Claim("/workspace/b.txt")); + + // Releasing the claim makes the path claimable again. + store.Release("/workspace/a.txt"); + EXPECT_EXPRESSION(store.Claim("/workspace/a.txt")); + store.Release("/workspace/a.txt"); + // Releasing an unheld path is a no-op. + store.Release("/workspace/missing.txt"); + + // A claim is not an observation: it never authorizes a mutation by itself. + EXPECT_EXPRESSION(store.Size() == 0U); + EXPECT_EXPRESSION(!store.Observed("/workspace/a.txt").has_value()); +} + +TEST(FileObservationStore, StaysBoundedByDroppingTheRecord) { + application::FileObservationStore store; + for (std::size_t index = 0; index < application::FileObservationStore:: + kMaximumObservedPaths; + ++index) { + store.Record("/workspace/file-" + std::to_string(index) + ".txt", + application::FileObservation{ + .content_hash = std::to_string(index), .complete = true}); + } + EXPECT_EXPRESSION(store.Size() == + application::FileObservationStore::kMaximumObservedPaths); + + // One more path drops the whole record, so a long-lived process cannot grow + // without bound; the newest path is still recorded. + store.Record("/workspace/overflow.txt", + application::FileObservation{.content_hash = "overflow", + .complete = false}); + EXPECT_EXPRESSION(store.Size() <= + application::FileObservationStore::kMaximumObservedPaths); + EXPECT_EXPRESSION(store.Observed("/workspace/overflow.txt")->content_hash == + "overflow"); + + // Dropping the record never drops the claims: a mutation in flight keeps its + // path reserved. + EXPECT_EXPRESSION(store.Claim("/workspace/held.txt")); + store.Record("/workspace/another.txt", + application::FileObservation{.content_hash = "another"}); + EXPECT_EXPRESSION(!store.Claim("/workspace/held.txt")); + store.Release("/workspace/held.txt"); +} + +} // namespace diff --git a/tests/file_tool_tests.cpp b/tests/file_tool_tests.cpp index 0d2b28d1..91101a75 100644 --- a/tests/file_tool_tests.cpp +++ b/tests/file_tool_tests.cpp @@ -21,6 +21,8 @@ #include #include +#include "application/file_content_hash.h" +#include "application/file_observation_store.h" #include "application/file_tool_registry.h" #include "application/ports/tool_file_access.h" #include "application/tool_text_catalog.h" @@ -537,10 +539,18 @@ struct Scenario final { files = std::make_shared(); files->root = root; SeedWorkspace(*files); - tools = std::make_shared(settings, workspace, - files); + // The registries own one record each. Injecting them lets a test start from a + // known state and read back what a call recorded, instead of relying on + // whatever an earlier call in the same suite happened to leave behind. + observations = std::make_shared(); + chinese_observations = + std::make_shared(); + tools = std::make_shared( + settings, workspace, files, application::ToolTextLanguage::english, + nullptr, observations); chinese_tools = std::make_shared( - settings, workspace, files, application::ToolTextLanguage::chinese); + settings, workspace, files, application::ToolTextLanguage::chinese, + nullptr, chinese_observations); } ~Scenario() { @@ -579,6 +589,8 @@ struct Scenario final { std::shared_ptr settings; std::shared_ptr workspace; std::shared_ptr files; + std::shared_ptr observations; + std::shared_ptr chinese_observations; std::shared_ptr tools; std::shared_ptr chinese_tools; bool done{}; @@ -905,7 +917,12 @@ huxerui::Task RunFileToolChecks(std::shared_ptr scenario) { EXPECT_EXPRESSION(invoked->content == "Failed to write file: injected write"); scenario->files->fail_write.reset(); - // 5. file_edit: unique match, no match, ambiguity and replace_all. + // 5. file_edit: unique match, no match, ambiguity and replace_all. Editing + // rewrites the file, so each target is read first -- the read-before-write + // rule covered in its own section below. + invoked = co_await scenario->tools->Invoke("file_read", + R"({"file_path":"edit.txt"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); invoked = co_await scenario->tools->Invoke( "file_edit", R"({"file_path":"edit.txt","old_string":"world","new_string":"there"})"); @@ -920,6 +937,9 @@ huxerui::Task RunFileToolChecks(std::shared_ptr scenario) { EXPECT_EXPRESSION(invoked && invoked->error); EXPECT_EXPRESSION(invoked->content == "No matching text found"); + invoked = co_await scenario->tools->Invoke("file_read", + R"({"file_path":"multi.txt"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); invoked = co_await scenario->tools->Invoke( "file_edit", R"({"file_path":"multi.txt","old_string":"cat","new_string":"dog"})"); @@ -1183,6 +1203,368 @@ huxerui::Task RunFileToolChecks(std::shared_ptr scenario) { EXPECT_EXPRESSION(failed.error().message.contains("injected")); scenario->settings->fail_load = false; + // 13. Read before write: an existing file may only be replaced or edited + // after a read whose recorded content digest still matches the file. + scenario->files->Seed("guard.txt", "original"); + invoked = co_await scenario->tools->Invoke( + "file_write", R"({"file_path":"guard.txt","content":"replaced"})"); + EXPECT_EXPRESSION(invoked && invoked->error); + EXPECT_EXPRESSION( + invoked->content == + "Cannot modify guard.txt: the file already exists but has not been read. " + "Read it with file_read first, then try again."); + EXPECT_EXPRESSION(scenario->files->Content("guard.txt") == "original"); + + // Reading the file authorizes the write, and the write the tool performs + // counts as read for the next one, so a sequence of edits needs no re-read. + invoked = + co_await scenario->tools->Invoke("file_read", R"({"file_path":"guard.txt"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + invoked = co_await scenario->tools->Invoke( + "file_write", R"({"file_path":"guard.txt","content":"one"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + EXPECT_EXPRESSION(invoked->content == + "Successfully updated file guard.txt (1 lines)"); + invoked = co_await scenario->tools->Invoke( + "file_write", R"({"file_path":"guard.txt","content":"two"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + EXPECT_EXPRESSION(scenario->files->Content("guard.txt") == "two"); + + // Rewriting the file outside the tools invalidates the recorded read. + scenario->files->Seed("guard.txt", "external"); + invoked = co_await scenario->tools->Invoke( + "file_write", R"({"file_path":"guard.txt","content":"clobber"})"); + EXPECT_EXPRESSION(invoked && invoked->error); + EXPECT_EXPRESSION( + invoked->content == + "Cannot modify guard.txt: the file changed after it was read. Read it " + "again with file_read, then try again."); + EXPECT_EXPRESSION(scenario->files->Content("guard.txt") == "external"); + + // Re-reading the changed file authorizes the write of the new revision. + invoked = + co_await scenario->tools->Invoke("file_read", R"({"file_path":"guard.txt"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + invoked = co_await scenario->tools->Invoke( + "file_write", R"({"file_path":"guard.txt","content":"three"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + EXPECT_EXPRESSION(scenario->files->Content("guard.txt") == "three"); + + // The rule compares content, not timestamps: touching the same bytes leaves + // the recorded revision valid. + invoked = + co_await scenario->tools->Invoke("file_read", R"({"file_path":"guard.txt"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + scenario->files->Seed("guard.txt", "three"); + invoked = co_await scenario->tools->Invoke( + "file_write", R"({"file_path":"guard.txt","content":"four"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + EXPECT_EXPRESSION(scenario->files->Content("guard.txt") == "four"); + + // A creation needs no read, and the created file is immediately writable. + invoked = co_await scenario->tools->Invoke( + "file_write", R"({"file_path":"guard-new.txt","content":"created"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + EXPECT_EXPRESSION(invoked->content == + "Successfully created file guard-new.txt (1 lines)"); + invoked = co_await scenario->tools->Invoke( + "file_write", R"({"file_path":"guard-new.txt","content":"again"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + EXPECT_EXPRESSION(scenario->files->Content("guard-new.txt") == "again"); + + // A read that failed records nothing, so the file stays unread. + scenario->files->Seed("guard-failed-read.txt", "untouched"); + scenario->files->fail_read = application::ToolFileError{ + .code = application::ToolFileErrorCode::io, .message = "injected read"}; + invoked = co_await scenario->tools->Invoke( + "file_read", R"({"file_path":"guard-failed-read.txt"})"); + EXPECT_EXPRESSION(invoked && invoked->error); + scenario->files->fail_read.reset(); + invoked = co_await scenario->tools->Invoke( + "file_write", R"({"file_path":"guard-failed-read.txt","content":"x"})"); + EXPECT_EXPRESSION(invoked && invoked->error); + EXPECT_EXPRESSION( + invoked->content == + "Cannot modify guard-failed-read.txt: the file already exists but has " + "not been read. Read it with file_read first, then try again."); + EXPECT_EXPRESSION(scenario->files->Content("guard-failed-read.txt") == + "untouched"); + + // A write that failed leaves the file as it was, so the recorded read still + // authorizes the retry. + scenario->files->Seed("guard-failed-write.txt", "original"); + invoked = co_await scenario->tools->Invoke( + "file_read", R"({"file_path":"guard-failed-write.txt"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + scenario->files->fail_write = application::ToolFileError{ + .code = application::ToolFileErrorCode::io, .message = "injected write"}; + invoked = co_await scenario->tools->Invoke( + "file_write", R"({"file_path":"guard-failed-write.txt","content":"x"})"); + EXPECT_EXPRESSION(invoked && invoked->error); + EXPECT_EXPRESSION(invoked->content == "Failed to write file: injected write"); + scenario->files->fail_write.reset(); + invoked = co_await scenario->tools->Invoke( + "file_write", R"({"file_path":"guard-failed-write.txt","content":"x"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + EXPECT_EXPRESSION(scenario->files->Content("guard-failed-write.txt") == "x"); + + // file_edit follows the same rule, including the stale case. + scenario->files->Seed("guard-edit.txt", "hello world"); + invoked = co_await scenario->tools->Invoke( + "file_edit", + R"({"file_path":"guard-edit.txt","old_string":"world","new_string":"there"})"); + EXPECT_EXPRESSION(invoked && invoked->error); + EXPECT_EXPRESSION( + invoked->content == + "Cannot modify guard-edit.txt: the file already exists but has not been " + "read. Read it with file_read first, then try again."); + EXPECT_EXPRESSION(scenario->files->Content("guard-edit.txt") == "hello world"); + + invoked = co_await scenario->tools->Invoke("file_read", + R"({"file_path":"guard-edit.txt"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + invoked = co_await scenario->tools->Invoke( + "file_edit", + R"({"file_path":"guard-edit.txt","old_string":"world","new_string":"there"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + EXPECT_EXPRESSION(invoked->content == + "Successfully edited guard-edit.txt (1 match(es) replaced)"); + EXPECT_EXPRESSION(scenario->files->Content("guard-edit.txt") == "hello there"); + + scenario->files->Seed("guard-edit.txt", "changed elsewhere"); + invoked = co_await scenario->tools->Invoke( + "file_edit", + R"({"file_path":"guard-edit.txt","old_string":"changed","new_string":"x"})"); + EXPECT_EXPRESSION(invoked && invoked->error); + EXPECT_EXPRESSION( + invoked->content == + "Cannot modify guard-edit.txt: the file changed after it was read. Read " + "it again with file_read, then try again."); + EXPECT_EXPRESSION(scenario->files->Content("guard-edit.txt") == + "changed elsewhere"); + + // A partial read identifies the revision, which is what lets the model edit + // the file, but it must not authorize replacing the whole file. The record is + // cleared first and a fresh path is read, so only this read can authorize + // anything that follows. + std::string wide; + wide.append(1024, 'w'); + wide.push_back('\n'); + wide.append(1024, 'x'); + wide.push_back('\n'); + wide.append(1024, 'y'); + wide.push_back('\n'); + scenario->files->Seed("guard-window.txt", std::move(wide)); + const std::string window_path = scenario->root + "/guard-window.txt"; + scenario->observations->Clear(); + invoked = co_await scenario->tools->Invoke( + "file_read", R"({"file_path":"guard-window.txt","start_kb":0,"end_kb":1})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + // The read did record something, and it recorded a partial view of exactly the + // bytes it delivered. + const auto window_observed = scenario->observations->Observed(window_path); + EXPECT_EXPRESSION(window_observed.has_value()); + EXPECT_EXPRESSION(window_observed->complete == false); + EXPECT_EXPRESSION(scenario->observations->Size() == 1U); + + invoked = co_await scenario->tools->Invoke( + "file_write", R"({"file_path":"guard-window.txt","content":"small now"})"); + EXPECT_EXPRESSION(invoked && invoked->error); + EXPECT_EXPRESSION( + invoked->content == + "Cannot replace guard-window.txt: the file was never read in full, so " + "replacing it could drop content that was never seen. Use file_edit for a " + "targeted change."); + EXPECT_EXPRESSION(scenario->files->Content("guard-window.txt").size() == 3075U); + + // The same partial view does authorize a localized edit, which keeps every + // byte it does not replace, and the edit inherits the partial view. + invoked = co_await scenario->tools->Invoke( + "file_edit", + R"({"file_path":"guard-window.txt","old_string":"w","new_string":"W","replace_all":true})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + EXPECT_EXPRESSION(scenario->files->Content("guard-window.txt") + .starts_with("WWWW")); + EXPECT_EXPRESSION(scenario->files->Content("guard-window.txt").ends_with("yyy\n")); + EXPECT_EXPRESSION(scenario->observations->Observed(window_path)->complete == + false); + + // An explicit range that covers the whole file is a complete view, so it + // authorizes a replacement. + invoked = co_await scenario->tools->Invoke( + "file_read", R"({"file_path":"segment.txt","start_kb":0,"end_kb":2})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + invoked = co_await scenario->tools->Invoke( + "file_write", R"({"file_path":"segment.txt","content":"replaced whole"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + EXPECT_EXPRESSION(scenario->files->Content("segment.txt") == + "replaced whole"); + + // A whole-file read whose rendered output is truncated only showed the model + // part of the file, so it is a partial view as well. + std::string truncated; + for (int line = 0; line < 4500; ++line) + truncated += "0123456789\n"; + EXPECT_EXPRESSION(truncated.size() < 50U * 1024U); + scenario->files->Seed("guard-truncated.txt", std::move(truncated)); + invoked = co_await scenario->tools->Invoke( + "file_read", R"({"file_path":"guard-truncated.txt"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + EXPECT_EXPRESSION(invoked->content.contains("chars truncated")); + invoked = co_await scenario->tools->Invoke( + "file_write", R"({"file_path":"guard-truncated.txt","content":"short"})"); + EXPECT_EXPRESSION(invoked && invoked->error); + EXPECT_EXPRESSION( + invoked->content == + "Cannot replace guard-truncated.txt: the file was never read in full, so " + "replacing it could drop content that was never seen. Use file_edit for a " + "targeted change."); + invoked = co_await scenario->tools->Invoke( + "file_edit", + R"({"file_path":"guard-truncated.txt","old_string":"0123456789","new_string":"9876543210","replace_all":true})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + + // The recorded digest is the digest of the text the read returned, not of a + // later look at the path: a read records exactly what the model was given. + scenario->observations->Clear(); + scenario->files->Seed("guard-digest.txt", "exact bytes"); + invoked = co_await scenario->tools->Invoke("file_read", + R"({"file_path":"guard-digest.txt"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + const auto digest_observed = + scenario->observations->Observed(scenario->root + "/guard-digest.txt"); + EXPECT_EXPRESSION(digest_observed.has_value()); + EXPECT_EXPRESSION(digest_observed->content_hash == + application::Sha256Hex("exact bytes")); + EXPECT_EXPRESSION(digest_observed->complete == true); + + // Another tool call already changing the file refuses the mutation instead of + // letting both pass the freshness check and one drop the other's change. + scenario->files->Seed("guard-busy.txt", "busy"); + invoked = co_await scenario->tools->Invoke("file_read", + R"({"file_path":"guard-busy.txt"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + const std::string busy_path = scenario->root + "/guard-busy.txt"; + EXPECT_EXPRESSION(scenario->observations->Claim(busy_path)); + invoked = co_await scenario->tools->Invoke( + "file_write", R"({"file_path":"guard-busy.txt","content":"mine"})"); + EXPECT_EXPRESSION(invoked && invoked->error); + EXPECT_EXPRESSION( + invoked->content == + "Cannot modify guard-busy.txt: another tool call is already changing this " + "file. Try again."); + invoked = co_await scenario->tools->Invoke( + "file_edit", + R"({"file_path":"guard-busy.txt","old_string":"busy","new_string":"idle"})"); + EXPECT_EXPRESSION(invoked && invoked->error); + EXPECT_EXPRESSION( + invoked->content == + "Cannot modify guard-busy.txt: another tool call is already changing this " + "file. Try again."); + EXPECT_EXPRESSION(scenario->files->Content("guard-busy.txt") == "busy"); + scenario->observations->Release(busy_path); + // A refused mutation released its own claim, so the path is free again. + invoked = co_await scenario->tools->Invoke( + "file_write", R"({"file_path":"guard-busy.txt","content":"mine"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + EXPECT_EXPRESSION(scenario->files->Content("guard-busy.txt") == "mine"); + + // A Stat failure that is not "not found" leaves the state of the path unknown, + // so the write fails instead of treating an existing file as a creation. + scenario->files->Seed("guard-stat.txt", "existing"); + invoked = co_await scenario->tools->Invoke("file_read", + R"({"file_path":"guard-stat.txt"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + scenario->files->fail_stat = application::ToolFileError{ + .code = application::ToolFileErrorCode::io, .message = "injected stat"}; + invoked = co_await scenario->tools->Invoke( + "file_write", R"({"file_path":"guard-stat.txt","content":"clobber"})"); + EXPECT_EXPRESSION(invoked && invoked->error); + EXPECT_EXPRESSION(invoked->content == "Failed to write file: injected stat"); + invoked = co_await scenario->tools->Invoke( + "file_edit", + R"({"file_path":"guard-stat.txt","old_string":"existing","new_string":"x"})"); + EXPECT_EXPRESSION(invoked && invoked->error); + EXPECT_EXPRESSION(invoked->content == "Failed to edit file: injected stat"); + scenario->files->fail_stat.reset(); + EXPECT_EXPRESSION(scenario->files->Content("guard-stat.txt") == "existing"); + + // A missing file is still a creation under a failing Stat only when the + // provider reports it as missing. + scenario->files->fail_stat = application::ToolFileError{ + .code = application::ToolFileErrorCode::not_found, + .message = "injected missing"}; + invoked = co_await scenario->tools->Invoke( + "file_write", R"({"file_path":"guard-stat-new.txt","content":"created"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + EXPECT_EXPRESSION(invoked->content == + "Successfully created file guard-stat-new.txt (1 lines)"); + scenario->files->fail_stat.reset(); + + // The record lives with one registry: the Chinese registry has read nothing, + // so it must read the file before it may change it -- and it reports the + // refusal in its own language. + scenario->files->Seed("guard-shared.txt", "shared"); + invoked = co_await scenario->tools->Invoke( + "file_read", R"({"file_path":"guard-shared.txt"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + chinese_invoked = co_await scenario->chinese_tools->Invoke( + "file_write", R"({"file_path":"guard-shared.txt","content":"zh"})"); + EXPECT_EXPRESSION(chinese_invoked && chinese_invoked->error); + EXPECT_EXPRESSION( + chinese_invoked->content == + "无法修改 guard-shared.txt:文件已存在但尚未读取。请先用 file_read " + "读取该文件,然后重试。"); + EXPECT_EXPRESSION(scenario->files->Content("guard-shared.txt") == "shared"); + chinese_invoked = co_await scenario->chinese_tools->Invoke( + "file_read", R"({"file_path":"guard-shared.txt"})"); + EXPECT_EXPRESSION(chinese_invoked && !chinese_invoked->error); + scenario->files->Seed("guard-shared.txt", "shared elsewhere"); + chinese_invoked = co_await scenario->chinese_tools->Invoke( + "file_write", R"({"file_path":"guard-shared.txt","content":"zh"})"); + EXPECT_EXPRESSION(chinese_invoked && chinese_invoked->error); + EXPECT_EXPRESSION( + chinese_invoked->content == + "无法修改 guard-shared.txt:文件在读取之后已被修改。请用 file_read " + "重新读取该文件,然后重试。"); + + // A file that disappeared after the read is a creation again, not an + // unverified overwrite. + scenario->files->Seed("guard-gone.txt", "temporary"); + invoked = co_await scenario->tools->Invoke("file_read", + R"({"file_path":"guard-gone.txt"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + invoked = co_await scenario->tools->Invoke( + "file_delete", R"({"paths":["guard-gone.txt"],"reason":"cleanup"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + invoked = co_await scenario->tools->Invoke( + "file_write", R"({"file_path":"guard-gone.txt","content":"recreated"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + EXPECT_EXPRESSION(invoked->content == + "Successfully created file guard-gone.txt (1 lines)"); + + // A shared record can be injected, which is how two registries agree on what + // has been read. + auto shared = std::make_shared(); + auto first = std::make_shared( + scenario->settings, scenario->workspace, scenario->files, + application::ToolTextLanguage::english, nullptr, shared); + auto second = std::make_shared( + scenario->settings, scenario->workspace, scenario->files, + application::ToolTextLanguage::english, nullptr, shared); + auto first_refreshed = co_await first->Refresh(); + EXPECT_EXPRESSION(first_refreshed); + scenario->files->Seed("guard-injected.txt", "before"); + invoked = co_await first->Invoke("file_read", + R"({"file_path":"guard-injected.txt"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + auto second_refreshed = co_await second->Refresh(); + EXPECT_EXPRESSION(second_refreshed); + invoked = co_await second->Invoke( + "file_write", R"({"file_path":"guard-injected.txt","content":"after"})"); + EXPECT_EXPRESSION(invoked && !invoked->error); + EXPECT_EXPRESSION(scenario->files->Content("guard-injected.txt") == "after"); + scenario->done = true; } diff --git a/update.md b/update.md index e473be14..20a2edb9 100644 --- a/update.md +++ b/update.md @@ -1,5 +1,39 @@ # 更新日志 +## v1.3.4 + +### 工作区与文件 + +- **改动文件前先读取** - 内置文件工具新增读后写保护:覆盖或编辑一个已存在的文件之前 + 必须先读取它。读取成功时记录本次返回内容的 SHA-256,写入或编辑前用**同一次读取拿到的 + 内容**重新比对(不再另查一次磁盘,因此记录的一定是模型看到的那一版): + 文件从未读取过就拒绝,并提示先用 `file_read` 读取后重试;读取之后文件又被外部改动 + (其他程序、其他 Agent 或用户)同样拒绝,并提示重新读取,不再覆盖 AI 从未看过的内容。 +- **整体替换要求完整读过** - `file_write` 会用新内容整份替换目标,所以只在模型确实看完整份 + 文件时才放行:窗口读取(`start_kb`/`end_kb`)或结果被截断的读取只记为「部分读取」, + 此时整体替换会被拒绝并提示改用 `file_edit`;`file_edit` 只替换匹配到的片段、其余字节 + 原样保留,因此部分读取也允许编辑。目标不存在时按新建处理,无需先读;工具自身写入或 + 编辑成功后刷新记录,连续修改不必反复重读。 +- **同一文件不会被两个工具调用同时改** - 变更期间按路径持有内存占用标记:并行子 Agent 或 + 同一轮里的并发调用正在改同一文件时,后来的变更直接拒绝并提示重试,不会出现两次变更 + 都通过校验、后者覆盖前者却都报成功。进程之外的写入者无法被拦住(平台文件能力没有 + compare-and-swap),这段窗口仍以最终的写入调用为界。 +- **读取失败的状态不再被当作新文件** - 写入前 `Stat` 失败且错误不是「不存在」(例如权限或 + I/O 错误)时现在直接失败,不再把已存在的文件当成新建文件而跳过读取保护。 +- **记录只存在内存中** - 已读记录按绝对路径保存在进程内存里,不写入数据库或磁盘: + 应用重启后从零开始,这只会让规则更严格;记录条数设有上限,超出后整表作废, + 代价仅是重新读取一次。 + +### 发布 + +- **版本** - `versionCode` 38、`versionName` 1.3.4。 +- **回归覆盖** - 原生测试新增 `file_observation_tests`(SHA-256 标准向量与分块一致性、 + 内存观察表语义、容量上限与变更占用标记)和 `file_tool_tests` 的读后写用例(未读拒绝、 + 外部改动拒绝、内容相同不算改动、部分读取只允许编辑、窗口覆盖整份文件才允许整体替换、 + 截断读取记为部分、读取失败不授权、`Stat` 失败不当作新文件、并发变更被拒绝后释放、 + 写入失败后可重试、文件被外部删除后可重建、中英文提示、共享记录注入); + `:app:assembleDebug` 通过,工具文案目录表由 `tools/gen_tool_text_catalog.py` 重新生成。 + ## v1.3.3 ### 工作区与文件